| Message ID | 20201022030133.19528-1-peter.chen@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] usb: host: add XHCI_CDNS_HOST flag | expand |
Hi Peter, On 22/10/2020 06:01, Peter Chen wrote: > The Cadence xHCI host has the same issue with Intel's, s/with/as > it is triggered by reboot stress test. Can you please provide some more details about the test so I can try at my end. Thanks. cheers, -roger > > Cc: Pawel Laszczak <pawell@cadence.com> > Cc: Roger Quadros <rogerq@ti.com> > Signed-off-by: Peter Chen <peter.chen@nxp.com> > --- > drivers/usb/host/xhci.c | 2 +- > drivers/usb/host/xhci.h | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 482fe8c5e3b4..fc72a03dc27f 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -193,7 +193,7 @@ int xhci_reset(struct xhci_hcd *xhci) > * Without this delay, the subsequent HC register access, > * may result in a system hang very rarely. > */ > - if (xhci->quirks & XHCI_INTEL_HOST) > + if (xhci->quirks & (XHCI_INTEL_HOST | XHCI_CDNS_HOST)) > udelay(1000); > > ret = xhci_handshake(&xhci->op_regs->command, > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > index 8be88379c0fb..4b7275c73ea5 100644 > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -1877,6 +1877,7 @@ struct xhci_hcd { > #define XHCI_SNPS_BROKEN_SUSPEND BIT_ULL(35) > #define XHCI_RENESAS_FW_QUIRK BIT_ULL(36) > #define XHCI_SKIP_PHY_INIT BIT_ULL(37) > +#define XHCI_CDNS_HOST BIT_ULL(38) > > unsigned int num_active_eps; > unsigned int limit_active_eps; >
> > On 22/10/2020 06:01, Peter Chen wrote: > > The Cadence xHCI host has the same issue with Intel's, > > s/with/as > > > it is triggered by reboot stress test. > > Can you please provide some more details about the test so I can try at my end. > Thanks. > Connect USB3 Drive at port, and reboot system over night. Peter
On 22.10.2020 6.01, Peter Chen wrote: > The Cadence xHCI host has the same issue with Intel's, > it is triggered by reboot stress test. > > Cc: Pawel Laszczak <pawell@cadence.com> > Cc: Roger Quadros <rogerq@ti.com> > Signed-off-by: Peter Chen <peter.chen@nxp.com> > --- > drivers/usb/host/xhci.c | 2 +- > drivers/usb/host/xhci.h | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 482fe8c5e3b4..fc72a03dc27f 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -193,7 +193,7 @@ int xhci_reset(struct xhci_hcd *xhci) > * Without this delay, the subsequent HC register access, > * may result in a system hang very rarely. > */ > - if (xhci->quirks & XHCI_INTEL_HOST) > + if (xhci->quirks & (XHCI_INTEL_HOST | XHCI_CDNS_HOST)) > udelay(1000); > > ret = xhci_handshake(&xhci->op_regs->command, > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > index 8be88379c0fb..4b7275c73ea5 100644 > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -1877,6 +1877,7 @@ struct xhci_hcd { > #define XHCI_SNPS_BROKEN_SUSPEND BIT_ULL(35) > #define XHCI_RENESAS_FW_QUIRK BIT_ULL(36) > #define XHCI_SKIP_PHY_INIT BIT_ULL(37) > +#define XHCI_CDNS_HOST BIT_ULL(38) > > unsigned int num_active_eps; > unsigned int limit_active_eps; > Is the XHCI_CDNS_HOST quirk bit set in some other patchseries? -Mathias
> > Cc: Pawel Laszczak <pawell@cadence.com> > > Cc: Roger Quadros <rogerq@ti.com> > > Signed-off-by: Peter Chen <peter.chen@nxp.com> > > --- > > drivers/usb/host/xhci.c | 2 +- > > drivers/usb/host/xhci.h | 1 + > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index > > 482fe8c5e3b4..fc72a03dc27f 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -193,7 +193,7 @@ int xhci_reset(struct xhci_hcd *xhci) > > * Without this delay, the subsequent HC register access, > > * may result in a system hang very rarely. > > */ > > - if (xhci->quirks & XHCI_INTEL_HOST) > > + if (xhci->quirks & (XHCI_INTEL_HOST | XHCI_CDNS_HOST)) > > udelay(1000); > > > > ret = xhci_handshake(&xhci->op_regs->command, > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index > > 8be88379c0fb..4b7275c73ea5 100644 > > --- a/drivers/usb/host/xhci.h > > +++ b/drivers/usb/host/xhci.h > > @@ -1877,6 +1877,7 @@ struct xhci_hcd { > > #define XHCI_SNPS_BROKEN_SUSPEND BIT_ULL(35) > > #define XHCI_RENESAS_FW_QUIRK BIT_ULL(36) > > #define XHCI_SKIP_PHY_INIT BIT_ULL(37) > > +#define XHCI_CDNS_HOST BIT_ULL(38) > > > > unsigned int num_active_eps; > > unsigned int limit_active_eps; > > > > Is the XHCI_CDNS_HOST quirk bit set in some other patchseries? > Currently, no, only at the function of xhci_reset in this commit. Peter
On 27.10.2020 3.50, Peter Chen wrote: > >>> Cc: Pawel Laszczak <pawell@cadence.com> >>> Cc: Roger Quadros <rogerq@ti.com> >>> Signed-off-by: Peter Chen <peter.chen@nxp.com> >>> --- >>> drivers/usb/host/xhci.c | 2 +- >>> drivers/usb/host/xhci.h | 1 + >>> 2 files changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index >>> 482fe8c5e3b4..fc72a03dc27f 100644 >>> --- a/drivers/usb/host/xhci.c >>> +++ b/drivers/usb/host/xhci.c >>> @@ -193,7 +193,7 @@ int xhci_reset(struct xhci_hcd *xhci) >>> * Without this delay, the subsequent HC register access, >>> * may result in a system hang very rarely. >>> */ >>> - if (xhci->quirks & XHCI_INTEL_HOST) >>> + if (xhci->quirks & (XHCI_INTEL_HOST | XHCI_CDNS_HOST)) >>> udelay(1000); >>> >>> ret = xhci_handshake(&xhci->op_regs->command, >>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index >>> 8be88379c0fb..4b7275c73ea5 100644 >>> --- a/drivers/usb/host/xhci.h >>> +++ b/drivers/usb/host/xhci.h >>> @@ -1877,6 +1877,7 @@ struct xhci_hcd { >>> #define XHCI_SNPS_BROKEN_SUSPEND BIT_ULL(35) >>> #define XHCI_RENESAS_FW_QUIRK BIT_ULL(36) >>> #define XHCI_SKIP_PHY_INIT BIT_ULL(37) >>> +#define XHCI_CDNS_HOST BIT_ULL(38) >>> >>> unsigned int num_active_eps; >>> unsigned int limit_active_eps; >>> >> >> Is the XHCI_CDNS_HOST quirk bit set in some other patchseries? >> > > Currently, no, only at the function of xhci_reset in this commit. I can only see it being checked, not set. Am I missing something? -Mathias
On 20-10-27 10:33:52, Mathias Nyman wrote: > On 27.10.2020 3.50, Peter Chen wrote: > > > >>> Cc: Pawel Laszczak <pawell@cadence.com> > >>> Cc: Roger Quadros <rogerq@ti.com> > >>> Signed-off-by: Peter Chen <peter.chen@nxp.com> > >>> --- > >>> drivers/usb/host/xhci.c | 2 +- > >>> drivers/usb/host/xhci.h | 1 + > >>> 2 files changed, 2 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index > >>> 482fe8c5e3b4..fc72a03dc27f 100644 > >>> --- a/drivers/usb/host/xhci.c > >>> +++ b/drivers/usb/host/xhci.c > >>> @@ -193,7 +193,7 @@ int xhci_reset(struct xhci_hcd *xhci) > >>> * Without this delay, the subsequent HC register access, > >>> * may result in a system hang very rarely. > >>> */ > >>> - if (xhci->quirks & XHCI_INTEL_HOST) > >>> + if (xhci->quirks & (XHCI_INTEL_HOST | XHCI_CDNS_HOST)) > >>> udelay(1000); > >>> > >>> ret = xhci_handshake(&xhci->op_regs->command, > >>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index > >>> 8be88379c0fb..4b7275c73ea5 100644 > >>> --- a/drivers/usb/host/xhci.h > >>> +++ b/drivers/usb/host/xhci.h > >>> @@ -1877,6 +1877,7 @@ struct xhci_hcd { > >>> #define XHCI_SNPS_BROKEN_SUSPEND BIT_ULL(35) > >>> #define XHCI_RENESAS_FW_QUIRK BIT_ULL(36) > >>> #define XHCI_SKIP_PHY_INIT BIT_ULL(37) > >>> +#define XHCI_CDNS_HOST BIT_ULL(38) > >>> > >>> unsigned int num_active_eps; > >>> unsigned int limit_active_eps; > >>> > >> > >> Is the XHCI_CDNS_HOST quirk bit set in some other patchseries? > >> > > > > Currently, no, only at the function of xhci_reset in this commit. > > I can only see it being checked, not set. Am I missing something? > > -Mathias > Oh, there is a missing patch for cdns3 host change, the code is located at drivers/usb/cdns3/host.c. I will merge them together, and send v2.
> > >>> > > >>> unsigned int num_active_eps; > > >>> unsigned int limit_active_eps; > > >>> > > >> > > >> Is the XHCI_CDNS_HOST quirk bit set in some other patchseries? > > >> > > > > > > Currently, no, only at the function of xhci_reset in this commit. > > > > I can only see it being checked, not set. Am I missing something? > > > > -Mathias > > > > Oh, there is a missing patch for cdns3 host change, the code is located at > drivers/usb/cdns3/host.c. I will merge them together, and send v2. > Mathias, adding XHCI_CDNS_HOST quirk at cdns3 part depends on some reviewing patches [1], these patches add the struct xhci_plat_priv with other quirks first. Would you help queue these cdns3 host patches after reviewing? Or what's the suggestion to handle this dependency? Thanks. [1] https://patchwork.kernel.org/project/linux-usb/patch/20201022013930.2166-1-peter.chen@nxp.com/ Peter
> > > > Oh, there is a missing patch for cdns3 host change, the code is > > located at drivers/usb/cdns3/host.c. I will merge them together, and send v2. > > > > Mathias, adding XHCI_CDNS_HOST quirk at cdns3 part depends on some > reviewing patches [1], these patches add the struct xhci_plat_priv with other > quirks first. Would you help queue these cdns3 host patches after reviewing? > Or what's the suggestion to handle this dependency? Thanks. > > [1] > https://patchwork.kernel.org/project/linux-usb/patch/20201022013930.2166-1 > -peter.chen@nxp.com/ > I think we may not need this patch at upstream kernel, I run the test overnight, and could not reproduce hang issue. @Roger Quadros, did you meet hang at the reboot stress test? Peter
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 482fe8c5e3b4..fc72a03dc27f 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -193,7 +193,7 @@ int xhci_reset(struct xhci_hcd *xhci) * Without this delay, the subsequent HC register access, * may result in a system hang very rarely. */ - if (xhci->quirks & XHCI_INTEL_HOST) + if (xhci->quirks & (XHCI_INTEL_HOST | XHCI_CDNS_HOST)) udelay(1000); ret = xhci_handshake(&xhci->op_regs->command, diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 8be88379c0fb..4b7275c73ea5 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1877,6 +1877,7 @@ struct xhci_hcd { #define XHCI_SNPS_BROKEN_SUSPEND BIT_ULL(35) #define XHCI_RENESAS_FW_QUIRK BIT_ULL(36) #define XHCI_SKIP_PHY_INIT BIT_ULL(37) +#define XHCI_CDNS_HOST BIT_ULL(38) unsigned int num_active_eps; unsigned int limit_active_eps;
The Cadence xHCI host has the same issue with Intel's, it is triggered by reboot stress test. Cc: Pawel Laszczak <pawell@cadence.com> Cc: Roger Quadros <rogerq@ti.com> Signed-off-by: Peter Chen <peter.chen@nxp.com> --- drivers/usb/host/xhci.c | 2 +- drivers/usb/host/xhci.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)