Commit a31b1c15 authored by Vladimir Oltean's avatar Vladimir Oltean Committed by Jakub Kicinski
Browse files

net: phy: aquantia: merge aqr113c_fill_interface_modes() into aqr107_fill_interface_modes()



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 a7f3abcf ("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: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
Link: https://patch.msgid.link/20250821152022.1065237-3-vladimir.oltean@nxp.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 7cd3597b
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -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)
+20 −21
Original line number Diff line number Diff line
@@ -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)