On Mon, Oct 28, 2024 at 4:55 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Mon, Oct 28, 2024 at 04:09:30PM +0100, Bart Van Severen wrote: > > On Fri, Oct 25, 2024 at 9:37 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > On Thu, Oct 24, 2024 at 11:37:50AM +0200, Bart Van Severen wrote: > > > > Hi, > > > > > > > > We have a use case to remote control a pc through a composite gadget > > > > consisting of keyboard, mouse and tablet functions. > > > > The problem we face is that when the pc is sent to sleep, we cannot > > > > wake it by writing to the hid device, which is what you > > > > would expect to work. > > > > > > > > We're running on Xilinx Zynqmp soc/DWC3 290A. > > > > > > > > We have set the bmAttributes ch9 USB_CONFIG_ATT_WAKEUP bit and do see > > > > the set_feature request to enable remote > > > > wakeup, just before the pc goes to sleep. > > > > > > Do you see a wakeup request from the gadget to the host? > > > > No, I don't see any wakeup request towards the host. > > As one would expect, given that there was no call to > usb_gadget_wakeup(). Correct > > > > > We noticed the recent relevant work > > > > https://lore.kernel.org/linux-usb/1679694482-16430-1-git-send-email-quic_eserrao@xxxxxxxxxxx/, > > > > regarding function suspend/resume and remote wakeup improvements. > > > > > > > > However, the main question we have is: what would be the right place > > > > in the gadget framework to fix the issue where a hid write > > > > doesn't trigger a remote wake up when the usb device is suspended and > > > > the host has enabled remote wakeup. > > > > > > Probably the drivers/usb/gadget/function/f_hid.c file. I don't know > > > anything about how this gadget driver works, but the file doesn't have > > > any calls to usb_gadget_wakeup(), which means it won't generate a wakeup > > > request no matter what you write to it. > > > > > > Alan Stern > > > > > > > That's what my initial thought was also, and that certainly works. > > But then I wondered if it isn't a better idea to handle this further down, to > > avoid having to write similar code in all function drivers, say in the > > gadget core? > > 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 suspended before calling usb_gadget_wakeup() on hid write. 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. Br, Bart
