| Message ID | 20200824110057.32089-1-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb-host: workaround libusb bug | expand |
Gerd Hoffmann <kraxel@redhat.com> writes: > libusb_get_device_speed() does not work for > libusb_wrap_sys_device() devices in v1.0.23. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1871090 > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/usb/host-libusb.c | 37 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 36 insertions(+), 1 deletion(-) > > diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c > index c474551d8456..77f1eaa5fe9e 100644 > --- a/hw/usb/host-libusb.c > +++ b/hw/usb/host-libusb.c > @@ -39,6 +39,11 @@ > #endif > #include <libusb.h> > > +#ifdef CONFIG_LINUX > +#include <sys/ioctl.h> > +#include <linux/usbdevice_fs.h> > +#endif > + > #include "qapi/error.h" > #include "migration/vmstate.h" > #include "monitor/monitor.h" > @@ -885,6 +890,7 @@ static void usb_host_ep_update(USBHostDevice *s) > static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd) > { > USBDevice *udev = USB_DEVICE(s); > + int libusb_speed; > int bus_num = 0; > int addr = 0; > int rc; > @@ -935,7 +941,36 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd) > usb_ep_init(udev); > usb_host_ep_update(s); > > - udev->speed = speed_map[libusb_get_device_speed(dev)]; > + libusb_speed = libusb_get_device_speed(dev); > +#ifdef CONFIG_LINUX > + if (hostfd && libusb_speed == 0) { > + /* > + * Workaround libusb bug: libusb_get_device_speed() does not > + * work for libusb_wrap_sys_device() devices in v1.0.23. > + * > + * Speeds are defined in linux/usb/ch9.h, file not included > + * due to name conflicts. > + */ > + int rc = ioctl(hostfd, USBDEVFS_GET_SPEED, NULL); This (further) breaks a bunch of the Travis jobs - I assume because libusb doesn't always have this symbol: Compiling C object libcommon.fa.p/hw_input_vhost-user-input.c.o ../hw/usb/host-libusb.c: In function ‘usb_host_open’: ../hw/usb/host-libusb.c:954:32: error: ‘USBDEVFS_GET_SPEED’ undeclared (first use in this function) int rc = ioctl(hostfd, USBDEVFS_GET_SPEED, NULL); ^ ../hw/usb/host-libusb.c:954:32: note: each undeclared identifier is reported only once for each function it appears in Makefile.ninja:961: recipe for target 'libcommon.fa.p/hw_usb_host-libusb.c.o' failed make: *** [libcommon.fa.p/hw_usb_host-libusb.c.o] Error 1 make: *** Waiting for unfinished jobs....
> > +#include <linux/usbdevice_fs.h> > > + int rc = ioctl(hostfd, USBDEVFS_GET_SPEED, NULL); > > This (further) breaks a bunch of the Travis jobs - I assume because libusb doesn't > always have this symbol: > > ../hw/usb/host-libusb.c:954:32: error: ‘USBDEVFS_GET_SPEED’ undeclared (first use in this function) It isn't libusb, it is the kernel (linux/usbdevice_fs.h). /me checks git ... Added in 4.13, so available for quite a while. I guess that is the prehistoric ubuntu version travis has? Given that we only need that workaround with rather new libusb versions (which have libusb_wrap_sys_device() support) which most likely isn't present in that old ubuntu version we can probably just do this: diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c index 08604f787fdf..c25102f3aca1 100644 --- a/hw/usb/host-libusb.c +++ b/hw/usb/host-libusb.c @@ -942,7 +942,7 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd) usb_host_ep_update(s); libusb_speed = libusb_get_device_speed(dev); -#ifdef CONFIG_LINUX +#if LIBUSB_API_VERSION >= 0x01000107 && defined(CONFIG_LINUX) if (hostfd && libusb_speed == 0) { /* * Workaround libusb bug: libusb_get_device_speed() does not
Gerd Hoffmann <kraxel@redhat.com> writes: >> > +#include <linux/usbdevice_fs.h> > >> > + int rc = ioctl(hostfd, USBDEVFS_GET_SPEED, NULL); >> >> This (further) breaks a bunch of the Travis jobs - I assume because libusb doesn't >> always have this symbol: >> >> ../hw/usb/host-libusb.c:954:32: error: ‘USBDEVFS_GET_SPEED’ undeclared (first use in this function) > > It isn't libusb, it is the kernel (linux/usbdevice_fs.h). > > /me checks git ... > Added in 4.13, so available for quite a while. > I guess that is the prehistoric ubuntu version travis has? > > Given that we only need that workaround with rather new libusb versions > (which have libusb_wrap_sys_device() support) which most likely isn't > present in that old ubuntu version we can probably just do this: > > diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c > index 08604f787fdf..c25102f3aca1 100644 > --- a/hw/usb/host-libusb.c > +++ b/hw/usb/host-libusb.c > @@ -942,7 +942,7 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd) > usb_host_ep_update(s); > > libusb_speed = libusb_get_device_speed(dev); > -#ifdef CONFIG_LINUX > +#if LIBUSB_API_VERSION >= 0x01000107 && defined(CONFIG_LINUX) > if (hostfd && libusb_speed == 0) { > /* > * Workaround libusb bug: libusb_get_device_speed() does not That works. Do you want to include that in your next PR or send a patch for me to include in my testing/next PR? In the meantime: Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org>
On Wed, Sep 02, 2020 at 03:31:46PM +0100, Alex Bennée wrote: > > Gerd Hoffmann <kraxel@redhat.com> writes: > > >> > +#include <linux/usbdevice_fs.h> > > > >> > + int rc = ioctl(hostfd, USBDEVFS_GET_SPEED, NULL); > >> > >> This (further) breaks a bunch of the Travis jobs - I assume because libusb doesn't > >> always have this symbol: > >> > >> ../hw/usb/host-libusb.c:954:32: error: ‘USBDEVFS_GET_SPEED’ undeclared (first use in this function) > > > > It isn't libusb, it is the kernel (linux/usbdevice_fs.h). > > > > /me checks git ... > > Added in 4.13, so available for quite a while. > > I guess that is the prehistoric ubuntu version travis has? > > > > Given that we only need that workaround with rather new libusb versions > > (which have libusb_wrap_sys_device() support) which most likely isn't > > present in that old ubuntu version we can probably just do this: > > > > diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c > > index 08604f787fdf..c25102f3aca1 100644 > > --- a/hw/usb/host-libusb.c > > +++ b/hw/usb/host-libusb.c > > @@ -942,7 +942,7 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd) > > usb_host_ep_update(s); > > > > libusb_speed = libusb_get_device_speed(dev); > > -#ifdef CONFIG_LINUX > > +#if LIBUSB_API_VERSION >= 0x01000107 && defined(CONFIG_LINUX) > > if (hostfd && libusb_speed == 0) { > > /* > > * Workaround libusb bug: libusb_get_device_speed() does not > > That works. Do you want to include that in your next PR or send a patch > for me to include in my testing/next PR? Patch is on the list already: https://patchwork.ozlabs.org/project/qemu-devel/patch/20200902081445.3291-1-kraxel@redhat.com/ Feel free to include it in testing pull. take care, Gerd
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c index c474551d8456..77f1eaa5fe9e 100644 --- a/hw/usb/host-libusb.c +++ b/hw/usb/host-libusb.c @@ -39,6 +39,11 @@ #endif #include <libusb.h> +#ifdef CONFIG_LINUX +#include <sys/ioctl.h> +#include <linux/usbdevice_fs.h> +#endif + #include "qapi/error.h" #include "migration/vmstate.h" #include "monitor/monitor.h" @@ -885,6 +890,7 @@ static void usb_host_ep_update(USBHostDevice *s) static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd) { USBDevice *udev = USB_DEVICE(s); + int libusb_speed; int bus_num = 0; int addr = 0; int rc; @@ -935,7 +941,36 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd) usb_ep_init(udev); usb_host_ep_update(s); - udev->speed = speed_map[libusb_get_device_speed(dev)]; + libusb_speed = libusb_get_device_speed(dev); +#ifdef CONFIG_LINUX + if (hostfd && libusb_speed == 0) { + /* + * Workaround libusb bug: libusb_get_device_speed() does not + * work for libusb_wrap_sys_device() devices in v1.0.23. + * + * Speeds are defined in linux/usb/ch9.h, file not included + * due to name conflicts. + */ + int rc = ioctl(hostfd, USBDEVFS_GET_SPEED, NULL); + switch(rc) { + case 1: /* low */ + libusb_speed = LIBUSB_SPEED_LOW; + break; + case 2: /* full */ + libusb_speed = LIBUSB_SPEED_FULL; + break; + case 3: /* high */ + case 4: /* wireless */ + libusb_speed = LIBUSB_SPEED_HIGH; + break; + case 5: /* super */ + case 6: /* super plus */ + libusb_speed = LIBUSB_SPEED_SUPER; + break; + } + } +#endif + udev->speed = speed_map[libusb_speed]; usb_host_speed_compat(s); if (s->ddesc.iProduct) {
libusb_get_device_speed() does not work for libusb_wrap_sys_device() devices in v1.0.23. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1871090 Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/usb/host-libusb.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)