RE: [PATCH net-next v3 5/5] net: phy: Move phy_init_hw() from phy_resume() to __phy_resume()

[Biju Das]

Hi Andrew/Russell,

> -----Original Message-----
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Sent: 14 April 2026 19:43
> Subject: RE: [PATCH net-next v3 5/5] net: phy: Move phy_init_hw() from phy_resume() to __phy_resume()
> 
> Hi Andrew,
> 
> > -----Original Message-----
> > From: Andrew Lunn <andrew@xxxxxxx>
> > Sent: 14 April 2026 17:03
> > Subject: Re: [PATCH net-next v3 5/5] net: phy: Move phy_init_hw() from
> > phy_resume() to __phy_resume()
> >
> > On Sun, Apr 12, 2026 at 03:00:27PM +0100, Biju wrote:
> > > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > >
> > > Now that redundant locking has been removed from PHY driver
> > > callbacks,
> > > phy_init_hw() can be called with phydev->lock held.
> > >
> > > Many MAC drivers and the phylink framework resume the PHY via
> > > phy_start(), which invokes __phy_resume() directly without going
> > > through phy_resume(). Keeping phy_init_hw() in phy_resume() means it
> > > is not called in this path.
> > >
> > > Move phy_init_hw() into __phy_resume() so that PHY soft reset and
> > > re-initialisation happen unconditionally on every resume, regardless
> > > of which code path triggers it.
> >
> > I would change the order of these patches. First remove the redundant
> > locks. You can then put
> > phy_init_hw() into __phy_resume(), rather than first moving it into
> > phy_resume() and then __phy_resume().
> 
> Agreed.

One of my colleague pointed out that this patch may break[1], but we don't have the hardware to
test this IP. According to him

 "It does a phy_init_hw(), resetting the PHY, it applies some ethtool settings and then it calls
phy_start(). Since we are calling phy_init_hw() again during phy_start(),we might be undoing the
ethtool settings"

This kind of cleanup/fixing will be in 7.2 cycle, right?

[1] https://elixir.bootlin.com/linux/v7.0/source/drivers/net/ethernet/marvell/mv643xx_eth.c#L2327

Cheers,
Biju