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. 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. > > usb_suspend_device() did close the required port/bus to maintain usb > > transfer. > > I don't know what you mean by that. > Nothing special here, I'm just echoing what you've mentioned and trying to bring up the discussion about usb_hcd_flush_endpoint(). > > However, there are additional usb_hcd_flush_endpoint() calls > > aside from usb_suspend_device(). Maybe we should consider not doing > > those also since some of the endpoints are now handled by the > > sideband. > > Those calls should not be skipped. If the sideband controller wants > to handle the endpoints on its own, it can go right ahead. The main > controller and the USB core need to know that they shouldn't use the > endpoint while the system is suspending. > > Alan Stern Got it, let me update the patch and put the related changes into usb_suspend_device()/usb_resume_device(). Regards, Guan-Yu
