[PATCH net] net: phy: micrel: fix LAN8814 QSGMII soft reset

[PATCH net] net: phy: micrel: fix LAN8814 QSGMII soft reset

[Robert Marko]

LAN8814 QSGMII soft reset was moved into the probe function to avoid
triggering it for each of 4 PHY-s in the package.

However, that broke QSGMII link between the MAC and PHY on most LAN8814
PHY-s, specificaly for us on the Microchip LAN969x switch.
Reading the QSGMII status registers it was visible that lanes were only
partially synced.

It looks like the reset timing is crucial, so lets move the reset back
into the .config_init function but guard it with phy_package_init_once()
to avoid it being triggered on each of 4 PHY-s in the package.
Change the probe function to use phy_package_probe_once() for coma and PtP
setup.

Fixes: 96a9178a29a6 ("net: phy: micrel: lan8814 fix reset of the QSGMII interface")
Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 drivers/net/phy/micrel.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 2aa1dedd21b8..e211a523c258 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -4548,6 +4548,13 @@ static int lan8814_config_init(struct phy_device *phydev)
 	struct kszphy_priv *lan8814 = phydev->priv;
 	int ret;
 
+	if (phy_package_init_once(phydev))
+		/* Reset the PHY */
+		lanphy_modify_page_reg(phydev, LAN8814_PAGE_COMMON_REGS,
+				       LAN8814_QSGMII_SOFT_RESET,
+				       LAN8814_QSGMII_SOFT_RESET_BIT,
+				       LAN8814_QSGMII_SOFT_RESET_BIT);
+
 	/* Based on the interface type select how the advertise ability is
 	 * encoded, to set as SGMII or as USGMII.
 	 */
@@ -4655,13 +4662,7 @@ static int lan8814_probe(struct phy_device *phydev)
 	priv->is_ptp_available = err == LAN8814_REV_LAN8814 ||
 				 err == LAN8814_REV_LAN8818;
 
-	if (phy_package_init_once(phydev)) {
-		/* Reset the PHY */
-		lanphy_modify_page_reg(phydev, LAN8814_PAGE_COMMON_REGS,
-				       LAN8814_QSGMII_SOFT_RESET,
-				       LAN8814_QSGMII_SOFT_RESET_BIT,
-				       LAN8814_QSGMII_SOFT_RESET_BIT);
-
+	if (phy_package_probe_once(phydev)) {
 		err = lan8814_release_coma_mode(phydev);
 		if (err)
 			return err;
-- 
2.54.0

Re: [PATCH net] net: phy: micrel: fix LAN8814 QSGMII soft reset

[Paolo Abeni]

On 4/28/26 3:41 PM, Robert Marko wrote:
> LAN8814 QSGMII soft reset was moved into the probe function to avoid
> triggering it for each of 4 PHY-s in the package.
> 
> However, that broke QSGMII link between the MAC and PHY on most LAN8814
> PHY-s, specificaly for us on the Microchip LAN969x switch.
> Reading the QSGMII status registers it was visible that lanes were only
> partially synced.
> 
> It looks like the reset timing is crucial, so lets move the reset back
> into the .config_init function but guard it with phy_package_init_once()
> to avoid it being triggered on each of 4 PHY-s in the package.
> Change the probe function to use phy_package_probe_once() for coma and PtP
> setup.
> 
> Fixes: 96a9178a29a6 ("net: phy: micrel: lan8814 fix reset of the QSGMII interface")
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
>  drivers/net/phy/micrel.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 2aa1dedd21b8..e211a523c258 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -4548,6 +4548,13 @@ static int lan8814_config_init(struct phy_device *phydev)
>  	struct kszphy_priv *lan8814 = phydev->priv;
>  	int ret;
>  
> +	if (phy_package_init_once(phydev))
> +		/* Reset the PHY */
> +		lanphy_modify_page_reg(phydev, LAN8814_PAGE_COMMON_REGS,
> +				       LAN8814_QSGMII_SOFT_RESET,
> +				       LAN8814_QSGMII_SOFT_RESET_BIT,
> +				       LAN8814_QSGMII_SOFT_RESET_BIT)

Sashiko says:

---
Could this introduce a race condition if multiple ports are brought up
concurrently?
Because phy_package_init_once() does not provide a synchronization
barrier for followers, they might proceed immediately to configure their
registers while the leader is still performing the reset.
---

on top of my head IDK if such race is possible at all.

/P

Re: [PATCH net] net: phy: micrel: fix LAN8814 QSGMII soft reset

[Andrew Lunn]

> > @@ -4548,6 +4548,13 @@ static int lan8814_config_init(struct phy_device *phydev)
> >  	struct kszphy_priv *lan8814 = phydev->priv;
> >  	int ret;
> >  
> > +	if (phy_package_init_once(phydev))
> > +		/* Reset the PHY */
> > +		lanphy_modify_page_reg(phydev, LAN8814_PAGE_COMMON_REGS,
> > +				       LAN8814_QSGMII_SOFT_RESET,
> > +				       LAN8814_QSGMII_SOFT_RESET_BIT,
> > +				       LAN8814_QSGMII_SOFT_RESET_BIT)
> 
> Sashiko says:
> 
> ---
> Could this introduce a race condition if multiple ports are brought up
> concurrently?
> Because phy_package_init_once() does not provide a synchronization
> barrier for followers, they might proceed immediately to configure their
> registers while the leader is still performing the reset.
> ---
> 
> on top of my head IDK if such race is possible at all.

config_init() is called from phy_init_hw(). That is called from
mdio_bus_phy_resume() and phy_attach_direct().

It seems unlikely resumes of devices on one bus is done in parallel,
same as probing of devices on one bus is not performed in parallel.

phy_attach_direct() is either used in the MAC drivers probe() or
open(). Again, probe should not be running in parallel especially
since this PHY is likely connect to a switch, and the ports are
created sequentially by the DSA core. open() should be protected by
RTNL.

So it seems unlikely to me.

lanphy_modify_page_reg() also takes the MDIO bus lock. That will
prevent any other MDIO operations being performed in parallel. This
does however make the assumption the software reset can be performed
within one MDIO bus cycle.

So a race here seems pretty theoretical to me.

    Andrew

Re: [PATCH net] net: phy: micrel: fix LAN8814 QSGMII soft reset

[Simon Horman]

On Tue, Apr 28, 2026 at 03:41:01PM +0200, Robert Marko wrote:
> LAN8814 QSGMII soft reset was moved into the probe function to avoid
> triggering it for each of 4 PHY-s in the package.
> 
> However, that broke QSGMII link between the MAC and PHY on most LAN8814
> PHY-s, specificaly for us on the Microchip LAN969x switch.
> Reading the QSGMII status registers it was visible that lanes were only
> partially synced.
> 
> It looks like the reset timing is crucial, so lets move the reset back
> into the .config_init function but guard it with phy_package_init_once()
> to avoid it being triggered on each of 4 PHY-s in the package.
> Change the probe function to use phy_package_probe_once() for coma and PtP
> setup.
> 
> Fixes: 96a9178a29a6 ("net: phy: micrel: lan8814 fix reset of the QSGMII interface")
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>

FTR: an AI generated review of this patch is available at sashiko.dev.
     I believe that both issues flagged there pre-date this patch
     and should not block progress of it.

Re: [PATCH net] net: phy: micrel: fix LAN8814 QSGMII soft reset

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 28 Apr 2026 15:41:01 +0200 you wrote:
> LAN8814 QSGMII soft reset was moved into the probe function to avoid
> triggering it for each of 4 PHY-s in the package.
> 
> However, that broke QSGMII link between the MAC and PHY on most LAN8814
> PHY-s, specificaly for us on the Microchip LAN969x switch.
> Reading the QSGMII status registers it was visible that lanes were only
> partially synced.
> 
> [...]

Here is the summary with links:
  - [net] net: phy: micrel: fix LAN8814 QSGMII soft reset
    https://git.kernel.org/netdev/net/c/e027c218c482

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html