On Wed, Oct 09, 2024 at 10:35:05AM +0800, duanchenghao wrote: > Hi Alan, > > These are two patches, each addressing the same issue. The current > patch is a direct solution to the problem of the interrupt bottom half > being frozen. The patch you replied with is another, alternative > solution to the same problem. Please review which solution is more > suitable, or if there are any other revised proposals. > > > Please review the patch I mentioned: > https://lore.kernel.org/all/0a4dc46ae767c28dd207ae29511ede747f05539a.camel@xxxxxxxxxx/ I still think your whole approach is wrong. There is no need to change the way the HCD_FLAG_WAKEUP_PENDING flag gets set or cleared; that's not the reason for the problem you're seeing. The problem occurs because when suspend_common() in drivers/usb/core/hcd-pci.c sets do_wakeup, it does so by calling device_may_wakeup(), and the value that function returns is not what we want. The value is based on whether the controller's power/wakeup attribute is set, but we also have to take into account the type of suspend that's happening. Basically, if msg is one of the suspend types for which wakeups should always be disabled, then do_wakeup should be set to false. There isn't a macro to test for these things, but there should be. Something like PMSG_IS_AUTO() in include/linux/pm.h; maybe call it PMSG_NO_WAKEUP(). This macro should return true if the PM_EVENT_FREEZE or PM_EVENT_QUIESCE bits are set in msg.event. Rafael, please correct me if I got any of this wrong. So the right way to fix the problem is to add to pm.h: #define PMSG_NO_WAKEUP(msg) (((msg.event) & \ (PM_EVENT_FREEZE | PM_EVENT_QUIESCE)) != 0) and in suspend_common(): if (PMSG_IS_AUTO(msg)) do_wakeup = true; else if (PMSG_NO_WAKEUP(msg)) do_wakeup = false; else do_wakeup = device_may_wakeup(dev); Then with do_wakeup set to false, none of the HCD_WAKEUP_PENDING() tests in the following code will be executed, so the routine won't fail during the freeze or restore phase with -EBUSY. You'll also have to define an hcd_pci_freeze() routine, just like hcd_pci_suspend() except that it uses PMSG_FREEZE instead of PMSG_SUSPEND. And the .freeze callback in usb_hcd_pci_pm_ops should be changed to hcd_pci_freeze. In fact, it looks like choose_wakeup() in drivers/usb/core/driver.c could also use the new macro, instead of checking for FREEZE or QUIESCE directly the way it does now. Alan Stern
