On Wed, Apr 16, 2025 at 02:43:03PM +0000, Guan-Yu Lin wrote:
> The existing sideband driver only registers sidebands without tracking
> their active usage. To address this, sideband will now record its active
> usage when it creates/removes interrupters. In addition, a new api is
> introduced to provide a means for other dirvers to fetch sideband
> activity information on a USB host controller.
>
> Signed-off-by: Guan-Yu Lin <guanyulin@xxxxxxxxxx>
> ---
> drivers/usb/host/xhci-sideband.c | 43 +++++++++++++++++++++++++++++++
> include/linux/usb/xhci-sideband.h | 9 +++++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
> index d49f9886dd84..67f31b0ec726 100644
> --- a/drivers/usb/host/xhci-sideband.c
> +++ b/drivers/usb/host/xhci-sideband.c
> @@ -266,6 +266,31 @@ xhci_sideband_get_event_buffer(struct xhci_sideband *sb)
> }
> EXPORT_SYMBOL_GPL(xhci_sideband_get_event_buffer);
>
> +#if IS_ENABLED(CONFIG_USB_XHCI_SIDEBAND_SUSPEND)
> +/**
> + * xhci_sideband_check - check the existence of active sidebands
> + * @hcd: the host controller driver associated with the target host controller
> + *
> + * Allow other drivers, such as usb controller driver, to check if there are
> + * any sideband activity on the host controller. This information could be used
> + * for power management or other forms of resource management.
> + *
> + * Returns true on any active sideband existence, false otherwise.
> + */
> +bool xhci_sideband_check(struct usb_hcd *hcd)
> +{
> + struct usb_device *udev = hcd->self.root_hub;
> + bool active;
> +
> + device_lock(&udev->dev);
> + active = usb_offload_check(udev);
> + device_unlock(&udev->dev);
> +
> + return active;
What happens if the value changes right after reading it? What are you
going to do with the value?
> +}
> +EXPORT_SYMBOL_GPL(xhci_sideband_check);
> +#endif /* IS_ENABLED(CONFIG_USB_XHCI_SIDEBAND_SUSPEND) */
> +
> /**
> * xhci_sideband_create_interrupter - creates a new interrupter for this sideband
> * @sb: sideband instance for this usb device
> @@ -286,6 +311,7 @@ xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
> bool ip_autoclear, u32 imod_interval, int intr_num)
> {
> int ret = 0;
> + struct usb_device *udev;
>
> if (!sb || !sb->xhci)
> return -ENODEV;
> @@ -304,6 +330,11 @@ xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
> goto out;
> }
>
> + udev = sb->vdev->udev;
> + device_lock(&udev->dev);
> + ret = usb_offload_get(udev);
> + device_unlock(&udev->dev);
A "raw" call to device_lock/unlock feels rough, and harsh, why doesn't
the function do that itself?
thanks,
greg k-h
