[PATCH] usb: dwc3: reset the address and run_stop on init

[PATCH] usb: dwc3: reset the address and run_stop on init

[Roman Kapl]

The address should be set to zero during reset according to the
documentation. Clearing RunStop ensures that the host disconnects from
the device (it was not cleared by CSFTRST, at least on ls1043).

This allows the dwc3 to properly initialize even if the previous
driver did not shutdown the device (e.g. when using virtualization).

Signed-off-by: Roman Kapl <rka@sysgo.com>
---
 drivers/usb/dwc3/core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c9bb93a2c81e..c633f5e0621d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -250,6 +250,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
 
 	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
 	reg |= DWC3_DCTL_CSFTRST;
+	reg &= ~DWC3_DCTL_RUN_STOP;
 	dwc3_writel(dwc->regs, DWC3_DCTL, reg);
 
 	do {
@@ -266,6 +267,10 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
 	return -ETIMEDOUT;
 
 done:
+	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
+	reg &= ~(DWC3_DCFG_DEVADDR_MASK);
+	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
+
 	/*
 	 * For DWC_usb31 controller, once DWC3_DCTL_CSFTRST bit is cleared,
 	 * we must wait at least 50ms before accessing the PHY domain
-- 
2.22.0

Re: [PATCH] usb: dwc3: reset the address and run_stop on init

[Thinh Nguyen]

Hi,

Roman Kapl wrote:
> The address should be set to zero during reset according to the
> documentation.

That is for usb reset and not core soft reset, and dwc3 already handles 
that case.

> Clearing RunStop ensures that the host disconnects from
> the device (it was not cleared by CSFTRST, at least on ls1043).
>
> This allows the dwc3 to properly initialize even if the previous
> driver did not shutdown the device (e.g. when using virtualization).

This sounds like a workaround to some issue that the function driver did 
not handle.

> Signed-off-by: Roman Kapl <rka@sysgo.com>
> ---
>   drivers/usb/dwc3/core.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index c9bb93a2c81e..c633f5e0621d 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -250,6 +250,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>   
>   	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>   	reg |= DWC3_DCTL_CSFTRST;
> +	reg &= ~DWC3_DCTL_RUN_STOP;
>   	dwc3_writel(dwc->regs, DWC3_DCTL, reg);
>   
>   	do {
> @@ -266,6 +267,10 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>   	return -ETIMEDOUT;
>   
>   done:
> +	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
> +	reg &= ~(DWC3_DCFG_DEVADDR_MASK);
> +	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
> +
>   	/*
>   	 * For DWC_usb31 controller, once DWC3_DCTL_CSFTRST bit is cleared,
>   	 * we must wait at least 50ms before accessing the PHY domain

BR,
Thinh

Re: [PATCH] usb: dwc3: reset the address and run_stop on init

[Roman Kapl]

Hello,

On 9/5/19 8:57 PM, Thinh Nguyen wrote:
> Hi,
> 
> Roman Kapl wrote:
>> The address should be set to zero during reset according to the
>> documentation.
> 
> That is for usb reset and not core soft reset, and dwc3 already handles
> that case.

I thought that core soft reset is part of the USB reset (it is done 
during the init, right?).

> 
>> Clearing RunStop ensures that the host disconnects from
>> the device (it was not cleared by CSFTRST, at least on ls1043).
>>
>> This allows the dwc3 to properly initialize even if the previous
>> driver did not shutdown the device (e.g. when using virtualization).
> 
> This sounds like a workaround to some issue that the function driver did
> not handle.

I am using the ACM function driver. However, as I said, the issue arises 
only when the dwc3 driver is in a guest OS that is rebooted (even if the 
reboot is initiated from within the guest OS, not a hard reset).

Maybe this is out of scope for Linux, trying to 'clean up' after the 
previous driver?

Thank you, Roman Kapl

> 
>> Signed-off-by: Roman Kapl <rka@sysgo.com>
>> ---
>>    drivers/usb/dwc3/core.c | 5 +++++
>>    1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index c9bb93a2c81e..c633f5e0621d 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -250,6 +250,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>>    
>>    	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>>    	reg |= DWC3_DCTL_CSFTRST;
>> +	reg &= ~DWC3_DCTL_RUN_STOP;
>>    	dwc3_writel(dwc->regs, DWC3_DCTL, reg);
>>    
>>    	do {
>> @@ -266,6 +267,10 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>>    	return -ETIMEDOUT;
>>    
>>    done:
>> +	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
>> +	reg &= ~(DWC3_DCFG_DEVADDR_MASK);
>> +	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
>> +
>>    	/*
>>    	 * For DWC_usb31 controller, once DWC3_DCTL_CSFTRST bit is cleared,
>>    	 * we must wait at least 50ms before accessing the PHY domain
> 
> BR,
> Thinh
> 

Re: [PATCH] usb: dwc3: reset the address and run_stop on init

[Thinh Nguyen]

Hi,

Roman Kapl wrote:
> Hello,
>
> On 9/5/19 8:57 PM, Thinh Nguyen wrote:
>> Hi,
>>
>> Roman Kapl wrote:
>>> The address should be set to zero during reset according to the
>>> documentation.
>>
>> That is for usb reset and not core soft reset, and dwc3 already handles
>> that case.
>
> I thought that core soft reset is part of the USB reset (it is done 
> during the init, right?).

A soft reset is done during device power-on init. It's not part of the 
USB reset.

>
>>
>>> Clearing RunStop ensures that the host disconnects from
>>> the device (it was not cleared by CSFTRST, at least on ls1043).
>>>
>>> This allows the dwc3 to properly initialize even if the previous
>>> driver did not shutdown the device (e.g. when using virtualization).
>>
>> This sounds like a workaround to some issue that the function driver did
>> not handle.
>
> I am using the ACM function driver. However, as I said, the issue 
> arises only when the dwc3 driver is in a guest OS that is rebooted 
> (even if the reboot is initiated from within the guest OS, not a hard 
> reset).

Check why it wasn't de-initialized on reboot. Also please describe what 
problem you saw in the patch.

>
> Maybe this is out of scope for Linux, trying to 'clean up' after the 
> previous driver?
>
> Thank you, Roman Kapl
>

BR,
Thinh