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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Felipe Balbi wrote:
> 
> Hi,
> 
> John Stultz <john.stultz@xxxxxxxxxx> writes:
>> On Fri, Apr 16, 2021 at 12:49 PM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
>>>
>>> John Stultz wrote:
>>>> On Fri, Apr 16, 2021 at 3:47 AM Felipe Balbi <balbi@xxxxxxxxxx> wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>>>>>
>>>>>> From: Yu Chen <chenyu56@xxxxxxxxxx>
>>>>>> From: John Stultz <john.stultz@xxxxxxxxxx>
>>>>>>
>>>>>> 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://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@xxxxxxxxxx/__;!!A4F2R9G_pg!L4TLb25Nkq0DF2qrCPWW13PUq4idhDn5QSZhgvnVAy7wJiYFOSSouSptwo9nOzIdPD4j$
>>>>>> [2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@xxxxxxxxx/__;!!A4F2R9G_pg!L4TLb25Nkq0DF2qrCPWW13PUq4idhDn5QSZhgvnVAy7wJiYFOSSouSptwo9nO21VT8q7$
>>>>>>
>>>>>> Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
>>>>>> Cc: Ferry Toth <fntoth@xxxxxxxxx>
>>>>>> Cc: Wesley Cheng <wcheng@xxxxxxxxxxxxxx>
>>>>>> Cc: <stable@xxxxxxxxxxxxxxx>
>>>>>> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly")
>>>>>> Signed-off-by: Yu Chen <chenyu56@xxxxxxxxxx>
>>>>>> Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx>
>>>>>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>
>>>>>
>>>>> 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)
>>>>
>>>> I unfortunately don't have an easy way to automate the switching right
>>>> off. But I'll try to hack up the mux switch driver to provide an
>>>> interface we can script against.
>>>>
>>>
>>> FYI, you can do the following:
>>>
>>> 1) Enable "usb-role-switch" DT property if not already done so
>>> 2) Add userspace control:
>>>
>>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>>> index e2b68bb770d1..b203e3d87291 100644
>>> --- a/drivers/usb/dwc3/drd.c
>>> +++ b/drivers/usb/dwc3/drd.c
>>> @@ -555,6 +555,7 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc)
>>>                 mode = DWC3_GCTL_PRTCAP_DEVICE;
>>>         }
>>>
>>> +       dwc3_role_switch.allow_userspace_control = true;
>>>         dwc3_role_switch.fwnode = dev_fwnode(dwc->dev);
>>>         dwc3_role_switch.set = dwc3_usb_role_switch_set;
>>>         dwc3_role_switch.get = dwc3_usb_role_switch_get;
>>>
>>> 3) Write a script to do the following:
>>>
>>> # echo host > /sys/class/usb_role/<UDC>/role
>>>
>>> and
>>>
>>> # echo device > /sys/class/usb_role/<UDC>/role
>>
>> Thanks so much for this. So I ran both of those commands in a while
>> loop for awhile and didn't see any trouble.
>>
>> HiKey960 is interesting as well because we have a mux switch, which is
>> sort of an intermediary roll switcher (it gets the role switch signal
>> from the tcpci_rt1711h, tweaks some gpios and then signals the dwc3).
>> So I also did the above tweaks to the mux-switch and had it switching
>> between device/none and validated the onboard hub came up and down
>> along with the dwc3 core.
>>
>> Everything still looks good here.
> 
> Sounds good, happy to see so many platforms supported by Thinh's
> change. Thanks for doing this work, Thinh :-)
> 

Thanks for the review Felipe :)

Thinh




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux