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

From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Udipto Goswami <quic_ugoswami@quicinc.com>,
	Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	Felipe Balbi <balbi@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Cc: John Youn <John.Youn@synopsys.com>
Subject: Re: [PATCH v2] usb: dwc3: core: Deprecate GCTL.CORESOFTRESET
Date: Mon, 27 Jun 2022 16:56:05 +0000	[thread overview]
Message-ID: <708f3063-cf2f-5d4e-6afd-ee1702b080e1@synopsys.com> (raw)
In-Reply-To: <400a84d5-4d23-bf67-4a80-773bf2129da0@quicinc.com>

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