[PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Ovidiu Panait]

During system suspend, phy_suspend() puts the PHY into Software Power-Down
(SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to the KSZ9131
datasheet, MMD register access is restricted during SPD:

  - Only access to the standard registers (0 through 31) is supported.
  - Access to MMD address spaces other than MMD address space 1 is
    possible if the spd_clock_gate_override bit is set.
  - Access to MMD address space 1 is not possible.

However, ksz9131_resume() calls ksz9131_config_rgmii_delay() before
kszphy_resume() clears BMCR_PDOWN. This means MMD registers are accessed
while the PHY is still in SPD, contrary to the datasheet.

Additionally, on platforms where the PHY loses power during suspend
(e.g. RZ/G3E), all settings from ksz9131_config_init(), not just the
RGMII delays, are lost and need to be restored. When the MAC driver
sets mac_managed_pm (e.g. stmmac), mdio_bus_phy_resume() is skipped,
so phy_init_hw() (which calls config_init to restore all PHY settings)
is never invoked during resume.

Fix this by replacing the RGMII delay restoration with a call to
phy_init_hw(), which takes the PHY out of SPD and performs full
reinitialization.

Fixes: f25a7eaa897f ("net: phy: micrel: Add ksz9131_resume()")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/net/phy/micrel.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 2aa1dedd21b8..4236dbf4ad6b 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -6016,8 +6016,13 @@ static int lan8841_suspend(struct phy_device *phydev)
 
 static int ksz9131_resume(struct phy_device *phydev)
 {
-	if (phydev->suspended && phy_interface_is_rgmii(phydev))
-		ksz9131_config_rgmii_delay(phydev);
+	int ret;
+
+	if (phydev->suspended) {
+		ret = phy_init_hw(phydev);
+		if (ret)
+			return ret;
+	}
 
 	return kszphy_resume(phydev);
 }
-- 
2.51.0

Re: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Nicolai Buchwitz]

On 3.4.2026 13:17, Ovidiu Panait wrote:
> During system suspend, phy_suspend() puts the PHY into Software 
> Power-Down
> (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to the 
> KSZ9131
> datasheet, MMD register access is restricted during SPD:
> 
>   - Only access to the standard registers (0 through 31) is supported.
>   - Access to MMD address spaces other than MMD address space 1 is
>     possible if the spd_clock_gate_override bit is set.
>   - Access to MMD address space 1 is not possible.
> 
> However, ksz9131_resume() calls ksz9131_config_rgmii_delay() before
> kszphy_resume() clears BMCR_PDOWN. This means MMD registers are 
> accessed
> while the PHY is still in SPD, contrary to the datasheet.
> 
> Additionally, on platforms where the PHY loses power during suspend
> (e.g. RZ/G3E), all settings from ksz9131_config_init(), not just the
> RGMII delays, are lost and need to be restored. When the MAC driver
> sets mac_managed_pm (e.g. stmmac), mdio_bus_phy_resume() is skipped,
> so phy_init_hw() (which calls config_init to restore all PHY settings)
> is never invoked during resume.
> 
> Fix this by replacing the RGMII delay restoration with a call to
> phy_init_hw(), which takes the PHY out of SPD and performs full
> reinitialization.
> 
> Fixes: f25a7eaa897f ("net: phy: micrel: Add ksz9131_resume()")
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> ---
>  drivers/net/phy/micrel.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 2aa1dedd21b8..4236dbf4ad6b 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -6016,8 +6016,13 @@ static int lan8841_suspend(struct phy_device 
> *phydev)
> 
>  static int ksz9131_resume(struct phy_device *phydev)
>  {
> -	if (phydev->suspended && phy_interface_is_rgmii(phydev))
> -		ksz9131_config_rgmii_delay(phydev);
> +	int ret;
> +
> +	if (phydev->suspended) {
> +		ret = phy_init_hw(phydev);
> +		if (ret)
> +			return ret;
> +	}

Fix looks correct.

nit: phy_init_hw() already clears PDOWN, so the PDOWN clear and
1ms sleep in kszphy_resume() become redundant. But this is probably
something for a future cleanup.

> 
>  	return kszphy_resume(phydev);
>  }

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Biju Das]

Hi Ovidiu Panait,

Thanks for the patch.

> -----Original Message-----
> From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> Sent: 03 April 2026 12:18
> Subject: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()
> 
> During system suspend, phy_suspend() puts the PHY into Software Power-Down
> (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to the KSZ9131 datasheet, MMD register
> access is restricted during SPD:
> 
>   - Only access to the standard registers (0 through 31) is supported.
>   - Access to MMD address spaces other than MMD address space 1 is
>     possible if the spd_clock_gate_override bit is set.
>   - Access to MMD address space 1 is not possible.
> 
> However, ksz9131_resume() calls ksz9131_config_rgmii_delay() before
> kszphy_resume() clears BMCR_PDOWN. This means MMD registers are accessed while the PHY is still in SPD,
> contrary to the datasheet.

SPD mode: This mode is used to power down the device when it is not in use after power-up.
          Previous register settings are maintained during and following the removal of SPD.

Suspend to Idle case, it is in SPD mode and the MMD register values are retained.

But in Suspend to RAM, PHY loses power and the reset value of Power Down bit 0 (ie normal mode), 
there is no restriction for accessing MMD registers in this mode.

Am I missing anything here??


Cheers,
Biju

RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Ovidiu Panait]

Hi,

> 
> Hi Ovidiu Panait,
> 
> Thanks for the patch.
> 
> > -----Original Message-----
> > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > Sent: 03 April 2026 12:18
> > Subject: [PATCH net] net: phy: micrel: Fix MMD register access during
> SPD in ksz9131_resume()
> >
> > During system suspend, phy_suspend() puts the PHY into Software Power-
> Down
> > (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to the
> KSZ9131 datasheet, MMD register
> > access is restricted during SPD:
> >
> >   - Only access to the standard registers (0 through 31) is supported.
> >   - Access to MMD address spaces other than MMD address space 1 is
> >     possible if the spd_clock_gate_override bit is set.
> >   - Access to MMD address space 1 is not possible.
> >
> > However, ksz9131_resume() calls ksz9131_config_rgmii_delay() before
> > kszphy_resume() clears BMCR_PDOWN. This means MMD registers are accessed
> while the PHY is still in SPD,
> > contrary to the datasheet.
> 
> SPD mode: This mode is used to power down the device when it is not in use
> after power-up.
>           Previous register settings are maintained during and following
> the removal of SPD.
> 
> Suspend to Idle case, it is in SPD mode and the MMD register values are
> retained.
> 

On resume from s2idle, ksz9131_resume() calls ksz9131_config_rgmii_delay()
which does MMD accesses, while the PHY is in SPD. According to the datasheet,
it shouldn't happen. See commit e398822c4751 ("net: phy: micrel: populate
.soft_reset for KSZ9131") which fixes the same issue.

> But in Suspend to RAM, PHY loses power and the reset value of Power Down
> bit 0 (ie normal mode),
> there is no restriction for accessing MMD registers in this mode.
> 

If the PHY loses power, it loses all the configuration that was done by
ksz9131_config_init(). Right now, only the RGMII delays are restored, which
is a subset of the configurations done by ksz9131_config_init().

Calling phy_init_hw() fixes both of these issues.

Ovidiu


> Am I missing anything here??
> 
> 
> Cheers,
> Biju

RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Biju Das]

> -----Original Message-----
> From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> Sent: 03 April 2026 14:23
> To: Biju Das <biju.das.jz@bp.renesas.com>; andrew@lunn.ch; hkallweit1@gmail.com; linux@armlinux.org.uk;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-renesas-soc@vger.kernel.org
> Subject: RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()
> 
> Hi,
> 
> >
> > Hi Ovidiu Panait,
> >
> > Thanks for the patch.
> >
> > > -----Original Message-----
> > > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > > Sent: 03 April 2026 12:18
> > > Subject: [PATCH net] net: phy: micrel: Fix MMD register access
> > > during
> > SPD in ksz9131_resume()
> > >
> > > During system suspend, phy_suspend() puts the PHY into Software
> > > Power-
> > Down
> > > (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to the
> > KSZ9131 datasheet, MMD register
> > > access is restricted during SPD:
> > >
> > >   - Only access to the standard registers (0 through 31) is supported.
> > >   - Access to MMD address spaces other than MMD address space 1 is
> > >     possible if the spd_clock_gate_override bit is set.
> > >   - Access to MMD address space 1 is not possible.
> > >
> > > However, ksz9131_resume() calls ksz9131_config_rgmii_delay() before
> > > kszphy_resume() clears BMCR_PDOWN. This means MMD registers are
> > > accessed
> > while the PHY is still in SPD,
> > > contrary to the datasheet.
> >
> > SPD mode: This mode is used to power down the device when it is not in
> > use after power-up.
> >           Previous register settings are maintained during and
> > following the removal of SPD.
> >
> > Suspend to Idle case, it is in SPD mode and the MMD register values
> > are retained.
> >
> 
> On resume from s2idle, ksz9131_resume() calls ksz9131_config_rgmii_delay() which does MMD accesses,
> while the PHY is in SPD. According to the datasheet, it shouldn't happen. See commit e398822c4751
> ("net: phy: micrel: populate .soft_reset for KSZ9131") which fixes the same issue.

On my board, while s2idle in SPD mode, it does not hang. The datasheet does not explain
the behaviour when it is SPD mode. But it states that it retains all previous register values
when it is out of SPD mode.

> 
> > But in Suspend to RAM, PHY loses power and the reset value of Power
> > Down bit 0 (ie normal mode), there is no restriction for accessing MMD
> > registers in this mode.
> >
> 
> If the PHY loses power, it loses all the configuration that was done by ksz9131_config_init(). Right
> now, only the RGMII delays are restored, which is a subset of the configurations done by
> ksz9131_config_init().

I agree, Only RZ/G3E reported this issue and with configuring only delays, it worked.

Calling ksz9131_config_init() restores more MMD registers or call phy_init() like this patch.

Cheers,
Biju

RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Ovidiu Panait]

> >
> > Hi,
> >
> > >
> > > Hi Ovidiu Panait,
> > >
> > > Thanks for the patch.
> > >
> > > > -----Original Message-----
> > > > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > > > Sent: 03 April 2026 12:18
> > > > Subject: [PATCH net] net: phy: micrel: Fix MMD register access
> > > > during
> > > SPD in ksz9131_resume()
> > > >
> > > > During system suspend, phy_suspend() puts the PHY into Software
> > > > Power-
> > > Down
> > > > (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to the
> > > KSZ9131 datasheet, MMD register
> > > > access is restricted during SPD:
> > > >
> > > >   - Only access to the standard registers (0 through 31) is
> supported.
> > > >   - Access to MMD address spaces other than MMD address space 1 is
> > > >     possible if the spd_clock_gate_override bit is set.
> > > >   - Access to MMD address space 1 is not possible.
> > > >
> > > > However, ksz9131_resume() calls ksz9131_config_rgmii_delay() before
> > > > kszphy_resume() clears BMCR_PDOWN. This means MMD registers are
> > > > accessed
> > > while the PHY is still in SPD,
> > > > contrary to the datasheet.
> > >
> > > SPD mode: This mode is used to power down the device when it is not in
> > > use after power-up.
> > >           Previous register settings are maintained during and
> > > following the removal of SPD.
> > >
> > > Suspend to Idle case, it is in SPD mode and the MMD register values
> > > are retained.
> > >
> >
> > On resume from s2idle, ksz9131_resume() calls
> ksz9131_config_rgmii_delay() which does MMD accesses,
> > while the PHY is in SPD. According to the datasheet, it shouldn't
> happen. See commit e398822c4751
> > ("net: phy: micrel: populate .soft_reset for KSZ9131") which fixes the
> same issue.
> 
> On my board, while s2idle in SPD mode, it does not hang. The datasheet
> does not explain
> the behaviour when it is SPD mode. But it states that it retains all
> previous register values
> when it is out of SPD mode.
> 

According to the KSZ9131 datasheet ([1]):

4.17.3 SOFTWARE POWER-DOWN MODE (SPD)
...
The following remain operational during SPD:
MII Management Interface
- Only access to the standard registers (0 through 31) is supported.
- Access to MMD address spaces other than MMD address space 1 is
  possible if the spd_clock_gate_override bit is set.
- Access to MMD address space 1 is not possible.


The spd_clock_gate_override bit is not used in the KSZ9131 driver.

While the datasheet does not specify exactly what happens if registers
from an unsupported address space are accessed while the PHY is in SPD,
I think it is correct for the driver to not do it in the first place.

[1] https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/00002841D.pdf

Thanks,
Ovidiu


> >
> > > But in Suspend to RAM, PHY loses power and the reset value of Power
> > > Down bit 0 (ie normal mode), there is no restriction for accessing MMD
> > > registers in this mode.
> > >
> >
> > If the PHY loses power, it loses all the configuration that was done by
> ksz9131_config_init(). Right
> > now, only the RGMII delays are restored, which is a subset of the
> configurations done by
> > ksz9131_config_init().
> 
> I agree, Only RZ/G3E reported this issue and with configuring only delays,
> it worked.
> 
> Calling ksz9131_config_init() restores more MMD registers or call
> phy_init() like this patch.
> 
> Cheers,
> Biju

RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Biju Das]

> -----Original Message-----
> From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> Sent: 03 April 2026 14:51
> To: Biju Das <biju.das.jz@bp.renesas.com>; andrew@lunn.ch; hkallweit1@gmail.com; linux@armlinux.org.uk;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-renesas-soc@vger.kernel.org
> Subject: RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()
> 
> 
> > >
> > > Hi,
> > >
> > > >
> > > > Hi Ovidiu Panait,
> > > >
> > > > Thanks for the patch.
> > > >
> > > > > -----Original Message-----
> > > > > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > > > > Sent: 03 April 2026 12:18
> > > > > Subject: [PATCH net] net: phy: micrel: Fix MMD register access
> > > > > during
> > > > SPD in ksz9131_resume()
> > > > >
> > > > > During system suspend, phy_suspend() puts the PHY into Software
> > > > > Power-
> > > > Down
> > > > > (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to
> > > > > the
> > > > KSZ9131 datasheet, MMD register
> > > > > access is restricted during SPD:
> > > > >
> > > > >   - Only access to the standard registers (0 through 31) is
> > supported.
> > > > >   - Access to MMD address spaces other than MMD address space 1 is
> > > > >     possible if the spd_clock_gate_override bit is set.
> > > > >   - Access to MMD address space 1 is not possible.
> > > > >
> > > > > However, ksz9131_resume() calls ksz9131_config_rgmii_delay()
> > > > > before
> > > > > kszphy_resume() clears BMCR_PDOWN. This means MMD registers are
> > > > > accessed
> > > > while the PHY is still in SPD,
> > > > > contrary to the datasheet.
> > > >
> > > > SPD mode: This mode is used to power down the device when it is
> > > > not in use after power-up.
> > > >           Previous register settings are maintained during and
> > > > following the removal of SPD.
> > > >
> > > > Suspend to Idle case, it is in SPD mode and the MMD register
> > > > values are retained.
> > > >
> > >
> > > On resume from s2idle, ksz9131_resume() calls
> > ksz9131_config_rgmii_delay() which does MMD accesses,
> > > while the PHY is in SPD. According to the datasheet, it shouldn't
> > happen. See commit e398822c4751
> > > ("net: phy: micrel: populate .soft_reset for KSZ9131") which fixes
> > > the
> > same issue.
> >
> > On my board, while s2idle in SPD mode, it does not hang. The datasheet
> > does not explain the behaviour when it is SPD mode. But it states that
> > it retains all previous register values when it is out of SPD mode.
> >
> 
> According to the KSZ9131 datasheet ([1]):
> 
> 4.17.3 SOFTWARE POWER-DOWN MODE (SPD)
> ...
> The following remain operational during SPD:
> MII Management Interface
> - Only access to the standard registers (0 through 31) is supported.
> - Access to MMD address spaces other than MMD address space 1 is
>   possible if the spd_clock_gate_override bit is set.
> - Access to MMD address space 1 is not possible.
> 
> 
> The spd_clock_gate_override bit is not used in the KSZ9131 driver.
> 
> While the datasheet does not specify exactly what happens if registers from an unsupported address
> space are accessed while the PHY is in SPD, I think it is correct for the driver to not do it in the
> first place.
> 
> [1]
> https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/00002841D.pd
> f

For s2idlecase: ie, PHY is in software power down state you don't need to restore
MMD register, as exiting software power down will restore those registers.

You need only restore MMD registers, when PHY loses power ie, suspend to RAM case.

Cheers,
Biju

RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Biju Das]

Hi Ovidiu Panait,

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> > Subject: RE: [PATCH net] net: phy: micrel: Fix MMD register access
> > during SPD in ksz9131_resume()
> >
> >
> > > >
> > > > Hi,
> > > >
> > > > >
> > > > > Hi Ovidiu Panait,
> > > > >
> > > > > Thanks for the patch.
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > > > > > Sent: 03 April 2026 12:18
> > > > > > Subject: [PATCH net] net: phy: micrel: Fix MMD register access
> > > > > > during
> > > > > SPD in ksz9131_resume()
> > > > > >
> > > > > > During system suspend, phy_suspend() puts the PHY into
> > > > > > Software
> > > > > > Power-
> > > > > Down
> > > > > > (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to
> > > > > > the
> > > > > KSZ9131 datasheet, MMD register
> > > > > > access is restricted during SPD:
> > > > > >
> > > > > >   - Only access to the standard registers (0 through 31) is
> > > supported.
> > > > > >   - Access to MMD address spaces other than MMD address space 1 is
> > > > > >     possible if the spd_clock_gate_override bit is set.
> > > > > >   - Access to MMD address space 1 is not possible.
> > > > > >
> > > > > > However, ksz9131_resume() calls ksz9131_config_rgmii_delay()
> > > > > > before
> > > > > > kszphy_resume() clears BMCR_PDOWN. This means MMD registers
> > > > > > are accessed
> > > > > while the PHY is still in SPD,
> > > > > > contrary to the datasheet.
> > > > >
> > > > > SPD mode: This mode is used to power down the device when it is
> > > > > not in use after power-up.
> > > > >           Previous register settings are maintained during and
> > > > > following the removal of SPD.
> > > > >
> > > > > Suspend to Idle case, it is in SPD mode and the MMD register
> > > > > values are retained.
> > > > >
> > > >
> > > > On resume from s2idle, ksz9131_resume() calls
> > > ksz9131_config_rgmii_delay() which does MMD accesses,
> > > > while the PHY is in SPD. According to the datasheet, it shouldn't
> > > happen. See commit e398822c4751
> > > > ("net: phy: micrel: populate .soft_reset for KSZ9131") which fixes
> > > > the
> > > same issue.
> > >
> > > On my board, while s2idle in SPD mode, it does not hang. The
> > > datasheet does not explain the behaviour when it is SPD mode. But it
> > > states that it retains all previous register values when it is out of SPD mode.
> > >
> >
> > According to the KSZ9131 datasheet ([1]):
> >
> > 4.17.3 SOFTWARE POWER-DOWN MODE (SPD)
> > ...
> > The following remain operational during SPD:
> > MII Management Interface
> > - Only access to the standard registers (0 through 31) is supported.
> > - Access to MMD address spaces other than MMD address space 1 is
> >   possible if the spd_clock_gate_override bit is set.
> > - Access to MMD address space 1 is not possible.
> >
> >
> > The spd_clock_gate_override bit is not used in the KSZ9131 driver.
> >
> > While the datasheet does not specify exactly what happens if registers
> > from an unsupported address space are accessed while the PHY is in
> > SPD, I think it is correct for the driver to not do it in the first place.
> >
> > [1]
> > https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/Product
> > Documents/DataSheets/00002841D.pd
> > f
> 
> For s2idlecase: ie, PHY is in software power down state you don't need to restore MMD register, as
> exiting software power down will restore those registers.
> 
> You need only restore MMD registers, when PHY loses power ie, suspend to RAM case.


I believe, You really don't need to call phy_init_hw() at all

SuspendtoRAM case: ###### ksz9131_resume 1140, software power down state is 0 (Normal mode)
SuspendtoIdle case: ###### ksz9131_resume 1940, software power down state  is 1 (SPD mode)

If (phydev->suspended && Normal mode)
  	Restore PHY specific MMD registers.

Cheers,
Biju

RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Biju Das]

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: 03 April 2026 17:29
> Subject: RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()
> 
> Hi Ovidiu Panait,
> 
> > -----Original Message-----
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > Subject: RE: [PATCH net] net: phy: micrel: Fix MMD register access
> > > during SPD in ksz9131_resume()
> > >
> > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > >
> > > > > > Hi Ovidiu Panait,
> > > > > >
> > > > > > Thanks for the patch.
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > > > > > > Sent: 03 April 2026 12:18
> > > > > > > Subject: [PATCH net] net: phy: micrel: Fix MMD register
> > > > > > > access during
> > > > > > SPD in ksz9131_resume()
> > > > > > >
> > > > > > > During system suspend, phy_suspend() puts the PHY into
> > > > > > > Software
> > > > > > > Power-
> > > > > > Down
> > > > > > > (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According
> > > > > > > to the
> > > > > > KSZ9131 datasheet, MMD register
> > > > > > > access is restricted during SPD:
> > > > > > >
> > > > > > >   - Only access to the standard registers (0 through 31) is
> > > > supported.
> > > > > > >   - Access to MMD address spaces other than MMD address space 1 is
> > > > > > >     possible if the spd_clock_gate_override bit is set.
> > > > > > >   - Access to MMD address space 1 is not possible.
> > > > > > >
> > > > > > > However, ksz9131_resume() calls ksz9131_config_rgmii_delay()
> > > > > > > before
> > > > > > > kszphy_resume() clears BMCR_PDOWN. This means MMD registers
> > > > > > > are accessed
> > > > > > while the PHY is still in SPD,
> > > > > > > contrary to the datasheet.
> > > > > >
> > > > > > SPD mode: This mode is used to power down the device when it
> > > > > > is not in use after power-up.
> > > > > >           Previous register settings are maintained during and
> > > > > > following the removal of SPD.
> > > > > >
> > > > > > Suspend to Idle case, it is in SPD mode and the MMD register
> > > > > > values are retained.
> > > > > >
> > > > >
> > > > > On resume from s2idle, ksz9131_resume() calls
> > > > ksz9131_config_rgmii_delay() which does MMD accesses,
> > > > > while the PHY is in SPD. According to the datasheet, it
> > > > > shouldn't
> > > > happen. See commit e398822c4751
> > > > > ("net: phy: micrel: populate .soft_reset for KSZ9131") which
> > > > > fixes the
> > > > same issue.
> > > >
> > > > On my board, while s2idle in SPD mode, it does not hang. The
> > > > datasheet does not explain the behaviour when it is SPD mode. But
> > > > it states that it retains all previous register values when it is out of SPD mode.
> > > >
> > >
> > > According to the KSZ9131 datasheet ([1]):
> > >
> > > 4.17.3 SOFTWARE POWER-DOWN MODE (SPD) ...
> > > The following remain operational during SPD:
> > > MII Management Interface
> > > - Only access to the standard registers (0 through 31) is supported.
> > > - Access to MMD address spaces other than MMD address space 1 is
> > >   possible if the spd_clock_gate_override bit is set.
> > > - Access to MMD address space 1 is not possible.
> > >
> > >
> > > The spd_clock_gate_override bit is not used in the KSZ9131 driver.
> > >
> > > While the datasheet does not specify exactly what happens if registers
> > > from an unsupported address space are accessed while the PHY is in
> > > SPD, I think it is correct for the driver to not do it in the first place.
> > >
> > > [1]
> > > https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/Product
> > > Documents/DataSheets/00002841D.pd
> > > f
> >
> > For s2idlecase: ie, PHY is in software power down state you don't need to restore MMD register, as
> > exiting software power down will restore those registers.
> >
> > You need only restore MMD registers, when PHY loses power ie, suspend to RAM case.
> 
> 
> I believe, You really don't need to call phy_init_hw() at all
> 
> SuspendtoRAM case: ###### ksz9131_resume 1140, software power down state is 0 (Normal mode)
> SuspendtoIdle case: ###### ksz9131_resume 1940, software power down state  is 1 (SPD mode)
> 
> If (phydev->suspended && Normal mode)
>   	Restore PHY specific MMD registers.


Please ignore the above, as I am not an expert in PHY sub system.

Cheers,
Biju

Re: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Russell King (Oracle)]

On Fri, Apr 03, 2026 at 11:17:38AM +0000, Ovidiu Panait wrote:
> During system suspend, phy_suspend() puts the PHY into Software Power-Down
> (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to the KSZ9131
> datasheet, MMD register access is restricted during SPD:
> 
>   - Only access to the standard registers (0 through 31) is supported.
>   - Access to MMD address spaces other than MMD address space 1 is
>     possible if the spd_clock_gate_override bit is set.
>   - Access to MMD address space 1 is not possible.
> 
> However, ksz9131_resume() calls ksz9131_config_rgmii_delay() before
> kszphy_resume() clears BMCR_PDOWN. This means MMD registers are accessed
> while the PHY is still in SPD, contrary to the datasheet.
> 
> Additionally, on platforms where the PHY loses power during suspend
> (e.g. RZ/G3E), all settings from ksz9131_config_init(), not just the
> RGMII delays, are lost and need to be restored. When the MAC driver
> sets mac_managed_pm (e.g. stmmac), mdio_bus_phy_resume() is skipped,
> so phy_init_hw() (which calls config_init to restore all PHY settings)
> is never invoked during resume.
> 
> Fix this by replacing the RGMII delay restoration with a call to
> phy_init_hw(), which takes the PHY out of SPD and performs full
> reinitialization.
> 
> Fixes: f25a7eaa897f ("net: phy: micrel: Add ksz9131_resume()")
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> ---
>  drivers/net/phy/micrel.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 2aa1dedd21b8..4236dbf4ad6b 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -6016,8 +6016,13 @@ static int lan8841_suspend(struct phy_device *phydev)
>  
>  static int ksz9131_resume(struct phy_device *phydev)
>  {
> -	if (phydev->suspended && phy_interface_is_rgmii(phydev))
> -		ksz9131_config_rgmii_delay(phydev);
> +	int ret;
> +
> +	if (phydev->suspended) {
> +		ret = phy_init_hw(phydev);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	return kszphy_resume(phydev);
>  }

mdio_bus_phy_resume():

        ret = phy_init_hw(phydev);
        if (ret < 0)
                return ret;

        ret = phy_resume(phydev);
        if (ret < 0)
                return ret;

where phy_resume() calls your resume function.

If a MAC driver is handling suspend/resume by setting
phydev->mac_managed_pm then maybe the MAC driver should also be
issuing phy_init_hw() before calling phy_resume() ?

Which MAC driver are you seeing a problem with?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Ovidiu Panait]

Hi,

> 
> On Fri, Apr 03, 2026 at 11:17:38AM +0000, Ovidiu Panait wrote:
> > During system suspend, phy_suspend() puts the PHY into Software Power-
> Down
> > (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to the
> KSZ9131
> > datasheet, MMD register access is restricted during SPD:
> >
> >   - Only access to the standard registers (0 through 31) is supported.
> >   - Access to MMD address spaces other than MMD address space 1 is
> >     possible if the spd_clock_gate_override bit is set.
> >   - Access to MMD address space 1 is not possible.
> >
> > However, ksz9131_resume() calls ksz9131_config_rgmii_delay() before
> > kszphy_resume() clears BMCR_PDOWN. This means MMD registers are accessed
> > while the PHY is still in SPD, contrary to the datasheet.
> >
> > Additionally, on platforms where the PHY loses power during suspend
> > (e.g. RZ/G3E), all settings from ksz9131_config_init(), not just the
> > RGMII delays, are lost and need to be restored. When the MAC driver
> > sets mac_managed_pm (e.g. stmmac), mdio_bus_phy_resume() is skipped,
> > so phy_init_hw() (which calls config_init to restore all PHY settings)
> > is never invoked during resume.
> >
> > Fix this by replacing the RGMII delay restoration with a call to
> > phy_init_hw(), which takes the PHY out of SPD and performs full
> > reinitialization.
> >
> > Fixes: f25a7eaa897f ("net: phy: micrel: Add ksz9131_resume()")
> > Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > ---
> >  drivers/net/phy/micrel.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index 2aa1dedd21b8..4236dbf4ad6b 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -6016,8 +6016,13 @@ static int lan8841_suspend(struct phy_device
> *phydev)
> >
> >  static int ksz9131_resume(struct phy_device *phydev)
> >  {
> > -	if (phydev->suspended && phy_interface_is_rgmii(phydev))
> > -		ksz9131_config_rgmii_delay(phydev);
> > +	int ret;
> > +
> > +	if (phydev->suspended) {
> > +		ret = phy_init_hw(phydev);
> > +		if (ret)
> > +			return ret;
> > +	}
> >
> >  	return kszphy_resume(phydev);
> >  }
> 
> mdio_bus_phy_resume():
> 
>         ret = phy_init_hw(phydev);
>         if (ret < 0)
>                 return ret;
> 
>         ret = phy_resume(phydev);
>         if (ret < 0)
>                 return ret;
> 
> where phy_resume() calls your resume function.
> 
> If a MAC driver is handling suspend/resume by setting
> phydev->mac_managed_pm then maybe the MAC driver should also be
> issuing phy_init_hw() before calling phy_resume() ?
> 
> Which MAC driver are you seeing a problem with?
> 

On my board the KSZ9131RNX PHY is paired to stmmac.

I could add phy_init_hw() before the phylink_prepare_resume() call, which
does the phy_resume() and remove the ksz9131_config_rgmii_delay() call from
ksz9131_resume(), as it is not correct/complete.

Ovidiu


> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Russell King (Oracle)]

On Fri, Apr 03, 2026 at 03:48:02PM +0000, Ovidiu Panait wrote:
> Hi,
> 
> > 
> > On Fri, Apr 03, 2026 at 11:17:38AM +0000, Ovidiu Panait wrote:
> > > During system suspend, phy_suspend() puts the PHY into Software Power-
> > Down
> > > (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to the
> > KSZ9131
> > > datasheet, MMD register access is restricted during SPD:
> > >
> > >   - Only access to the standard registers (0 through 31) is supported.
> > >   - Access to MMD address spaces other than MMD address space 1 is
> > >     possible if the spd_clock_gate_override bit is set.
> > >   - Access to MMD address space 1 is not possible.
> > >
> > > However, ksz9131_resume() calls ksz9131_config_rgmii_delay() before
> > > kszphy_resume() clears BMCR_PDOWN. This means MMD registers are accessed
> > > while the PHY is still in SPD, contrary to the datasheet.
> > >
> > > Additionally, on platforms where the PHY loses power during suspend
> > > (e.g. RZ/G3E), all settings from ksz9131_config_init(), not just the
> > > RGMII delays, are lost and need to be restored. When the MAC driver
> > > sets mac_managed_pm (e.g. stmmac), mdio_bus_phy_resume() is skipped,
> > > so phy_init_hw() (which calls config_init to restore all PHY settings)
> > > is never invoked during resume.
> > >
> > > Fix this by replacing the RGMII delay restoration with a call to
> > > phy_init_hw(), which takes the PHY out of SPD and performs full
> > > reinitialization.
> > >
> > > Fixes: f25a7eaa897f ("net: phy: micrel: Add ksz9131_resume()")
> > > Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > > ---
> > >  drivers/net/phy/micrel.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > > index 2aa1dedd21b8..4236dbf4ad6b 100644
> > > --- a/drivers/net/phy/micrel.c
> > > +++ b/drivers/net/phy/micrel.c
> > > @@ -6016,8 +6016,13 @@ static int lan8841_suspend(struct phy_device
> > *phydev)
> > >
> > >  static int ksz9131_resume(struct phy_device *phydev)
> > >  {
> > > -	if (phydev->suspended && phy_interface_is_rgmii(phydev))
> > > -		ksz9131_config_rgmii_delay(phydev);
> > > +	int ret;
> > > +
> > > +	if (phydev->suspended) {
> > > +		ret = phy_init_hw(phydev);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > >
> > >  	return kszphy_resume(phydev);
> > >  }
> > 
> > mdio_bus_phy_resume():
> > 
> >         ret = phy_init_hw(phydev);
> >         if (ret < 0)
> >                 return ret;
> > 
> >         ret = phy_resume(phydev);
> >         if (ret < 0)
> >                 return ret;
> > 
> > where phy_resume() calls your resume function.
> > 
> > If a MAC driver is handling suspend/resume by setting
> > phydev->mac_managed_pm then maybe the MAC driver should also be
> > issuing phy_init_hw() before calling phy_resume() ?
> > 
> > Which MAC driver are you seeing a problem with?
> > 
> 
> On my board the KSZ9131RNX PHY is paired to stmmac.
> 
> I could add phy_init_hw() before the phylink_prepare_resume() call, which
> does the phy_resume() and remove the ksz9131_config_rgmii_delay() call from
> ksz9131_resume(), as it is not correct/complete.

Yes, I think we should add phy_init_hw() before calling phy_resume()
in phylink's prepare_resume() path to ensure that the PHY state is
the same as when the PHY is resumed via the MDIO bus. Please prepare
a patch to that end, thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!