Re: [PATCH 2/2] usb: dwc3: gadget: Preserve UDC max speed setting

From: Thinh Nguyen
Date: Thu Oct 29 2020 - 03:46:51 EST


Thinh Nguyen wrote:
> Wesley Cheng wrote:
>> On 10/28/2020 5:43 PM, Thinh Nguyen wrote:
>>> Hi,
>>>
>>> Wesley Cheng wrote:
>>>> The USB gadget/UDC driver can restrict the DWC3 controller speed using
>>>> dwc3_gadget_set_speed(). Store this setting into a variable, in order for
>>>> this setting to persist across controller resets due to runtime PM.
>>> Why do we need to do this? DCFG should persist unless we overwrite it.
>>> The current PM shouldn't update the current speed setting.
>>>
>>> BR,
>>> Thinh
>>>
>> Hi Thinh,
>>
>> During runtime PM suspend, the dwc3_suspend_common() will call
>> dwc3_core_exit(). On some platforms they register the DWC3 reset
>> control to the DWC3 core driver (otherwise could be handled in the DWC3
>> glue drivers), which will be asserted here:
>>
>> static void dwc3_core_exit(struct dwc3 *dwc)
>> {
>> ...
>> reset_control_assert(dwc->reset);
>>
>> From the SNPS databook (Table 2-2 Resets for Registers) it mentions that
>> assertion of the reset signal will reset the DCFG register.
> I see. There's a hard reset on some platforms.
>
>> In addition to the above, with the change to allow runtime PM suspend
>> during UDC unbind, we need a way to avoid writing to the DCFG during the
>> UDC bind path. (if we entered suspend before re-binding the UDC) If we
>> add an early exit based on the PM state (in
>> dwc3_gadget_set_udc_speed()), then we basically ignore the max speed
>> request from the UDC/gadget layer.
> Then shouldn't we restore the speed setting when dwc3_gadget_resume()
> instead of in dwc3_gadget_run_stop()?

Actually, ignore this comment. This is fine, and it may save some
register read/write operations. I was thinking of save/restore from
suspend and resume similar to hibernation.

Thanks,
Thinh

>
>> Since it looks like the DWC3 gadget driver doesn't like using
>> synchronous PM runtime resumes, by going this route, we can allow the
>> async runtime resume handler to do everything, such as writing the speed
>> config and re-enabling the controller.
>>
>> Thanks
>>
>> Regards,
>> Wesley Cheng
> Thanks,
> Thinh