Re: [Qemu-devel] [PATCH 29/37] ehci: kick async schedule on wakeup

[Hans de Goede <hdegoede@redhat.com>]

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