[PATCH net 0/3] net: stmmac: socfpga: 1000BaseX support and cleanups

[PATCH net 0/3] net: stmmac: socfpga: 1000BaseX support and cleanups

[Maxime Chevallier]

Hello everyone,

This small series sorts-out 1000BaseX support and does a bit of cleanup
for the Lynx conversion.

Patch 1 makes sure that we set the right phy_mode when working in
1000BaseX mode, so that the internal GMII is configured correctly.

Patch 2 removes a check for phy_device upon calling fix_mac_speed(). As
the SGMII adapter may be chained to a Lynx PCS, checking for a
phy_device to be attached to the netdev before enabling the SGMII
adapter doesn't make sense, as we won't have a downstream PHY when using
1000BaseX.

Patch 3 cleans an unused field from the PCS conversion.

Maxime Chevallier (3):
  net: stmmac: socfpga: Enable internal GMII when using 1000BaseX
  net: stmmac: socfpga: Don't check for phy to enable the SGMII adapter
  net: stmmac: socfpga: Remove unused pcs-mdiodev field

 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

-- 
2.49.0

[PATCH net 1/3] net: stmmac: socfpga: Enable internal GMII when using 1000BaseX

[Maxime Chevallier]

Dwmac Socfpga may be used with an instance of a Lynx / Altera TSE PCS,
in which case it gains support for 1000BaseX.

It appears that the PCS is wired to the MAC through an internal GMII
bus. Make sure that we enable the GMII_MII mode for the internal MAC when
using 1000BaseX.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 59f90b123c5b..027356033e5e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -256,6 +256,7 @@ static int socfpga_set_phy_mode_common(int phymode, u32 *val)
 	case PHY_INTERFACE_MODE_MII:
 	case PHY_INTERFACE_MODE_GMII:
 	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
 		*val = SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII;
 		break;
 	case PHY_INTERFACE_MODE_RMII:
-- 
2.49.0

[PATCH net 2/3] net: stmmac: socfpga: Don't check for phy to enable the SGMII adapter

[Maxime Chevallier]

The SGMII adapter needs to be enabled for both Cisco SGMII and 1000BaseX
operations. It doesn't make sense to check for an attached phydev here,
as we simply might not have any, in particular if we're using the
1000BaseX interface mode.

So, check only for the presence of a SGMII adapter to re-enable it after
a link config.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 027356033e5e..cc9946b58236 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -67,9 +67,6 @@ static void socfpga_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode
 	struct socfpga_dwmac *dwmac = (struct socfpga_dwmac *)priv;
 	void __iomem *splitter_base = dwmac->splitter_base;
 	void __iomem *sgmii_adapter_base = dwmac->sgmii_adapter_base;
-	struct device *dev = dwmac->dev;
-	struct net_device *ndev = dev_get_drvdata(dev);
-	struct phy_device *phy_dev = ndev->phydev;
 	u32 val;
 
 	if (sgmii_adapter_base)
@@ -96,7 +93,7 @@ static void socfpga_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode
 		writel(val, splitter_base + EMAC_SPLITTER_CTRL_REG);
 	}
 
-	if (phy_dev && sgmii_adapter_base)
+	if (sgmii_adapter_base)
 		writew(SGMII_ADAPTER_ENABLE,
 		       sgmii_adapter_base + SGMII_ADAPTER_CTRL_REG);
 }
-- 
2.49.0

[PATCH net 3/3] net: stmmac: socfpga: Remove unused pcs-mdiodev field

[Maxime Chevallier]

When dwmac-socfpga was converted to using the Lynx PCS (previously
referred to in the driver as the Altera TSE PCS), the
lynx_pcs_create_mdiodev() was used to create the pcs instance.

As this function didn't exist in the early versions of the series, a
local mdiodev object was stored for PCS creation. It was never used, but
still made it into the driver, so remove it.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index cc9946b58236..cecfe77c8b72 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -59,7 +59,6 @@ struct socfpga_dwmac {
 	void __iomem *sgmii_adapter_base;
 	bool f2h_ptp_ref_clk;
 	const struct socfpga_dwmac_ops *ops;
-	struct mdio_device *pcs_mdiodev;
 };
 
 static void socfpga_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode)
-- 
2.49.0

Re: [PATCH net 1/3] net: stmmac: socfpga: Enable internal GMII when using 1000BaseX

[Russell King (Oracle)]

On Tue, Apr 22, 2025 at 11:46:55AM +0200, Maxime Chevallier wrote:
> Dwmac Socfpga may be used with an instance of a Lynx / Altera TSE PCS,
> in which case it gains support for 1000BaseX.
> 
> It appears that the PCS is wired to the MAC through an internal GMII
> bus. Make sure that we enable the GMII_MII mode for the internal MAC when
> using 1000BaseX.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net 2/3] net: stmmac: socfpga: Don't check for phy to enable the SGMII adapter

[Russell King (Oracle)]

On Tue, Apr 22, 2025 at 11:46:56AM +0200, Maxime Chevallier wrote:
> The SGMII adapter needs to be enabled for both Cisco SGMII and 1000BaseX
> operations. It doesn't make sense to check for an attached phydev here,
> as we simply might not have any, in particular if we're using the
> 1000BaseX interface mode.
> 
> So, check only for the presence of a SGMII adapter to re-enable it after
> a link config.

I wonder whether:

	struct stmmac_priv *priv = netdev_priv(dev_get_drvdata(dwmac->dev));

	if ((priv->plat->phy_interface == PHY_INTERFACE_MODE_SGMII ||
	     priv->plat->phy_interface == PHY_INTERFACE_MODE_1000BASEX) &&
	    sgmii_adapter_base)
 		writew(SGMII_ADAPTER_ENABLE,
 		       sgmii_adapter_base + SGMII_ADAPTER_CTRL_REG);

would describe the required functionality here better, making it clear
under which interface modes we expect to set the enable bits.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net 3/3] net: stmmac: socfpga: Remove unused pcs-mdiodev field

[Russell King (Oracle)]

On Tue, Apr 22, 2025 at 11:46:57AM +0200, Maxime Chevallier wrote:
> When dwmac-socfpga was converted to using the Lynx PCS (previously
> referred to in the driver as the Altera TSE PCS), the
> lynx_pcs_create_mdiodev() was used to create the pcs instance.
> 
> As this function didn't exist in the early versions of the series, a
> local mdiodev object was stored for PCS creation. It was never used, but
> still made it into the driver, so remove it.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net 2/3] net: stmmac: socfpga: Don't check for phy to enable the SGMII adapter

[Maxime Chevallier]

Hi Russell,

On Tue, 22 Apr 2025 11:26:23 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Tue, Apr 22, 2025 at 11:46:56AM +0200, Maxime Chevallier wrote:
> > The SGMII adapter needs to be enabled for both Cisco SGMII and 1000BaseX
> > operations. It doesn't make sense to check for an attached phydev here,
> > as we simply might not have any, in particular if we're using the
> > 1000BaseX interface mode.
> > 
> > So, check only for the presence of a SGMII adapter to re-enable it after
> > a link config.  
> 
> I wonder whether:
> 
> 	struct stmmac_priv *priv = netdev_priv(dev_get_drvdata(dwmac->dev));
> 
> 	if ((priv->plat->phy_interface == PHY_INTERFACE_MODE_SGMII ||
> 	     priv->plat->phy_interface == PHY_INTERFACE_MODE_1000BASEX) &&
> 	    sgmii_adapter_base)
>  		writew(SGMII_ADAPTER_ENABLE,
>  		       sgmii_adapter_base + SGMII_ADAPTER_CTRL_REG);
> 
> would describe the required functionality here better, making it clear
> under which interface modes we expect to set the enable bits.

That's true indeed. Thanks for the suggestion, I'll include that in V2
with your suggested-by :)

Thanks,

Maxime