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

[Thinh Nguyen]

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()?

>
> 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