Message ID | 1435033463-13185-1-git-send-email-lma@suse.com |
---|---|
State | New |
Headers | show |
On 2015/6/23 12:24, Lin Ma wrote: > Signed-off-by: Lin Ma <lma@suse.com> > --- > hw/usb/host-libusb.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c > index 10f4735..7258c4d 100644 > --- a/hw/usb/host-libusb.c > +++ b/hw/usb/host-libusb.c > @@ -888,6 +888,11 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev) > fail: > trace_usb_host_open_failure(bus_num, addr); > if (s->dh != NULL) { > + qemu_remove_exit_notifier(&s->exit); > + QTAILQ_REMOVE(&hostdevs, s, next); This change will cause a regression. For example, if an usb device's (assume that it's usb1.0 device) speed does not match the ehci adapter, then then invoking usb_host_open failed. if somebody changes the usb device to an usb 2.0 device, it will not auto check because the device has removed from the global hostdevs list. So I think we don't need do the above operations. Regards, -Gonglei > + usb_host_release_interfaces(s); > + libusb_reset_device(s->dh); > + usb_host_attach_kernel(s); > libusb_close(s->dh); > s->dh = NULL; > s->dev = NULL; >
在 2015年06月23日 16:29, Gonglei 写道: > On 2015/6/23 12:24, Lin Ma wrote: >> Signed-off-by: Lin Ma <lma@suse.com> >> --- >> hw/usb/host-libusb.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c >> index 10f4735..7258c4d 100644 >> --- a/hw/usb/host-libusb.c >> +++ b/hw/usb/host-libusb.c >> @@ -888,6 +888,11 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev) >> fail: >> trace_usb_host_open_failure(bus_num, addr); >> if (s->dh != NULL) { >> + qemu_remove_exit_notifier(&s->exit); >> + QTAILQ_REMOVE(&hostdevs, s, next); > This change will cause a regression. For example, if an usb device's > (assume that it's usb1.0 device) speed does not match the ehci adapter, > then then invoking usb_host_open failed. if somebody changes the usb > device to an usb 2.0 device, it will not auto check because the device has > removed from the global hostdevs list. > > So I think we don't need do the above operations. In this situation, in my opinion, User should perform device_add to add the device again, Then the device will be added to hostdevs list and can be auto checked again, right? In addition, The usb_host_open will be performed 3 times until 's->errcount >= 3' while invoking usb_host_open failed. If the usb device isn't removed from hostdevs list, It will be detached & re-attached 3 times. Thanks, Lin >> + usb_host_release_interfaces(s); >> + libusb_reset_device(s->dh); >> + usb_host_attach_kernel(s); >> libusb_close(s->dh); >> s->dh = NULL; >> s->dev = NULL; >> > >
On 2015/6/23 22:49, Lin Ma wrote: > > 在 2015年06月23日 16:29, Gonglei 写道: >> On 2015/6/23 12:24, Lin Ma wrote: >>> Signed-off-by: Lin Ma <lma@suse.com> >>> --- >>> hw/usb/host-libusb.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c >>> index 10f4735..7258c4d 100644 >>> --- a/hw/usb/host-libusb.c >>> +++ b/hw/usb/host-libusb.c >>> @@ -888,6 +888,11 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev) >>> fail: >>> trace_usb_host_open_failure(bus_num, addr); >>> if (s->dh != NULL) { >>> + qemu_remove_exit_notifier(&s->exit); >>> + QTAILQ_REMOVE(&hostdevs, s, next); >> This change will cause a regression. For example, if an usb device's >> (assume that it's usb1.0 device) speed does not match the ehci adapter, >> then then invoking usb_host_open failed. if somebody changes the usb >> device to an usb 2.0 device, it will not auto check because the device has >> removed from the global hostdevs list. >> >> So I think we don't need do the above operations. > In this situation, in my opinion, User should perform device_add to add the device again, Then the device will be added > to hostdevs list and can be auto checked again, right? > No, that's a regression if you ask users call device_add again. The purpose that we use bus/port to identify one usb device is can handle this scenario. After passing through one usb device by usb/port, the guest can auto check the device though you changed a usb device in the same physical port. > In addition, The usb_host_open will be performed 3 times until 's->errcount >= 3' while invoking usb_host_open failed. > If the usb device isn't removed from hostdevs list, It will be detached & re-attached 3 times. > Yes, that's no problem as a protecting logic. Regards, -Gonglei > > Thanks, > Lin >>> + usb_host_release_interfaces(s); >>> + libusb_reset_device(s->dh); >>> + usb_host_attach_kernel(s); >>> libusb_close(s->dh); >>> s->dh = NULL; >>> s->dev = NULL; >>> >> >> >
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c index 10f4735..7258c4d 100644 --- a/hw/usb/host-libusb.c +++ b/hw/usb/host-libusb.c @@ -888,6 +888,11 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev) fail: trace_usb_host_open_failure(bus_num, addr); if (s->dh != NULL) { + qemu_remove_exit_notifier(&s->exit); + QTAILQ_REMOVE(&hostdevs, s, next); + usb_host_release_interfaces(s); + libusb_reset_device(s->dh); + usb_host_attach_kernel(s); libusb_close(s->dh); s->dh = NULL; s->dev = NULL;
Signed-off-by: Lin Ma <lma@suse.com> --- hw/usb/host-libusb.c | 5 +++++ 1 file changed, 5 insertions(+)