netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] net: phy: mscc: consolidate a common RGMII delay implementation
@ 2020-03-24 14:13 Vladimir Oltean
  2020-03-24 14:18 ` Andrew Lunn
  0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2020-03-24 14:13 UTC (permalink / raw)
  To: davem; +Cc: netdev, andrew, f.fainelli, hkallweit1, antoine.tenart

From: Vladimir Oltean <vladimir.oltean@nxp.com>

It looks like the VSC8584 PHY driver is rolling its own RGMII delay
configuration code, despite the fact that the logic is mostly the same.

In fact only the register layout and position for the RGMII controls has
changed. So we need to adapt and parameterize the PHY-dependent bit
fields when calling the new generic function.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
Added back a comment containing the information from Antoine's patch.

 drivers/net/phy/mscc/mscc.h      | 28 ++++--------
 drivers/net/phy/mscc/mscc_main.c | 78 ++++++++++++++++----------------
 2 files changed, 49 insertions(+), 57 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index d983d3af66d6..030bf8b600df 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -161,25 +161,15 @@ enum rgmii_clock_delay {
 /* Extended Page 2 Registers */
 #define MSCC_PHY_CU_PMD_TX_CNTL		  16
 
-#define MSCC_PHY_RGMII_SETTINGS		  18
-#define RGMII_SKEW_RX_POS		  1
-#define RGMII_SKEW_TX_POS		  4
-
-/* RGMII skew values, in ns */
-#define VSC8584_RGMII_SKEW_0_2		  0
-#define VSC8584_RGMII_SKEW_0_8		  1
-#define VSC8584_RGMII_SKEW_1_1		  2
-#define VSC8584_RGMII_SKEW_1_7		  3
-#define VSC8584_RGMII_SKEW_2_0		  4
-#define VSC8584_RGMII_SKEW_2_3		  5
-#define VSC8584_RGMII_SKEW_2_6		  6
-#define VSC8584_RGMII_SKEW_3_4		  7
-
-#define MSCC_PHY_RGMII_CNTL		  20
-#define RGMII_RX_CLK_DELAY_MASK		  0x0070
-#define RGMII_RX_CLK_DELAY_POS		  4
-#define RGMII_TX_CLK_DELAY_MASK		  0x0007
-#define RGMII_TX_CLK_DELAY_POS		  0
+/* RGMII setting controls at address 18E2, for VSC8572 and similar */
+#define VSC8572_RGMII_CNTL		  18
+#define VSC8572_RGMII_RX_DELAY_MASK	  0x000E
+#define VSC8572_RGMII_TX_DELAY_MASK	  0x0070
+
+/* RGMII controls at address 20E2, for VSC8502 and similar */
+#define VSC8502_RGMII_CNTL		  20
+#define VSC8502_RGMII_RX_DELAY_MASK	  0x0070
+#define VSC8502_RGMII_TX_DELAY_MASK	  0x0007
 
 #define MSCC_PHY_WOL_LOWER_MAC_ADDR	  21
 #define MSCC_PHY_WOL_MID_MAC_ADDR	  22
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 19603081dd14..acddef79f4e8 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -520,28 +520,34 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
 	return rc;
 }
 
-static int vsc85xx_default_config(struct phy_device *phydev)
+/* Set the RGMII RX and TX clock skews individually, according to the PHY
+ * interface type, to:
+ *  * 0.2 ns (their default, and lowest, hardware value) if delays should
+ *    not be enabled
+ *  * 2.0 ns (which causes the data to be sampled at exactly half way between
+ *    clock transitions at 1000 Mbps) if delays should be enabled
+ */
+static int vsc85xx_rgmii_set_skews(struct phy_device *phydev, u32 rgmii_cntl,
+				   u16 rgmii_rx_delay_mask,
+				   u16 rgmii_tx_delay_mask)
 {
+	u16 rgmii_rx_delay_pos = ffs(rgmii_rx_delay_mask) - 1;
+	u16 rgmii_tx_delay_pos = ffs(rgmii_tx_delay_mask) - 1;
 	u16 reg_val = 0;
 	int rc;
 
-	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
-
-	if (!phy_interface_mode_is_rgmii(phydev->interface))
-		return 0;
-
 	mutex_lock(&phydev->lock);
 
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
-		reg_val |= RGMII_CLK_DELAY_2_0_NS << RGMII_RX_CLK_DELAY_POS;
+		reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_rx_delay_pos;
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
-		reg_val |= RGMII_CLK_DELAY_2_0_NS << RGMII_TX_CLK_DELAY_POS;
+		reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_tx_delay_pos;
 
 	rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
-			      MSCC_PHY_RGMII_CNTL,
-			      RGMII_RX_CLK_DELAY_MASK | RGMII_TX_CLK_DELAY_MASK,
+			      rgmii_cntl,
+			      rgmii_rx_delay_mask | rgmii_tx_delay_mask,
 			      reg_val);
 
 	mutex_unlock(&phydev->lock);
@@ -549,6 +555,23 @@ static int vsc85xx_default_config(struct phy_device *phydev)
 	return rc;
 }
 
+static int vsc85xx_default_config(struct phy_device *phydev)
+{
+	int rc;
+
+	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+
+	if (phy_interface_mode_is_rgmii(phydev->interface)) {
+		rc = vsc85xx_rgmii_set_skews(phydev, VSC8502_RGMII_CNTL,
+					     VSC8502_RGMII_RX_DELAY_MASK,
+					     VSC8502_RGMII_TX_DELAY_MASK);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
 static int vsc85xx_get_tunable(struct phy_device *phydev,
 			       struct ethtool_tunable *tuna, void *data)
 {
@@ -1301,32 +1324,6 @@ static bool vsc8584_is_pkg_init(struct phy_device *phydev, bool reversed)
 	return false;
 }
 
-static void vsc8584_rgmii_set_skews(struct phy_device *phydev)
-{
-	u32 skew_rx, skew_tx;
-
-	/* We first set the Rx and Tx skews to their default value in h/w
-	 * (0.2 ns).
-	 */
-	skew_rx = VSC8584_RGMII_SKEW_0_2;
-	skew_tx = VSC8584_RGMII_SKEW_0_2;
-
-	/* We then set the skews based on the interface mode. */
-	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
-	    phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
-		skew_rx = VSC8584_RGMII_SKEW_2_0;
-	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
-	    phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
-		skew_tx = VSC8584_RGMII_SKEW_2_0;
-
-	/* Finally we apply the skews configuration. */
-	phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
-			 MSCC_PHY_RGMII_SETTINGS,
-			 (0x7 << RGMII_SKEW_RX_POS) | (0x7 << RGMII_SKEW_TX_POS),
-			 (skew_rx << RGMII_SKEW_RX_POS) |
-			 (skew_tx << RGMII_SKEW_TX_POS));
-}
-
 static int vsc8584_config_init(struct phy_device *phydev)
 {
 	struct vsc8531_private *vsc8531 = phydev->priv;
@@ -1461,8 +1458,13 @@ static int vsc8584_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	if (phy_interface_is_rgmii(phydev))
-		vsc8584_rgmii_set_skews(phydev);
+	if (phy_interface_is_rgmii(phydev)) {
+		ret = vsc85xx_rgmii_set_skews(phydev, VSC8572_RGMII_CNTL,
+					      VSC8572_RGMII_RX_DELAY_MASK,
+					      VSC8572_RGMII_TX_DELAY_MASK);
+		if (ret)
+			return ret;
+	}
 
 	ret = genphy_soft_reset(phydev);
 	if (ret)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next] net: phy: mscc: consolidate a common RGMII delay implementation
  2020-03-24 14:13 [PATCH net-next] net: phy: mscc: consolidate a common RGMII delay implementation Vladimir Oltean
@ 2020-03-24 14:18 ` Andrew Lunn
  2020-03-24 14:49   ` Antoine Tenart
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2020-03-24 14:18 UTC (permalink / raw)
  To: Vladimir Oltean; +Cc: davem, netdev, f.fainelli, hkallweit1, antoine.tenart

On Tue, Mar 24, 2020 at 04:13:58PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> It looks like the VSC8584 PHY driver is rolling its own RGMII delay
> configuration code, despite the fact that the logic is mostly the same.
> 
> In fact only the register layout and position for the RGMII controls has
> changed. So we need to adapt and parameterize the PHY-dependent bit
> fields when calling the new generic function.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next] net: phy: mscc: consolidate a common RGMII delay implementation
  2020-03-24 14:18 ` Andrew Lunn
@ 2020-03-24 14:49   ` Antoine Tenart
  2020-03-24 14:51     ` Vladimir Oltean
  2020-03-24 23:36     ` David Miller
  0 siblings, 2 replies; 8+ messages in thread
From: Antoine Tenart @ 2020-03-24 14:49 UTC (permalink / raw)
  To: Andrew Lunn, Vladimir Oltean; +Cc: davem, netdev, f.fainelli, hkallweit1

Hi Vladimir,

Quoting Andrew Lunn (2020-03-24 15:18:29)
> On Tue, Mar 24, 2020 at 04:13:58PM +0200, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > 
> > It looks like the VSC8584 PHY driver is rolling its own RGMII delay
> > configuration code, despite the fact that the logic is mostly the same.
> > 
> > In fact only the register layout and position for the RGMII controls has
> > changed. So we need to adapt and parameterize the PHY-dependent bit
> > fields when calling the new generic function.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Tested-by: Antoine Tenart <antoine.tenart@bootlin.com>

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next] net: phy: mscc: consolidate a common RGMII delay implementation
  2020-03-24 14:49   ` Antoine Tenart
@ 2020-03-24 14:51     ` Vladimir Oltean
  2020-03-24 23:36     ` David Miller
  1 sibling, 0 replies; 8+ messages in thread
From: Vladimir Oltean @ 2020-03-24 14:51 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: Andrew Lunn, David S. Miller, netdev, Florian Fainelli, Heiner Kallweit

On Tue, 24 Mar 2020 at 16:49, Antoine Tenart <antoine.tenart@bootlin.com> wrote:
>
> Hi Vladimir,
>
> Quoting Andrew Lunn (2020-03-24 15:18:29)
> > On Tue, Mar 24, 2020 at 04:13:58PM +0200, Vladimir Oltean wrote:
> > > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > >
> > > It looks like the VSC8584 PHY driver is rolling its own RGMII delay
> > > configuration code, despite the fact that the logic is mostly the same.
> > >
> > > In fact only the register layout and position for the RGMII controls has
> > > changed. So we need to adapt and parameterize the PHY-dependent bit
> > > fields when calling the new generic function.
> > >
> > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
> Tested-by: Antoine Tenart <antoine.tenart@bootlin.com>
>
> Thanks!
> Antoine
>
> --
> Antoine Ténart, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Thanks for testing, Antoine!

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next] net: phy: mscc: consolidate a common RGMII delay implementation
  2020-03-24 14:49   ` Antoine Tenart
  2020-03-24 14:51     ` Vladimir Oltean
@ 2020-03-24 23:36     ` David Miller
  1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2020-03-24 23:36 UTC (permalink / raw)
  To: antoine.tenart; +Cc: andrew, olteanv, netdev, f.fainelli, hkallweit1

From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Tue, 24 Mar 2020 15:49:37 +0100

> Hi Vladimir,
> 
> Quoting Andrew Lunn (2020-03-24 15:18:29)
>> On Tue, Mar 24, 2020 at 04:13:58PM +0200, Vladimir Oltean wrote:
>> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
>> > 
>> > It looks like the VSC8584 PHY driver is rolling its own RGMII delay
>> > configuration code, despite the fact that the logic is mostly the same.
>> > 
>> > In fact only the register layout and position for the RGMII controls has
>> > changed. So we need to adapt and parameterize the PHY-dependent bit
>> > fields when calling the new generic function.
>> > 
>> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>> 
>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
> Tested-by: Antoine Tenart <antoine.tenart@bootlin.com>

Applied, thanks everyone.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next] net: phy: mscc: consolidate a common RGMII delay implementation
  2020-03-24 13:40 ` Andrew Lunn
@ 2020-03-24 14:13   ` Vladimir Oltean
  0 siblings, 0 replies; 8+ messages in thread
From: Vladimir Oltean @ 2020-03-24 14:13 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S. Miller, netdev, Florian Fainelli, Heiner Kallweit,
	Antoine Tenart

On Tue, 24 Mar 2020 at 15:40, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Tue, Mar 24, 2020 at 02:48:37PM +0200, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > It looks like the VSC8584 PHY driver is rolling its own RGMII delay
> > configuration code, despite the fact that the logic is mostly the same.
> >
> > In fact only the register layout and position for the RGMII controls has
> > changed. So we need to adapt and parameterize the PHY-dependent bit
> > fields when calling the new generic function.
>
> Nice.
>
> > -static void vsc8584_rgmii_set_skews(struct phy_device *phydev)
> > -{
> > -     u32 skew_rx, skew_tx;
> > -
> > -     /* We first set the Rx and Tx skews to their default value in h/w
> > -      * (0.2 ns).
> > -      */
>
> I like seeing this comment. It makes it clear that
> PHY_INTERFACE_MODE_RGMII does not actually mean 0ns, but 0.2ns.  It
> also makes it clear that if PHY_INTERFACE_MODE_RGMII_ID,
> PHY_INTERFACE_MODE_RGMII_RXID or PHY_INTERFACE_MODE_RGMII_TXID is not
> given, the delay is set to something. We have had PHY drivers which
> get this wrong and leave the bootloader/strapping value in place.
>
> So if you can keep the comment in some form, that would be good.
>
> Thanks
>         Andrew

I find the comments fairly redundant since the code that does that is
already explicit, but anyway I'll send a v2 for the 0.2 ns thing.

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next] net: phy: mscc: consolidate a common RGMII delay implementation
  2020-03-24 12:48 Vladimir Oltean
@ 2020-03-24 13:40 ` Andrew Lunn
  2020-03-24 14:13   ` Vladimir Oltean
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2020-03-24 13:40 UTC (permalink / raw)
  To: Vladimir Oltean; +Cc: davem, netdev, f.fainelli, hkallweit1, antoine.tenart

On Tue, Mar 24, 2020 at 02:48:37PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> It looks like the VSC8584 PHY driver is rolling its own RGMII delay
> configuration code, despite the fact that the logic is mostly the same.
> 
> In fact only the register layout and position for the RGMII controls has
> changed. So we need to adapt and parameterize the PHY-dependent bit
> fields when calling the new generic function.

Nice.

> -static void vsc8584_rgmii_set_skews(struct phy_device *phydev)
> -{
> -	u32 skew_rx, skew_tx;
> -
> -	/* We first set the Rx and Tx skews to their default value in h/w
> -	 * (0.2 ns).
> -	 */

I like seeing this comment. It makes it clear that
PHY_INTERFACE_MODE_RGMII does not actually mean 0ns, but 0.2ns.  It
also makes it clear that if PHY_INTERFACE_MODE_RGMII_ID,
PHY_INTERFACE_MODE_RGMII_RXID or PHY_INTERFACE_MODE_RGMII_TXID is not
given, the delay is set to something. We have had PHY drivers which
get this wrong and leave the bootloader/strapping value in place.

So if you can keep the comment in some form, that would be good.

Thanks
	Andrew

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next] net: phy: mscc: consolidate a common RGMII delay implementation
@ 2020-03-24 12:48 Vladimir Oltean
  2020-03-24 13:40 ` Andrew Lunn
  0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2020-03-24 12:48 UTC (permalink / raw)
  To: davem; +Cc: netdev, andrew, f.fainelli, hkallweit1, antoine.tenart

From: Vladimir Oltean <vladimir.oltean@nxp.com>

It looks like the VSC8584 PHY driver is rolling its own RGMII delay
configuration code, despite the fact that the logic is mostly the same.

In fact only the register layout and position for the RGMII controls has
changed. So we need to adapt and parameterize the PHY-dependent bit
fields when calling the new generic function.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/mscc/mscc.h      | 28 ++++---------
 drivers/net/phy/mscc/mscc_main.c | 71 +++++++++++++++-----------------
 2 files changed, 42 insertions(+), 57 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index d983d3af66d6..030bf8b600df 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -161,25 +161,15 @@ enum rgmii_clock_delay {
 /* Extended Page 2 Registers */
 #define MSCC_PHY_CU_PMD_TX_CNTL		  16
 
-#define MSCC_PHY_RGMII_SETTINGS		  18
-#define RGMII_SKEW_RX_POS		  1
-#define RGMII_SKEW_TX_POS		  4
-
-/* RGMII skew values, in ns */
-#define VSC8584_RGMII_SKEW_0_2		  0
-#define VSC8584_RGMII_SKEW_0_8		  1
-#define VSC8584_RGMII_SKEW_1_1		  2
-#define VSC8584_RGMII_SKEW_1_7		  3
-#define VSC8584_RGMII_SKEW_2_0		  4
-#define VSC8584_RGMII_SKEW_2_3		  5
-#define VSC8584_RGMII_SKEW_2_6		  6
-#define VSC8584_RGMII_SKEW_3_4		  7
-
-#define MSCC_PHY_RGMII_CNTL		  20
-#define RGMII_RX_CLK_DELAY_MASK		  0x0070
-#define RGMII_RX_CLK_DELAY_POS		  4
-#define RGMII_TX_CLK_DELAY_MASK		  0x0007
-#define RGMII_TX_CLK_DELAY_POS		  0
+/* RGMII setting controls at address 18E2, for VSC8572 and similar */
+#define VSC8572_RGMII_CNTL		  18
+#define VSC8572_RGMII_RX_DELAY_MASK	  0x000E
+#define VSC8572_RGMII_TX_DELAY_MASK	  0x0070
+
+/* RGMII controls at address 20E2, for VSC8502 and similar */
+#define VSC8502_RGMII_CNTL		  20
+#define VSC8502_RGMII_RX_DELAY_MASK	  0x0070
+#define VSC8502_RGMII_TX_DELAY_MASK	  0x0007
 
 #define MSCC_PHY_WOL_LOWER_MAC_ADDR	  21
 #define MSCC_PHY_WOL_MID_MAC_ADDR	  22
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 19603081dd14..d14d4c7263f4 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -520,28 +520,27 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
 	return rc;
 }
 
-static int vsc85xx_default_config(struct phy_device *phydev)
+static int vsc85xx_rgmii_set_skews(struct phy_device *phydev, u32 rgmii_cntl,
+				   u16 rgmii_rx_delay_mask,
+				   u16 rgmii_tx_delay_mask)
 {
+	u16 rgmii_rx_delay_pos = ffs(rgmii_rx_delay_mask) - 1;
+	u16 rgmii_tx_delay_pos = ffs(rgmii_tx_delay_mask) - 1;
 	u16 reg_val = 0;
 	int rc;
 
-	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
-
-	if (!phy_interface_mode_is_rgmii(phydev->interface))
-		return 0;
-
 	mutex_lock(&phydev->lock);
 
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
-		reg_val |= RGMII_CLK_DELAY_2_0_NS << RGMII_RX_CLK_DELAY_POS;
+		reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_rx_delay_pos;
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
-		reg_val |= RGMII_CLK_DELAY_2_0_NS << RGMII_TX_CLK_DELAY_POS;
+		reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_tx_delay_pos;
 
 	rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
-			      MSCC_PHY_RGMII_CNTL,
-			      RGMII_RX_CLK_DELAY_MASK | RGMII_TX_CLK_DELAY_MASK,
+			      rgmii_cntl,
+			      rgmii_rx_delay_mask | rgmii_tx_delay_mask,
 			      reg_val);
 
 	mutex_unlock(&phydev->lock);
@@ -549,6 +548,23 @@ static int vsc85xx_default_config(struct phy_device *phydev)
 	return rc;
 }
 
+static int vsc85xx_default_config(struct phy_device *phydev)
+{
+	int rc;
+
+	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+
+	if (phy_interface_mode_is_rgmii(phydev->interface)) {
+		rc = vsc85xx_rgmii_set_skews(phydev, VSC8502_RGMII_CNTL,
+					     VSC8502_RGMII_RX_DELAY_MASK,
+					     VSC8502_RGMII_TX_DELAY_MASK);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
 static int vsc85xx_get_tunable(struct phy_device *phydev,
 			       struct ethtool_tunable *tuna, void *data)
 {
@@ -1301,32 +1317,6 @@ static bool vsc8584_is_pkg_init(struct phy_device *phydev, bool reversed)
 	return false;
 }
 
-static void vsc8584_rgmii_set_skews(struct phy_device *phydev)
-{
-	u32 skew_rx, skew_tx;
-
-	/* We first set the Rx and Tx skews to their default value in h/w
-	 * (0.2 ns).
-	 */
-	skew_rx = VSC8584_RGMII_SKEW_0_2;
-	skew_tx = VSC8584_RGMII_SKEW_0_2;
-
-	/* We then set the skews based on the interface mode. */
-	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
-	    phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
-		skew_rx = VSC8584_RGMII_SKEW_2_0;
-	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
-	    phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
-		skew_tx = VSC8584_RGMII_SKEW_2_0;
-
-	/* Finally we apply the skews configuration. */
-	phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
-			 MSCC_PHY_RGMII_SETTINGS,
-			 (0x7 << RGMII_SKEW_RX_POS) | (0x7 << RGMII_SKEW_TX_POS),
-			 (skew_rx << RGMII_SKEW_RX_POS) |
-			 (skew_tx << RGMII_SKEW_TX_POS));
-}
-
 static int vsc8584_config_init(struct phy_device *phydev)
 {
 	struct vsc8531_private *vsc8531 = phydev->priv;
@@ -1461,8 +1451,13 @@ static int vsc8584_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	if (phy_interface_is_rgmii(phydev))
-		vsc8584_rgmii_set_skews(phydev);
+	if (phy_interface_is_rgmii(phydev)) {
+		ret = vsc85xx_rgmii_set_skews(phydev, VSC8572_RGMII_CNTL,
+					      VSC8572_RGMII_RX_DELAY_MASK,
+					      VSC8572_RGMII_TX_DELAY_MASK);
+		if (ret)
+			return ret;
+	}
 
 	ret = genphy_soft_reset(phydev);
 	if (ret)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-03-24 23:36 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-24 14:13 [PATCH net-next] net: phy: mscc: consolidate a common RGMII delay implementation Vladimir Oltean
2020-03-24 14:18 ` Andrew Lunn
2020-03-24 14:49   ` Antoine Tenart
2020-03-24 14:51     ` Vladimir Oltean
2020-03-24 23:36     ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2020-03-24 12:48 Vladimir Oltean
2020-03-24 13:40 ` Andrew Lunn
2020-03-24 14:13   ` Vladimir Oltean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).