On Wed, Oct 16, 2024 at 10:45 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Wed, Oct 16, 2024 at 03:40:00PM +0800, Guan-Yu Lin wrote: > > On Tue, Oct 15, 2024 at 10:43 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > On Tue, Oct 15, 2024 at 11:56:00AM +0800, Guan-Yu Lin wrote: > > > > On Mon, Oct 14, 2024 at 11:56 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > I'm not so sure about this. By returning early, you prevent the drivers > > > > > bound to this device from suspending. But they can't operate properly > > > > > when the system is in a low-power mode. Won't that cause problems? > > > > > > > > > > Maybe this really belongs in usb_suspend_device(), and its counterpart > > > > > belongs in usb_resume_device(). > > > > > > > > > > > > > To my understanding, after the system is suspended, the USB driver > > > > will do nothing as the main processor has been suspended. May I check > > > > what forms of low power mode and operation we are discussing here? > > > > > > S3 suspend. You are right that the driver will do nothing while the > > > CPU is suspended. But what about the times before and after that, > > > while the suspend and resume procedures are underway? The driver > > > needs to be told to cancel any ongoing transfers while the system > > > suspends and then restart them while the system resumes. > > > > > > > Regarding the cancellation of ongoing transfers during suspend, I > > believe usb_hcd_flush_endpoint() handles this as discussed below. > > There's more to it than that. If you cancel ongoing transfers by > calling usb_hcd_flush_endpoint() without informing the driver first, the > driver will get very confused and think the device has failed. > > > Besides calling usb_hcd_flush_endpoint(), are there any other > > necessary changes before suspending the driver in our scenario? Maybe > > we could discuss setting usb_device_state to USB_STATE_SUSPENDED. > > However, my understanding is that this variable reflects the actual > > device state. Since the device remains active via the sideband in our > > case, changing usb_device_state seems unnecessary. > > That's right. > > I don't think anything else is needed. Just call > usb_suspend_interface() like the normal pathway in usb_suspend_both() > does, but skip calling usb_suspend_device(). > > Alan Stern Thanks for the suggestions, let me address them in the next version. After some local development, our experiments suggest it may be necessary to skip usb_suspend_interface() & usb_hcd_flush_endpoint() for connection changes behind a hub and HID events in our scenario. Typically, when the system sleeps, the hub uses remote wakeup to reactivate upstream devices and resume the interface to handle connection changes. However, our current conclusion is to maintain the device in an active state while suspending the interface. This deviates from the norm, as remote wakeup is designed to function when devices and links are suspended. We're concerned that this discrepancy might interfere with the remote wakeup mechanism. To address this, we're currently bypassing usb_suspend_interface() and usb_hcd_flush_endpoint(). This effectively simulates an "active system" state, allowing the USB controller to notify the kernel about connection changes via interrupts. This workaround applies to HID events as well. Which approach do you recommend? Should we invest in integrating with the remote wakeup framework, or is it acceptable to keep necessary components active, mirroring an "active system" state? Regards, Guan-Yu
