Re: [PATCH 01/15] usb: dwc3: Device SoftReset PHY synchonization delay

[Thinh Nguyen]

On 1/8/2018 4:01 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> Thinh Nguyen  writes:
>>  From DWC_usb31 databook section 1.3.2, once DWC3_DCTL_CSFTRST bit is
>> cleared, we must wait at least 50ms before accessing the PHY domain
>> (synchronization delay).
>>
>> Signed-off-by: Thinh Nguyen 
>> ---
>>   drivers/usb/dwc3/core.c | 13 -
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 07832509584f..7d21f21707f5 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -228,12 +228,23 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>>  do {
>>  reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>>  if (!(reg & DWC3_DCTL_CSFTRST))
>> -return 0;
>> +goto done;
>>   
>>  udelay(1);
>>  } while (--retries);
>>   
>>  return -ETIMEDOUT;
>> +
>> +done:
>> +/*
>> + * For DWC_usb31 controller, once DWC3_DCTL_CSFTRST bit is cleared,
>> + * we must wait at least 50ms before accessing the PHY domain
>> + * (synchronization delay). DWC_usb31 Databook section 1.3.2.
>> + */
>> +if (dwc3_is_usb31(dwc))
>> +mdelay(50);
> 
> we don't have any locks held. Why are you busy looping for an entire
> 50ms? Why don't you msleep() here?
> 

You're right. I'll make an update.

Thanks,
Thinh
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] usb: dwc3: Device SoftReset PHY synchonization delay

[Felipe Balbi]

Hi,

Thinh Nguyen  writes:
> From DWC_usb31 databook section 1.3.2, once DWC3_DCTL_CSFTRST bit is
> cleared, we must wait at least 50ms before accessing the PHY domain
> (synchronization delay).
>
> Signed-off-by: Thinh Nguyen 
> ---
>  drivers/usb/dwc3/core.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 07832509584f..7d21f21707f5 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -228,12 +228,23 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>   do {
>   reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>   if (!(reg & DWC3_DCTL_CSFTRST))
> - return 0;
> + goto done;
>  
>   udelay(1);
>   } while (--retries);
>  
>   return -ETIMEDOUT;
> +
> +done:
> + /*
> +  * For DWC_usb31 controller, once DWC3_DCTL_CSFTRST bit is cleared,
> +  * we must wait at least 50ms before accessing the PHY domain
> +  * (synchronization delay). DWC_usb31 Databook section 1.3.2.
> +  */
> + if (dwc3_is_usb31(dwc))
> + mdelay(50);

we don't have any locks held. Why are you busy looping for an entire
50ms? Why don't you msleep() here?

-- 
balbi


signature.asc
Description: PGP signature

[PATCH 01/15] usb: dwc3: Device SoftReset PHY synchonization delay

[Thinh Nguyen]

>From DWC_usb31 databook section 1.3.2, once DWC3_DCTL_CSFTRST bit is
cleared, we must wait at least 50ms before accessing the PHY domain
(synchronization delay).

Signed-off-by: Thinh Nguyen 
---
 drivers/usb/dwc3/core.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 07832509584f..7d21f21707f5 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -228,12 +228,23 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
do {
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
if (!(reg & DWC3_DCTL_CSFTRST))
-   return 0;
+   goto done;
 
udelay(1);
} while (--retries);
 
return -ETIMEDOUT;
+
+done:
+   /*
+* For DWC_usb31 controller, once DWC3_DCTL_CSFTRST bit is cleared,
+* we must wait at least 50ms before accessing the PHY domain
+* (synchronization delay). DWC_usb31 Databook section 1.3.2.
+*/
+   if (dwc3_is_usb31(dwc))
+   mdelay(50);
+
+   return 0;
 }
 
 /*
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html