On Tue, May 03, 2022 at 03:15:04PM +0200, Lukas Wunner wrote: > When a PHY interrupt is signaled, the SMSC LAN95xx driver updates the > MAC full duplex mode and PHY flow control registers based on cached data > in struct phy_device: > > smsc95xx_status() # raises EVENT_LINK_RESET > usbnet_deferred_kevent() > smsc95xx_link_reset() # uses cached data in phydev > > Simultaneously, phylib polls link status once per second and updates > that cached data: > > phy_state_machine() > phy_check_link_status() > phy_read_status() > lan87xx_read_status() > genphy_read_status() # updates cached data in phydev > > If smsc95xx_link_reset() wins the race against genphy_read_status(), > the registers may be updated based on stale data. > > E.g. if the link was previously down, phydev->duplex is set to > DUPLEX_UNKNOWN and that's what smsc95xx_link_reset() will use, even > though genphy_read_status() may update it to DUPLEX_FULL afterwards. > > PHY interrupts are currently only enabled on suspend to trigger wakeup, > so the impact of the race is limited, but we're about to enable them > perpetually. > > Avoid the race by delaying execution of smsc95xx_link_reset() until > phy_state_machine() has done its job and calls back via > smsc95xx_handle_link_change(). > > Signaling EVENT_LINK_RESET on wakeup is not necessary because phylib > picks up link status changes through polling. So drop the declaration > of a ->link_reset() callback. > > Note that the semicolon on a line by itself added in smsc95xx_status() > is a placeholder for a function call which will be added in a subsequent > commit. That function call will actually handle the INT_ENP_PHY_INT_ > interrupt. > > Tested-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> # LAN9514/9512/9500 > Tested-by: Ferry Toth <fntoth@xxxxxxxxx> # LAN9514 > Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx> Reviewed-by: Andrew Lunn <andrew@xxxxxxx> Andrew