* [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery
@ 2026-10-16 3:23 Hongmei Xu
2026-10-16 3:23 ` [PATCH net-next v3 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks Hongmei Xu
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Hongmei Xu @ 2026-10-16 3:23 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Hongmei Xu
This series fixes two related issues in the Aquantia PHY driver:
- Patch 1 restores EEE discovery for AQR111/114C/115C by replacing
the manual linkmode_or() with genphy_c45_pma_read_abilities().
- Patch 2 adds get_features() for AQR113C to correctly report
10G/5G/2.5G/1G PMA capabilities.
---
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 | 40 ++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 10 deletions(-)
---
base-commit: 944ae66642b726bd6b25ae71b1e9ff88a0e0bdb0
change-id: 20260917-net-phy-aqr113c-fix-pma-capability-d7d08cbc9879
Best regards,
--
Hongmei Xu <hongmei.xu@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH net-next v3 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks 2026-10-16 3:23 [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery Hongmei Xu @ 2026-10-16 3:23 ` Hongmei Xu 2026-10-16 21:19 ` Andrew Lunn 2026-10-19 18:26 ` netdev-bot+sashiko 2026-10-16 3:23 ` [PATCH net-next v3 2/2] net: phy: aquantia: add get_features() for AQR113C Hongmei Xu 2026-10-21 20:29 ` [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery Jakub Kicinski 2 siblings, 2 replies; 12+ messages in thread From: Hongmei Xu @ 2026-10-16 3:23 UTC (permalink / raw) To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni Cc: netdev, linux-kernel, 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_pma_read_abilities(), which is responsible for reading EEE capabilities and filling phydev->supported_eee. As a result, phy_probe() never populates supported_eee for AQR111, AQR114C 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. Fix this by following the pattern used in aqr105_get_features(): call genphy_c45_pma_read_abilities() as the base, then set the 2.5G bit explicitly since the PHY does not report it correctly in PMA registers. Also fix aqr111_get_features() to check and propagate the return value of aqr115c_get_features(), which can now fail since it calls genphy_c45_pma_read_abilities(). Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com> --- drivers/net/phy/aquantia/aquantia_main.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c index e95d391a6dc2..ed5631f2956e 100644 --- a/drivers/net/phy/aquantia/aquantia_main.c +++ b/drivers/net/phy/aquantia/aquantia_main.c @@ -1071,23 +1071,25 @@ static int aqr_gen1_resume(struct phy_device *phydev) static int aqr115c_get_features(struct phy_device *phydev) { - unsigned long *supported = phydev->supported; + int ret; - /* 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); + ret = genphy_c45_pma_read_abilities(phydev); + if (ret) + return ret; + + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported); return 0; } 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks 2026-10-16 3:23 ` [PATCH net-next v3 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks Hongmei Xu @ 2026-10-16 21:19 ` Andrew Lunn 2026-10-19 18:26 ` netdev-bot+sashiko 1 sibling, 0 replies; 12+ messages in thread From: Andrew Lunn @ 2026-10-16 21:19 UTC (permalink / raw) To: Hongmei Xu Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Fri, Oct 16, 2026 at 11:23:18AM +0800, Hongmei Xu wrote: > aqr115c_get_features() populates phydev->supported by ORing in > phy_gbit_features and setting the 2.5G bit directly. This skips > genphy_c45_pma_read_abilities(), which is responsible for reading > EEE capabilities and filling phydev->supported_eee. > > As a result, phy_probe() never populates supported_eee for AQR111, > AQR114C 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. > > Fix this by following the pattern used in aqr105_get_features(): call > genphy_c45_pma_read_abilities() as the base, then set the 2.5G bit > explicitly since the PHY does not report it correctly in PMA registers. > > Also fix aqr111_get_features() to check and propagate the return value > of aqr115c_get_features(), which can now fail since it calls > genphy_c45_pma_read_abilities(). > > Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks @ 2026-10-19 18:26 ` netdev-bot+sashiko 2026-10-22 10:19 ` Hongmei Xu 0 siblings, 1 reply; 12+ messages in thread From: netdev-bot+sashiko @ 2026-10-19 18:26 UTC (permalink / raw) To: hongmei.xu Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev, linux-kernel Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider. Critical: 0 · High: 1 · Medium: 2 · 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). - [High] This patch makes aqr115c_get_features() (drivers/net/phy/aquantia/aquantia_main.c:1072) take phydev->supported… - [Medium] The commit message misdescribes the hardware and understates the impact. - [Medium] genphy_c45_read_eee_abilities() runs at the end of genphy_c45_pma_read_abilities(). - [Low] The patch fixes a user-visible regression (EEE not reported or configurable through ethtool on… -- net: phy: aquantia: restore EEE discovery in get_features callbacks This patch changes aqr115c_get_features() to build phydev->supported with genphy_c45_pma_read_abilities() and then set the 2.5G bit. Previously it used phy_gbit_features plus 2.5G. The aim is to fill phydev->supported_eee so ethtool can show and configure EEE. aqr111_get_features() now also returns the error from aqr115c_get_features(). > As a result, phy_probe() never populates supported_eee for AQR111, > AQR114C and AQR115C. The kernel then clears advertising_eee: [ ... ] > Fix this by following the pattern used in aqr105_get_features(): call > genphy_c45_pma_read_abilities() as the base, then set the 2.5G bit > explicitly since the PHY does not report it correctly in PMA registers. [Severity: Medium] Is this description of the hardware accurate? aqr105_get_features() is for a PHY that fails to report 2.5G and 5G: /* The AQR105 PHY misses to indicate the 2.5G and 5G modes, so add them * here */ Earlier commits for this PHY family describe the opposite problem. Commit 17cbfcdd85f6 ("net: phy: aquantia: AQR115c fix up PMA capabilities") says: AQR115c reports incorrect PMA capabilities which includes 10G/5G and also incorrectly disables capabilities like autoneg and 10Mbps support. Commit 0974f1f03b07 says AQCS109 reports 10G and 5G but doesn't support them. Commit 8f61d73306c6 removed an aqr111_config_init() comment saying AQR111 reports speeds up to 10G but only supports up to 5G. That history is the reason for the "PMA capabilities are not useful" comments. The patch removes them without saying why the PMA registers can now be trusted. The list of affected parts also looks incomplete. In aqr_driver[], AQCS109, AQR111B0 and AQR115 use these same callbacks. The message describes an EEE-only change, but the supported and advertised link modes change too. 10G/5G may be added, and Autoneg, 10baseT, 1000baseT_Half and TP may be lost. Should the commit message say so? > Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com> [Severity: Low] Should this patch have a Fixes: tag? The missing EEE discovery seems to come from 17cbfcdd85f6 ("net: phy: aquantia: AQR115c fix up PMA capabilities") and 8f61d73306c6 ("net: phy: aquantia: remove usage of phy_set_max_speed"). Those commits added ->get_features callbacks that skip genphy_c45_pma_read_abilities(). phy_probe() only reads EEE abilities on the static ->features path: if (phydrv->features) { linkmode_copy(phydev->supported, phydrv->features); genphy_c45_read_eee_abilities(phydev); } else if (phydrv->get_features) err = phydrv->get_features(phydev); Before 8f61d73306c6, AQR111, AQR114C and AQCS109 had no get_features callback. They used the default genphy_c45_pma_read_abilities() path, which does read the EEE abilities. > diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c > index e95d391a6dc23..ed5631f2956ee 100644 > --- a/drivers/net/phy/aquantia/aquantia_main.c > +++ b/drivers/net/phy/aquantia/aquantia_main.c > @@ -1071,23 +1071,25 @@ static int aqr_gen1_resume(struct phy_device *phydev) > > static int aqr115c_get_features(struct phy_device *phydev) > { > - unsigned long *supported = phydev->supported; > + int ret; > > - /* 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); > + ret = genphy_c45_pma_read_abilities(phydev); > + if (ret) > + return ret; [Severity: High] Does this bring back the wrong speed capabilities that 17cbfcdd85f6, 0974f1f03b07 and 8f61d73306c6 fixed? AQCS109, AQR115 and AQR115C use this callback. aqr111_get_features() calls it too, so AQR111, AQR111B0 and AQR114C are also affected. genphy_c45_pma_read_ext_abilities() sets 10000baseT_Full from MDIO_PMA_EXTABLE_10GBT and 5000baseT_Full from MDIO_PMA_NG_EXTABLE_5GBT. The new code only sets bits after that and never clears any. As a result: - the 2.5G parts (AQCS109, AQR115, AQR115C) would report 5G and 10G again - the 5G parts (AQR111, AQR111B0, AQR114C) would report 10G again The modes that phy_gbit_features used to supply (Autoneg, 10baseT, 1000baseT_Half, TP) are also no longer forced on. genphy_c45_pma_read_abilities() starts with: linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported); It sets the bit again only if AN STAT1 reports ABLE. 17cbfcdd85f6 says AQR115C misreports this. If it does, phy_probe() runs: if (!linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported)) phydev->autoneg = 0; aqr_config_aneg() then calls genphy_c45_pma_setup_forced(), and 2.5GBASE-T can't link without autoneg. If autoneg stays enabled, phy_advertise_supported() copies the false 10G/5G modes into advertising. aqr_config_aneg() then advertises them to the link partner through genphy_c45_an_config_aneg(). The next patch in this series, "net: phy: aquantia: add get_features() for AQR113C", builds on aqr111_get_features() and doesn't fix this. Could the hard-coded speed mask stay, with a call to genphy_c45_read_eee_abilities(phydev) after it is built? phy_probe() already does this for drivers with static ->features. > + > + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported); [Severity: Medium] Can supported_eee still be missing the 2.5G and 5G EEE modes? genphy_c45_read_eee_abilities() runs at the end of genphy_c45_pma_read_abilities(). That happens before this line sets 2500baseT_Full, and before aqr111_get_features() sets 5000baseT_Full. It reads the EEE capability 2 register (2.5G/5G EEE) only in this case: if (linkmode_intersects(phydev->supported, PHY_EEE_CAP2_FEATURES)) { val = genphy_c45_read_eee_cap2(phydev); PHY_EEE_CAP2_FEATURES contains only 2500baseT_Full and 5000baseT_Full. The commit message says the PHY doesn't report 2.5G in the PMA registers. If MDIO_PMA_NG_EXTABLE reports neither 2.5G nor 5G, cap2 is never read. phy_probe() then drops these modes from advertising_eee, and ethtool --set-eee can't enable EEE at 2.5G or 5G. Would calling genphy_c45_read_eee_abilities() after the final supported mask is built avoid this? [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-net-phy-aqr113c-fix-pma-capability-v3-0-0bbb1b84035d%40oss.qualcomm.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks 2026-10-19 18:26 ` netdev-bot+sashiko @ 2026-10-22 10:19 ` Hongmei Xu 0 siblings, 0 replies; 12+ messages in thread From: Hongmei Xu @ 2026-10-22 10:19 UTC (permalink / raw) To: netdev-bot+sashiko Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev, linux-kernel The High issue is legitimate. The v4 keeps the phy_gbit_features mask and calls genphy_c45_read_eee_abilities() explicitly after the supported mask is final. Commit message and Fixes: tags corrected. pw-bot: cr On 10/20/2026 2:26 AM, netdev-bot+sashiko@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 4 potential > issue(s) to consider. > > Critical: 0 · High: 1 · Medium: 2 · 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). > > - [High] This patch makes aqr115c_get_features() > (drivers/net/phy/aquantia/aquantia_main.c:1072) take phydev->supported… > - [Medium] The commit message misdescribes the hardware and understates > the impact. > - [Medium] genphy_c45_read_eee_abilities() runs at the end of > genphy_c45_pma_read_abilities(). > - [Low] The patch fixes a user-visible regression (EEE not reported or > configurable through ethtool on… > > -- > > net: phy: aquantia: restore EEE discovery in get_features callbacks > > This patch changes aqr115c_get_features() to build phydev->supported with > genphy_c45_pma_read_abilities() and then set the 2.5G bit. Previously it > used phy_gbit_features plus 2.5G. The aim is to fill > phydev->supported_eee so ethtool can show and configure EEE. > aqr111_get_features() now also returns the error from > aqr115c_get_features(). > >> As a result, phy_probe() never populates supported_eee for AQR111, >> AQR114C and AQR115C. The kernel then clears advertising_eee: > > [ ... ] > >> Fix this by following the pattern used in aqr105_get_features(): call >> genphy_c45_pma_read_abilities() as the base, then set the 2.5G bit >> explicitly since the PHY does not report it correctly in PMA registers. > > [Severity: Medium] > Is this description of the hardware accurate? aqr105_get_features() is > for a PHY that fails to report 2.5G and 5G: > > /* The AQR105 PHY misses to indicate the 2.5G and 5G modes, so add them > * here > */ > > Earlier commits for this PHY family describe the opposite problem. Commit > 17cbfcdd85f6 ("net: phy: aquantia: AQR115c fix up PMA capabilities") says: > > AQR115c reports incorrect PMA capabilities which includes 10G/5G and > also incorrectly disables capabilities like autoneg and 10Mbps support. > > Commit 0974f1f03b07 says AQCS109 reports 10G and 5G but doesn't support > them. Commit 8f61d73306c6 removed an aqr111_config_init() comment saying > AQR111 reports speeds up to 10G but only supports up to 5G. > > That history is the reason for the "PMA capabilities are not useful" > comments. The patch removes them without saying why the PMA registers can > now be trusted. > > The list of affected parts also looks incomplete. In aqr_driver[], > AQCS109, AQR111B0 and AQR115 use these same callbacks. > > The message describes an EEE-only change, but the supported and > advertised link modes change too. 10G/5G may be added, and Autoneg, > 10baseT, 1000baseT_Half and TP may be lost. Should the commit message > say so? > >> Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com> > > [Severity: Low] > Should this patch have a Fixes: tag? The missing EEE discovery seems to > come from 17cbfcdd85f6 ("net: phy: aquantia: AQR115c fix up PMA > capabilities") and 8f61d73306c6 ("net: phy: aquantia: remove usage of > phy_set_max_speed"). Those commits added ->get_features callbacks that > skip genphy_c45_pma_read_abilities(). > > phy_probe() only reads EEE abilities on the static ->features path: > > if (phydrv->features) { > linkmode_copy(phydev->supported, phydrv->features); > genphy_c45_read_eee_abilities(phydev); > } > else if (phydrv->get_features) > err = phydrv->get_features(phydev); > > Before 8f61d73306c6, AQR111, AQR114C and AQCS109 had no get_features > callback. They used the default genphy_c45_pma_read_abilities() path, > which does read the EEE abilities. > >> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c >> index e95d391a6dc23..ed5631f2956ee 100644 >> --- a/drivers/net/phy/aquantia/aquantia_main.c >> +++ b/drivers/net/phy/aquantia/aquantia_main.c >> @@ -1071,23 +1071,25 @@ static int aqr_gen1_resume(struct phy_device *phydev) >> >> static int aqr115c_get_features(struct phy_device *phydev) >> { >> - unsigned long *supported = phydev->supported; >> + int ret; >> >> - /* 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); >> + ret = genphy_c45_pma_read_abilities(phydev); >> + if (ret) >> + return ret; > > [Severity: High] > Does this bring back the wrong speed capabilities that 17cbfcdd85f6, > 0974f1f03b07 and 8f61d73306c6 fixed? > > AQCS109, AQR115 and AQR115C use this callback. aqr111_get_features() > calls it too, so AQR111, AQR111B0 and AQR114C are also affected. > > genphy_c45_pma_read_ext_abilities() sets 10000baseT_Full from > MDIO_PMA_EXTABLE_10GBT and 5000baseT_Full from MDIO_PMA_NG_EXTABLE_5GBT. > The new code only sets bits after that and never clears any. As a > result: > > - the 2.5G parts (AQCS109, AQR115, AQR115C) would report 5G and 10G > again > - the 5G parts (AQR111, AQR111B0, AQR114C) would report 10G again > > The modes that phy_gbit_features used to supply (Autoneg, 10baseT, > 1000baseT_Half, TP) are also no longer forced on. > genphy_c45_pma_read_abilities() starts with: > > linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported); > > It sets the bit again only if AN STAT1 reports ABLE. 17cbfcdd85f6 says > AQR115C misreports this. If it does, phy_probe() runs: > > if (!linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, > phydev->supported)) > phydev->autoneg = 0; > > aqr_config_aneg() then calls genphy_c45_pma_setup_forced(), and > 2.5GBASE-T can't link without autoneg. > > If autoneg stays enabled, phy_advertise_supported() copies the false > 10G/5G modes into advertising. aqr_config_aneg() then advertises them to > the link partner through genphy_c45_an_config_aneg(). > > The next patch in this series, "net: phy: aquantia: add get_features() > for AQR113C", builds on aqr111_get_features() and doesn't fix this. > > Could the hard-coded speed mask stay, with a call to > genphy_c45_read_eee_abilities(phydev) after it is built? phy_probe() > already does this for drivers with static ->features. > >> + >> + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported); > > [Severity: Medium] > Can supported_eee still be missing the 2.5G and 5G EEE modes? > genphy_c45_read_eee_abilities() runs at the end of > genphy_c45_pma_read_abilities(). That happens before this line sets > 2500baseT_Full, and before aqr111_get_features() sets 5000baseT_Full. > > It reads the EEE capability 2 register (2.5G/5G EEE) only in this case: > > if (linkmode_intersects(phydev->supported, PHY_EEE_CAP2_FEATURES)) { > val = genphy_c45_read_eee_cap2(phydev); > > PHY_EEE_CAP2_FEATURES contains only 2500baseT_Full and 5000baseT_Full. > > The commit message says the PHY doesn't report 2.5G in the PMA > registers. If MDIO_PMA_NG_EXTABLE reports neither 2.5G nor 5G, cap2 is > never read. phy_probe() then drops these modes from advertising_eee, and > ethtool --set-eee can't enable EEE at 2.5G or 5G. > > Would calling genphy_c45_read_eee_abilities() after the final supported > mask is built avoid this? > > [ ... ] > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next v3 2/2] net: phy: aquantia: add get_features() for AQR113C 2026-10-16 3:23 [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery Hongmei Xu 2026-10-16 3:23 ` [PATCH net-next v3 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks Hongmei Xu @ 2026-10-16 3:23 ` Hongmei Xu 2026-10-16 21:20 ` Andrew Lunn 2026-10-19 18:26 ` netdev-bot+sashiko 2026-10-21 20:29 ` [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery Jakub Kicinski 2 siblings, 2 replies; 12+ messages in thread From: Hongmei Xu @ 2026-10-16 3:23 UTC (permalink / raw) To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni Cc: netdev, linux-kernel, Hongmei Xu AQR113c does not correctly report its supported PMA capabilities which includes speeds like 10G/5G/2.5G. Add get_features() to explicitly declare 10G/5G/2.5G/1G support instead of relying on PMA register reads. 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. 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 ed5631f2956e..8213d9a15b4f 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) +{ + /* Generic C45 PMA abilities do not fully describe the PHY's copper + * autonegotiation modes, so add the 10G mode 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net: phy: aquantia: add get_features() for AQR113C 2026-10-16 3:23 ` [PATCH net-next v3 2/2] net: phy: aquantia: add get_features() for AQR113C Hongmei Xu @ 2026-10-16 21:20 ` Andrew Lunn 2026-10-19 18:26 ` netdev-bot+sashiko 1 sibling, 0 replies; 12+ messages in thread From: Andrew Lunn @ 2026-10-16 21:20 UTC (permalink / raw) To: Hongmei Xu Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Fri, Oct 16, 2026 at 11:23:19AM +0800, Hongmei Xu wrote: > AQR113c does not correctly report its supported PMA capabilities > which includes speeds like 10G/5G/2.5G. Add get_features() to > explicitly declare 10G/5G/2.5G/1G support instead of relying on > PMA register reads. > > 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. > > Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net: phy: aquantia: add get_features() for AQR113C @ 2026-10-19 18:26 ` netdev-bot+sashiko 2026-10-22 10:20 ` Hongmei Xu 0 siblings, 1 reply; 12+ messages in thread From: netdev-bot+sashiko @ 2026-10-19 18:26 UTC (permalink / raw) To: hongmei.xu Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev, linux-kernel 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] The commit message says the patch adds get_features() to "explicitly declare 10G/5G/2.5G/1G support instead of relying on PMA… - [Low] The patch is written as a fix for a hardware-reporting defect ("AQR113c does not correctly report its supported PMA capabilities"),… -- net: phy: aquantia: add get_features() for AQR113C This adds an aqr113c_get_features() callback to the AQR113C entry in aqr_driver[]. The callback builds on aqr111_get_features() and also sets 10000baseT_Full in phydev->supported. The commit message says the PHY does not correctly report its PMA capabilities. > diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c > index ed5631f2956ee..8213d9a15b4fe 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) > +{ > + /* Generic C45 PMA abilities do not fully describe the PHY's copper > + * autonegotiation modes, so add the 10G mode 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; > +} [Severity: Medium] Does the commit message match what this code does? It says: Add get_features() to explicitly declare 10G/5G/2.5G/1G support instead of relying on PMA register reads. The call chain still starts with the generic PMA reads: aqr113c_get_features() aqr111_get_features() aqr115c_get_features() genphy_c45_pma_read_abilities() So phydev->supported is still built from the PMA registers. On top of that, the chain only adds 2500baseT_Full in aqr115c_get_features(), 5000baseT_Full in aqr111_get_features() and 10000baseT_Full here. Nothing in this chain sets 1000baseT_Full explicitly. It still comes only from the PMA extended abilities register: drivers/net/phy/phy-c45.c:genphy_c45_pma_read_ext_abilities() { ... linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, phydev->supported, val & MDIO_PMA_EXTABLE_1000BT); ... } The comment in the function gives a third version ("add the 10G mode explicitly"). It does not mention that 5G and 2.5G are also forced through the callees. Could the commit message and the comment say which abilities the AQR113C PMA registers (MDIO_PMA_EXTABLE / MDIO_PMA_NG_EXTABLE) actually misreport? For example, if the PHY already reports 10GBASE-T in MDIO_PMA_EXTABLE, the extra linkmode_set_bit() here does nothing. [ ... ] > @@ -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, [Severity: Low] The commit message describes this as a fix for a hardware reporting problem. Should it have a Fixes: tag? This commit added the AQR113C entry in aqr_driver[] without a get_features callback: commit 12cf1b89a668 ("net: phy: Add support for AQR113C EPHY") Because of that, phy_probe() has always used genphy_c45_pma_read_abilities() for this PHY: drivers/net/phy/phy_device.c:phy_probe() { ... else if (phydrv->get_features) err = phydrv->get_features(phydev); else if (phydev->is_c45) err = genphy_c45_pma_read_abilities(phydev); ... } Earlier fixes for the same quirk on related Aquantia PHYs had Fixes: tags. Two examples are 17cbfcdd85f6 ("net: phy: aquantia: AQR115c fix up PMA capabilities") and 8f61d73306c6 ("net: phy: aquantia: remove usage of phy_set_max_speed"). Would something like this be appropriate? Fixes: 12cf1b89a668 ("net: phy: Add support for AQR113C EPHY") Without the tag, stable kernels that support AQR113C may not pick up this change. [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-net-phy-aqr113c-fix-pma-capability-v3-0-0bbb1b84035d%40oss.qualcomm.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net: phy: aquantia: add get_features() for AQR113C 2026-10-19 18:26 ` netdev-bot+sashiko @ 2026-10-22 10:20 ` Hongmei Xu 0 siblings, 0 replies; 12+ messages in thread From: Hongmei Xu @ 2026-10-22 10:20 UTC (permalink / raw) To: netdev-bot+sashiko Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev, linux-kernel The commit message is inaccurate in v3 — genphy_c45_pma_read_abilities() is still called via the chain. The v4 commit message corrects this. Fixes: tag added. On 10/20/2026 2:26 AM, 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] The commit message says the patch adds get_features() to > "explicitly declare 10G/5G/2.5G/1G support instead of relying on PMA… > - [Low] The patch is written as a fix for a hardware-reporting defect > ("AQR113c does not correctly report its supported PMA capabilities"),… > > -- > > net: phy: aquantia: add get_features() for AQR113C > > This adds an aqr113c_get_features() callback to the AQR113C entry in > aqr_driver[]. The callback builds on aqr111_get_features() and also sets > 10000baseT_Full in phydev->supported. The commit message says the PHY > does not correctly report its PMA capabilities. > >> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c >> index ed5631f2956ee..8213d9a15b4fe 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) >> +{ >> + /* Generic C45 PMA abilities do not fully describe the PHY's copper >> + * autonegotiation modes, so add the 10G mode 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; >> +} > > [Severity: Medium] > Does the commit message match what this code does? It says: > > Add get_features() to explicitly declare 10G/5G/2.5G/1G support > instead of relying on PMA register reads. > > The call chain still starts with the generic PMA reads: > > aqr113c_get_features() > aqr111_get_features() > aqr115c_get_features() > genphy_c45_pma_read_abilities() > > So phydev->supported is still built from the PMA registers. On top of > that, the chain only adds 2500baseT_Full in aqr115c_get_features(), > 5000baseT_Full in aqr111_get_features() and 10000baseT_Full here. > > Nothing in this chain sets 1000baseT_Full explicitly. It still comes only > from the PMA extended abilities register: > > drivers/net/phy/phy-c45.c:genphy_c45_pma_read_ext_abilities() { > ... > linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, > phydev->supported, > val & MDIO_PMA_EXTABLE_1000BT); > ... > } > > The comment in the function gives a third version ("add the 10G mode > explicitly"). It does not mention that 5G and 2.5G are also forced > through the callees. > > Could the commit message and the comment say which abilities the AQR113C > PMA registers (MDIO_PMA_EXTABLE / MDIO_PMA_NG_EXTABLE) actually > misreport? For example, if the PHY already reports 10GBASE-T in > MDIO_PMA_EXTABLE, the extra linkmode_set_bit() here does nothing. > > [ ... ] > >> @@ -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, > > [Severity: Low] > The commit message describes this as a fix for a hardware reporting > problem. Should it have a Fixes: tag? > > This commit added the AQR113C entry in aqr_driver[] without a > get_features callback: > > commit 12cf1b89a668 ("net: phy: Add support for AQR113C EPHY") > > Because of that, phy_probe() has always used > genphy_c45_pma_read_abilities() for this PHY: > > drivers/net/phy/phy_device.c:phy_probe() { > ... > else if (phydrv->get_features) > err = phydrv->get_features(phydev); > else if (phydev->is_c45) > err = genphy_c45_pma_read_abilities(phydev); > ... > } > > Earlier fixes for the same quirk on related Aquantia PHYs had Fixes: > tags. Two examples are 17cbfcdd85f6 ("net: phy: aquantia: AQR115c fix up > PMA capabilities") and 8f61d73306c6 ("net: phy: aquantia: remove usage > of phy_set_max_speed"). > > Would something like this be appropriate? > > Fixes: 12cf1b89a668 ("net: phy: Add support for AQR113C EPHY") > > Without the tag, stable kernels that support AQR113C may not pick up this > change. > > [ ... ] > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery 2026-10-16 3:23 [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery Hongmei Xu 2026-10-16 3:23 ` [PATCH net-next v3 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks Hongmei Xu 2026-10-16 3:23 ` [PATCH net-next v3 2/2] net: phy: aquantia: add get_features() for AQR113C Hongmei Xu @ 2026-10-21 20:29 ` Jakub Kicinski 2026-10-22 10:22 ` Hongmei Xu 2 siblings, 1 reply; 12+ messages in thread From: Jakub Kicinski @ 2026-10-21 20:29 UTC (permalink / raw) To: Hongmei Xu Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet, Paolo Abeni, netdev, linux-kernel On Fri, 16 Oct 2026 11:23:17 +0800 Hongmei Xu wrote: > This series fixes two related issues in the Aquantia PHY driver: > - Patch 1 restores EEE discovery for AQR111/114C/115C by replacing > the manual linkmode_or() with genphy_c45_pma_read_abilities(). > - Patch 2 adds get_features() for AQR113C to correctly report > 10G/5G/2.5G/1G PMA capabilities. You need to respond to AI robots in a timely fashion. Since you haven't and the merge window has started on Monday we have to drop this, please repost for the next release cycle ## Form letter - net-next-closed The merge window for Linux v7.4 has begun, and the net-next tree is now closed for new drivers, features, code refactoring and optimizations. We are currently accepting bug fixes only. Please repost when net-next reopens. There is some uncertainty about how long this merge window will last, so please check https://netdev.bots.linux.dev/net-next.html before reposting. net-next _may_ reopen on Nov 2nd. See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle -- pw-bot: defer ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery 2026-10-21 20:29 ` [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery Jakub Kicinski @ 2026-10-22 10:22 ` Hongmei Xu 0 siblings, 0 replies; 12+ messages in thread From: Hongmei Xu @ 2026-10-22 10:22 UTC (permalink / raw) To: Jakub Kicinski Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet, Paolo Abeni, netdev, linux-kernel Thank you for the explanation. I apologize for not responding to the netdev-bot+sashiko review in time. The v4 series addresses all the raised issues. I will repost when net-next reopens. On 10/22/2026 4:29 AM, Jakub Kicinski wrote: > On Fri, 16 Oct 2026 11:23:17 +0800 Hongmei Xu wrote: >> This series fixes two related issues in the Aquantia PHY driver: >> - Patch 1 restores EEE discovery for AQR111/114C/115C by replacing >> the manual linkmode_or() with genphy_c45_pma_read_abilities(). >> - Patch 2 adds get_features() for AQR113C to correctly report >> 10G/5G/2.5G/1G PMA capabilities. > > You need to respond to AI robots in a timely fashion. > Since you haven't and the merge window has started on Monday > we have to drop this, please repost for the next release > cycle > > ## Form letter - net-next-closed > > The merge window for Linux v7.4 has begun, and the net-next > tree is now closed for new drivers, features, code refactoring > and optimizations. We are currently accepting bug fixes only. > > Please repost when net-next reopens. There is some uncertainty > about how long this merge window will last, so please check > https://netdev.bots.linux.dev/net-next.html > before reposting. net-next _may_ reopen on Nov 2nd. > > See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery @ 2026-10-16 3:28 netdev-bot+sinfo 0 siblings, 0 replies; 12+ messages in thread From: netdev-bot+sinfo @ 2026-10-16 3:28 UTC (permalink / raw) To: Hongmei Xu Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel Hi! This is an automated message. This series looks like a fix, but its commit messages seem to be missing some information: - How the issue was discovered, e.g. hit in production, hit during development, syzbot report, manual code inspection, LLM or static analysis tool scan. - Whether the issue was actually triggered, or is only theoretical (e.g. found by code inspection). If it was triggered please include the symptoms, like the stack trace or error messages. Please do not repost the series just to address the above. Instead, reply to this email with the missing information, so that reviewers can take it into account. If the series needs another revision for other reasons, please include the information in the commit messages then. The evaluation is done by an LLM so it may be wrong, if you think that is the case please reply and explain. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-10-22 10:22 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-16 3:23 [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery Hongmei Xu 2026-10-16 3:23 ` [PATCH net-next v3 1/2] net: phy: aquantia: restore EEE discovery in get_features callbacks Hongmei Xu 2026-10-16 21:19 ` Andrew Lunn 2026-10-19 18:26 ` netdev-bot+sashiko 2026-10-22 10:19 ` Hongmei Xu 2026-10-16 3:23 ` [PATCH net-next v3 2/2] net: phy: aquantia: add get_features() for AQR113C Hongmei Xu 2026-10-16 21:20 ` Andrew Lunn 2026-10-19 18:26 ` netdev-bot+sashiko 2026-10-22 10:20 ` Hongmei Xu 2026-10-21 20:29 ` [PATCH net-next v3 0/2] net: phy: aquantia: fix PMA capability reporting and EEE discovery Jakub Kicinski 2026-10-22 10:22 ` Hongmei Xu 2026-10-16 3:28 netdev-bot+sinfo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®