[PATCH net-next v3 0/2] net: phy: mscc: add support for RGMII MAC mode

[PATCH net-next v3 0/2] net: phy: mscc: add support for RGMII MAC mode

[Antoine Tenart]

Hello,

This series adds support for the RGMII MAC mode for the VSC8584 PHY
family and for RGMII_ID modes (Tx and/or Rx).

I decided to drop the custom delay for now. I made some tests and it
seemed to be working quite well. If we find out we really need to lower
the delay, which I doubt, I'll send support for it.

Thanks!
Antoine

Since v2:
  - Dropped support for custom dt bindings.
  - Add the 2ns delay based on the interface mode.

Since v1:
  - The RGMII skew delays are now set based on the PHY interface mode
    being used (RGMII vs ID vs RXID vs TXID).
  - When RGMII is used, the skew delays are set to their default value,
    meaning we do not rely anymore on the bootloader's configuration.
  - Improved the commit messages.
  - s/phy_interface_mode_is_rgmii/phy_interface_is_rgmii/

Antoine Tenart (2):
  net: phy: mscc: add support for RGMII MAC mode
  net: phy: mscc: RGMII skew delay configuration

 drivers/net/phy/mscc/mscc.h      | 15 ++++++++
 drivers/net/phy/mscc/mscc_main.c | 61 +++++++++++++++++++++++++-------
 2 files changed, 64 insertions(+), 12 deletions(-)

-- 
2.25.1

[PATCH net-next v3 1/2] net: phy: mscc: add support for RGMII MAC mode

[Antoine Tenart]

This patch adds support for connecting VSC8584 PHYs to the MAC using
RGMII.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc.h      |  1 +
 drivers/net/phy/mscc/mscc_main.c | 32 ++++++++++++++++++++------------
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index 29ccb2c9c095..d1b8bbe8acca 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -241,6 +241,7 @@ enum rgmii_rx_clock_delay {
 #define MAC_CFG_MASK			  0xc000
 #define MAC_CFG_SGMII			  0x0000
 #define MAC_CFG_QSGMII			  0x4000
+#define MAC_CFG_RGMII			  0x8000
 
 /* Test page Registers */
 #define MSCC_PHY_TEST_PAGE_5		  5
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index bc6beec8aff0..86bb5c3c911a 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1360,27 +1360,35 @@ static int vsc8584_config_init(struct phy_device *phydev)
 
 	val = phy_base_read(phydev, MSCC_PHY_MAC_CFG_FASTLINK);
 	val &= ~MAC_CFG_MASK;
-	if (phydev->interface == PHY_INTERFACE_MODE_QSGMII)
+	if (phydev->interface == PHY_INTERFACE_MODE_QSGMII) {
 		val |= MAC_CFG_QSGMII;
-	else
+	} else if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
 		val |= MAC_CFG_SGMII;
+	} else if (phy_interface_is_rgmii(phydev)) {
+		val |= MAC_CFG_RGMII;
+	} else {
+		ret = -EINVAL;
+		goto err;
+	}
 
 	ret = phy_base_write(phydev, MSCC_PHY_MAC_CFG_FASTLINK, val);
 	if (ret)
 		goto err;
 
-	val = PROC_CMD_MCB_ACCESS_MAC_CONF | PROC_CMD_RST_CONF_PORT |
-		PROC_CMD_READ_MOD_WRITE_PORT;
-	if (phydev->interface == PHY_INTERFACE_MODE_QSGMII)
-		val |= PROC_CMD_QSGMII_MAC;
-	else
-		val |= PROC_CMD_SGMII_MAC;
+	if (!phy_interface_is_rgmii(phydev)) {
+		val = PROC_CMD_MCB_ACCESS_MAC_CONF | PROC_CMD_RST_CONF_PORT |
+			PROC_CMD_READ_MOD_WRITE_PORT;
+		if (phydev->interface == PHY_INTERFACE_MODE_QSGMII)
+			val |= PROC_CMD_QSGMII_MAC;
+		else
+			val |= PROC_CMD_SGMII_MAC;
 
-	ret = vsc8584_cmd(phydev, val);
-	if (ret)
-		goto err;
+		ret = vsc8584_cmd(phydev, val);
+		if (ret)
+			goto err;
 
-	usleep_range(10000, 20000);
+		usleep_range(10000, 20000);
+	}
 
 	/* Disable SerDes for 100Base-FX */
 	ret = vsc8584_cmd(phydev, PROC_CMD_FIBER_MEDIA_CONF |
-- 
2.25.1

Re: [PATCH net-next v3 1/2] net: phy: mscc: add support for RGMII MAC mode

[Andrew Lunn]

On Thu, Mar 19, 2020 at 03:19:57PM +0100, Antoine Tenart wrote:
> This patch adds support for connecting VSC8584 PHYs to the MAC using
> RGMII.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>

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

    Andrew

[PATCH net-next v3 2/2] net: phy: mscc: RGMII skew delay configuration

[Antoine Tenart]

This patch adds support for configuring the RGMII skew delays in Rx and
Tx. The Rx and Tx skews are set based on the interface mode. By default
their configuration is set to the default value in hardware (0.2ns);
this means the driver do not rely anymore on the bootloader
configuration.

Then based on the interface mode being used, a 2ns delay is added:
- RGMII_ID adds it for both Rx and Tx.
- RGMII_RXID adds it for Rx.
- RGMII_TXID adds it for Tx.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc.h      | 14 ++++++++++++++
 drivers/net/phy/mscc/mscc_main.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index d1b8bbe8acca..25729302714c 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -161,6 +161,20 @@ enum rgmii_rx_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
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 86bb5c3c911a..5d78732de702 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1288,6 +1288,32 @@ 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;
@@ -1422,6 +1448,9 @@ static int vsc8584_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
+	if (phy_interface_is_rgmii(phydev))
+		vsc8584_rgmii_set_skews(phydev);
+
 	ret = genphy_soft_reset(phydev);
 	if (ret)
 		return ret;
-- 
2.25.1

Re: [PATCH net-next v3 2/2] net: phy: mscc: RGMII skew delay configuration

[Andrew Lunn]

On Thu, Mar 19, 2020 at 03:19:58PM +0100, Antoine Tenart wrote:
> This patch adds support for configuring the RGMII skew delays in Rx and
> Tx. The Rx and Tx skews are set based on the interface mode. By default
> their configuration is set to the default value in hardware (0.2ns);
> this means the driver do not rely anymore on the bootloader
> configuration.
> 
> Then based on the interface mode being used, a 2ns delay is added:
> - RGMII_ID adds it for both Rx and Tx.
> - RGMII_RXID adds it for Rx.
> - RGMII_TXID adds it for Tx.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
> ---
>  drivers/net/phy/mscc/mscc.h      | 14 ++++++++++++++
>  drivers/net/phy/mscc/mscc_main.c | 29 +++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
> index d1b8bbe8acca..25729302714c 100644
> --- a/drivers/net/phy/mscc/mscc.h
> +++ b/drivers/net/phy/mscc/mscc.h
> @@ -161,6 +161,20 @@ enum rgmii_rx_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

  
> +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;

Hi Antoine

Does this mean it is impossible to have a skew of 0ns?

     Andrew

Re: [PATCH net-next v3 2/2] net: phy: mscc: RGMII skew delay configuration

[Antoine Tenart]

Hi Andrew,

Quoting Andrew Lunn (2020-03-19 17:05:05)
> On Thu, Mar 19, 2020 at 03:19:58PM +0100, Antoine Tenart wrote:
> > This patch adds support for configuring the RGMII skew delays in Rx and
> > Tx. The Rx and Tx skews are set based on the interface mode. By default
> > their configuration is set to the default value in hardware (0.2ns);
> > this means the driver do not rely anymore on the bootloader
> > configuration.
> > 
> > Then based on the interface mode being used, a 2ns delay is added:
> > - RGMII_ID adds it for both Rx and Tx.
> > - RGMII_RXID adds it for Rx.
> > - RGMII_TXID adds it for Tx.
> > 
> > Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
> > ---
> >  drivers/net/phy/mscc/mscc.h      | 14 ++++++++++++++
> >  drivers/net/phy/mscc/mscc_main.c | 29 +++++++++++++++++++++++++++++
> >  2 files changed, 43 insertions(+)
> > 
> > diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
> > index d1b8bbe8acca..25729302714c 100644
> > --- a/drivers/net/phy/mscc/mscc.h
> > +++ b/drivers/net/phy/mscc/mscc.h
> > @@ -161,6 +161,20 @@ enum rgmii_rx_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
> 
>   
> > +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;
> 
> Does this mean it is impossible to have a skew of 0ns?

It seems to be the case, the lowest value this register accepts is 0,
which means a 0.2ns delay, according to the datasheet. And I'm not
seeing any register disabling this when RGMII is used.

Thanks!
Antoine

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

Re: [PATCH net-next v3 2/2] net: phy: mscc: RGMII skew delay configuration

[Andrew Lunn]

On Thu, Mar 19, 2020 at 03:19:58PM +0100, Antoine Tenart wrote:
> This patch adds support for configuring the RGMII skew delays in Rx and
> Tx. The Rx and Tx skews are set based on the interface mode. By default
> their configuration is set to the default value in hardware (0.2ns);
> this means the driver do not rely anymore on the bootloader
> configuration.
> 
> Then based on the interface mode being used, a 2ns delay is added:
> - RGMII_ID adds it for both Rx and Tx.
> - RGMII_RXID adds it for Rx.
> - RGMII_TXID adds it for Tx.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>

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

    Andrew

Re: [PATCH net-next v3 0/2] net: phy: mscc: add support for RGMII MAC mode

[David Miller]

From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Thu, 19 Mar 2020 15:19:56 +0100

> This series adds support for the RGMII MAC mode for the VSC8584 PHY
> family and for RGMII_ID modes (Tx and/or Rx).
> 
> I decided to drop the custom delay for now. I made some tests and it
> seemed to be working quite well. If we find out we really need to lower
> the delay, which I doubt, I'll send support for it.
 ...
> Since v2:
>   - Dropped support for custom dt bindings.
>   - Add the 2ns delay based on the interface mode.
> 
> Since v1:
>   - The RGMII skew delays are now set based on the PHY interface mode
>     being used (RGMII vs ID vs RXID vs TXID).
>   - When RGMII is used, the skew delays are set to their default value,
>     meaning we do not rely anymore on the bootloader's configuration.
>   - Improved the commit messages.
>   - s/phy_interface_mode_is_rgmii/phy_interface_is_rgmii/

Series applied, thanks.