diff mbox series

[v2] usb: dwc3: core: Deprecate GCTL.CORESOFTRESET

Message ID 9df529fde6e55f5508321b6bc26e92848044ef2b.1655338967.git.Thinh.Nguyen@synopsys.com (mailing list archive)
State Accepted
Commit afbd04e66e5d16ca3c7ea2e3c56eca25558eacf3
Headers show
Series [v2] usb: dwc3: core: Deprecate GCTL.CORESOFTRESET | expand

Commit Message

Thinh Nguyen June 16, 2022, 12:24 a.m. UTC
Synopsys IP DWC_usb32 and DWC_usb31 version 1.90a and above deprecated
GCTL.CORESOFTRESET. The DRD mode switching flow is updated to remove the
GCTL soft reset. Add version checks to prevent using deprecated setting
in mode switching flow.

Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
---
 Changes in v2:
 - Rebase on Greg's usb-testing branch.

 drivers/usb/dwc3/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


base-commit: 235a6d80f021d9c3bb5652fb6b19d092a7339248

Comments

Udipto Goswami June 27, 2022, 10:28 a.m. UTC | #1
Hi Thinh,

On 6/16/22 5:54 AM, Thinh Nguyen wrote:
> Synopsys IP DWC_usb32 and DWC_usb31 version 1.90a and above deprecated
> GCTL.CORESOFTRESET. The DRD mode switching flow is updated to remove the
> GCTL soft reset. Add version checks to prevent using deprecated setting
> in mode switching flow.
>
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
>   Changes in v2:
>   - Rebase on Greg's usb-testing branch.
>
>   drivers/usb/dwc3/core.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 2c12bbbcd55c..91278d2a72b8 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -159,7 +159,8 @@ static void __dwc3_set_mode(struct work_struct *work)
>   	}
>   
>   	/* For DRD host or device mode only */
> -	if (dwc->desired_dr_role != DWC3_GCTL_PRTCAP_OTG) {
> +	if ((DWC3_IP_IS(DWC3) || DWC3_VER_IS_PRIOR(DWC31, 190A)) &&
just curious, i might be wrong here but, did you meant to use

(DWC3_IP_IS(DWC3) && DWC3_VER_IS_PRIOR(DWC31, 190A) ?

because from the commit text it looks like we are trying to avoid doing GCTL core soft reset for GEN1 above 190A
and GEN2. But the check fails for GEN1 controller with version above 190A.

> +	    dwc->desired_dr_role != DWC3_GCTL_PRTCAP_OTG) {
>   		reg = dwc3_readl(dwc->regs, DWC3_GCTL);
>   		reg |= DWC3_GCTL_CORESOFTRESET;
>   		dwc3_writel(dwc->regs, DWC3_GCTL, reg);
>
> base-commit: 235a6d80f021d9c3bb5652fb6b19d092a7339248

Thanks,

-Udipto
Thinh Nguyen June 27, 2022, 4:56 p.m. UTC | #2
On 6/27/2022 3:28 AM, Udipto Goswami wrote:
> Hi Thinh,
>
> On 6/16/22 5:54 AM, Thinh Nguyen wrote:
>> Synopsys IP DWC_usb32 and DWC_usb31 version 1.90a and above deprecated
>> GCTL.CORESOFTRESET. The DRD mode switching flow is updated to remove the
>> GCTL soft reset. Add version checks to prevent using deprecated setting
>> in mode switching flow.
>>
>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>> ---
>>   Changes in v2:
>>   - Rebase on Greg's usb-testing branch.
>>
>>   drivers/usb/dwc3/core.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 2c12bbbcd55c..91278d2a72b8 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -159,7 +159,8 @@ static void __dwc3_set_mode(struct work_struct 
>> *work)
>>       }
>>         /* For DRD host or device mode only */
>> -    if (dwc->desired_dr_role != DWC3_GCTL_PRTCAP_OTG) {
>> +    if ((DWC3_IP_IS(DWC3) || DWC3_VER_IS_PRIOR(DWC31, 190A)) &&
> just curious, i might be wrong here but, did you meant to use
>
> (DWC3_IP_IS(DWC3) && DWC3_VER_IS_PRIOR(DWC31, 190A) ?


No. The check above should always be false right? The controller can't 
be both DWC_usb3 and DWC_usb31 IP at the same time.


>
> because from the commit text it looks like we are trying to avoid 
> doing GCTL core soft reset for GEN1 above 190A
> and GEN2. But the check fails for GEN1 controller with version above 
> 190A.
>

I'm not clear what you meant by GEN1/GEN2 here. We're not doing any 
GEN1/GEN2 check here. And what fails?


BR,

Thinh
Udipto Goswami June 28, 2022, 6:43 a.m. UTC | #3
Hi Thinh,

On 6/27/22 10:26 PM, Thinh Nguyen wrote:
> On 6/27/2022 3:28 AM, Udipto Goswami wrote:
>> Hi Thinh,
>>
>> On 6/16/22 5:54 AM, Thinh Nguyen wrote:
>>> Synopsys IP DWC_usb32 and DWC_usb31 version 1.90a and above deprecated
>>> GCTL.CORESOFTRESET. The DRD mode switching flow is updated to remove the
>>> GCTL soft reset. Add version checks to prevent using deprecated setting
>>> in mode switching flow.
>>>
>>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>>> ---
>>>    Changes in v2:
>>>    - Rebase on Greg's usb-testing branch.
>>>
>>>    drivers/usb/dwc3/core.c | 3 ++-
>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 2c12bbbcd55c..91278d2a72b8 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -159,7 +159,8 @@ static void __dwc3_set_mode(struct work_struct
>>> *work)
>>>        }
>>>          /* For DRD host or device mode only */
>>> -    if (dwc->desired_dr_role != DWC3_GCTL_PRTCAP_OTG) {
>>> +    if ((DWC3_IP_IS(DWC3) || DWC3_VER_IS_PRIOR(DWC31, 190A)) &&
>> just curious, i might be wrong here but, did you meant to use
>>
>> (DWC3_IP_IS(DWC3) && DWC3_VER_IS_PRIOR(DWC31, 190A) ?
>
> No. The check above should always be false right? The controller can't
> be both DWC_usb3 and DWC_usb31 IP at the same time.

got it, i misunderstood DWC_usb3 to be same as DWC_usb31. Apologies!


>
>> because from the commit text it looks like we are trying to avoid
>> doing GCTL core soft reset for GEN1 above 190A
>> and GEN2. But the check fails for GEN1 controller with version above
>> 190A.
>>
> I'm not clear what you meant by GEN1/GEN2 here. We're not doing any
> GEN1/GEN2 check here. And what fails?

i meant DWC_usb3 & DWC_usb31 by Gen1/Gen2, sorry for the confusion.

>
> BR,
>
> Thinh
>
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 2c12bbbcd55c..91278d2a72b8 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -159,7 +159,8 @@  static void __dwc3_set_mode(struct work_struct *work)
 	}
 
 	/* For DRD host or device mode only */
-	if (dwc->desired_dr_role != DWC3_GCTL_PRTCAP_OTG) {
+	if ((DWC3_IP_IS(DWC3) || DWC3_VER_IS_PRIOR(DWC31, 190A)) &&
+	    dwc->desired_dr_role != DWC3_GCTL_PRTCAP_OTG) {
 		reg = dwc3_readl(dwc->regs, DWC3_GCTL);
 		reg |= DWC3_GCTL_CORESOFTRESET;
 		dwc3_writel(dwc->regs, DWC3_GCTL, reg);