2021-11-19 09:27:27

by Kai-Heng Feng

[permalink] [raw]
Subject: [PATCH] xhci: Remove CONFIG_USB_DEFAULT_PERSIST to prevent xHCI from runtime suspending

When the xHCI is quirked with XHCI_RESET_ON_RESUME, runtime resume
routine also resets the controller.

This is bad for USB drivers without reset_resume callback, because
there's no subsequent call of usb_dev_complete() ->
usb_resume_complete() to force rebinding the driver to the device. For
instance, btusb device stops working after xHCI controller is runtime
resumed, if the controlled is quirked with XHCI_RESET_ON_RESUME.

So always take XHCI_RESET_ON_RESUME into account to solve the issue.

Signed-off-by: Kai-Heng Feng <[email protected]>
---
drivers/usb/host/xhci.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 902f410874e8e..af92a9f8ed670 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3934,7 +3934,6 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
struct xhci_slot_ctx *slot_ctx;
int i, ret;

-#ifndef CONFIG_USB_DEFAULT_PERSIST
/*
* We called pm_runtime_get_noresume when the device was attached.
* Decrement the counter here to allow controller to runtime suspend
@@ -3942,7 +3941,6 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
*/
if (xhci->quirks & XHCI_RESET_ON_RESUME)
pm_runtime_put_noidle(hcd->self.controller);
-#endif

ret = xhci_check_args(hcd, udev, NULL, 0, true, __func__);
/* If the host is halted due to driver unload, we still need to free the
@@ -4094,14 +4092,12 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)

xhci_debugfs_create_slot(xhci, slot_id);

-#ifndef CONFIG_USB_DEFAULT_PERSIST
/*
* If resetting upon resume, we can't put the controller into runtime
* suspend if there is a device attached.
*/
if (xhci->quirks & XHCI_RESET_ON_RESUME)
pm_runtime_get_noresume(hcd->self.controller);
-#endif

/* Is this a LS or FS device under a HS hub? */
/* Hub or peripherial? */
--
2.32.0



2021-12-01 00:19:52

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] xhci: Remove CONFIG_USB_DEFAULT_PERSIST to prevent xHCI from runtime suspending

On Fri, Nov 19, 2021 at 5:27 PM Kai-Heng Feng
<[email protected]> wrote:
>
> When the xHCI is quirked with XHCI_RESET_ON_RESUME, runtime resume
> routine also resets the controller.
>
> This is bad for USB drivers without reset_resume callback, because
> there's no subsequent call of usb_dev_complete() ->
> usb_resume_complete() to force rebinding the driver to the device. For
> instance, btusb device stops working after xHCI controller is runtime
> resumed, if the controlled is quirked with XHCI_RESET_ON_RESUME.
>
> So always take XHCI_RESET_ON_RESUME into account to solve the issue.
>
> Signed-off-by: Kai-Heng Feng <[email protected]>

A gentle ping...

> ---
> drivers/usb/host/xhci.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 902f410874e8e..af92a9f8ed670 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -3934,7 +3934,6 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
> struct xhci_slot_ctx *slot_ctx;
> int i, ret;
>
> -#ifndef CONFIG_USB_DEFAULT_PERSIST
> /*
> * We called pm_runtime_get_noresume when the device was attached.
> * Decrement the counter here to allow controller to runtime suspend
> @@ -3942,7 +3941,6 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
> */
> if (xhci->quirks & XHCI_RESET_ON_RESUME)
> pm_runtime_put_noidle(hcd->self.controller);
> -#endif
>
> ret = xhci_check_args(hcd, udev, NULL, 0, true, __func__);
> /* If the host is halted due to driver unload, we still need to free the
> @@ -4094,14 +4092,12 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
>
> xhci_debugfs_create_slot(xhci, slot_id);
>
> -#ifndef CONFIG_USB_DEFAULT_PERSIST
> /*
> * If resetting upon resume, we can't put the controller into runtime
> * suspend if there is a device attached.
> */
> if (xhci->quirks & XHCI_RESET_ON_RESUME)
> pm_runtime_get_noresume(hcd->self.controller);
> -#endif
>
> /* Is this a LS or FS device under a HS hub? */
> /* Hub or peripherial? */
> --
> 2.32.0
>

2021-12-01 09:00:20

by Mathias Nyman

[permalink] [raw]
Subject: Re: [PATCH] xhci: Remove CONFIG_USB_DEFAULT_PERSIST to prevent xHCI from runtime suspending

On 1.12.2021 2.19, Kai-Heng Feng wrote:
> On Fri, Nov 19, 2021 at 5:27 PM Kai-Heng Feng
> <[email protected]> wrote:
>>
>> When the xHCI is quirked with XHCI_RESET_ON_RESUME, runtime resume
>> routine also resets the controller.
>>
>> This is bad for USB drivers without reset_resume callback, because
>> there's no subsequent call of usb_dev_complete() ->
>> usb_resume_complete() to force rebinding the driver to the device. For
>> instance, btusb device stops working after xHCI controller is runtime
>> resumed, if the controlled is quirked with XHCI_RESET_ON_RESUME.
>>
>> So always take XHCI_RESET_ON_RESUME into account to solve the issue.
>>
>> Signed-off-by: Kai-Heng Feng <[email protected]>
>
> A gentle ping...

Thanks
Adding to queue

-Mathias

2021-12-09 06:42:22

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] xhci: Remove CONFIG_USB_DEFAULT_PERSIST to prevent xHCI from runtime suspending

On Wed, Dec 1, 2021 at 5:00 PM Mathias Nyman
<[email protected]> wrote:
>
> On 1.12.2021 2.19, Kai-Heng Feng wrote:
> > On Fri, Nov 19, 2021 at 5:27 PM Kai-Heng Feng
> > <[email protected]> wrote:
> >>
> >> When the xHCI is quirked with XHCI_RESET_ON_RESUME, runtime resume
> >> routine also resets the controller.
> >>
> >> This is bad for USB drivers without reset_resume callback, because
> >> there's no subsequent call of usb_dev_complete() ->
> >> usb_resume_complete() to force rebinding the driver to the device. For
> >> instance, btusb device stops working after xHCI controller is runtime
> >> resumed, if the controlled is quirked with XHCI_RESET_ON_RESUME.
> >>
> >> So always take XHCI_RESET_ON_RESUME into account to solve the issue.
> >>
> >> Signed-off-by: Kai-Heng Feng <[email protected]>
> >
> > A gentle ping...
>
> Thanks
> Adding to queue

I haven't found this patch in your repo. Can you please push it so I
can backport it to downstream kernel?

Kai-Heng

>
> -Mathias

2021-12-10 14:20:52

by Mathias Nyman

[permalink] [raw]
Subject: Re: [PATCH] xhci: Remove CONFIG_USB_DEFAULT_PERSIST to prevent xHCI from runtime suspending

On 9.12.2021 8.42, Kai-Heng Feng wrote:
> On Wed, Dec 1, 2021 at 5:00 PM Mathias Nyman
> <[email protected]> wrote:
>>
>> On 1.12.2021 2.19, Kai-Heng Feng wrote:
>>> On Fri, Nov 19, 2021 at 5:27 PM Kai-Heng Feng
>>> <[email protected]> wrote:
>>>>
>>>> When the xHCI is quirked with XHCI_RESET_ON_RESUME, runtime resume
>>>> routine also resets the controller.
>>>>
>>>> This is bad for USB drivers without reset_resume callback, because
>>>> there's no subsequent call of usb_dev_complete() ->
>>>> usb_resume_complete() to force rebinding the driver to the device. For
>>>> instance, btusb device stops working after xHCI controller is runtime
>>>> resumed, if the controlled is quirked with XHCI_RESET_ON_RESUME.
>>>>
>>>> So always take XHCI_RESET_ON_RESUME into account to solve the issue.
>>>>
>>>> Signed-off-by: Kai-Heng Feng <[email protected]>
>>>
>>> A gentle ping...
>>
>> Thanks
>> Adding to queue
>
> I haven't found this patch in your repo. Can you please push it so I
> can backport it to downstream kernel?

Patch got shuffled around a bit.
It's now in my for-usb-linus branch, and sent to Greg

Thanks
-Mathias