Messages in this thread |  | | Date | Sat, 11 Apr 2026 17:46:51 +0100 | | From | "Russell King (Oracle)" <> | | 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.
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
|  |