Re: [PATCH v11 4/4] usb: host: enable USB offload during system sleep

[Alan Stern]

On Tue, Apr 08, 2025 at 03:57:24AM +0000, Guan-Yu Lin wrote:
> Sharing a USB controller with another entity via xhci-sideband driver
> creates power management complexities. To prevent the USB controller
> from being inadvertently deactivated while in use by the other entity, a
> usage-count based mechanism is implemented. This allows the system to
> manage power effectively, ensuring the controller remains available
> whenever needed.
> In order to maintain full functionality of an offloaded USB devices,
> several changes are made within the suspend flow of such devices:
> - skip usb_suspend_device() so that the port/hub are still active for
>   USB transfers via offloaded path.
> - not suspending the endpoints which are used by USB interfaces marked
>   with needs_remote_wakeup. Namely, skip usb_suspend_interface() and
>   usb_hcd_flush_endpoint() on associated USB interfaces. This reserves a
>   pending interrupt urb during system suspend for handling the interrupt
>   transfer, which is necessary since remote wakeup doesn't apply in the
>   offloaded USB devices when controller is still active.
> - not flushing the endpoints of actively offloaded USB devices. Given
>   that the USB devices is used by another entity, unilaterally flush the
>   endpoint might lead to unexpected behavior on another entity.
> - not suspending the xhci controller. This is done by skipping the
>   suspend/resume callbacks in the xhci platform driver.
> 
> Signed-off-by: Guan-Yu Lin <guanyulin@xxxxxxxxxx>
> ---
>  drivers/usb/core/driver.c    | 43 +++++++++++++++++++++++++++++++-----
>  drivers/usb/host/xhci-plat.c | 19 ++++++++++++++++
>  drivers/usb/host/xhci-plat.h |  1 +
>  include/linux/usb.h          |  2 ++
>  4 files changed, 59 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index 316526a05250..59b33e5e9a87 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -1420,11 +1420,25 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
>  			udev->state == USB_STATE_SUSPENDED)
>  		goto done;
>  
> +	if (msg.event == PM_EVENT_SUSPEND && usb_offload_check(udev)) {
> +		dev_dbg(&udev->dev, "device offload active, skip suspend.\n");
> +		udev->offload_at_suspend = 1;
> +	}
> +
>  	/* Suspend all the interfaces and then udev itself */
>  	if (udev->actconfig) {
>  		n = udev->actconfig->desc.bNumInterfaces;
>  		for (i = n - 1; i >= 0; --i) {
>  			intf = udev->actconfig->interface[i];
> +			/*
> +			 * Don't suspend interfaces with remote wakeup while the controller is
> +			 * active. This preserves pending interrupt urbs, allowing interrupt
> +			 * events to be handled during system suspend.
> +			 */

Here and below, please try to limit the line lengths to below 80 
columns, even in comments.

Alan Stern