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]

Date:   Thu, 29 Oct 2020 06:29:10 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Wesley Cheng <wcheng@...eaurora.org>,
        "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

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