[PATCH net-next] net: phy: call phy_init_hw() in phy resume path

[PATCH net-next] net: phy: call phy_init_hw() in phy resume path

[Biju]

From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>

When mac_managed_pm flag is set, mdio_bus_phy_resume() is skipped, so
phy_init_hw(), which performs soft_reset and config_init, is not called
during resume.

This is inconsistent with the non-mac_managed_pm path, where
mdio_bus_phy_resume() calls phy_init_hw() before phy_resume() on every
resume.

To align both paths, add a phy_init_hw() call at the top of
__phy_resume(), before invoking the driver's resume callback. This
guarantees the PHY undergoes soft reset and re-initialization regardless
of whether PM is managed by the MAC or the MDIO bus.

Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/net/phy/phy_device.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0edff47478c2..8255f4208d66 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2008,6 +2008,10 @@ int __phy_resume(struct phy_device *phydev)
 	if (!phydrv || !phydrv->resume)
 		return 0;
 
+	ret = phy_init_hw(phydev);
+	if (ret)
+		return ret;
+
 	ret = phydrv->resume(phydev);
 	if (!ret)
 		phydev->suspended = false;
-- 
2.43.0

Re: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path

[Russell King (Oracle)]

On Fri, Apr 10, 2026 at 03:29:01PM +0100, Biju wrote:
> From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> 
> When mac_managed_pm flag is set, mdio_bus_phy_resume() is skipped, so
> phy_init_hw(), which performs soft_reset and config_init, is not called
> during resume.
> 
> This is inconsistent with the non-mac_managed_pm path, where
> mdio_bus_phy_resume() calls phy_init_hw() before phy_resume() on every
> resume.
> 
> To align both paths, add a phy_init_hw() call at the top of
> __phy_resume(), before invoking the driver's resume callback. This
> guarantees the PHY undergoes soft reset and re-initialization regardless
> of whether PM is managed by the MAC or the MDIO bus.
> 
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/net/phy/phy_device.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 0edff47478c2..8255f4208d66 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -2008,6 +2008,10 @@ int __phy_resume(struct phy_device *phydev)
>  	if (!phydrv || !phydrv->resume)
>  		return 0;
>  
> +	ret = phy_init_hw(phydev);
> +	if (ret)
> +		return ret;

Do we want to do this even when phydrv->resume is NULL?

Apart from that, looks fine to me - it seems some paths call
phy_init_hw() can be called with or without phydev->lock held, and
this one will call it with the lock held which seems to be okay.

-- 
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-next] net: phy: call phy_init_hw() in phy resume path

[Russell King (Oracle)]

On Fri, Apr 10, 2026 at 03:51:18PM +0100, Russell King (Oracle) wrote:
> On Fri, Apr 10, 2026 at 03:29:01PM +0100, Biju wrote:
> > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > 
> > When mac_managed_pm flag is set, mdio_bus_phy_resume() is skipped, so
> > phy_init_hw(), which performs soft_reset and config_init, is not called
> > during resume.
> > 
> > This is inconsistent with the non-mac_managed_pm path, where
> > mdio_bus_phy_resume() calls phy_init_hw() before phy_resume() on every
> > resume.
> > 
> > To align both paths, add a phy_init_hw() call at the top of
> > __phy_resume(), before invoking the driver's resume callback. This
> > guarantees the PHY undergoes soft reset and re-initialization regardless
> > of whether PM is managed by the MAC or the MDIO bus.
> > 
> > Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  drivers/net/phy/phy_device.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index 0edff47478c2..8255f4208d66 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -2008,6 +2008,10 @@ int __phy_resume(struct phy_device *phydev)
> >  	if (!phydrv || !phydrv->resume)
> >  		return 0;
> >  
> > +	ret = phy_init_hw(phydev);
> > +	if (ret)
> > +		return ret;
> 
> Do we want to do this even when phydrv->resume is NULL?

I should've also added (sorry, busy packing) - with it always being
called even when phydrv->resume is NULL, it means that the call sites
to phy_resume() in phylib which are preceeded by a call to
phy_init_hw() should have that call removed, otherwise we're going to
be calling phy_init_hw() twice.

As the patch currently stands, that's the case when phydrv->resume
is populated, and I think we should avoid that.

> Apart from that, looks fine to me - it seems some paths call
> phy_init_hw() can be called with or without phydev->lock held, and
> this one will call it with the lock held which seems to be okay.

-- 
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-next] net: phy: call phy_init_hw() in phy resume path

[Biju Das]

Hi Russell King/ Andrew,

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: 10 April 2026 15:55
> Subject: Re: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path
> 
> On Fri, Apr 10, 2026 at 03:51:18PM +0100, Russell King (Oracle) wrote:
> > On Fri, Apr 10, 2026 at 03:29:01PM +0100, Biju wrote:
> > > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > >
> > > When mac_managed_pm flag is set, mdio_bus_phy_resume() is skipped,
> > > so phy_init_hw(), which performs soft_reset and config_init, is not
> > > called during resume.
> > >
> > > This is inconsistent with the non-mac_managed_pm path, where
> > > mdio_bus_phy_resume() calls phy_init_hw() before phy_resume() on
> > > every resume.
> > >
> > > To align both paths, add a phy_init_hw() call at the top of
> > > __phy_resume(), before invoking the driver's resume callback. This
> > > guarantees the PHY undergoes soft reset and re-initialization
> > > regardless of whether PM is managed by the MAC or the MDIO bus.
> > >
> > > Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > >  drivers/net/phy/phy_device.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/net/phy/phy_device.c
> > > b/drivers/net/phy/phy_device.c index 0edff47478c2..8255f4208d66
> > > 100644
> > > --- a/drivers/net/phy/phy_device.c
> > > +++ b/drivers/net/phy/phy_device.c
> > > @@ -2008,6 +2008,10 @@ int __phy_resume(struct phy_device *phydev)
> > >  	if (!phydrv || !phydrv->resume)
> > >  		return 0;
> > >
> > > +	ret = phy_init_hw(phydev);
> > > +	if (ret)
> > > +		return ret;
> >
> > Do we want to do this even when phydrv->resume is NULL?
> 
> I should've also added (sorry, busy packing) - with it always being called even when phydrv->resume is
> NULL, it means that the call sites to phy_resume() in phylib which are preceeded by a call to
> phy_init_hw() should have that call removed, otherwise we're going to be calling phy_init_hw() twice.
> 
> As the patch currently stands, that's the case when phydrv->resume is populated, and I think we should
> avoid that.
> 
> > Apart from that, looks fine to me - it seems some paths call
> > phy_init_hw() can be called with or without phydev->lock held, and
> > this one will call it with the lock held which seems to be okay.


The new patch will be like this, after moving phy_init_hw() without
phydev->lock held. Please let me know are you ok with this?


diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0edff47478c2..4a2b19d39373 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -396,10 +396,6 @@ static __maybe_unused int mdio_bus_phy_resume(struct device *dev)
 	WARN_ON(phydev->state != PHY_HALTED && phydev->state != PHY_READY &&
 		phydev->state != PHY_UP);
 
-	ret = phy_init_hw(phydev);
-	if (ret < 0)
-		return ret;
-
 	ret = phy_resume(phydev);
 	if (ret < 0)
 		return ret;
@@ -1857,16 +1853,14 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	if (dev)
 		netif_carrier_off(phydev->attached_dev);
 
-	/* Do initial configuration here, now that
+	/* Do initial configuration inside phy_init_hw(), now that
 	 * we have certain key parameters
 	 * (dev_flags and interface)
 	 */
-	err = phy_init_hw(phydev);
+	err = phy_resume(phydev);
 	if (err)
 		goto error;
 
-	phy_resume(phydev);
-
 	/**
 	 * If the external phy used by current mac interface is managed by
 	 * another mac interface, so we should create a device link between
@@ -2020,6 +2014,10 @@ int phy_resume(struct phy_device *phydev)
 {
 	int ret;
 
+	ret = phy_init_hw(phydev);
+	if (ret)
+		return ret;
+
 	mutex_lock(&phydev->lock);
 	ret = __phy_resume(phydev);
 	mutex_unlock(&phydev->lock);
--

Re: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path

[Andrew Lunn]

> Apart from that, looks fine to me - it seems some paths call
> phy_init_hw() can be called with or without phydev->lock held, and
> this one will call it with the lock held which seems to be okay.

Haven't we had deadlocks in this area before?

Please test with CONFIG_PROVE_LOCKING enabled.

       Andrew

Re: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path

[Russell King (Oracle)]

On Fri, Apr 10, 2026 at 05:15:21PM +0200, Andrew Lunn wrote:
> > Apart from that, looks fine to me - it seems some paths call
> > phy_init_hw() can be called with or without phydev->lock held, and
> > this one will call it with the lock held which seems to be okay.
> 
> Haven't we had deadlocks in this area before?

If we have a problem calling phy_init_hw() with phydev->lock held, then:

phy_state_machine():
        mutex_lock(&phydev->lock);
        state_work = _phy_state_machine(phydev);

_phy_state_machine():
	switch (phydev->state) {
...
        case PHY_CABLETEST:
                err = phydev->drv->cable_test_get_status(phydev, &finished);
                if (err) {
                        phy_abort_cable_test(phydev);

phy_abort_cable_test():
        err = phy_init_hw(phydev);

that path has a problem and needs fixing.


-- 
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-next] net: phy: call phy_init_hw() in phy resume path

[Biju Das]

Hi Russell King,

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: 10 April 2026 16:22
> Subject: Re: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path
> 
> On Fri, Apr 10, 2026 at 05:15:21PM +0200, Andrew Lunn wrote:
> > > Apart from that, looks fine to me - it seems some paths call
> > > phy_init_hw() can be called with or without phydev->lock held, and
> > > this one will call it with the lock held which seems to be okay.
> >
> > Haven't we had deadlocks in this area before?
> 
> If we have a problem calling phy_init_hw() with phydev->lock held, then:
> 
> phy_state_machine():
>         mutex_lock(&phydev->lock);
>         state_work = _phy_state_machine(phydev);
> 
> _phy_state_machine():
> 	switch (phydev->state) {
> ...
>         case PHY_CABLETEST:
>                 err = phydev->drv->cable_test_get_status(phydev, &finished);
>                 if (err) {
>                         phy_abort_cable_test(phydev);
> 
> phy_abort_cable_test():
>         err = phy_init_hw(phydev);
> 
> that path has a problem and needs fixing.

These 3 Phy drivers are using the same lock, and it can lead to dead lock.

drivers/net/phy/microchip_t1.c
drivers/net/phy/marvell-88x2222.c
drivers/net/phy/mscc/mscc_main.c

Maybe as you said earlier, moving to phy_resume() will be safer solution.

Cheers,
Biju

Re: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path

[Andrew Lunn]

On Fri, Apr 10, 2026 at 04:22:23PM +0100, Russell King (Oracle) wrote:
> On Fri, Apr 10, 2026 at 05:15:21PM +0200, Andrew Lunn wrote:
> > > Apart from that, looks fine to me - it seems some paths call
> > > phy_init_hw() can be called with or without phydev->lock held, and
> > > this one will call it with the lock held which seems to be okay.
> > 
> > Haven't we had deadlocks in this area before?
> 
> If we have a problem calling phy_init_hw() with phydev->lock held, then:

I thought it was an AB-BA with RTNL?

Lets see what PROVE_LOCKING actually says. I could be remembering
wrongly.

	Andrew

RE: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path

[Biju Das]

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: 10 April 2026 16:15
> Subject: Re: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path
> 
> > Apart from that, looks fine to me - it seems some paths call
> > phy_init_hw() can be called with or without phydev->lock held, and
> > this one will call it with the lock held which seems to be okay.
> 
> Haven't we had deadlocks in this area before?
> 
> Please test with CONFIG_PROVE_LOCKING enabled.

I have n't faced any issue with micrel phy. But my collegue
got the below issue with Microsemi phy. It doesn't finish the boot.

drivers/net/phy/mscc/mscc_main.c 


[    5.125699] ============================================
[    5.131849] WARNING: possible recursive locking detected
[    5.137996] 7.0.0-rc7-next-20260409+ #614 Not tainted
[    5.143847] --------------------------------------------
[    5.149991] swapper/0/1 is trying to acquire lock:
[    5.155333] ffff000182bbe7c0 (&dev->lock#2){+.+.}-{4:4}, at: vsc85xx_config_init+0x68/0x344
[    5.164771] 
[    5.164771] but task is already holding lock:
[    5.171520] ffff000182bbe7c0 (&dev->lock#2){+.+.}-{4:4}, at: phy_attach_direct+0x19c/0x3d0
[    5.180984] 
[    5.180984] other info that might help us debug this:
[    5.188237]  Possible unsafe locking scenario:
[    5.188237] 
[    5.195089]        CPU0
[    5.197914]        ----
[    5.200670]   lock(&dev->lock#2);
[    5.204425]   lock(&dev->lock#2);
[    5.208273] 
[    5.208273]  *** DEADLOCK ***
[    5.208273] 
[    5.214920]  May be due to missing lock nesting notation
[    5.214920] 
[    5.222677] 2 locks held by swapper/0/1:
[    5.227217]  #0: ffff800082f051f0 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_lock+0x1c/0x28
[    5.236041]  #1: ffff000182bbe7c0 (&dev->lock#2){+.+.}-{4:4}, at: phy_attach_direct+0x19c/0x3d0
[    5.245880] 
[    5.245880] stack backtrace:
[    5.250824] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 7.0.0-rc7-next-20260409+ #614 PREEMPT 
[    5.250840] Hardware name: Renesas RZ/T2H EVK Board based on r9a09g077m44 (DT)
[    5.250847] Call trace:
[    5.250853]  show_stack+0x18/0x30 (C)
[    5.250876]  dump_stack_lvl+0x70/0x98
[    5.250894]  dump_stack+0x18/0x24
[    5.250910]  print_deadlock_bug+0x220/0x234
[    5.250931]  __lock_acquire+0xe10/0x1594
[    5.250950]  lock_acquire+0x284/0x400
[    5.250967]  __mutex_lock+0xa8/0x804
[    5.250983]  mutex_lock_nested+0x24/0x3c
[    5.250998]  vsc85xx_config_init+0x68/0x344
[    5.251013]  phy_init_hw+0x68/0xa8
[    5.251030]  __phy_resume+0x3c/0x98
[    5.251047]  phy_attach_direct+0x1a4/0x3d0
[    5.251064]  phylink_fwnode_phy_connect+0x98/0x140
[    5.251085]  stmmac_open+0x120/0x38c
[    5.251103]  __dev_open+0x13c/0x280
[    5.251117]  __dev_change_flags+0x19c/0x21c
[    5.251131]  netif_change_flags+0x24/0x6c
[    5.251144]  dev_change_flags+0x48/0x80
[    5.251159]  ip_auto_config+0x264/0xec0
[    5.251176]  do_one_initcall+0x7c/0x4d0
[    5.251194]  kernel_init_freeable+0x2b4/0x33c
[    5.251212]  kernel_init+0x24/0x140
[    5.251231]  ret_from_fork+0x10/0x20


Cheers,
Biju

Re: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path

[Florian Fainelli]

On 4/10/26 07:51, Russell King (Oracle) wrote:
> On Fri, Apr 10, 2026 at 03:29:01PM +0100, Biju wrote:
>> From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
>>
>> When mac_managed_pm flag is set, mdio_bus_phy_resume() is skipped, so
>> phy_init_hw(), which performs soft_reset and config_init, is not called
>> during resume.
>>
>> This is inconsistent with the non-mac_managed_pm path, where
>> mdio_bus_phy_resume() calls phy_init_hw() before phy_resume() on every
>> resume.
>>
>> To align both paths, add a phy_init_hw() call at the top of
>> __phy_resume(), before invoking the driver's resume callback. This
>> guarantees the PHY undergoes soft reset and re-initialization regardless
>> of whether PM is managed by the MAC or the MDIO bus.
>>
>> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>> ---
>>   drivers/net/phy/phy_device.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 0edff47478c2..8255f4208d66 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -2008,6 +2008,10 @@ int __phy_resume(struct phy_device *phydev)
>>   	if (!phydrv || !phydrv->resume)
>>   		return 0;
>>   
>> +	ret = phy_init_hw(phydev);
>> +	if (ret)
>> +		return ret;
> 
> Do we want to do this even when phydrv->resume is NULL?

Seems to me we would want that, but this gets into the territory of 
potentially creating many soft-reset of the PHYs, something that I 
regret having introduced years ago.

> 
> Apart from that, looks fine to me - it seems some paths call
> phy_init_hw() can be called with or without phydev->lock held, and
> this one will call it with the lock held which seems to be okay.
> 

Agreed.
-- 
Florian