linux-kernel - Re: [PATCH 2/5] usb: gadget: Add function wakeup support

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: <e3bcfd4c-efdb-c7b0-4e94-1afcd3b8eb73@synopsys.com>
Date:   Fri, 12 Aug 2022 02:19:01 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Elson Serrao <quic_eserrao@...cinc.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>,
        "quic_wcheng@...cinc.com" <quic_wcheng@...cinc.com>,
        "quic_jackp@...cinc.com" <quic_jackp@...cinc.com>,
        "quic_mrana@...cinc.com" <quic_mrana@...cinc.com>
Subject: Re: [PATCH 2/5] usb: gadget: Add function wakeup support

On 8/11/2022, Thinh Nguyen wrote:
> On 8/11/2022, Elson Serrao wrote:
>>
>>
>> On 8/9/2022 6:08 PM, Thinh Nguyen wrote:
> 
> <snip>
> 
> 
>>> To summarize the points:
>>>
>>> 1) The host only arms function remote wakeup if the device is capable of
>>> remote wakeup (check USB_CONFIG_ATT_WAKEUP in bmAttributes and hardware
>>> capability)
>>>
>>> 2) If the device is in suspend, the device can do remote wakeup (through
>>> LFPS handshake) if the function is armed for remote wakeup (through
>>> SET_FEATURE(FUNC_SUSPEND)).
>>>
>>> 3) If the link transitions to U0 after the device triggering a remote
>>> wakeup, the device will also send device notification function wake for
>>> all the interfaces armed with remote wakeup.
>>>
>>> 4) If the device is not in suspend, the device can send device
>>> notification function wake if it's in U0.
>>>
>>>
>>> Now, remote wakeup and function wake device notification are 2 separate
>>> operations. We have the usb_gadget_ops->wakeup() for remote wakeup. I
>>> suggested to maybe add usb_gadget_ops->send_wakeup_notification(gadget,
>>> intf_id) for the device notification. What you did was combining both
>>> operations in usb_gadget_ops->func_wakeup(). That may only work for
>>> point 4) (assuming you fix the U0 check), but not point 3).
>>
>> Thank you for your feedback and summary. I will rename func_wakeup to
>> send_wakeup_notification to better align with the approach. The reason I
>> have combined remote_wakeup and function wake notification in
>> usb_gadget_ops->func_wakeup() is because since the implementation is at
>> function/composite level it has no knowledge on the link state. So I
>> have delegated that task to controller driver to handle the notification
>> accordingly. That is do a LFPS handshake first if the device is
>> suspended and then send notification (explained below). But we can
>> definitely separate this by adding an additional flag in the composite
>> layer to set the link state based on the gadget suspend callback called
>> when U3 SUSPEND interrupt is received. Let me know if you feel
>> separating the two is a better approach.
>>
> 
> The reason I think we need to separate it is because of point 3. As I
> note earlier, the spec states that "If remote wake event occurs in
> multiple functions, each function shall send a Function Wake Notification."
> 
> But if there's no remote wake event, and the host brought the device up
> instead, then the function suspend state is retained.
> 
> If we separate these 2 operations, the caller can check whether the
> operation went through properly. For example, if the wakeup() is
> initiated properly, but the function wake device notification didn't go
> through. We would only need to resend the device notification rather
> than initiate remote wakeup again.

If we don't have to send device notification for other interfaces, we
can combine the operations here as you did.

BR,
Thinh