* [PATCH v6] xhci: re-initialize the HC during resume if HCE was set @ 2022-01-29 9:30 Puma Hsu 2022-02-02 19:12 ` Mathias Nyman 0 siblings, 1 reply; 6+ messages in thread From: Puma Hsu @ 2022-01-29 9:30 UTC (permalink / raw) To: mathias.nyman, gregkh Cc: s.shtylyov, albertccwang, linux-usb, linux-kernel, Puma Hsu, stable When HCE(Host Controller Error) is set, it means an internal error condition has been detected. Software needs to re-initialize the HC, so add this check in xhci resume. Cc: stable@vger.kernel.org Signed-off-by: Puma Hsu <pumahsu@google.com> --- v2: Follow Sergey Shtylyov <s.shtylyov@omp.ru>'s comment. v3: Add stable@vger.kernel.org for stable release. v4: Refine the commit message. v5: Add a debug log. Follow Mathias Nyman <mathias.nyman@linux.intel.com>'s comment. v6: Fix the missing declaration for str. drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357cabb265..6f1198068004 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) int retval = 0; bool comp_timer_running = false; bool pending_portevent = false; + char str[XHCI_MSG_MAX]; if (!hcd->state) return 0; @@ -1146,8 +1147,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) temp = readl(&xhci->op_regs->status); } - /* If restore operation fails, re-initialize the HC during resume */ - if ((temp & STS_SRE) || hibernated) { + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ + if ((temp & (STS_SRE | STS_HCE)) || hibernated) { + xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n", + xhci_decode_usbsts(str, temp)); if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) { -- 2.35.0.rc2.247.g8bbb082509-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v6] xhci: re-initialize the HC during resume if HCE was set 2022-01-29 9:30 [PATCH v6] xhci: re-initialize the HC during resume if HCE was set Puma Hsu @ 2022-02-02 19:12 ` Mathias Nyman 2022-02-08 7:11 ` Puma Hsu 0 siblings, 1 reply; 6+ messages in thread From: Mathias Nyman @ 2022-02-02 19:12 UTC (permalink / raw) To: Puma Hsu, mathias.nyman, gregkh Cc: s.shtylyov, albertccwang, linux-usb, linux-kernel, stable On 29.1.2022 11.30, Puma Hsu wrote: > When HCE(Host Controller Error) is set, it means an internal > error condition has been detected. Software needs to re-initialize > the HC, so add this check in xhci resume. > > Cc: stable@vger.kernel.org > Signed-off-by: Puma Hsu <pumahsu@google.com> > --- > v2: Follow Sergey Shtylyov <s.shtylyov@omp.ru>'s comment. > v3: Add stable@vger.kernel.org for stable release. > v4: Refine the commit message. > v5: Add a debug log. Follow Mathias Nyman <mathias.nyman@linux.intel.com>'s comment. > v6: Fix the missing declaration for str. > > drivers/usb/host/xhci.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index dc357cabb265..6f1198068004 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > int retval = 0; > bool comp_timer_running = false; > bool pending_portevent = false; > + char str[XHCI_MSG_MAX]; > > if (!hcd->state) > return 0; > @@ -1146,8 +1147,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > temp = readl(&xhci->op_regs->status); > } > > - /* If restore operation fails, re-initialize the HC during resume */ > - if ((temp & STS_SRE) || hibernated) { > + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ > + if ((temp & (STS_SRE | STS_HCE)) || hibernated) { > + xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n", > + xhci_decode_usbsts(str, temp)); > > if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && > !(xhci_all_ports_seen_u0(xhci))) { > Ended up modifying this patch a bit more than I first intended, - don't print warning in hibernation case, only error. - maybe using a lot of stack for a debug string isn't really needed. - make sure we read the usbsts register before checking for the HCE bit. Does the below work for you? If yes, and you agree I'll apply it instead --- diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357cabb265..04ec2de158bf 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) int retval = 0; bool comp_timer_running = false; bool pending_portevent = false; + bool reinit_xhc = false; if (!hcd->state) return 0; @@ -1107,10 +1108,11 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags); spin_lock_irq(&xhci->lock); - if ((xhci->quirks & XHCI_RESET_ON_RESUME) || xhci->broken_suspend) - hibernated = true; - if (!hibernated) { + if (hibernated || xhci->quirks & XHCI_RESET_ON_RESUME || xhci->broken_suspend) + reinit_xhc = true; + + if (!reinit_xhc) { /* * Some controllers might lose power during suspend, so wait * for controller not ready bit to clear, just as in xHC init. @@ -1143,12 +1145,17 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) spin_unlock_irq(&xhci->lock); return -ETIMEDOUT; } - temp = readl(&xhci->op_regs->status); } - /* If restore operation fails, re-initialize the HC during resume */ - if ((temp & STS_SRE) || hibernated) { + temp = readl(&xhci->op_regs->status); + + /* re-initialize the HC on Restore Error, or Host Controller Error */ + if (temp & (STS_SRE | STS_HCE)) { + reinit_xhc = true; + xhci_warn(xhci, "xHC error in resume, USBSTS 0x%x, Reinit\n", temp); + } + if (reinit_xhc) { if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) { del_timer_sync(&xhci->comp_mode_recovery_timer); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v6] xhci: re-initialize the HC during resume if HCE was set 2022-02-02 19:12 ` Mathias Nyman @ 2022-02-08 7:11 ` Puma Hsu 2022-02-10 11:09 ` Mathias Nyman 0 siblings, 1 reply; 6+ messages in thread From: Puma Hsu @ 2022-02-08 7:11 UTC (permalink / raw) To: Mathias Nyman Cc: mathias.nyman, Greg KH, Sergey Shtylyov, Albert Wang, linux-usb, linux-kernel, stable On Thu, Feb 3, 2022 at 3:11 AM Mathias Nyman <mathias.nyman@linux.intel.com> wrote: > > On 29.1.2022 11.30, Puma Hsu wrote: > > When HCE(Host Controller Error) is set, it means an internal > > error condition has been detected. Software needs to re-initialize > > the HC, so add this check in xhci resume. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Puma Hsu <pumahsu@google.com> > > --- > > v2: Follow Sergey Shtylyov <s.shtylyov@omp.ru>'s comment. > > v3: Add stable@vger.kernel.org for stable release. > > v4: Refine the commit message. > > v5: Add a debug log. Follow Mathias Nyman <mathias.nyman@linux.intel.com>'s comment. > > v6: Fix the missing declaration for str. > > > > drivers/usb/host/xhci.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > > index dc357cabb265..6f1198068004 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > > int retval = 0; > > bool comp_timer_running = false; > > bool pending_portevent = false; > > + char str[XHCI_MSG_MAX]; > > > > if (!hcd->state) > > return 0; > > @@ -1146,8 +1147,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > > temp = readl(&xhci->op_regs->status); > > } > > > > - /* If restore operation fails, re-initialize the HC during resume */ > > - if ((temp & STS_SRE) || hibernated) { > > + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ > > + if ((temp & (STS_SRE | STS_HCE)) || hibernated) { > > + xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n", > > + xhci_decode_usbsts(str, temp)); > > > > if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && > > !(xhci_all_ports_seen_u0(xhci))) { > > > > Ended up modifying this patch a bit more than I first intended, > - don't print warning in hibernation case, only error. > - maybe using a lot of stack for a debug string isn't really needed. > - make sure we read the usbsts register before checking for the HCE bit. > > Does the below work for you? If yes, and you agree I'll apply it instead Hi Mathias, Yes, your patch works for me, thanks! Will you submit a new patch? or should I update to a new version? Thanks. > --- > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index dc357cabb265..04ec2de158bf 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > int retval = 0; > bool comp_timer_running = false; > bool pending_portevent = false; > + bool reinit_xhc = false; > > if (!hcd->state) > return 0; > @@ -1107,10 +1108,11 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags); > > spin_lock_irq(&xhci->lock); > - if ((xhci->quirks & XHCI_RESET_ON_RESUME) || xhci->broken_suspend) > - hibernated = true; > > - if (!hibernated) { > + if (hibernated || xhci->quirks & XHCI_RESET_ON_RESUME || xhci->broken_suspend) > + reinit_xhc = true; > + > + if (!reinit_xhc) { > /* > * Some controllers might lose power during suspend, so wait > * for controller not ready bit to clear, just as in xHC init. > @@ -1143,12 +1145,17 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > spin_unlock_irq(&xhci->lock); > return -ETIMEDOUT; > } > - temp = readl(&xhci->op_regs->status); > } > > - /* If restore operation fails, re-initialize the HC during resume */ > - if ((temp & STS_SRE) || hibernated) { > + temp = readl(&xhci->op_regs->status); > + > + /* re-initialize the HC on Restore Error, or Host Controller Error */ > + if (temp & (STS_SRE | STS_HCE)) { > + reinit_xhc = true; > + xhci_warn(xhci, "xHC error in resume, USBSTS 0x%x, Reinit\n", temp); > + } > > + if (reinit_xhc) { > if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && > !(xhci_all_ports_seen_u0(xhci))) { > del_timer_sync(&xhci->comp_mode_recovery_timer); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] xhci: re-initialize the HC during resume if HCE was set 2022-02-08 7:11 ` Puma Hsu @ 2022-02-10 11:09 ` Mathias Nyman 2022-02-15 12:05 ` Puma Hsu 0 siblings, 1 reply; 6+ messages in thread From: Mathias Nyman @ 2022-02-10 11:09 UTC (permalink / raw) To: Puma Hsu Cc: mathias.nyman, Greg KH, Sergey Shtylyov, Albert Wang, linux-usb, linux-kernel, stable On 8.2.2022 9.11, Puma Hsu wrote: > On Thu, Feb 3, 2022 at 3:11 AM Mathias Nyman > <mathias.nyman@linux.intel.com> wrote: >> >> On 29.1.2022 11.30, Puma Hsu wrote: >>> When HCE(Host Controller Error) is set, it means an internal >>> error condition has been detected. Software needs to re-initialize >>> the HC, so add this check in xhci resume. >>> >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Puma Hsu <pumahsu@google.com> >>> --- >>> v2: Follow Sergey Shtylyov <s.shtylyov@omp.ru>'s comment. >>> v3: Add stable@vger.kernel.org for stable release. >>> v4: Refine the commit message. >>> v5: Add a debug log. Follow Mathias Nyman <mathias.nyman@linux.intel.com>'s comment. >>> v6: Fix the missing declaration for str. >>> >>> drivers/usb/host/xhci.c | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >>> index dc357cabb265..6f1198068004 100644 >>> --- a/drivers/usb/host/xhci.c >>> +++ b/drivers/usb/host/xhci.c >>> @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) >>> int retval = 0; >>> bool comp_timer_running = false; >>> bool pending_portevent = false; >>> + char str[XHCI_MSG_MAX]; >>> >>> if (!hcd->state) >>> return 0; >>> @@ -1146,8 +1147,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) >>> temp = readl(&xhci->op_regs->status); >>> } >>> >>> - /* If restore operation fails, re-initialize the HC during resume */ >>> - if ((temp & STS_SRE) || hibernated) { >>> + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ >>> + if ((temp & (STS_SRE | STS_HCE)) || hibernated) { >>> + xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n", >>> + xhci_decode_usbsts(str, temp)); >>> >>> if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && >>> !(xhci_all_ports_seen_u0(xhci))) { >>> >> >> Ended up modifying this patch a bit more than I first intended, >> - don't print warning in hibernation case, only error. >> - maybe using a lot of stack for a debug string isn't really needed. >> - make sure we read the usbsts register before checking for the HCE bit. >> >> Does the below work for you? If yes, and you agree I'll apply it instead > > Hi Mathias, > Yes, your patch works for me, thanks! > Will you submit a new patch? or should I update to a new version? > Thanks. I'll submit it Thanks -Mathias ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] xhci: re-initialize the HC during resume if HCE was set 2022-02-10 11:09 ` Mathias Nyman @ 2022-02-15 12:05 ` Puma Hsu 2022-02-15 12:35 ` Mathias Nyman 0 siblings, 1 reply; 6+ messages in thread From: Puma Hsu @ 2022-02-15 12:05 UTC (permalink / raw) To: Mathias Nyman Cc: mathias.nyman, Greg KH, Sergey Shtylyov, Albert Wang, linux-usb, linux-kernel, stable On Thu, Feb 10, 2022 at 7:08 PM Mathias Nyman <mathias.nyman@linux.intel.com> wrote: > > On 8.2.2022 9.11, Puma Hsu wrote: > > On Thu, Feb 3, 2022 at 3:11 AM Mathias Nyman > > <mathias.nyman@linux.intel.com> wrote: > >> > >> On 29.1.2022 11.30, Puma Hsu wrote: > >>> When HCE(Host Controller Error) is set, it means an internal > >>> error condition has been detected. Software needs to re-initialize > >>> the HC, so add this check in xhci resume. > >>> > >>> Cc: stable@vger.kernel.org > >>> Signed-off-by: Puma Hsu <pumahsu@google.com> > >>> --- > >>> v2: Follow Sergey Shtylyov <s.shtylyov@omp.ru>'s comment. > >>> v3: Add stable@vger.kernel.org for stable release. > >>> v4: Refine the commit message. > >>> v5: Add a debug log. Follow Mathias Nyman <mathias.nyman@linux.intel.com>'s comment. > >>> v6: Fix the missing declaration for str. > >>> > >>> drivers/usb/host/xhci.c | 7 +++++-- > >>> 1 file changed, 5 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > >>> index dc357cabb265..6f1198068004 100644 > >>> --- a/drivers/usb/host/xhci.c > >>> +++ b/drivers/usb/host/xhci.c > >>> @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > >>> int retval = 0; > >>> bool comp_timer_running = false; > >>> bool pending_portevent = false; > >>> + char str[XHCI_MSG_MAX]; > >>> > >>> if (!hcd->state) > >>> return 0; > >>> @@ -1146,8 +1147,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > >>> temp = readl(&xhci->op_regs->status); > >>> } > >>> > >>> - /* If restore operation fails, re-initialize the HC during resume */ > >>> - if ((temp & STS_SRE) || hibernated) { > >>> + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ > >>> + if ((temp & (STS_SRE | STS_HCE)) || hibernated) { > >>> + xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n", > >>> + xhci_decode_usbsts(str, temp)); > >>> > >>> if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && > >>> !(xhci_all_ports_seen_u0(xhci))) { > >>> > >> > >> Ended up modifying this patch a bit more than I first intended, > >> - don't print warning in hibernation case, only error. > >> - maybe using a lot of stack for a debug string isn't really needed. > >> - make sure we read the usbsts register before checking for the HCE bit. > >> > >> Does the below work for you? If yes, and you agree I'll apply it instead > > > > Hi Mathias, > > Yes, your patch works for me, thanks! > > Will you submit a new patch? or should I update to a new version? > > Thanks. > > I'll submit it Hi Mathias, Could I know when you will submit the new patch? We will sync the patch to our project. Thank you for your effort! > > Thanks > -Mathias ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] xhci: re-initialize the HC during resume if HCE was set 2022-02-15 12:05 ` Puma Hsu @ 2022-02-15 12:35 ` Mathias Nyman 0 siblings, 0 replies; 6+ messages in thread From: Mathias Nyman @ 2022-02-15 12:35 UTC (permalink / raw) To: Puma Hsu Cc: mathias.nyman, Greg KH, Sergey Shtylyov, Albert Wang, linux-usb, linux-kernel, stable On 15.2.2022 14.05, Puma Hsu wrote: > On Thu, Feb 10, 2022 at 7:08 PM Mathias Nyman > <mathias.nyman@linux.intel.com> wrote: >> >> On 8.2.2022 9.11, Puma Hsu wrote: >>> On Thu, Feb 3, 2022 at 3:11 AM Mathias Nyman >>> <mathias.nyman@linux.intel.com> wrote: >>>> >>>> On 29.1.2022 11.30, Puma Hsu wrote: >>>>> When HCE(Host Controller Error) is set, it means an internal >>>>> error condition has been detected. Software needs to re-initialize >>>>> the HC, so add this check in xhci resume. >>>>> >>>>> Cc: stable@vger.kernel.org >>>>> Signed-off-by: Puma Hsu <pumahsu@google.com> >>>>> --- >>>>> v2: Follow Sergey Shtylyov <s.shtylyov@omp.ru>'s comment. >>>>> v3: Add stable@vger.kernel.org for stable release. >>>>> v4: Refine the commit message. >>>>> v5: Add a debug log. Follow Mathias Nyman <mathias.nyman@linux.intel.com>'s comment. >>>>> v6: Fix the missing declaration for str. >>>>> >>>>> drivers/usb/host/xhci.c | 7 +++++-- >>>>> 1 file changed, 5 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >>>>> index dc357cabb265..6f1198068004 100644 >>>>> --- a/drivers/usb/host/xhci.c >>>>> +++ b/drivers/usb/host/xhci.c >>>>> @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) >>>>> int retval = 0; >>>>> bool comp_timer_running = false; >>>>> bool pending_portevent = false; >>>>> + char str[XHCI_MSG_MAX]; >>>>> >>>>> if (!hcd->state) >>>>> return 0; >>>>> @@ -1146,8 +1147,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) >>>>> temp = readl(&xhci->op_regs->status); >>>>> } >>>>> >>>>> - /* If restore operation fails, re-initialize the HC during resume */ >>>>> - if ((temp & STS_SRE) || hibernated) { >>>>> + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ >>>>> + if ((temp & (STS_SRE | STS_HCE)) || hibernated) { >>>>> + xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n", >>>>> + xhci_decode_usbsts(str, temp)); >>>>> >>>>> if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && >>>>> !(xhci_all_ports_seen_u0(xhci))) { >>>>> >>>> >>>> Ended up modifying this patch a bit more than I first intended, >>>> - don't print warning in hibernation case, only error. >>>> - maybe using a lot of stack for a debug string isn't really needed. >>>> - make sure we read the usbsts register before checking for the HCE bit. >>>> >>>> Does the below work for you? If yes, and you agree I'll apply it instead >>> >>> Hi Mathias, >>> Yes, your patch works for me, thanks! >>> Will you submit a new patch? or should I update to a new version? >>> Thanks. >> >> I'll submit it > > Hi Mathias, > Could I know when you will submit the new patch? > We will sync the patch to our project. > Thank you for your effort! Just did, thanks for the reminder. -Mathias ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-15 12:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-29 9:30 [PATCH v6] xhci: re-initialize the HC during resume if HCE was set Puma Hsu 2022-02-02 19:12 ` Mathias Nyman 2022-02-08 7:11 ` Puma Hsu 2022-02-10 11:09 ` Mathias Nyman 2022-02-15 12:05 ` Puma Hsu 2022-02-15 12:35 ` Mathias Nyman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox