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

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

 



Greg Kroah-Hartman wrote:
> On Thu, Apr 15, 2021 at 07:10:34AM +0000, Thinh Nguyen wrote:
>> Felipe Balbi 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
>>>
>>> we're not really missing, it was a deliberate choice :-) The only reason
>>> why we need the soft reset is because host and gadget registers map to
>>> the same physical space within dwc3 core. If we cache and restore the
>>> affected registers, we're good ;-)
>>
>> It's part of the programming model. I've already discussed with internal
>> RTL designers. This is needed, and I've provided the discussion we had
>> prior also. We have several different devices in the wild that need
>> this. What is the concern?
>>
>>>
>>> IMHO, that's a better compromise than doing a full soft reset.
>>>
>>>> @@ -40,6 +41,8 @@
>>>>  
>>>>  #define DWC3_DEFAULT_AUTOSUSPEND_DELAY	5000 /* ms */
>>>>  
>>>> +static DEFINE_MUTEX(mode_switch_lock);
>>>
>>> there are several platforms which more than one DWC3 instance. Sure this
>>> won't break on such systems?
>>>
>>
>> How? Am I missing something? Please let me know so I can make the change.
> 
> All data needs to be per-device, not "global for the codebase" like the
> way you declared this lock.
> 

Sure. I can make the change. Thanks for the review.

BR,
Thinh




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux