| Message ID | 1339061486-28513-30-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi, On 06/07/2012 11:31 AM, Gerd Hoffmann wrote: > Kick async schedule when we get a wakeup > notification from a usb device. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/usb/hcd-ehci.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c > index 8b2dfed..f8ed80d 100644 > --- a/hw/usb/hcd-ehci.c > +++ b/hw/usb/hcd-ehci.c > @@ -852,6 +852,8 @@ static void ehci_wakeup(USBPort *port) > USBPort *companion = s->companion_ports[port->index]; > if (companion->ops->wakeup) { > companion->ops->wakeup(companion); > + } else { > + qemu_bh_schedule(s->async_bh); > } > } > } > This is wrong, this puts the added else inside the "if (portsc & PORTSC_POWNER) {" block, iow it will only trigger when the port is redirected to the companion controller, it should be the else of the "if (portsc & PORTSC_POWNER)" so that the async schedule gets kicked on wakeup when the device is connected to the ehci (rather then to an companion uhci). Notice how the if where the else now belongs to is unconditionally dereferencing companion in the form of companion->ops, it can do this because the "if (portsc & PORTSC_POWNER)" check ensures there is a companion port for this device, the second level if is to see if the companion has wakeup support. Regards, Hans
Hi, On 07/06/2012 04:30 PM, Hans de Goede wrote: > Hi, > > On 06/07/2012 11:31 AM, Gerd Hoffmann wrote: >> Kick async schedule when we get a wakeup >> notification from a usb device. >> >> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> >> --- >> hw/usb/hcd-ehci.c | 2 ++ >> 1 files changed, 2 insertions(+), 0 deletions(-) >> >> diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c >> index 8b2dfed..f8ed80d 100644 >> --- a/hw/usb/hcd-ehci.c >> +++ b/hw/usb/hcd-ehci.c >> @@ -852,6 +852,8 @@ static void ehci_wakeup(USBPort *port) >> USBPort *companion = s->companion_ports[port->index]; >> if (companion->ops->wakeup) { >> companion->ops->wakeup(companion); >> + } else { >> + qemu_bh_schedule(s->async_bh); >> } >> } >> } >> > > This is wrong, this puts the added else inside the > "if (portsc & PORTSC_POWNER) {" block, iow it will only > trigger when the port is redirected to the companion controller, > it should be the else of the "if (portsc & PORTSC_POWNER)" so that > the async schedule gets kicked on wakeup when the device is connected > to the ehci (rather then to an companion uhci). > > Notice how the if where the else now belongs to is unconditionally > dereferencing companion in the form of companion->ops, it can do > this because the "if (portsc & PORTSC_POWNER)" check ensures there is > a companion port for this device, the second level if is to see if > the companion has wakeup support. I've just send a patch fixing this. One last remark though, do we support wakeup in the EHCI-code at all? Regards, Hans
Hi, > do we > support wakeup in the EHCI-code at all? uhci and ohci do, could be ehci doesn't, all our hid emulated devices which need remote wakeup most are usb 1.1 devices ... cheers, Gerd
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 8b2dfed..f8ed80d 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -852,6 +852,8 @@ static void ehci_wakeup(USBPort *port) USBPort *companion = s->companion_ports[port->index]; if (companion->ops->wakeup) { companion->ops->wakeup(companion); + } else { + qemu_bh_schedule(s->async_bh); } } }
Kick async schedule when we get a wakeup notification from a usb device. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/usb/hcd-ehci.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)