diff mbox series

[1/2] usb: host: add XHCI_CDNS_HOST flag

Message ID 20201022030133.19528-1-peter.chen@nxp.com
State New
Headers show
Series [1/2] usb: host: add XHCI_CDNS_HOST flag | expand

Commit Message

Peter Chen Oct. 22, 2020, 3:01 a.m. UTC
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(-)

Comments

Roger Quadros Oct. 22, 2020, 6:48 a.m. UTC | #1
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;

> 


-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Peter Chen Oct. 22, 2020, 7:17 a.m. UTC | #2
> 

> 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
Mathias Nyman Oct. 26, 2020, 3:02 p.m. UTC | #3
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
Peter Chen Oct. 27, 2020, 1:50 a.m. UTC | #4
> > 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
Mathias Nyman Oct. 27, 2020, 8:33 a.m. UTC | #5
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
Peter Chen Oct. 27, 2020, 9:50 a.m. UTC | #6
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.
Peter Chen Oct. 28, 2020, 7:02 a.m. UTC | #7
> > >>>
> > >>>  	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
Peter Chen Oct. 30, 2020, 11:14 p.m. UTC | #8
> >

> > 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 mbox series

Patch

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;