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

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!

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