From e1fc248fb8d1833ce396c5838608c0b0dbc84148 Mon Sep 17 00:00:00 2001 From: Kelly Byrd Date: Sun, 5 Nov 2023 17:12:55 -0800 Subject: [PATCH] Add usb_config_index to usb_communication_subdriver_s, default to 0. The tactical goal of this change is to change ret = libusb_get_config_descriptor(device, (uint8_t)usb_subdriver.hid_rep_index, &conf_desc); to ret = libusb_get_config_descriptor(device, (uint8_t)usb_subdriver.usb_config_index, &conf_desc); Before this change, libusb1.c did libusb_get_config_descriptor() with a config index equal to the interface number. For composite devices using an interface index > 0, this is usally the wrong choice. Concretely, I'm using an Arduino for a DIY UPS project and these are composite devices with multiple interfaces under the first (and only) config descriptor. In the USB descriptor heirarchy, deivce descriptors have config descriptors which have interface descriptors. Also, nearly all USB devices have a single configuration (index 0). In order to do this, I added a new member alongside the existing hid_rep_idex and hid_desc_index. I chose to do this instead of using the add_var method because this member is used in places in very similar ways to how hid_rep_index and hid_desc_index is used. This new member defaults to 0 which covers the majority of USB devices. Any future subdriver is able to use this if a device requires it. For existing subdrives, we'll just use an index of 0. I also changed some debug logging to print out the config index where the code was already printing the interface index. Signed-off-by: Kelly Byrd --- drivers/libusb0.c | 12 ++++++------ drivers/libusb1.c | 17 ++++++++++++----- drivers/nut_libusb.h | 10 ++++++++++ drivers/usb-common.h | 8 ++++++++ 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/drivers/libusb0.c b/drivers/libusb0.c index 0dd46b0139..8851ae4ac6 100644 --- a/drivers/libusb0.c +++ b/drivers/libusb0.c @@ -435,8 +435,9 @@ static int libusb_open(usb_dev_handle **udevp, } fatalx(EXIT_FAILURE, - "Can't claim USB device [%04x:%04x]@%d/%d: %s", + "Can't claim USB device [%04x:%04x]@%d/%d/%d: %s", curDevice->VendorID, curDevice->ProductID, + usb_subdriver.usb_config_index, usb_subdriver.hid_rep_index, usb_subdriver.hid_desc_index, usb_strerror()); @@ -449,8 +450,9 @@ static int libusb_open(usb_dev_handle **udevp, } fatalx(EXIT_FAILURE, - "Can't claim USB device [%04x:%04x]@%d/%d: %s", + "Can't claim USB device [%04x:%04x]@%d/%d/%d: %s", curDevice->VendorID, curDevice->ProductID, + usb_subdriver.usb_config_index, usb_subdriver.hid_rep_index, usb_subdriver.hid_desc_index, usb_strerror()); @@ -508,10 +510,7 @@ static int libusb_open(usb_dev_handle **udevp, /* Note: on some broken UPS's (e.g. Tripp Lite Smart1000LCD), only this second method gives the correct result */ - - /* for now, we always assume configuration 0, interface 0, - altsetting 0, as above. */ - iface = &dev->config[0].interface[usb_subdriver.hid_rep_index].altsetting[0]; + iface = &dev->config[usb_subdriver.usb_config_index].interface[usb_subdriver.hid_rep_index].altsetting[0]; for (i=0; iextralen; i+=iface->extra[i]) { upsdebugx(4, "i=%d, extra[i]=%02x, extra[i+1]=%02x", i, iface->extra[i], iface->extra[i+1]); @@ -904,6 +903,7 @@ usb_communication_subdriver_t usb_subdriver = { libusb_set_report, libusb_get_string, libusb_get_interrupt, + LIBUSB_DEFAULT_CONF_INDEX, LIBUSB_DEFAULT_INTERFACE, LIBUSB_DEFAULT_DESC_INDEX, LIBUSB_DEFAULT_HID_EP_IN, diff --git a/drivers/libusb1.c b/drivers/libusb1.c index 17f4b8f740..349e59985a 100644 --- a/drivers/libusb1.c +++ b/drivers/libusb1.c @@ -418,9 +418,10 @@ static int nut_libusb_open(libusb_device_handle **udevp, * that the device is not what we want. */ upsdebugx(2, "Device matches"); - upsdebugx(2, "Reading first configuration descriptor"); + upsdebugx(2, "Reading configuration descriptor %d of %d", + usb_subdriver.usb_config_index+1, dev_desc.bNumConfigurations); ret = libusb_get_config_descriptor(device, - (uint8_t)usb_subdriver.hid_rep_index, + (uint8_t)usb_subdriver.usb_config_index, &conf_desc); /*ret = libusb_get_active_config_descriptor(device, &conf_desc);*/ if (ret < 0) @@ -497,8 +498,9 @@ static int nut_libusb_open(libusb_device_handle **udevp, libusb_free_config_descriptor(conf_desc); libusb_free_device_list(devlist, 1); fatalx(EXIT_FAILURE, - "Can't claim USB device [%04x:%04x]@%d/%d: %s", + "Can't claim USB device [%04x:%04x]@%d/%d/%d: %s", curDevice->VendorID, curDevice->ProductID, + usb_subdriver.usb_config_index, usb_subdriver.hid_rep_index, usb_subdriver.hid_desc_index, libusb_strerror((enum libusb_error)ret)); @@ -513,8 +515,9 @@ static int nut_libusb_open(libusb_device_handle **udevp, libusb_free_config_descriptor(conf_desc); libusb_free_device_list(devlist, 1); fatalx(EXIT_FAILURE, - "Can't claim USB device [%04x:%04x]@%d/%d: %s", + "Can't claim USB device [%04x:%04x]@%d/%d/%d: %s", curDevice->VendorID, curDevice->ProductID, + usb_subdriver.usb_config_index, usb_subdriver.hid_rep_index, usb_subdriver.hid_desc_index, libusb_strerror((enum libusb_error)ret)); @@ -533,7 +536,10 @@ static int nut_libusb_open(libusb_device_handle **udevp, } if (!conf_desc) { /* ?? this should never happen */ - upsdebugx(2, " Couldn't retrieve descriptors"); + upsdebugx(2, " Couldn't retrieve config descriptor [%04x:%04x]@%d", + curDevice->VendorID, curDevice->ProductID, + usb_subdriver.usb_config_index + ); goto next_device; } @@ -1025,6 +1031,7 @@ usb_communication_subdriver_t usb_subdriver = { nut_libusb_set_report, nut_libusb_get_string, nut_libusb_get_interrupt, + LIBUSB_DEFAULT_CONF_INDEX, LIBUSB_DEFAULT_INTERFACE, LIBUSB_DEFAULT_DESC_INDEX, LIBUSB_DEFAULT_HID_EP_IN, diff --git a/drivers/nut_libusb.h b/drivers/nut_libusb.h index 8b2a644341..376e653037 100644 --- a/drivers/nut_libusb.h +++ b/drivers/nut_libusb.h @@ -37,6 +37,7 @@ * and for libusb headers and 0.1/1.0 mapping */ /* Used in drivers/libusb*.c sources: */ +#define LIBUSB_DEFAULT_CONF_INDEX 0 #define LIBUSB_DEFAULT_INTERFACE 0 #define LIBUSB_DEFAULT_DESC_INDEX 0 #define LIBUSB_DEFAULT_HID_EP_IN 1 @@ -73,6 +74,15 @@ typedef struct usb_communication_subdriver_s { usb_ctrl_charbuf buf, usb_ctrl_charbufsize bufsize, usb_ctrl_timeout_msec timeout); + /* Nearly all devices use a single configuration descriptor, index 0. + * But, it is possible for a device have more than one, check bNumConfigration + * on the device descriptor for the total. + * + * In USB, the descriptor heirarchy is + * device -> configuration(s) -> interface(s) -> endpoint(s) + */ + usb_ctrl_cfgindex usb_config_index; /* index of the device config we use. Almost always 0; see comments above */ + /* Used for Powervar UPS or similar cases to make sure * we use the right interface in the Composite device. * In a few cases our libusb*.c sets the value for specific diff --git a/drivers/usb-common.h b/drivers/usb-common.h index d1de6d23d5..4f151ee8ac 100644 --- a/drivers/usb-common.h +++ b/drivers/usb-common.h @@ -122,6 +122,10 @@ #define USB_CTRL_MSGVALUE_MIN 0 #define USB_CTRL_MSGVALUE_MAX UINT16_MAX + typedef uint8_t usb_ctrl_cfgindex; + #define USB_CTRL_CFGINDEX_MIN 0 + #define USB_CTRL_CFGINDEX_MAX UINT8_MAX + typedef uint16_t usb_ctrl_repindex; #define USB_CTRL_REPINDEX_MIN 0 #define USB_CTRL_REPINDEX_MAX UINT16_MAX @@ -411,6 +415,10 @@ #define USB_CTRL_MSGVALUE_MIN INT_MIN #define USB_CTRL_MSGVALUE_MAX INT_MAX + typedef uint8_t usb_ctrl_cfgindex; + #define USB_CTRL_CFGINDEX_MIN 0 + #define USB_CTRL_CFGINDEX_MAX UINT8_MAX + typedef int usb_ctrl_repindex; #define USB_CTRL_REPINDEX_MIN INT_MIN #define USB_CTRL_REPINDEX_MAX INT_MAX