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

From: Udipto Goswami <quic_ugoswami@quicinc.com>
To: 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: Tue, 28 Jun 2022 12:13:57 +0530	[thread overview]
Message-ID: <817f66cc-8971-ee48-0a4a-4d442b2e7451@quicinc.com> (raw)
In-Reply-To: <708f3063-cf2f-5d4e-6afd-ee1702b080e1@synopsys.com>

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
>