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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <902d2180-cae0-6cc9-1549-a3f711198a2b@synopsys.com>
Date:   Thu, 29 Oct 2020 02:57:42 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Wesley Cheng <wcheng@...eaurora.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        "balbi@...nel.org" <balbi@...nel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "jackp@...eaurora.org" <jackp@...eaurora.org>
Subject: Re: [PATCH 2/2] usb: dwc3: gadget: Preserve UDC max speed setting

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