Subject: Re: [PATCH net-next v3 5/7] usbnet: smsc95xx: Forward PHY interrupts to PHY driver to avoid polling

From: Lukas Wunner <[email-masked]>
Date: Fri, 26 Aug 2022 09:19:24 +0200

On Fri, Aug 26, 2022 at 08:51:58AM +0200, Marek Szyprowski wrote:
> On 19.05.2022 23:22, Marek Szyprowski wrote:
> > On 19.05.2022 21:08, Lukas Wunner wrote:
> >> On Tue, May 17, 2022 at 12:18:45PM +0200, Marek Szyprowski wrote:
> >>> This patch landed in the recent linux next-20220516 as commit
> >>> 1ce8b37241ed ("usbnet: smsc95xx: Forward PHY interrupts to PHY 
> >>> driver to
> >>> avoid polling"). Unfortunately it breaks smsc95xx usb ethernet 
> >>> operation
> >>> after system suspend-resume cycle. On the Odroid XU3 board I got the
> >>> following warning in the kernel log:
> >>>
> >>> # time rtcwake -s10 -mmem
> >>> rtcwake: wakeup from "mem" using /dev/rtc0 at Tue May 17 09:16:07 2022
> >>> PM: suspend entry (deep)
> >>> Filesystems sync: 0.001 seconds
> >>> Freezing user space processes ... (elapsed 0.002 seconds) done.
> >>> OOM killer disabled.
> >>> Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> >>> printk: Suspending console(s) (use no_console_suspend to debug)
> >>> smsc95xx 4-1.1:1.0 eth0: entering SUSPEND2 mode
> >>> smsc95xx 4-1.1:1.0 eth0: Failed to read reg index 0x00000114: -113
> >>> smsc95xx 4-1.1:1.0 eth0: Error reading MII_ACCESS
> >>> smsc95xx 4-1.1:1.0 eth0: __smsc95xx_mdio_read: MII is busy
> >>> ------------[ cut here ]------------
> >>> WARNING: CPU: 2 PID: 73 at drivers/net/phy/phy.c:946
> >>> phy_state_machine+0x98/0x28c
> >> [...]
> >>> It looks that the driver's suspend/resume operations might need some
> >>> adjustments. After the system suspend/resume cycle the driver is not
> >>> operational anymore. Reverting the $subject patch on top of linux
> >>> next-20220516 restores ethernet operation after system suspend/resume.
> >> Thanks a lot for the report. It seems the PHY is signaling a link 
> >> change
> >> shortly before system sleep and by the time the phy_state_machine() 
> >> worker
> >> gets around to handle it, the device has already been suspended and thus
> >> refuses any further USB requests with -EHOSTUNREACH (-113):
[...]
> >> Assuming the above theory is correct, calling phy_stop_machine()
> >> after usbnet_suspend() would be sufficient to fix the issue.
> >> It cancels the phy_state_machine() worker.
> >>
> >> The small patch below does that. Could you give it a spin?
> >
> > That's it. Your analysis is right and the patch fixes the issue. Thanks!
> >
> > Feel free to add:
> >
> > Reported-by: Marek Szyprowski <[email-masked]>
> >
> > Tested-by: Marek Szyprowski <[email-masked]>
> 
> Gentle ping for the final patch...

Hm?  Actually this issue is supposed to be fixed by mainline commit
1758bde2e4aa ("net: phy: Don't trigger state machine while in suspend").

The initial fix attempt that you're replying to should not be necessary
with that commit.

Are you still seeing issues even with 1758bde2e4aa applied?
Or are you maybe using a custom downstream tree which is missing that commit?

Thanks,

Lukas