diff mbox series

[v2,6/6] drivers: net: fsl_enetc: force the RGMII MAC speed/duplex instead of using in-band signaling

Message ID 20210617222604.2563375-7-vladimir.oltean@nxp.com
State Superseded
Delegated to: Ramon Fried
Headers show
Series Fixes for the NXP LS1028A-QDS boards | expand

Commit Message

Vladimir Oltean June 17, 2021, 10:26 p.m. UTC
The RGMII spec supports optional in-band status reporting for the speed
and duplex negotiated on the copper side, and the ENETC driver enables
this feature by default.

However, this does not work when the PHY does not implement the in-band
reporting, or when there is a MAC-to-MAC connection described using a
fixed-link. In that case, it would be better to disable the feature in
the ENETC MAC and always force the speed and duplex to the values that
were negotiated and retrieved over MDIO once the autoneg is finished.
Since this works always, we just do it unconditionally and drop the
in-band code.

Note that because we need to wait for the autoneg to complete, we need
to move enetc_setup_mac_iface() after phy_startup() returns, and then
pass the phydev pointer all the way to enetc_init_rgmii().

The same considerations have led to a similar Linux driver patch as well:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=c76a97218dcbb2cb7cec1404ace43ef96c87d874

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: reword commit message

 drivers/net/fsl_enetc.c | 44 ++++++++++++++++++++++++++++++-----------
 drivers/net/fsl_enetc.h |  5 +++++
 2 files changed, 38 insertions(+), 11 deletions(-)

Comments

Ramon Fried June 18, 2021, 8:19 a.m. UTC | #1
On Fri, Jun 18, 2021 at 1:26 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> The RGMII spec supports optional in-band status reporting for the speed
> and duplex negotiated on the copper side, and the ENETC driver enables
> this feature by default.
>
> However, this does not work when the PHY does not implement the in-band
> reporting, or when there is a MAC-to-MAC connection described using a
> fixed-link. In that case, it would be better to disable the feature in
> the ENETC MAC and always force the speed and duplex to the values that
> were negotiated and retrieved over MDIO once the autoneg is finished.
> Since this works always, we just do it unconditionally and drop the
> in-band code.
>
> Note that because we need to wait for the autoneg to complete, we need
> to move enetc_setup_mac_iface() after phy_startup() returns, and then
> pass the phydev pointer all the way to enetc_init_rgmii().
>
> The same considerations have led to a similar Linux driver patch as well:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=c76a97218dcbb2cb7cec1404ace43ef96c87d874
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> v1->v2: reword commit message
>
>  drivers/net/fsl_enetc.c | 44 ++++++++++++++++++++++++++++++-----------
>  drivers/net/fsl_enetc.h |  5 +++++
>  2 files changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
> index 5961775024ff..de1e42f907e7 100644
> --- a/drivers/net/fsl_enetc.c
> +++ b/drivers/net/fsl_enetc.c
> @@ -178,21 +178,43 @@ static int enetc_init_sgmii(struct udevice *dev)
>  }
>
>  /* set up MAC for RGMII */
> -static int enetc_init_rgmii(struct udevice *dev)
> +static void enetc_init_rgmii(struct udevice *dev, struct phy_device *phydev)
>  {
>         struct enetc_priv *priv = dev_get_priv(dev);
> -       u32 if_mode;
> +       u32 old_val, val;
>
> -       /* enable RGMII AN */
> -       if_mode = enetc_read_port(priv, ENETC_PM_IF_MODE);
> -       if_mode |= ENETC_PM_IF_MODE_AN_ENA;
> -       enetc_write_port(priv, ENETC_PM_IF_MODE, if_mode);
> +       old_val = val = enetc_read_port(priv, ENETC_PM_IF_MODE);
>
> -       return 0;
> +       /* disable unreliable RGMII in-band signaling and force the MAC into
> +        * the speed negotiated by the PHY.
> +        */
> +       val &= ~ENETC_PM_IF_MODE_AN_ENA;
> +
> +       if (phydev->speed == SPEED_1000) {
> +               val &= ~ENETC_PM_IFM_SSP_MASK;
> +               val |= ENETC_PM_IFM_SSP_1000;
> +       } else if (phydev->speed == SPEED_100) {
> +               val &= ~ENETC_PM_IFM_SSP_MASK;
> +               val |= ENETC_PM_IFM_SSP_100;
> +       } else if (phydev->speed == SPEED_10) {
> +               val &= ~ENETC_PM_IFM_SSP_MASK;
> +               val |= ENETC_PM_IFM_SSP_10;
> +       }
> +
> +       if (phydev->duplex == DUPLEX_FULL)
> +               val |= ENETC_PM_IFM_FULL_DPX;
> +       else
> +               val &= ~ENETC_PM_IFM_FULL_DPX;
> +
> +       if (val == old_val)
> +               return;
> +
> +       enetc_write_port(priv, ENETC_PM_IF_MODE, val);
>  }
>
>  /* set up MAC configuration for the given interface type */
> -static void enetc_setup_mac_iface(struct udevice *dev)
> +static void enetc_setup_mac_iface(struct udevice *dev,
> +                                 struct phy_device *phydev)
>  {
>         struct enetc_priv *priv = dev_get_priv(dev);
>         u32 if_mode;
> @@ -202,7 +224,7 @@ static void enetc_setup_mac_iface(struct udevice *dev)
>         case PHY_INTERFACE_MODE_RGMII_ID:
>         case PHY_INTERFACE_MODE_RGMII_RXID:
>         case PHY_INTERFACE_MODE_RGMII_TXID:
> -               enetc_init_rgmii(dev);
> +               enetc_init_rgmii(dev, phydev);
>                 break;
>         case PHY_INTERFACE_MODE_XGMII:
>         case PHY_INTERFACE_MODE_USXGMII:
> @@ -546,10 +568,10 @@ static int enetc_start(struct udevice *dev)
>         enetc_setup_tx_bdr(dev);
>         enetc_setup_rx_bdr(dev);
>
> -       enetc_setup_mac_iface(dev);
> -
>         phy_startup(priv->phy);
>
> +       enetc_setup_mac_iface(dev, priv->phy);
> +
>         return 0;
>  }
>
> diff --git a/drivers/net/fsl_enetc.h b/drivers/net/fsl_enetc.h
> index 110c1d78fbc6..a4409505e043 100644
> --- a/drivers/net/fsl_enetc.h
> +++ b/drivers/net/fsl_enetc.h
> @@ -77,6 +77,11 @@ enum enetc_bdr_type {TX, RX};
>  #define ENETC_PM_IF_MODE               0x8300
>  #define  ENETC_PM_IF_MODE_RG           BIT(2)
>  #define  ENETC_PM_IF_MODE_AN_ENA       BIT(15)
> +#define  ENETC_PM_IFM_SSP_MASK         GENMASK(14, 13)
> +#define  ENETC_PM_IFM_SSP_1000         (2 << 13)
> +#define  ENETC_PM_IFM_SSP_100          (0 << 13)
> +#define  ENETC_PM_IFM_SSP_10           (1 << 13)
> +#define  ENETC_PM_IFM_FULL_DPX         BIT(12)
>  #define  ENETC_PM_IF_IFMODE_MASK       GENMASK(1, 0)
>
>  /* buffer descriptors count must be multiple of 8 and aligned to 128 bytes */
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
Vladimir Oltean June 22, 2021, 9:20 a.m. UTC | #2
On Fri, Jun 18, 2021 at 11:19:11AM +0300, Ramon Fried wrote:
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Ramon, could you please take these patches through your tree? Thanks.
Ramon Fried June 29, 2021, 10:58 p.m. UTC | #3
On Tue, Jun 22, 2021 at 12:20 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Fri, Jun 18, 2021 at 11:19:11AM +0300, Ramon Fried wrote:
> > Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
>
> Ramon, could you please take these patches through your tree? Thanks.
Done.
diff mbox series

Patch

diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
index 5961775024ff..de1e42f907e7 100644
--- a/drivers/net/fsl_enetc.c
+++ b/drivers/net/fsl_enetc.c
@@ -178,21 +178,43 @@  static int enetc_init_sgmii(struct udevice *dev)
 }
 
 /* set up MAC for RGMII */
-static int enetc_init_rgmii(struct udevice *dev)
+static void enetc_init_rgmii(struct udevice *dev, struct phy_device *phydev)
 {
 	struct enetc_priv *priv = dev_get_priv(dev);
-	u32 if_mode;
+	u32 old_val, val;
 
-	/* enable RGMII AN */
-	if_mode = enetc_read_port(priv, ENETC_PM_IF_MODE);
-	if_mode |= ENETC_PM_IF_MODE_AN_ENA;
-	enetc_write_port(priv, ENETC_PM_IF_MODE, if_mode);
+	old_val = val = enetc_read_port(priv, ENETC_PM_IF_MODE);
 
-	return 0;
+	/* disable unreliable RGMII in-band signaling and force the MAC into
+	 * the speed negotiated by the PHY.
+	 */
+	val &= ~ENETC_PM_IF_MODE_AN_ENA;
+
+	if (phydev->speed == SPEED_1000) {
+		val &= ~ENETC_PM_IFM_SSP_MASK;
+		val |= ENETC_PM_IFM_SSP_1000;
+	} else if (phydev->speed == SPEED_100) {
+		val &= ~ENETC_PM_IFM_SSP_MASK;
+		val |= ENETC_PM_IFM_SSP_100;
+	} else if (phydev->speed == SPEED_10) {
+		val &= ~ENETC_PM_IFM_SSP_MASK;
+		val |= ENETC_PM_IFM_SSP_10;
+	}
+
+	if (phydev->duplex == DUPLEX_FULL)
+		val |= ENETC_PM_IFM_FULL_DPX;
+	else
+		val &= ~ENETC_PM_IFM_FULL_DPX;
+
+	if (val == old_val)
+		return;
+
+	enetc_write_port(priv, ENETC_PM_IF_MODE, val);
 }
 
 /* set up MAC configuration for the given interface type */
-static void enetc_setup_mac_iface(struct udevice *dev)
+static void enetc_setup_mac_iface(struct udevice *dev,
+				  struct phy_device *phydev)
 {
 	struct enetc_priv *priv = dev_get_priv(dev);
 	u32 if_mode;
@@ -202,7 +224,7 @@  static void enetc_setup_mac_iface(struct udevice *dev)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		enetc_init_rgmii(dev);
+		enetc_init_rgmii(dev, phydev);
 		break;
 	case PHY_INTERFACE_MODE_XGMII:
 	case PHY_INTERFACE_MODE_USXGMII:
@@ -546,10 +568,10 @@  static int enetc_start(struct udevice *dev)
 	enetc_setup_tx_bdr(dev);
 	enetc_setup_rx_bdr(dev);
 
-	enetc_setup_mac_iface(dev);
-
 	phy_startup(priv->phy);
 
+	enetc_setup_mac_iface(dev, priv->phy);
+
 	return 0;
 }
 
diff --git a/drivers/net/fsl_enetc.h b/drivers/net/fsl_enetc.h
index 110c1d78fbc6..a4409505e043 100644
--- a/drivers/net/fsl_enetc.h
+++ b/drivers/net/fsl_enetc.h
@@ -77,6 +77,11 @@  enum enetc_bdr_type {TX, RX};
 #define ENETC_PM_IF_MODE		0x8300
 #define  ENETC_PM_IF_MODE_RG		BIT(2)
 #define  ENETC_PM_IF_MODE_AN_ENA	BIT(15)
+#define  ENETC_PM_IFM_SSP_MASK		GENMASK(14, 13)
+#define  ENETC_PM_IFM_SSP_1000		(2 << 13)
+#define  ENETC_PM_IFM_SSP_100		(0 << 13)
+#define  ENETC_PM_IFM_SSP_10		(1 << 13)
+#define  ENETC_PM_IFM_FULL_DPX		BIT(12)
 #define  ENETC_PM_IF_IFMODE_MASK	GENMASK(1, 0)
 
 /* buffer descriptors count must be multiple of 8 and aligned to 128 bytes */