On Thu, Sep 11, 2025 at 10:30:09PM -0400, Alan Stern wrote:
On Thu, Sep 11, 2025 at 09:46:35PM +0100, Russell King (Oracle) wrote:
On Thu, Sep 11, 2025 at 07:55:13AM -0700, Jakub Kicinski wrote:
We keep having issues with rtnl_lock taken from resume. Honestly, I'm not sure anyone has found a good solution, yet. Mostly people just don't implement runtime PM.
If we were able to pass optional context to suspend/resume we could implement conditional locking. We'd lose a lot of self-respect but it'd make fixing such bugs easier..
Normal drivers have the option of separate callbacks for runtime PM vs system suspend/resume states. It seems USB doesn't, just munging everything into one pair of suspend and resume ops without any way of telling them apart. I suggest that is part of the problem here.
However, I'm not a USB expert, so...
The USB subsystem uses only one pair of callbacks for suspend and resume because USB hardware has only one suspend state. However, the callbacks do get an extra pm_message_t parameter which they can use to distinguish between system sleep transitions and runtime PM transitions.
Unfortunately, this isn't the case. While a struct usb_device_driver's suspend()/resume() methods get the pm_message_t, a struct usb_driver's suspend()/resume() methods do not:
static int usb_resume_interface(struct usb_device *udev, struct usb_interface *intf, pm_message_t msg, int reset_resume) { struct usb_driver *driver; ... if (reset_resume) { if (driver->reset_resume) { status = driver->reset_resume(intf); ... } else { status = driver->resume(intf);
vs
static int usb_resume_device(struct usb_device *udev, pm_message_t msg) { struct usb_device_driver *udriver; ... if (status == 0 && udriver->resume) status = udriver->resume(udev, msg);
and in drivers/net/usb/asix_devices.c:
static struct usb_driver asix_driver = { ... .suspend = asix_suspend, .resume = asix_resume, .reset_resume = asix_resume,
where asix_resume() only takes one argument:
static int asix_resume(struct usb_interface *intf) {
Thanks.