| Message ID | 20211119092628.677935-1-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 811ae81320da53a5670c36970cefacca8519f90e |
| Headers | show |
| Series | xhci: Remove CONFIG_USB_DEFAULT_PERSIST to prevent xHCI from runtime suspending | expand |
On Fri, Nov 19, 2021 at 5:27 PM Kai-Heng Feng <kai.heng.feng@canonical.com> 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 <kai.heng.feng@canonical.com> 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 >
On 1.12.2021 2.19, Kai-Heng Feng wrote: > On Fri, Nov 19, 2021 at 5:27 PM Kai-Heng Feng > <kai.heng.feng@canonical.com> 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 <kai.heng.feng@canonical.com> > > A gentle ping... Thanks Adding to queue -Mathias
On Wed, Dec 1, 2021 at 5:00 PM Mathias Nyman <mathias.nyman@linux.intel.com> wrote: > > On 1.12.2021 2.19, Kai-Heng Feng wrote: > > On Fri, Nov 19, 2021 at 5:27 PM Kai-Heng Feng > > <kai.heng.feng@canonical.com> 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 <kai.heng.feng@canonical.com> > > > > 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
On 9.12.2021 8.42, Kai-Heng Feng wrote: > On Wed, Dec 1, 2021 at 5:00 PM Mathias Nyman > <mathias.nyman@linux.intel.com> wrote: >> >> On 1.12.2021 2.19, Kai-Heng Feng wrote: >>> On Fri, Nov 19, 2021 at 5:27 PM Kai-Heng Feng >>> <kai.heng.feng@canonical.com> 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 <kai.heng.feng@canonical.com> >>> >>> 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
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? */
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 <kai.heng.feng@canonical.com> --- drivers/usb/host/xhci.c | 4 ---- 1 file changed, 4 deletions(-)