I'm unsure whether intentionate or not, but I think the (partially observed) naming convention in this driver is that function prefixes denote the earliest generation when a feature is available. In case of aqr107_fill_interface_modes(), that means that the GLOBAL_CFG registers are a Gen2 feature. Supporting evidence: the AQR105, a Gen1 PHY, does not have these registers, thus the function is not named aqr105_*. Based on this inferred naming scheme, I am proposing a refinement of commit a7f3abcf6357 ("net: phy: aquantia: only poll GLOBAL_CFG regs on aqr113, aqr113c and aqr115c") which introduced aqr113c_fill_interface_modes(), suggesting this may be a Gen4 PHY feature. The long-term goal is for aqr107_config_init() to tail-call aqr107_fill_interface_modes(), such that the latter function is also called by AQR107 itself, and many other PHY drivers. Currently it can't, because aqr113c_config_init() calls aqr107_config_init() and then aqr113c_fill_interface_modes(). So this would lead to a duplicate call to aqr107_fill_interface_modes() for AQR113C. Centralize the reading of GLOBAL_CFG registers in the AQR107 method, and create a boolean, set to true by AQR113C, which tests whether waiting for a non-zero value in the GLOBAL_CFG_100M register is necessary. Signed-off-by: Vladimir Oltean --- drivers/net/phy/aquantia/aquantia.h | 1 + drivers/net/phy/aquantia/aquantia_main.c | 41 ++++++++++++------------ 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h index 0c78bfabace5..67ec6f7484af 100644 --- a/drivers/net/phy/aquantia/aquantia.h +++ b/drivers/net/phy/aquantia/aquantia.h @@ -178,6 +178,7 @@ struct aqr107_priv { u64 sgmii_stats[AQR107_SGMII_STAT_SZ]; unsigned long leds_active_low; unsigned long leds_active_high; + bool wait_on_global_cfg; }; #if IS_REACHABLE(CONFIG_HWMON) diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c index 52facd318c83..b9b58c6ce686 100644 --- a/drivers/net/phy/aquantia/aquantia_main.c +++ b/drivers/net/phy/aquantia/aquantia_main.c @@ -991,9 +991,24 @@ static const u16 aqr_global_cfg_regs[] = { static int aqr107_fill_interface_modes(struct phy_device *phydev) { unsigned long *possible = phydev->possible_interfaces; + struct aqr107_priv *priv = phydev->priv; unsigned int serdes_mode, rate_adapt; phy_interface_t interface; - int i, val; + int i, val, ret; + + /* It's been observed on some models that - when coming out of suspend + * - the FW signals that the PHY is ready but the GLOBAL_CFG registers + * continue on returning zeroes for some time. Let's poll the 100M + * register until it returns a real value as both 113c and 115c support + * this mode. + */ + if (priv->wait_on_global_cfg) { + ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1, + VEND1_GLOBAL_CFG_100M, val, + val != 0, 1000, 100000, false); + if (ret) + return ret; + } /* Walk the media-speed configuration registers to determine which * host-side serdes modes may be used by the PHY depending on the @@ -1042,25 +1057,6 @@ static int aqr107_fill_interface_modes(struct phy_device *phydev) return 0; } -static int aqr113c_fill_interface_modes(struct phy_device *phydev) -{ - int val, ret; - - /* It's been observed on some models that - when coming out of suspend - * - the FW signals that the PHY is ready but the GLOBAL_CFG registers - * continue on returning zeroes for some time. Let's poll the 100M - * register until it returns a real value as both 113c and 115c support - * this mode. - */ - ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1, - VEND1_GLOBAL_CFG_100M, val, val != 0, - 1000, 100000, false); - if (ret) - return ret; - - return aqr107_fill_interface_modes(phydev); -} - static int aqr115c_get_features(struct phy_device *phydev) { unsigned long *supported = phydev->supported; @@ -1088,8 +1084,11 @@ static int aqr111_get_features(struct phy_device *phydev) static int aqr113c_config_init(struct phy_device *phydev) { + struct aqr107_priv *priv = phydev->priv; int ret; + priv->wait_on_global_cfg = true; + ret = aqr107_config_init(phydev); if (ret < 0) return ret; @@ -1103,7 +1102,7 @@ static int aqr113c_config_init(struct phy_device *phydev) if (ret) return ret; - return aqr113c_fill_interface_modes(phydev); + return aqr107_fill_interface_modes(phydev); } static int aqr107_probe(struct phy_device *phydev) -- 2.34.1