diff mbox series

[net] net: phy: dp83867: Fix various styling and space issues

Message ID 20200903141510.20212-1-dmurphy@ti.com
State New
Headers show
Series [net] net: phy: dp83867: Fix various styling and space issues | expand

Commit Message

Dan Murphy Sept. 3, 2020, 2:15 p.m. UTC 
Fix spacing issues reported for misaligned switch..case and extra new
lines.

Also updated the file header to comply with networking commet style.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/phy/dp83867.c | 47 ++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

Comments

Joe Perches Sept. 3, 2020, 4:54 p.m. UTC | #1 
On Thu, 2020-09-03 at 11:41 -0500, Dan Murphy wrote:
> On 9/3/20 11:34 AM, Florian Fainelli wrote:
> > On 9/3/2020 7:15 AM, Dan Murphy wrote:
> > > Fix spacing issues reported for misaligned switch..case and extra new
> > > lines.
[]
> > > diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
[]
> > > @@ -35,7 +34,7 @@
> > >   #define DP83867_CFG4_SGMII_ANEG_MASK (BIT(5) | BIT(6))
> > >   #define DP83867_CFG4_SGMII_ANEG_TIMER_11MS   (3 << 5)
> > >   #define DP83867_CFG4_SGMII_ANEG_TIMER_800US  (2 << 5)
> > > -#define DP83867_CFG4_SGMII_ANEG_TIMER_2US    (1 << 5)
> > > +#define DP83867_CFG4_SGMII_ANEG_TIMER_2US    BIT(5)

> > Now the definitions are inconsistent, you would want to drop this one 
> > and stick to the existing style.
> 
> OK I was a little conflicted making that change due to the reasons you 
> mentioned.  But if that is an acceptable warning I am ok with it.

Maybe use GENMASK for DP83867_CFG4_SGMII_ANEG_MASK instead.
Andrew Lunn Sept. 3, 2020, 8:42 p.m. UTC | #2 
> > >   #define DP83867_CFG4_SGMII_ANEG_MASK (BIT(5) | BIT(6))
> > >   #define DP83867_CFG4_SGMII_ANEG_TIMER_11MS   (3 << 5)
> > >   #define DP83867_CFG4_SGMII_ANEG_TIMER_800US  (2 << 5)
> > > -#define DP83867_CFG4_SGMII_ANEG_TIMER_2US    (1 << 5)
> > > +#define DP83867_CFG4_SGMII_ANEG_TIMER_2US    BIT(5)
> > 
> > Now the definitions are inconsistent, you would want to drop this one
> > and stick to the existing style.
> 
> OK I was a little conflicted making that change due to the reasons you
> mentioned.  But if that is an acceptable warning I am ok with it.

Hi Dan

I work around this by using hex:

#define DP83867_CFG4_SGMII_ANEG_TIMER_11MS   (0x3 << 5)
#define DP83867_CFG4_SGMII_ANEG_TIMER_800US  (0x2 << 5)
#define DP83867_CFG4_SGMII_ANEG_TIMER_2US    (0x1 << 5)

checkpatch does not complain about that.

	   Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index cd7032628a28..f182a8d767c6 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -1,6 +1,5 @@ 
 // SPDX-License-Identifier: GPL-2.0
-/*
- * Driver for the Texas Instruments DP83867 PHY
+/* Driver for the Texas Instruments DP83867 PHY
  *
  * Copyright (C) 2015 Texas Instruments Inc.
  */
@@ -35,7 +34,7 @@ 
 #define DP83867_CFG4_SGMII_ANEG_MASK (BIT(5) | BIT(6))
 #define DP83867_CFG4_SGMII_ANEG_TIMER_11MS   (3 << 5)
 #define DP83867_CFG4_SGMII_ANEG_TIMER_800US  (2 << 5)
-#define DP83867_CFG4_SGMII_ANEG_TIMER_2US    (1 << 5)
+#define DP83867_CFG4_SGMII_ANEG_TIMER_2US    BIT(5)
 #define DP83867_CFG4_SGMII_ANEG_TIMER_16MS   (0 << 5)
 
 #define DP83867_RGMIICTL	0x0032
@@ -113,7 +112,6 @@ 
 #define DP83867_RGMII_RX_CLK_DELAY_SHIFT	0
 #define DP83867_RGMII_RX_CLK_DELAY_INV	(DP83867_RGMII_RX_CLK_DELAY_MAX + 1)
 
-
 /* IO_MUX_CFG bits */
 #define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MASK	0x1f
 #define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX	0x0
@@ -384,22 +382,22 @@  static int dp83867_set_downshift(struct phy_device *phydev, u8 cnt)
 				      DP83867_DOWNSHIFT_EN);
 
 	switch (cnt) {
-		case DP83867_DOWNSHIFT_1_COUNT:
-			count = DP83867_DOWNSHIFT_1_COUNT_VAL;
-			break;
-		case DP83867_DOWNSHIFT_2_COUNT:
-			count = DP83867_DOWNSHIFT_2_COUNT_VAL;
-			break;
-		case DP83867_DOWNSHIFT_4_COUNT:
-			count = DP83867_DOWNSHIFT_4_COUNT_VAL;
-			break;
-		case DP83867_DOWNSHIFT_8_COUNT:
-			count = DP83867_DOWNSHIFT_8_COUNT_VAL;
-			break;
-		default:
-			phydev_err(phydev,
-				   "Downshift count must be 1, 2, 4 or 8\n");
-			return -EINVAL;
+	case DP83867_DOWNSHIFT_1_COUNT:
+		count = DP83867_DOWNSHIFT_1_COUNT_VAL;
+		break;
+	case DP83867_DOWNSHIFT_2_COUNT:
+		count = DP83867_DOWNSHIFT_2_COUNT_VAL;
+		break;
+	case DP83867_DOWNSHIFT_4_COUNT:
+		count = DP83867_DOWNSHIFT_4_COUNT_VAL;
+		break;
+	case DP83867_DOWNSHIFT_8_COUNT:
+		count = DP83867_DOWNSHIFT_8_COUNT_VAL;
+		break;
+	default:
+		phydev_err(phydev,
+			   "Downshift count must be 1, 2, 4 or 8\n");
+		return -EINVAL;
 	}
 
 	val = DP83867_DOWNSHIFT_EN;
@@ -411,7 +409,7 @@  static int dp83867_set_downshift(struct phy_device *phydev, u8 cnt)
 }
 
 static int dp83867_get_tunable(struct phy_device *phydev,
-				struct ethtool_tunable *tuna, void *data)
+			       struct ethtool_tunable *tuna, void *data)
 {
 	switch (tuna->id) {
 	case ETHTOOL_PHY_DOWNSHIFT:
@@ -422,7 +420,7 @@  static int dp83867_get_tunable(struct phy_device *phydev,
 }
 
 static int dp83867_set_tunable(struct phy_device *phydev,
-				struct ethtool_tunable *tuna, const void *data)
+			       struct ethtool_tunable *tuna, const void *data)
 {
 	switch (tuna->id) {
 	case ETHTOOL_PHY_DOWNSHIFT:
@@ -524,11 +522,10 @@  static int dp83867_of_init(struct phy_device *phydev)
 		dp83867->io_impedance = -1; /* leave at default */
 
 	dp83867->rxctrl_strap_quirk = of_property_read_bool(of_node,
-					"ti,dp83867-rxctrl-strap-quirk");
+							    "ti,dp83867-rxctrl-strap-quirk");
 
 	dp83867->sgmii_ref_clk_en = of_property_read_bool(of_node,
-					"ti,sgmii-ref-clock-output-enable");
-
+							  "ti,sgmii-ref-clock-output-enable");
 
 	dp83867->rx_id_delay = DP83867_RGMII_RX_CLK_DELAY_INV;
 	ret = of_property_read_u32(of_node, "ti,rx-internal-delay",