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
