On Mon, Oct 28, 2024, Alan Stern wrote: > On Mon, Oct 28, 2024 at 05:15:25PM +0100, Bart Van Severen wrote: > > On Mon, Oct 28, 2024 at 4:55 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > The gadget core doesn't know when the user wants to issue a wakeup > > > request; only the function driver knows this. (For instance, only > > > f_hid.c knows when there has been an hid write.) And the whole point of > > > usb_gadget_wakeup() is that it provides a way for the function drivers > > > to tell the gadget core to issue a wakeup request. > > > > > > Alan Stern > > > > Agree, best to handle it in the function driver. > > > > Unfortunately, as stated earlier, the dwc3 gadget driver doesn't > > enable link status interrupts. > > That should be enabled again, so that we can test if the gadget is If you're referring to the link status change event, then we only need it for certain scenarios. > > suspended before > > calling usb_gadget_wakeup() on hid write. We do enable suspend event for that. > > Dwc3_gadget_wakeup() does fetch and checks the link state explicitly > > to return early > > when in U0, so might as well always call usb_gadget_wakeup() on hid > > write, but it feels ugly. > > The test has to be done somewhere; I don't see that it makes much > difference whether it's in the function driver or the UDC driver. But > if dwc3 doesn't enable link status interrupts, how does it know when to > invoke the gadget driver's ->suspend callback? The dwc3 does enable suspend event to support that callback. > > And either way, it looks like there is a potential for races. What if > the host puts the link into U3 just after an hid write occurs but before Regarding the potential race condition you mentioned, the f_hid can track when the ->suspend() and ->resume() callbacks are called to handle these corner cases. How it should handle that is up to the f_hid. > f_hid has had a chance to queue a packet informing the host? Maybe we > need to add a flag to the usb_request structure, to let the UDC driver > know that it should issue a wakeup signal if the request is queued while > the link is suspended. > The host will sync with the gadget via SetFeature(remote_wakeup) control request before entering U3/L2 to enable remote wakeup, and it should disable remote wakeup after resume. We have the flag gadget->wakeup_armed to track that. The dwc3_gadget_wakeup() will not trigger remote wakeup if wakeup_armed is disabled. > This part of the Gadget API has never been tested very much... > > Alan Stern > The f_hid just need to properly implement the handling of remote wakeup as Alan noted. BR, Thinh
