[PATCH] usb: dwc3: Fix timeout issue during controller enter/exit from halt state

[PATCH] usb: dwc3: Fix timeout issue during controller enter/exit from halt state

[Selvarasu Ganesan]

There is a frequent timeout during controller enter/exit from halt state
after toggling the run_stop bit by SW. This timeout occurs when
performing frequent role switches between host and device, causing
device enumeration issues due to the timeout. This issue was not present
when USB2 suspend PHY was disabled by passing the SNPS quirks
(snps,dis_u2_susphy_quirk and snps,dis_enblslpm_quirk) from the DTS.
However, there is a requirement to enable USB2 suspend PHY by setting of
GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY bits when controller starts
in gadget or host mode results in the timeout issue.

This commit addresses this timeout issue by ensuring that the bits
GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY are cleared before starting
the dwc3_gadget_run_stop sequence and restoring them after the
dwc3_gadget_run_stop sequence is completed.

Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Selvarasu Ganesan <selvarasu.g@samsung.com>
---
 drivers/usb/dwc3/gadget.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index d27af65eb08a..4a158f703d64 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2629,10 +2629,25 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
 {
 	u32			reg;
 	u32			timeout = 2000;
+	u32			saved_config = 0;
 
 	if (pm_runtime_suspended(dwc->dev))
 		return 0;
 
+	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+	if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {
+		saved_config |= DWC3_GUSB2PHYCFG_SUSPHY;
+		reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
+	}
+
+	if (reg & DWC3_GUSB2PHYCFG_ENBLSLPM) {
+		saved_config |= DWC3_GUSB2PHYCFG_ENBLSLPM;
+		reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
+	}
+
+	if (saved_config)
+		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+
 	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
 	if (is_on) {
 		if (DWC3_VER_IS_WITHIN(DWC3, ANY, 187A)) {
@@ -2660,6 +2675,12 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
 		reg &= DWC3_DSTS_DEVCTRLHLT;
 	} while (--timeout && !(!is_on ^ !reg));
 
+	if (saved_config) {
+		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+		reg |= saved_config;
+		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+	}
+
 	if (!timeout)
 		return -ETIMEDOUT;
 
-- 
2.17.1

Re: [PATCH] usb: dwc3: Fix timeout issue during controller enter/exit from halt state

[Thinh Nguyen]

On Fri, Jan 31, 2025, Selvarasu Ganesan wrote:
> There is a frequent timeout during controller enter/exit from halt state
> after toggling the run_stop bit by SW. This timeout occurs when
> performing frequent role switches between host and device, causing
> device enumeration issues due to the timeout. This issue was not present
> when USB2 suspend PHY was disabled by passing the SNPS quirks
> (snps,dis_u2_susphy_quirk and snps,dis_enblslpm_quirk) from the DTS.
> However, there is a requirement to enable USB2 suspend PHY by setting of
> GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY bits when controller starts
> in gadget or host mode results in the timeout issue.
> 
> This commit addresses this timeout issue by ensuring that the bits
> GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY are cleared before starting
> the dwc3_gadget_run_stop sequence and restoring them after the
> dwc3_gadget_run_stop sequence is completed.
> 
> Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Selvarasu Ganesan <selvarasu.g@samsung.com>
> ---
>  drivers/usb/dwc3/gadget.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index d27af65eb08a..4a158f703d64 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2629,10 +2629,25 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>  {
>  	u32			reg;
>  	u32			timeout = 2000;
> +	u32			saved_config = 0;
>  
>  	if (pm_runtime_suspended(dwc->dev))
>  		return 0;
>  

Can you add some comments here that this was added through experiment
since it is not documented in the programming guide. It would be great
to also note which platform you used to test this with.

> +	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> +	if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {

I know we have "unlikely" in the other check, but we should not need it
here or the other place. Please remove this here.

> +		saved_config |= DWC3_GUSB2PHYCFG_SUSPHY;
> +		reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> +	}
> +
> +	if (reg & DWC3_GUSB2PHYCFG_ENBLSLPM) {
> +		saved_config |= DWC3_GUSB2PHYCFG_ENBLSLPM;
> +		reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
> +	}
> +
> +	if (saved_config)
> +		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> +
>  	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>  	if (is_on) {
>  		if (DWC3_VER_IS_WITHIN(DWC3, ANY, 187A)) {
> @@ -2660,6 +2675,12 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>  		reg &= DWC3_DSTS_DEVCTRLHLT;
>  	} while (--timeout && !(!is_on ^ !reg));
>  
> +	if (saved_config) {
> +		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> +		reg |= saved_config;
> +		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> +	}
> +
>  	if (!timeout)
>  		return -ETIMEDOUT;
>  
> -- 
> 2.17.1
> 

Thanks,
Thinh

Re: [PATCH] usb: dwc3: Fix timeout issue during controller enter/exit from halt state

[Selvarasu Ganesan]

On 2/1/2025 5:45 AM, Thinh Nguyen wrote:
> On Fri, Jan 31, 2025, Selvarasu Ganesan wrote:
>> There is a frequent timeout during controller enter/exit from halt state
>> after toggling the run_stop bit by SW. This timeout occurs when
>> performing frequent role switches between host and device, causing
>> device enumeration issues due to the timeout. This issue was not present
>> when USB2 suspend PHY was disabled by passing the SNPS quirks
>> (snps,dis_u2_susphy_quirk and snps,dis_enblslpm_quirk) from the DTS.
>> However, there is a requirement to enable USB2 suspend PHY by setting of
>> GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY bits when controller starts
>> in gadget or host mode results in the timeout issue.
>>
>> This commit addresses this timeout issue by ensuring that the bits
>> GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY are cleared before starting
>> the dwc3_gadget_run_stop sequence and restoring them after the
>> dwc3_gadget_run_stop sequence is completed.
>>
>> Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Selvarasu Ganesan <selvarasu.g@samsung.com>
>> ---
>>   drivers/usb/dwc3/gadget.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index d27af65eb08a..4a158f703d64 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2629,10 +2629,25 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>>   {
>>   	u32			reg;
>>   	u32			timeout = 2000;
>> +	u32			saved_config = 0;
>>   
>>   	if (pm_runtime_suspended(dwc->dev))
>>   		return 0;
>>   
> Can you add some comments here that this was added through experiment
> since it is not documented in the programming guide. It would be great
> to also note which platform you used to test this with.
>
>> +	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> +	if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {
> I know we have "unlikely" in the other check, but we should not need it
> here or the other place. Please remove this here.


Hi Thinh,

Thanks for your review comments. I updated below newer version with 
addressed your comments. Please review it.

https://lore.kernel.org/linux-usb/20250201163903.459-1-selvarasu.g@samsung.com/

Thanks,
Selva
>> +		saved_config |= DWC3_GUSB2PHYCFG_SUSPHY;
>> +		reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>> +	}
>> +
>> +	if (reg & DWC3_GUSB2PHYCFG_ENBLSLPM) {
>> +		saved_config |= DWC3_GUSB2PHYCFG_ENBLSLPM;
>> +		reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>> +	}
>> +
>> +	if (saved_config)
>> +		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> +
>>   	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>>   	if (is_on) {
>>   		if (DWC3_VER_IS_WITHIN(DWC3, ANY, 187A)) {
>> @@ -2660,6 +2675,12 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>>   		reg &= DWC3_DSTS_DEVCTRLHLT;
>>   	} while (--timeout && !(!is_on ^ !reg));
>>   
>> +	if (saved_config) {
>> +		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> +		reg |= saved_config;
>> +		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> +	}
>> +
>>   	if (!timeout)
>>   		return -ETIMEDOUT;
>>   
>> -- 
>> 2.17.1
>>
> Thanks,
> Thinh

Re: [PATCH] usb: dwc3: Fix timeout issue during controller enter/exit from halt state

[Krishna Kurapati]

On 2/1/2025 5:45 AM, Thinh Nguyen wrote:
> On Fri, Jan 31, 2025, Selvarasu Ganesan wrote:
>> There is a frequent timeout during controller enter/exit from halt state
>> after toggling the run_stop bit by SW. This timeout occurs when
>> performing frequent role switches between host and device, causing
>> device enumeration issues due to the timeout. This issue was not present
>> when USB2 suspend PHY was disabled by passing the SNPS quirks
>> (snps,dis_u2_susphy_quirk and snps,dis_enblslpm_quirk) from the DTS.
>> However, there is a requirement to enable USB2 suspend PHY by setting of
>> GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY bits when controller starts
>> in gadget or host mode results in the timeout issue.
>>
>> This commit addresses this timeout issue by ensuring that the bits
>> GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY are cleared before starting
>> the dwc3_gadget_run_stop sequence and restoring them after the
>> dwc3_gadget_run_stop sequence is completed.
>>
>> Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Selvarasu Ganesan <selvarasu.g@samsung.com>
>> ---
>>   drivers/usb/dwc3/gadget.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index d27af65eb08a..4a158f703d64 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2629,10 +2629,25 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>>   {
>>   	u32			reg;
>>   	u32			timeout = 2000;
>> +	u32			saved_config = 0;
>>   
>>   	if (pm_runtime_suspended(dwc->dev))
>>   		return 0;
>>   
> 
> Can you add some comments here that this was added through experiment
> since it is not documented in the programming guide. It would be great
> to also note which platform you used to test this with.
> 

I did see this issue during pullup_exit() in SM6375 and SM8150 targets. 
The exact code logic worked out for me downstream.

Regards,
Krishna,

Re: [PATCH] usb: dwc3: Fix timeout issue during controller enter/exit from halt state

[Thinh Nguyen]

On Sat, Feb 01, 2025, Krishna Kurapati wrote:
> 
> 
> On 2/1/2025 5:45 AM, Thinh Nguyen wrote:
> > On Fri, Jan 31, 2025, Selvarasu Ganesan wrote:
> > > There is a frequent timeout during controller enter/exit from halt state
> > > after toggling the run_stop bit by SW. This timeout occurs when
> > > performing frequent role switches between host and device, causing
> > > device enumeration issues due to the timeout. This issue was not present
> > > when USB2 suspend PHY was disabled by passing the SNPS quirks
> > > (snps,dis_u2_susphy_quirk and snps,dis_enblslpm_quirk) from the DTS.
> > > However, there is a requirement to enable USB2 suspend PHY by setting of
> > > GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY bits when controller starts
> > > in gadget or host mode results in the timeout issue.
> > > 
> > > This commit addresses this timeout issue by ensuring that the bits
> > > GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY are cleared before starting
> > > the dwc3_gadget_run_stop sequence and restoring them after the
> > > dwc3_gadget_run_stop sequence is completed.
> > > 
> > > Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Selvarasu Ganesan <selvarasu.g@samsung.com>
> > > ---
> > >   drivers/usb/dwc3/gadget.c | 21 +++++++++++++++++++++
> > >   1 file changed, 21 insertions(+)
> > > 
> > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > index d27af65eb08a..4a158f703d64 100644
> > > --- a/drivers/usb/dwc3/gadget.c
> > > +++ b/drivers/usb/dwc3/gadget.c
> > > @@ -2629,10 +2629,25 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
> > >   {
> > >   	u32			reg;
> > >   	u32			timeout = 2000;
> > > +	u32			saved_config = 0;
> > >   	if (pm_runtime_suspended(dwc->dev))
> > >   		return 0;
> > 
> > Can you add some comments here that this was added through experiment
> > since it is not documented in the programming guide. It would be great
> > to also note which platform you used to test this with.
> > 
> 
> I did see this issue during pullup_exit() in SM6375 and SM8150 targets. The
> exact code logic worked out for me downstream.
> 

Noted. This is good info.

Thanks,
Thinh