Re: [PATCH v3] usb: dwc3: core: Do core softreset when switch mode

From: Wesley Cheng <wcheng@codeaurora.org>
To: Felipe Balbi <balbi@kernel.org>,
	Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org
Cc: John Youn <John.Youn@synopsys.com>,
	stable@vger.kernel.org,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	John Stultz <john.stultz@linaro.org>,
	Ferry Toth <fntoth@gmail.com>, Yu Chen <chenyu56@huawei.com>
Subject: Re: [PATCH v3] usb: dwc3: core: Do core softreset when switch mode
Date: Fri, 16 Apr 2021 12:05:48 -0700	[thread overview]
Message-ID: <63f0d8c2-1272-51a5-8f0b-8ae663ab5b94@codeaurora.org> (raw)
In-Reply-To: <87zgxymrph.fsf@kernel.org>

On 4/16/2021 3:47 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> 
>> From: Yu Chen <chenyu56@huawei.com>
>> From: John Stultz <john.stultz@linaro.org>
>>
>> According to the programming guide, to switch mode for DRD controller,
>> the driver needs to do the following.
>>
>> To switch from device to host:
>> 1. Reset controller with GCTL.CoreSoftReset
>> 2. Set GCTL.PrtCapDir(host mode)
>> 3. Reset the host with USBCMD.HCRESET
>> 4. Then follow up with the initializing host registers sequence
>>
>> To switch from host to device:
>> 1. Reset controller with GCTL.CoreSoftReset
>> 2. Set GCTL.PrtCapDir(device mode)
>> 3. Reset the device with DCTL.CSftRst
>> 4. Then follow up with the initializing registers sequence
>>
>> Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of
>> switching from host to device. John Stult reported a lockup issue seen
>> with HiKey960 platform without these steps[1]. Similar issue is observed
>> with Ferry's testing platform[2].
>>
>> So, apply the required steps along with some fixes to Yu Chen's and John
>> Stultz's version. The main fixes to their versions are the missing wait
>> for clocks synchronization before clearing GCTL.CoreSoftReset and only
>> apply DCTL.CSftRst when switching from host to device.
>>
>> [1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.org/
>> [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail.com/
>>
>> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Cc: Ferry Toth <fntoth@gmail.com>
>> Cc: Wesley Cheng <wcheng@codeaurora.org>
>> Cc: <stable@vger.kernel.org>
>> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly")
>> Signed-off-by: Yu Chen <chenyu56@huawei.com>
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> 
> I still have concerns about the soft reset, but I won't block you guys
> from fixing Hikey's problem :-)
> 
> The only thing I would like to confirm is that this has been verified
> with hundreds of swaps happening as quickly as possible. DWC3 should
> still be functional after several hundred swaps.
> 
> Can someone confirm this is the case? (I'm assuming this can be
> scripted)
> 
Hi Thinh/Felipe,

Thanks Thinh for this change.  Will verify this on our platform as well
with a mode switch loop over the weekend.

Thanks
Wesley Cheng
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project