[PATCH net-next v4 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery

[PATCH net-next v4 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery

[Hongmei Xu]

This series fixes two related issues in the Aquantia PHY driver:
- Patch 1 restores EEE discovery for AQR111, AQR111B0, AQR114C,
  AQR115, AQR115C and AQCS109 by calling genphy_c45_read_eee_abilities()
  after the final supported mask is built. The phy_gbit_features base is
  preserved because these PHYs misreport PMA capabilities.
- Patch 2 adds get_features() for AQR113C to correctly declare 10G/5G/
  2.5G/1G support, since the PHY misreports these speeds in PMA extended
  abilities registers.

---
Changes in v4:
- Fix the comments from netdev-bot+sashiko
- Patch 1: keep phy_gbit_features base instead of genphy_c45_pma_read_abilities();
  call genphy_c45_read_eee_abilities() explicitly after supported mask is built
- Patch 1: update commit message to accurately describe the fix; add missing
  affected PHYs (AQR111B0, AQR115, AQCS109); add Fixes: tags
- Patch 2: fix commit message PMA reads still used as base and add Fixes: tag
- Link to v3: https://patch.msgid.link/20261016-net-phy-aqr113c-fix-pma-capability-v3-0-0bbb1b84035d@oss.qualcomm.com

Changes in v3:
- Add patch 1 to fix EEE discovery regression in aqr115c_get_features()
  affecting AQR111, AQR114C and AQR115C
- Extend commit message to note AQR113 may have the same PMA issue
- Link to v2: https://patch.msgid.link/20260924-net-phy-aqr113c-fix-pma-capability-v2-1-29b06283b0f3@oss.qualcomm.com

Changes in v2:
- Fix subject prefix to [PATCH net-next]
- Limit scope to AQR113C only as AQR113 not tested
- Link to v1: https://patch.msgid.link/20260923-net-phy-aqr113c-fix-pma-capability-v1-1-497359310f06@oss.qualcomm.com

To: Andrew Lunn <andrew@lunn.ch>
To: Heiner Kallweit <hkallweit1@gmail.com>
To: Russell King <linux@armlinux.org.uk>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Hongmei Xu (2):
      net: phy: aquantia: restore EEE discovery in get_features callbacks
      net: phy: aquantia: add get_features() for AQR113C

 drivers/net/phy/aquantia/aquantia_main.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)
---
base-commit: 944ae66642b726bd6b25ae71b1e9ff88a0e0bdb0
change-id: 20260917-net-phy-aqr113c-fix-pma-capability-d7d08cbc9879

Best regards,
--  
Hongmei Xu <hongmei.xu@oss.qualcomm.com>

[PATCH net-next v4 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks

[Hongmei Xu]

aqr115c_get_features() populates phydev->supported by ORing in
phy_gbit_features and setting the 2.5G bit directly. This skips
genphy_c45_read_eee_abilities(), which fills phydev->supported_eee.

As a result, phy_probe() never populates supported_eee for AQR111,
AQR111B0, AQR114C, AQR115, AQCS109 and AQR115C. The kernel then
clears advertising_eee:

  linkmode_and(phydev->advertising_eee, phydev->supported_eee,
               phydev->advertising_eee);

This means EEE advertisement registers cannot be changed via ethtool
and ethtool --show-eee reports no supported modes.

genphy_c45_pma_read_abilities() is not used to build the supported mask
because these PHYs misreport PMA capabilities. The supported and
advertised link modes are intentionally unchanged; only EEE discovery
is restored.

Fix this by calling genphy_c45_read_eee_abilities() at the end of
aqr115c_get_features(), after the final supported mask is built so that
2500baseT_Full is already set when the EEE cap2 register is read.

Also fix aqr111_get_features() to propagate the return value of
aqr115c_get_features().

Fixes: 17cbfcdd85f6 ("net: phy: aquantia: AQR115c fix up PMA capabilities")
Fixes: 8f61d73306c6 ("net: phy: aquantia: remove usage of phy_set_max_speed")
Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com>
---
 drivers/net/phy/aquantia/aquantia_main.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index e95d391a6dc2..099ee919de94 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -1079,15 +1079,17 @@ static int aqr115c_get_features(struct phy_device *phydev)
 	linkmode_or(supported, supported, phy_gbit_features);
 	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);
 
-	return 0;
+	return genphy_c45_read_eee_abilities(phydev);
 }
 
 static int aqr111_get_features(struct phy_device *phydev)
 {
-	/* PHY supports speeds up to 5G with autoneg. PMA capabilities
-	 * are not useful.
-	 */
-	aqr115c_get_features(phydev);
+	int ret;
+
+	ret = aqr115c_get_features(phydev);
+	if (ret)
+		return ret;
+
 	linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
 			 phydev->supported);
 

-- 
2.34.1

Re: [PATCH net-next v4 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] net: phy: aquantia: premature and unmasked EEE read in
  aqr115c_get_features()
- [Low] aqr115c_get_features()
  (drivers/net/phy/aquantia/aquantia_main.c:1082) now ends with `return…

--

net: phy: aquantia: restore EEE discovery in get_features callbacks

This patch makes aqr115c_get_features() call
genphy_c45_read_eee_abilities() after it builds the supported mask, so
supported_eee gets filled in for AQR111, AQR111B0, AQR114C, AQR115,
AQCS109 and AQR115C. It also makes aqr111_get_features() pass on the
return value of aqr115c_get_features().

> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
> index e95d391a6dc2..099ee919de94 100644
> --- a/drivers/net/phy/aquantia/aquantia_main.c
> +++ b/drivers/net/phy/aquantia/aquantia_main.c
> @@ -1079,15 +1079,17 @@ static int aqr115c_get_features(struct phy_device *phydev)
>  	linkmode_or(supported, supported, phy_gbit_features);
>  	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);
>  
> -	return 0;
> +	return genphy_c45_read_eee_abilities(phydev);
>  }
>  
>  static int aqr111_get_features(struct phy_device *phydev)
>  {
> -	/* PHY supports speeds up to 5G with autoneg. PMA capabilities
> -	 * are not useful.
> -	 */
> -	aqr115c_get_features(phydev);
> +	int ret;
> +
> +	ret = aqr115c_get_features(phydev);
> +	if (ret)
> +		return ret;

[Severity: Medium]
Two related problems here. The 2.5G-only parts can end up with
supported_eee out of sync with supported. In aqr111_get_features(),
the EEE read also runs before the final supported mask is built.

genphy_c45_read_eee_abilities() reads 3.20 via
genphy_c45_read_eee_cap1(). That function masks the result against
supported:

	linkmode_and(phydev->supported_eee, phydev->supported_eee,
		     phydev->supported);

It then reads 3.21 via genphy_c45_read_eee_cap2(), which only does:

	mii_eee_cap2_mod_linkmode_sup_t(phydev->supported_eee, val);

That helper sets 2500baseT_Full and 5000baseT_Full straight from
MDIO_EEE_2_5GT and MDIO_EEE_5GT. Nothing masks that result against
supported afterwards, either here or in phy_probe().

For AQR115C, AQR115 and AQCS109, supported stops at 2500baseT_Full.
The commit message itself says these PHYs misreport their
capabilities. If 3.21 also reports 5GT on them, supported_eee ends
up with 5000baseT_Full on a 2.5G PHY. phy_probe() only intersects
advertising_eee with supported_eee, so a 5G EEE advertisement can
survive into advertising_eee. ethtool --show-eee would then list
5000baseT/Full on a PHY that cannot link at 5G.

The ordering in aqr111_get_features() does not match the commit
message, which says the EEE read happens after the final supported
mask is built. Here aqr115c_get_features(), and with it the EEE
read, runs before 5000baseT_Full is added to supported. For 5G this
only works because cap2 is unmasked.

For cap1 modes it breaks. Patch 2/2 has the AQR113C get_features()
chain onto aqr111_get_features() before it sets 10000baseT_Full. So
the linkmode_and() in genphy_c45_read_eee_cap1() strips
10000baseT_Full from supported_eee while 10G is not yet in
supported, and it never comes back.

Could the EEE read be done once per driver, after that driver's
supported mask is complete? For example, keep the mask building in a
shared helper and call genphy_c45_read_eee_abilities() last in each
get_features(). A linkmode_and() of supported_eee against supported
after that would keep 3.21 from adding modes the PHY lacks. Or do
you think genphy_c45_read_eee_cap2() should apply the same mask that
cap1 already does?

[Severity: Low]
Should the result of genphy_c45_read_eee_abilities() be passed back as
the ->get_features() result here?

With this change, if reading the optional EEE ability registers (3.20
MDIO_PCS_EEE_ABLE or 3.21 MDIO_PCS_EEE_ABLE2) fails, aqr115c_get_features()
fails, and so does aqr111_get_features(). phy_probe() then aborts:

drivers/net/phy/phy_device.c:phy_probe() {
    ...
	else if (phydrv->get_features)
		err = phydrv->get_features(phydev);
    ...
}

The phylib core ignores the return value of this helper on purpose at
all three of its call sites. genphy_c45_pma_read_abilities() and
genphy_read_abilities() both have:

	/* This is optional functionality. If not supported, we may get an error
	 * which should be ignored.
	 */
	genphy_c45_read_eee_abilities(phydev);

The phydrv->features branch of phy_probe() also ignores it:

	if (phydrv->features) {
		linkmode_copy(phydev->supported, phydrv->features);
		genphy_c45_read_eee_abilities(phydev);
	}

The commit message says it restores EEE discovery. The code path it
restores, genphy_c45_pma_read_abilities() as used before 17cbfcdd85f6
and 8f61d73306c6, never failed probe because of EEE discovery.

In practice this is hard to hit. aqr107_probe() already reads VEND1
registers in aqr_wait_reset_complete() and fails probe on read errors.
That mostly leaves a transient MDIO bus error during this exact read as
the newly fatal case.

Would it match the core better to call genphy_c45_read_eee_abilities()
without passing its result back, and keep return 0?

> +
>  	linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
>  			 phydev->supported);
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022-net-phy-aqr113c-fix-pma-capability-v4-0-95a17f75dd7a%40oss.qualcomm.com

Re: [PATCH net-next v4 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks

[Hongmei Xu]

On 10/26/2026 5:48 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] net: phy: aquantia: premature and unmasked EEE read in
> aqr115c_get_features()
> - [Low] aqr115c_get_features()
> (drivers/net/phy/aquantia/aquantia_main.c:1082) now ends with `return…
>
> --
>
> net: phy: aquantia: restore EEE discovery in get_features callbacks
>
> This patch makes aqr115c_get_features() call
> genphy_c45_read_eee_abilities() after it builds the supported mask, so
> supported_eee gets filled in for AQR111, AQR111B0, AQR114C, AQR115,
> AQCS109 and AQR115C. It also makes aqr111_get_features() pass on the
> return value of aqr115c_get_features().
>
>> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
>> index e95d391a6dc2..099ee919de94 100644
>> --- a/drivers/net/phy/aquantia/aquantia_main.c
>> +++ b/drivers/net/phy/aquantia/aquantia_main.c
>> @@ -1079,15 +1079,17 @@ static int aqr115c_get_features(struct phy_device *phydev)
>> linkmode_or(supported, supported, phy_gbit_features);
>> linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);
>>
>> - return 0;
>> + return genphy_c45_read_eee_abilities(phydev);
>> }
>>
>> static int aqr111_get_features(struct phy_device *phydev)
>> {
>> - /* PHY supports speeds up to 5G with autoneg. PMA capabilities
>> - * are not useful.
>> - */
>> - aqr115c_get_features(phydev);
>> + int ret;
>> +
>> + ret = aqr115c_get_features(phydev);
>> + if (ret)
>> + return ret;
>
> [Severity: Medium]
> Two related problems here. The 2.5G-only parts can end up with
> supported_eee out of sync with supported. In aqr111_get_features(),
> the EEE read also runs before the final supported mask is built.
>
> genphy_c45_read_eee_abilities() reads 3.20 via
> genphy_c45_read_eee_cap1(). That function masks the result against
> supported:
>
> linkmode_and(phydev->supported_eee, phydev->supported_eee,
> phydev->supported);
>
> It then reads 3.21 via genphy_c45_read_eee_cap2(), which only does:
>
> mii_eee_cap2_mod_linkmode_sup_t(phydev->supported_eee, val);
>
> That helper sets 2500baseT_Full and 5000baseT_Full straight from
> MDIO_EEE_2_5GT and MDIO_EEE_5GT. Nothing masks that result against
> supported afterwards, either here or in phy_probe().
>
> For AQR115C, AQR115 and AQCS109, supported stops at 2500baseT_Full.
> The commit message itself says these PHYs misreport their
> capabilities. If 3.21 also reports 5GT on them, supported_eee ends
> up with 5000baseT_Full on a 2.5G PHY. phy_probe() only intersects
> advertising_eee with supported_eee, so a 5G EEE advertisement can
> survive into advertising_eee. ethtool --show-eee would then list
> 5000baseT/Full on a PHY that cannot link at 5G.
>
> The ordering in aqr111_get_features() does not match the commit
> message, which says the EEE read happens after the final supported
> mask is built. Here aqr115c_get_features(), and with it the EEE
> read, runs before 5000baseT_Full is added to supported. For 5G this
> only works because cap2 is unmasked.
>
> For cap1 modes it breaks. Patch 2/2 has the AQR113C get_features()
> chain onto aqr111_get_features() before it sets 10000baseT_Full. So
> the linkmode_and() in genphy_c45_read_eee_cap1() strips
> 10000baseT_Full from supported_eee while 10G is not yet in
> supported, and it never comes back.
>
> Could the EEE read be done once per driver, after that driver's
> supported mask is complete? For example, keep the mask building in a
> shared helper and call genphy_c45_read_eee_abilities() last in each
> get_features(). A linkmode_and() of supported_eee against supported
> after that would keep 3.21 from adding modes the PHY lacks. Or do
> you think genphy_c45_read_eee_cap2() should apply the same mask that
> cap1 already does?
>
Agreed. genphy_c45_read_eee_abilities() should only be called after the
supported parameter is built. So it should be called per driver. Adding
it to aqr115c_get_features() is wrong as aqr115c_get_features() is
called in aqr111_get_features() and coming aqr113c_get_features() before
the supported mask is been built in those PHYs.

> [Severity: Low]
> Should the result of genphy_c45_read_eee_abilities() be passed back as
> the ->get_features() result here?
>
> With this change, if reading the optional EEE ability registers (3.20
> MDIO_PCS_EEE_ABLE or 3.21 MDIO_PCS_EEE_ABLE2) fails, aqr115c_get_features()
> fails, and so does aqr111_get_features(). phy_probe() then aborts:
>
> drivers/net/phy/phy_device.c:phy_probe() {
> ...
> else if (phydrv->get_features)
> err = phydrv->get_features(phydev);
> ...
> }
>
> The phylib core ignores the return value of this helper on purpose at
> all three of its call sites. genphy_c45_pma_read_abilities() and
> genphy_read_abilities() both have:
>
> /* This is optional functionality. If not supported, we may get an error
> * which should be ignored.
> */
> genphy_c45_read_eee_abilities(phydev);
>
> The phydrv->features branch of phy_probe() also ignores it:
>
> if (phydrv->features) {
> linkmode_copy(phydev->supported, phydrv->features);
> genphy_c45_read_eee_abilities(phydev);
> }
>
Agreed. Should not take genphy_c45_pma_read_abilities() return value as
.get_features result.

> The commit message says it restores EEE discovery. The code path it
> restores, genphy_c45_pma_read_abilities() as used before 17cbfcdd85f6
> and 8f61d73306c6, never failed probe because of EEE discovery.
>
> In practice this is hard to hit. aqr107_probe() already reads VEND1
> registers in aqr_wait_reset_complete() and fails probe on read errors.
> That mostly leaves a transient MDIO bus error during this exact read as
> the newly fatal case.
>
> Would it match the core better to call genphy_c45_read_eee_abilities()
> without passing its result back, and keep return 0?
>
>> +
>> linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
>> phydev->supported);
>>
>

Since my goal is just to add .get_features for aqr113c phy, I should not
touch the existing logic of aqr115c_get_features which will impact
AQR111，AQR111，AQR114C PHYs. I should revert this patch.

pw-bot: cr


Regards,
Hongmei Xu

[PATCH net-next v4 2/2] net: phy: aquantia: add get_features() for AQR113C

[Hongmei Xu]

AQR113C does not correctly report its supported speeds in the PMA
extended abilities registers. Without a get_features callback,
phy_probe() uses genphy_c45_pma_read_abilities() directly, which
produces an incorrect supported mask.

Add aqr113c_get_features() that calls aqr111_get_features() as a base
(which reads PMA abilities and forces 1G/5G/2.5G that the PHY misreports)
and sets 10000baseT_Full explicitly since the PHY also misreports 10G.

The AQR113 likely has the same issue since it is a closely related
variant, but this fix is limited to AQR113C as that is the only
hardware available for testing.

Fixes: 12cf1b89a668 ("net: phy: Add support for AQR113C EPHY")

Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com>
---
 drivers/net/phy/aquantia/aquantia_main.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 099ee919de94..153adfbda53f 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -1096,6 +1096,23 @@ static int aqr111_get_features(struct phy_device *phydev)
 	return 0;
 }
 
+static int aqr113c_get_features(struct phy_device *phydev)
+{
+	/* AQR113C does not correctly report 10G in PMA extended abilities.
+	 * Build on aqr111_get_features() for 5G/2.5G and add 10G explicitly.
+	 */
+	int ret;
+
+	ret = aqr111_get_features(phydev);
+	if (ret)
+		return ret;
+
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+			 phydev->supported);
+
+	return 0;
+}
+
 static int aqr_gen4_config_init(struct phy_device *phydev)
 {
 	struct aqr107_priv *priv = phydev->priv;
@@ -1446,6 +1463,7 @@ static struct phy_driver aqr_driver[] = {
 	.get_sset_count = aqr107_get_sset_count,
 	.get_strings    = aqr107_get_strings,
 	.get_stats      = aqr107_get_stats,
+	.get_features   = aqr113c_get_features,
 	.link_change_notify = aqr107_link_change_notify,
 	.led_brightness_set = aqr_phy_led_brightness_set,
 	.led_hw_is_supported = aqr_phy_led_hw_is_supported,

-- 
2.34.1

Re: [PATCH net-next v4 2/2] net: phy: aquantia: add get_features() for AQR113C

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] aqr113c_get_features() adds 10000baseT_Full to
  phydev->supported only after EEE discovery has already run, so…
- [Medium] The commit message describes the fix's mechanism incorrectly.

--

net: phy: aquantia: add get_features() for AQR113C

This adds an aqr113c_get_features() callback for the AQR113C. It builds on
aqr111_get_features() and then sets 10000baseT_Full in the supported mask.
Previously phy_probe() fell back to genphy_c45_pma_read_abilities(), which
the commit says produces an incorrect supported mask for this PHY.

> Add aqr113c_get_features() that calls aqr111_get_features() as a base
> (which reads PMA abilities and forces 1G/5G/2.5G that the PHY misreports)
> and sets 10000baseT_Full explicitly since the PHY also misreports 10G.

[Severity: Medium]
Is this description of aqr111_get_features() accurate? It calls
aqr115c_get_features(), which does not read any PMA ability registers:

drivers/net/phy/aquantia/aquantia_main.c:aqr115c_get_features() {
	/* PHY supports speeds up to 2.5G with autoneg. PMA capabilities
	 * are not useful.
	 */
	linkmode_or(supported, supported, phy_gbit_features);
	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);

	return genphy_c45_read_eee_abilities(phydev);
}

The only hardware read in this path is for the EEE abilities. MDIO_STAT2,
MDIO_PMA_EXTABLE and MDIO_PMA_NG_EXTABLE are never read, so this patch
moves AQR113C from a mask built from the registers to a fully static one.

The message also leaves out a side effect. phy_gbit_features includes
1000baseT_Half and the 10/100 half and full duplex modes, so these are now
always in supported. aqr_config_aneg() will then advertise 1000BASE-T half
duplex:

	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
			      phydev->advertising))
		reg |= MDIO_AN_VEND_PROV_1000BASET_HALF;

The previous genphy_c45_pma_read_ext_abilities() path never set this bit.
AQR111, AQR114C and AQR115C already behave this way, but could the change
be mentioned in the message?

The new comment in aqr113c_get_features() says only 10G is misreported in
the PMA extended abilities. The commit message says 1G, 2.5G, 5G and 10G
are all misreported. Could the message and the comment be brought in line
with what the code does?

The inherited "up to 2.5G" comment in aqr115c_get_features() is also stale
now that a 10G PHY uses it.

> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
> index 099ee919de94..153adfbda53f 100644
> --- a/drivers/net/phy/aquantia/aquantia_main.c
> +++ b/drivers/net/phy/aquantia/aquantia_main.c
> @@ -1096,6 +1096,23 @@ static int aqr111_get_features(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +static int aqr113c_get_features(struct phy_device *phydev)
> +{
> +	/* AQR113C does not correctly report 10G in PMA extended abilities.
> +	 * Build on aqr111_get_features() for 5G/2.5G and add 10G explicitly.
> +	 */
> +	int ret;
> +
> +	ret = aqr111_get_features(phydev);
> +	if (ret)
> +		return ret;
> +
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
> +			 phydev->supported);

[Severity: Medium]
10000baseT_Full is set here only after EEE discovery has already run. Does
that mean AQR113C always loses 10GBASE-T EEE?

The call chain is:

aqr113c_get_features()
  aqr111_get_features()
    aqr115c_get_features()
      genphy_c45_read_eee_abilities()
        genphy_c45_read_eee_cap1()

At that point supported holds only the gbit modes and 2500baseT_Full.
genphy_c45_read_eee_cap1() maps MDIO_EEE_10GT from register 3.20 into
supported_eee and then masks it against supported:

	mii_eee_cap1_mod_linkmode_t(phydev->supported_eee, val);
	...
	linkmode_and(phydev->supported_eee, phydev->supported_eee,
		     phydev->supported);

10000baseT_Full is not in supported yet, so the 10G EEE bit is cleared.
Nothing reads the EEE abilities again afterwards: phy_probe() only calls
genphy_c45_read_eee_abilities() in its drv->features branch.

After that, phy_probe() ANDs advertising_eee with supported_eee. Then
genphy_c45_an_config_eee_aneg() writes the reduced mask to MDIO_AN_EEE_ADV,
which looks like it would clear any default 10GT EEE advertisement in the
hardware.

genphy_c45_ethtool_set_eee() would also return -EINVAL for a 10G EEE
request:

	if (linkmode_andnot(tmp, adv, phydev->supported_eee)) {

5G is not affected, because cap2 is not masked against supported.

Before this patch 10G was missing from supported entirely, so this is not
a loss compared to the old state. However, supported and supported_eee now
disagree on 10G.

The earlier commit "net: phy: aquantia: restore EEE discovery in
get_features callbacks" put EEE discovery at the end of
aqr115c_get_features() so that supported would be final by then. With this
nesting, that no longer holds for a cap1 mode.

Would it work to set all the forced modes before calling
genphy_c45_read_eee_abilities()? For example, the EEE read could move to
the end of each top-level get_features callback.

> +
> +	return 0;
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022-net-phy-aqr113c-fix-pma-capability-v4-0-95a17f75dd7a%40oss.qualcomm.com

Re: [PATCH net-next v4 2/2] net: phy: aquantia: add get_features() for AQR113C

[Hongmei Xu]

On 10/26/2026 5:48 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 2 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] aqr113c_get_features() adds 10000baseT_Full to
>    phydev->supported only after EEE discovery has already run, so…
> - [Medium] The commit message describes the fix's mechanism incorrectly.
>
> --
>
> net: phy: aquantia: add get_features() for AQR113C
>
> This adds an aqr113c_get_features() callback for the AQR113C. It builds on
> aqr111_get_features() and then sets 10000baseT_Full in the supported mask.
> Previously phy_probe() fell back to genphy_c45_pma_read_abilities(), which
> the commit says produces an incorrect supported mask for this PHY.
>
>> Add aqr113c_get_features() that calls aqr111_get_features() as a base
>> (which reads PMA abilities and forces 1G/5G/2.5G that the PHY misreports)
>> and sets 10000baseT_Full explicitly since the PHY also misreports 10G.
>
> [Severity: Medium]
> Is this description of aqr111_get_features() accurate? It calls
> aqr115c_get_features(), which does not read any PMA ability registers:
>
> drivers/net/phy/aquantia/aquantia_main.c:aqr115c_get_features() {
> /* PHY supports speeds up to 2.5G with autoneg. PMA capabilities
> * are not useful.
> */
> linkmode_or(supported, supported, phy_gbit_features);
> linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);
>
> return genphy_c45_read_eee_abilities(phydev);
> }
>
> The only hardware read in this path is for the EEE abilities. MDIO_STAT2,
> MDIO_PMA_EXTABLE and MDIO_PMA_NG_EXTABLE are never read, so this patch
> moves AQR113C from a mask built from the registers to a fully static one.
>
> The message also leaves out a side effect. phy_gbit_features includes
> 1000baseT_Half and the 10/100 half and full duplex modes, so these are now
> always in supported. aqr_config_aneg() will then advertise 1000BASE-T half
> duplex:
>
> if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
>      phydev->advertising))
> reg |= MDIO_AN_VEND_PROV_1000BASET_HALF;
>
> The previous genphy_c45_pma_read_ext_abilities() path never set this bit.
> AQR111, AQR114C and AQR115C already behave this way, but could the change
> be mentioned in the message?
>
> The new comment in aqr113c_get_features() says only 10G is misreported in
> the PMA extended abilities. The commit message says 1G, 2.5G, 5G and 10G
> are all misreported. Could the message and the comment be brought in line
> with what the code does?
>
> The inherited "up to 2.5G" comment in aqr115c_get_features() is also stale
> now that a 10G PHY uses it.
>
>> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
>> index 099ee919de94..153adfbda53f 100644
>> --- a/drivers/net/phy/aquantia/aquantia_main.c
>> +++ b/drivers/net/phy/aquantia/aquantia_main.c
>> @@ -1096,6 +1096,23 @@ static int aqr111_get_features(struct phy_device *phydev)
>>   return 0;
>>   }
>>
>> +static int aqr113c_get_features(struct phy_device *phydev)
>> +{
>> + /* AQR113C does not correctly report 10G in PMA extended abilities.
>> + * Build on aqr111_get_features() for 5G/2.5G and add 10G explicitly.
>> + */
>> + int ret;
>> +
>> + ret = aqr111_get_features(phydev);
>> + if (ret)
>> + return ret;
>> +
>> + linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
>> + phydev->supported);

The patch [net: phy: aquantia: restore EEE discovery in get_features
callbacks] will be reverted as it changed the existing logic for AQR111，
AQR111，AQR114C PHYs. I should not touch these unrealted PHYs.
And commit message should be more accurate.

> [Severity: Medium]
> 10000baseT_Full is set here only after EEE discovery has already run. Does
> that mean AQR113C always loses 10GBASE-T EEE?
>
> The call chain is:
>
> aqr113c_get_features()
>    aqr111_get_features()
>      aqr115c_get_features()
>        genphy_c45_read_eee_abilities()
>          genphy_c45_read_eee_cap1()
>
> At that point supported holds only the gbit modes and 2500baseT_Full.
> genphy_c45_read_eee_cap1() maps MDIO_EEE_10GT from register 3.20 into
> supported_eee and then masks it against supported:
>
> mii_eee_cap1_mod_linkmode_t(phydev->supported_eee, val);
> ...
> linkmode_and(phydev->supported_eee, phydev->supported_eee,
>     phydev->supported);
>
> 10000baseT_Full is not in supported yet, so the 10G EEE bit is cleared.
> Nothing reads the EEE abilities again afterwards: phy_probe() only calls
> genphy_c45_read_eee_abilities() in its drv->features branch.
>
> After that, phy_probe() ANDs advertising_eee with supported_eee. Then
> genphy_c45_an_config_eee_aneg() writes the reduced mask to MDIO_AN_EEE_ADV,
> which looks like it would clear any default 10GT EEE advertisement in the
> hardware.
>
> genphy_c45_ethtool_set_eee() would also return -EINVAL for a 10G EEE
> request:
>
> if (linkmode_andnot(tmp, adv, phydev->supported_eee)) {
>
> 5G is not affected, because cap2 is not masked against supported.
>
> Before this patch 10G was missing from supported entirely, so this is not
> a loss compared to the old state. However, supported and supported_eee now
> disagree on 10G.
>
> The earlier commit "net: phy: aquantia: restore EEE discovery in
> get_features callbacks" put EEE discovery at the end of
> aqr115c_get_features() so that supported would be final by then. With this
> nesting, that no longer holds for a cap1 mode.
>
> Would it work to set all the forced modes before calling
> genphy_c45_read_eee_abilities()? For example, the EEE read could move to
> the end of each top-level get_features callback.
>
>> +
>> + return 0;
>> +}
>
> [ ... ]
>

Agreed, will call genphy_c45_read_eee_abilities() at the the end of each
top-level get_features after all supported mask is built per driver.

pw-bot: cr


Regards,
Hongmei Xu