RE: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path
From: Biju Das
Date: Sun Apr 12 2026 - 08:07:03 EST
Hi Russell King,
> -----Original Message-----
> From: Russell King <linux@xxxxxxxxxxxxxxx>
> Sent: 11 April 2026 17:47
> Subject: Re: [PATCH net-next] net: phy: call phy_init_hw() in phy resume path
>
> On Sat, Apr 11, 2026 at 03:50:13PM +0200, Andrew Lunn wrote:
> > > So, I question whether any of the functions in this driver actually
> > > have a valid reason to take phydev->lock - looks to me like a not
> > > very well written driver.
> > >
> > > In cases like this, I don't think we should make things more
> > > difficult in the core just because we have a lockdep splat when that
> > > can be avoided by killing off unnecessary locking.
> >
> > Agreed. This patchset should cleanup these locks.
> >
> > We also need to look at lan937x_dsp_workaround(). I also don't see
> > what that mutex lock/unlock is protecting. Accessing bank registers
> > need to be protected, so doing one additional access within that
> > should not need additional protection.
>
> Looking at access_ereg(), shouldn't it be taking the MDIO bus lock and using the __phy_* accessors
> anyway because it's writing various registers which determine what is being read via the
> LAN87XX_EXT_REG_RD_DATA register or the value written via the LAN87XX_EXT_REG_WR_DATA register.
>
> Also, as it has access_ereg_modify_changed(), that entire sequence needs to take the MDIO bus lock to
> safely do the read-modify-write.
>
> Then there's lan87xx_config_rgmii_delay() which is a large open coded read-modify-write for the
> PHYACC_ATTR_BANK_MISC, LAN87XX_CTRL_1 register.
>
> To me, this looks like a racy driver, and it also looks like it's using the wrong lock to try and
> protect hardware accesses.
OK, will replace it with MDIO bus lock.
Cheers,
Biju