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

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

[Peter Chen]

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;
-- 
2.17.1

[PATCH 2/2] usb: host: xhci-plat: forbid runtime pm conditionally

[Peter Chen]

Some users may want to enable runtime pm by default, using the quirk
to judge it.

Reviewed-by: Jun Li <jun.li@nxp.com>
Reviewed-by: Ran Wang <ran.wang_1@nxp.com>
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/host/xhci-plat.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index aa2d35f98200..effb4f0586ae 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -351,7 +351,8 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	 * Prevent runtime pm from being on as default, users should enable
 	 * runtime pm using power/control in sysfs.
 	 */
-	pm_runtime_forbid(&pdev->dev);
+	if (!(xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW))
+		pm_runtime_forbid(&pdev->dev);
 
 	return 0;
 
-- 
2.17.1

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

[Roger Quadros]

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

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

[Peter Chen]

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

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

[Mathias Nyman]

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

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

[Peter Chen]

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

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

[Mathias Nyman]

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

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

[Peter Chen]

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.

-- 

Thanks,
Peter Chen

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

[Peter Chen]

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

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

[Peter Chen]

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