[PATCH net-next v2] net: phy: aquantia: Poll global status register for provision ID

[PATCH net-next v2] net: phy: aquantia: Poll global status register for provision ID

[Hongmei Xu]

According to the AQR113 PHY databook, after firmware reset, a 20ms
timer needs to be engaged to allow the supplies to settle prior to
allowing the PHY to boot. The working flow is:
  -After the 20 ms timer has completed, hardware state machines
   designed to guarantee PLL and band-gap stability engage.
  -After the PLLs and band-gap are locked and functioning properly,
   the processor and digital circuitry are released from reset.
  -The PHY image is then loaded through MDIO boot-load and the
   processor boots.
  -After the processor boots, any provisioned register values are
   set, and the PHY enters the provisioned operating state.

Starting subsequent firmware operations before the provision ID is
valid leads to steady link failures; this has been observed during
AQR113 PHY bring-up.

Also the provisioning ID is a component of the firmware fingerprint
used to identify the host interface mode for PHYs like AQR412C. When
such a PHY is provisioned with MUSX firmware, the fingerprint
determines whether the host interface should be reported as 10G-QXGMII
rather than USXGMII. Without a valid provisioning ID, the fingerprint
cannot be correctly built, making USXGMII and 10G-QXGMII
indistinguishable and potentially causing an interface mismatch.

Poll VEND1_GLOBAL_RSVD_STAT1 until the provision ID field is non-zero
before returning from aqr_wait_reset_complete(). A timeout indicates
a hardware-level provisioning failure that software cannot remediate;
the warning is logged to flag the hardware problem, but probe is not
failed since not all PHYs require a valid provision ID.

Co-developed-by: Sagar Cheluvegowda <sagar.cheluvegowda@oss.qualcomm.com>
Signed-off-by: Sagar Cheluvegowda <sagar.cheluvegowda@oss.qualcomm.com>
Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com>
---
Changes in v2:
- Change the log when provisioning ID poll times out to hint the
  mode mismatch possibility
- Update commit message to describe the AQR113 databook boot sequence
  and explain why polling PROV_ID is necessary
- Update commit message to explain that timeout prevents fingerprint
  construction for PHYs like AQR412C with MUSX firmware which in a result cause mode mismatch
- Link to v1: https://patch.msgid.link/20261029-aquantia-poll-global-status-register-v1-1-808089824a3d@oss.qualcomm.com
---
 drivers/net/phy/aquantia/aquantia_main.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index e95d391a6dc2..bed891cfaaa1 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -702,7 +702,23 @@ int aqr_wait_reset_complete(struct phy_device *phydev)
 		return val;
 	}
 
-	return ret;
+	if (ret)
+		return ret;
+
+	ret = read_poll_timeout(phy_read_mmd, val,
+				val < 0 ||
+				FIELD_GET(VEND1_GLOBAL_RSVD_STAT1_PROV_ID, val),
+				AQR_FW_WAIT_SLEEP_US, AQR_FW_WAIT_TIMEOUT_US,
+				false, phydev, MDIO_MMD_VEND1,
+				VEND1_GLOBAL_RSVD_STAT1);
+	if (val < 0)
+		phydev_warn(phydev, "Failed to read VEND1_GLOBAL_RSVD_STAT1: %pe\n",
+			    ERR_PTR(val));
+	else if (ret)
+		phydev_warn(phydev, "Firmware provisioning ID not ready; "
+			    "interface mode detection may be incorrect\n");
+
+	return 0;
 }
 
 static int aqr_build_fingerprint(struct phy_device *phydev)

---
base-commit: 944ae66642b726bd6b25ae71b1e9ff88a0e0bdb0
change-id: 20260923-aquantia-poll-global-status-register-d8966a46e56d

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

Re: [PATCH net-next v2] net: phy: aquantia: Poll global status register for provision ID

[Andrew Lunn]

> +	else if (ret)
> +		phydev_warn(phydev, "Firmware provisioning ID not ready; "
> +			    "interface mode detection may be incorrect\n");

Please don't split strings. Somebody is going to grep for "Firmware
provisioning ID not ready; interface mode detection may be incorrect"
when they see it in their logs and not be able to find it.

This is one exception we make to the 80 character rule.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next v2] net: phy: aquantia: Poll global status register for provision ID

[Hongmei Xu]

On Wed, Nov 11, 2026 at 9:40 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +     else if (ret)
> > +             phydev_warn(phydev, "Firmware provisioning ID not ready; "
> > +                         "interface mode detection may be incorrect\n");
>
> Please don't split strings. Somebody is going to grep for "Firmware
> provisioning ID not ready; interface mode detection may be incorrect"
> when they see it in their logs and not be able to find it.
>
> This is one exception we make to the 80 character rule.
Sure, make sense, will merge the log into one single line.

Regards,
Hongmei Xu

Re: [PATCH net-next v2] net: phy: aquantia: Poll global status register for provision ID

[netdev-bot+sashiko]

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

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

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] In aqr_wait_reset_complete()
  (drivers/net/phy/aquantia/aquantia_main.c:708-721), the new second…
- [Medium] net: phy: aquantia: propagate MDIO read error in
  aqr_wait_reset_complete()
- [Low] The comment above aqr_wait_reset_complete()
  (aquantia_main.c:684-690) was not updated.
- [Low] On a provisioning-ID timeout, aqr_wait_reset_complete()
  (aquantia_main.c:717-719) prints 'Firmware provisioning ID not ready;…
- [Low] net: phy: aquantia: missing Fixes trailer for
  aqr_wait_reset_complete() fix

--

net: phy: aquantia: Poll global status register for provision ID

This commit adds a second poll to aqr_wait_reset_complete(). Once the
firmware ID shows up, the poll waits for the provisioning ID field in
VEND1_GLOBAL_RSVD_STAT1 to become non-zero. If the poll times out, it
logs a warning and the function still returns 0. The stated reasons
are link failures seen on AQR113 and correct fingerprinting of AQR412C
MUSX firmware.

> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
> index e95d391a6dc23..bed891cfaaa1b 100644
> --- a/drivers/net/phy/aquantia/aquantia_main.c
> +++ b/drivers/net/phy/aquantia/aquantia_main.c
> @@ -702,7 +702,23 @@ int aqr_wait_reset_complete(struct phy_device *phydev)
>  		return val;
>  	}
>  
> -	return ret;
> +	if (ret)
> +		return ret;
> +
> +	ret = read_poll_timeout(phy_read_mmd, val,
> +				val < 0 ||
> +				FIELD_GET(VEND1_GLOBAL_RSVD_STAT1_PROV_ID, val),
> +				AQR_FW_WAIT_SLEEP_US, AQR_FW_WAIT_TIMEOUT_US,
> +				false, phydev, MDIO_MMD_VEND1,
> +				VEND1_GLOBAL_RSVD_STAT1);

[Severity: Low]
The commit message describes two problems: link failures seen during
AQR113 bring-up, and an AQR412C MUSX fingerprint that can't be built
correctly. Both come from aqr_wait_reset_complete() treating a
non-zero VEND1_GLOBAL_FW_ID as the end of chip initialization, before
the provisioning ID in VEND1_GLOBAL_RSVD_STAT1 is set.

If this is meant as a bug fix, should it carry a Fixes tag? The
FW_ID-only poll appears to go back to commit 43429a0353af ("net: phy:
aquantia: report PHY details like firmware version"). That commit
added aqr107_wait_reset_complete(), which was later renamed to
aqr_wait_reset_complete().

Without the tag, stable maintainers and backporters have no easy way
to tell that this change addresses a problem in released kernels. And
if it is a fix, should it go to net rather than net-next?

[Severity: Medium]
This poll only exits on a non-zero PROV_ID or a read error. If a
firmware image reports PROV_ID == 0, will every call sleep the full
AQR_FW_WAIT_TIMEOUT_US (2 s) and then return 0 anyway?

The commit message says "not all PHYs require a valid provision ID",
which suggests PROV_ID can stay 0 on some images.

In the driver, prov_id only matters for one fingerprint:

  AQR_G3_V4_3_C_AQR_NXP_SPF_30841_MUSX_ID40019_VER1198
      AQR_FW_FINGERPRINT(4, 3, 0xc, 1, 40019, 1198)

That fingerprint is only checked for PHY_ID_AQR412C, in
aqr_translate_interface().

aqr_wait_reset_complete() has two callers, and both now pay for this
wait:

  aqr107_probe()
    aqr_firmware_load()
      aqr_wait_reset_complete()

  phy_attach_direct() / mdio_bus_phy_resume()
    phy_init_hw()
      aqr_gen1_config_init()
        aqr_wait_reset_complete()

With such an image, would each port lose about 4 s across probe and
attach, and then 2 s plus a warning on every later attach and resume?
On quad-port parts like AQR412/AQR412C, that cost applies to each port.

Before this patch, both paths returned as soon as FW_ID was non-zero.
Could the PROV_ID wait be limited to the PHYs or firmware images that
need it?

> +	if (val < 0)
> +		phydev_warn(phydev, "Failed to read VEND1_GLOBAL_RSVD_STAT1: %pe\n",
> +			    ERR_PTR(val));
> +	else if (ret)
> +		phydev_warn(phydev, "Firmware provisioning ID not ready; "
> +			    "interface mode detection may be incorrect\n");

[Severity: Medium]
If phy_read_mmd() fails while polling VEND1_GLOBAL_RSVD_STAT1, the
val < 0 term ends read_poll_timeout() right away with ret == 0. The
code then logs a warning and returns 0, so the MDIO error is lost.

The VEND1_GLOBAL_FW_ID poll just above handles the same case with
phydev_err() and returns val. Is it intended that a read error here
is non-fatal? The commit message only says that the timeout case
should not fail probe.

Both callers act on the return value:

  aqr107_probe()
    aqr_firmware_load()
      aqr_wait_reset_complete()

aqr_firmware_load() treats 0 as "Some firmware is loaded => do
nothing". Any other negative value goes to the default branch, which
passes the PHY read error back to aqr107_probe(). With this change, an
MDIO failure on RSVD_STAT1 looks like ready firmware, and probe goes
on.

  phy_init_hw()
    aqr_gen1_config_init()
      aqr_wait_reset_complete()

aqr_gen1_config_init() only builds the fingerprint when the return
value is 0. After a read error it now goes on to
aqr_build_fingerprint() and the rest of the PHY configuration, even
though the provisioning ID was never seen.

Could the val < 0 case return val, the same way the FW_ID poll does,
and leave only the timeout as a warning?

[Severity: Low]
This warning can fire on every model that uses aqr107_probe() and
aqr_gen1_config_init(), such as AQR105, AQR107, AQCS109, AQR111,
AQR112, AQR113, AQR114C, AQR115 and AQR813. Is "interface mode
detection may be incorrect" accurate for those?

The provisioning ID only affects the interface mode in
aqr_translate_interface():

	if (phy_id_compare(phydev->drv->phy_id, PHY_ID_AQR412C, phydev->drv->phy_id_mask) &&
	    priv->fingerprint == AQR_G3_V4_3_C_AQR_NXP_SPF_30841_MUSX_ID40019_VER1198 &&
	    interface == PHY_INTERFACE_MODE_USXGMII)
		return PHY_INTERFACE_MODE_10G_QXGMII;

So on other models, a missing provisioning ID can't cause wrong
interface detection.

The message also doesn't mention the link failures that the commit
message describes for AQR113.

> +
> +	return 0;
>  }

[Severity: Low]
This isn't a bug, but the comment above aqr_wait_reset_complete() was
not updated:

/* If we configure settings whilst firmware is still initializing the chip,
 * then these settings may be overwritten. Therefore make sure chip
 * initialization has completed. Use presence of the firmware ID as
 * indicator for initialization having completed.
 * The chip also provides a "reset completed" bit, but it's cleared after
 * read. Therefore function would time out if called again.
 */

It still names FW_ID as the only sign that initialization is complete.
It doesn't mention the second, non-fatal wait on the provisioning ID.

Also, on PHYs where PROV_ID stays 0, every call from
aqr_firmware_load() and aqr_gen1_config_init() now times out again.
Isn't that the repeated-call timeout this comment says the design
avoids?

Could the comment be updated to match the new behavior?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111-aquantia-poll-global-status-register-v2-1-b602fc7e80a9%40oss.qualcomm.com

Re: [PATCH net-next v2] net: phy: aquantia: Poll global status register for provision ID

[Andrew Lunn]

> > +	if (val < 0)
> > +		phydev_warn(phydev, "Failed to read VEND1_GLOBAL_RSVD_STAT1: %pe\n",
> > +			    ERR_PTR(val));
> > +	else if (ret)
> > +		phydev_warn(phydev, "Firmware provisioning ID not ready; "
> > +			    "interface mode detection may be incorrect\n");
> 
> [Severity: Medium]
> If phy_read_mmd() fails while polling VEND1_GLOBAL_RSVD_STAT1, the
> val < 0 term ends read_poll_timeout() right away with ret == 0. The
> code then logs a warning and returns 0, so the MDIO error is lost.

This is one of the earliest reads from the device. If the MDIO bus is
going to fail, it will fail here, rather than later. So the error
handling should be correct. Please address this issue.

	 Andrew