net: phy: realtek: create rtl8211f_config_rgmii_delay()

The control flow in rtl8211f_config_init() has some pitfalls which were
probably unintended. Specifically it has an early return:

	switch (phydev->interface) {
	...
	default: /* the rest of the modes imply leaving delay as is. */
		return 0;
	}

which exits the entire config_init() function. This means it also skips
doing things such as disabling CLKOUT or disabling PHY-mode EEE.

For the RTL8211FS, which uses PHY_INTERFACE_MODE_SGMII, this might be a
problem. However, I don't know that it is, so there is no Fixes: tag.
The issue was observed through code inspection.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Link: https://patch.msgid.link/20251117234033.345679-2-vladimir.oltean@nxp.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 417f9a88aab66f9074f5a6c18615378d695066e3..896351022682b38926d897cb3591bf93cd0bb78a 100644 (file)
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -587,22 +587,11 @@ static int rtl8211c_config_init(struct phy_device *phydev)
                            CTL1000_ENABLE_MASTER | CTL1000_AS_MASTER);
 }
 
-static int rtl8211f_config_init(struct phy_device *phydev)
+static int rtl8211f_config_rgmii_delay(struct phy_device *phydev)
 {
-       struct rtl821x_priv *priv = phydev->priv;
-       struct device *dev = &phydev->mdio.dev;
        u16 val_txdly, val_rxdly;
        int ret;
 
-       ret = phy_modify_paged_changed(phydev, RTL8211F_PHYCR_PAGE, RTL8211F_PHYCR1,
-                                      RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF,
-                                      priv->phycr1);
-       if (ret < 0) {
-               dev_err(dev, "aldps mode  configuration failed: %pe\n",
-                       ERR_PTR(ret));
-               return ret;
-       }
-
        switch (phydev->interface) {
        case PHY_INTERFACE_MODE_RGMII:
                val_txdly = 0;
@@ -632,34 +621,58 @@ static int rtl8211f_config_init(struct phy_device *phydev)
                                       RTL8211F_TXCR, RTL8211F_TX_DELAY,
                                       val_txdly);
        if (ret < 0) {
-               dev_err(dev, "Failed to update the TX delay register\n");
+               phydev_err(phydev, "Failed to update the TX delay register: %pe\n",
+                          ERR_PTR(ret));
                return ret;
        } else if (ret) {
-               dev_dbg(dev,
-                       "%s 2ns TX delay (and changing the value from pin-strapping RXD1 or the bootloader)\n",
-                       str_enable_disable(val_txdly));
+               phydev_dbg(phydev,
+                          "%s 2ns TX delay (and changing the value from pin-strapping RXD1 or the bootloader)\n",
+                          str_enable_disable(val_txdly));
        } else {
-               dev_dbg(dev,
-                       "2ns TX delay was already %s (by pin-strapping RXD1 or bootloader configuration)\n",
-                       str_enabled_disabled(val_txdly));
+               phydev_dbg(phydev,
+                          "2ns TX delay was already %s (by pin-strapping RXD1 or bootloader configuration)\n",
+                          str_enabled_disabled(val_txdly));
        }
 
        ret = phy_modify_paged_changed(phydev, RTL8211F_RGMII_PAGE,
                                       RTL8211F_RXCR, RTL8211F_RX_DELAY,
                                       val_rxdly);
        if (ret < 0) {
-               dev_err(dev, "Failed to update the RX delay register\n");
+               phydev_err(phydev, "Failed to update the RX delay register: %pe\n",
+                          ERR_PTR(ret));
                return ret;
        } else if (ret) {
-               dev_dbg(dev,
-                       "%s 2ns RX delay (and changing the value from pin-strapping RXD0 or the bootloader)\n",
-                       str_enable_disable(val_rxdly));
+               phydev_dbg(phydev,
+                          "%s 2ns RX delay (and changing the value from pin-strapping RXD0 or the bootloader)\n",
+                          str_enable_disable(val_rxdly));
        } else {
-               dev_dbg(dev,
-                       "2ns RX delay was already %s (by pin-strapping RXD0 or bootloader configuration)\n",
-                       str_enabled_disabled(val_rxdly));
+               phydev_dbg(phydev,
+                          "2ns RX delay was already %s (by pin-strapping RXD0 or bootloader configuration)\n",
+                          str_enabled_disabled(val_rxdly));
        }
 
+       return 0;
+}
+
+static int rtl8211f_config_init(struct phy_device *phydev)
+{
+       struct rtl821x_priv *priv = phydev->priv;
+       struct device *dev = &phydev->mdio.dev;
+       int ret;
+
+       ret = phy_modify_paged_changed(phydev, RTL8211F_PHYCR_PAGE, RTL8211F_PHYCR1,
+                                      RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF,
+                                      priv->phycr1);
+       if (ret < 0) {
+               dev_err(dev, "aldps mode  configuration failed: %pe\n",
+                       ERR_PTR(ret));
+               return ret;
+       }
+
+       ret = rtl8211f_config_rgmii_delay(phydev);
+       if (ret)
+               return ret;
+
        if (!priv->has_phycr2)
                return 0;