From fc7ffa7f503f589e4914b6ca735f2d7b8d141cd8 Mon Sep 17 00:00:00 2001 From: Matt Merhar Date: Sun, 10 Dec 2023 00:59:58 -0500 Subject: [PATCH 1/3] drivers/libusb{0,1}.c: reset driver parameters while enumerating With an Arduino Leonardo compatible board attached alongside a CyberPower CP1500PFCLCD, usbhid-ups would incorrectly conclude that there was no kernel driver attached to the UPS and then fail subsequent steps accordingly. The cause is the global usb_communication_subdriver_t struct; when a subdriver (e.g. arduino-hid) sets different values during the enumeration loop, they're not set back to defaults afterwards, causing issues with other subdrivers (e.g. cps-hid). So, if a subdriver doesn't match, set a selection of fields back to their default values using the newly added nut_usb_subdriver_defaults() before the next attempt. Signed-off-by: Matt Merhar --- NEWS.adoc | 5 +++++ drivers/libusb0.c | 16 ++++++++++++++++ drivers/libusb1.c | 16 ++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/NEWS.adoc b/NEWS.adoc index f4d8929bfc..2932d9f9a1 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -149,6 +149,11 @@ https://github.com/networkupstools/nut/milestone/11 by default: `powercom_sdcmd_byte_order_fallback`. [PR #2480] * `cps-hid` subdriver now supports more variables, as available on e.g. CP1350EPFCLCD model. [PR #2540] + * USB parameters (per `usb_communication_subdriver_t`) are now set back to + their default values during enumeration after probing each subdriver. + Having an unrelated device connected with a VID:PID matching the + `arduino-hid` subdriver prevented use of an actual `usb-hid` device due to + changes made to this struct during probe. [#2611] - bicker_ser: added new driver for Bicker 12/24Vdc UPS via RS-232 serial communication protocol, which supports any UPS shipped with the PSZ-1053 diff --git a/drivers/libusb0.c b/drivers/libusb0.c index 2cc617a239..472a37220d 100644 --- a/drivers/libusb0.c +++ b/drivers/libusb0.c @@ -186,6 +186,20 @@ static int nut_libusb_set_altinterface(usb_dev_handle *udev) return ret; } +static void nut_usb_subdriver_defaults(usb_communication_subdriver_t *subdriver) +{ + if (!getval("usb_config_index")) + subdriver->usb_config_index = LIBUSB_DEFAULT_CONF_INDEX; + if (!getval("usb_hid_rep_index")) + subdriver->hid_rep_index = LIBUSB_DEFAULT_INTERFACE; + if (!getval("usb_hid_desc_index")) + subdriver->hid_desc_index = LIBUSB_DEFAULT_DESC_INDEX; + if (!getval("usb_hid_ep_in")) + subdriver->hid_ep_in = LIBUSB_DEFAULT_HID_EP_IN; + if (!getval("usb_hid_ep_out")) + subdriver->hid_ep_out = LIBUSB_DEFAULT_HID_EP_OUT; +} + #define usb_control_msg typesafe_control_msg /* On success, fill in the curDevice structure and return the report @@ -711,6 +725,8 @@ static int nut_libusb_open(usb_dev_handle **udevp, /* if (if_claimed) usb_release_interface(udev, 0); */ usb_close(udev); + /* reset any parameters modified by unmatched drivers back to defaults */ + nut_usb_subdriver_defaults(&usb_subdriver); } } diff --git a/drivers/libusb1.c b/drivers/libusb1.c index 6035f65cb9..e4fdbbf95d 100644 --- a/drivers/libusb1.c +++ b/drivers/libusb1.c @@ -152,6 +152,20 @@ static int nut_libusb_set_altinterface(libusb_device_handle *udev) return ret; } +static void nut_usb_subdriver_defaults(usb_communication_subdriver_t *subdriver) +{ + if (!getval("usb_config_index")) + subdriver->usb_config_index = LIBUSB_DEFAULT_CONF_INDEX; + if (!getval("usb_hid_rep_index")) + subdriver->hid_rep_index = LIBUSB_DEFAULT_INTERFACE; + if (!getval("usb_hid_desc_index")) + subdriver->hid_desc_index = LIBUSB_DEFAULT_DESC_INDEX; + if (!getval("usb_hid_ep_in")) + subdriver->hid_ep_in = LIBUSB_DEFAULT_HID_EP_IN; + if (!getval("usb_hid_ep_out")) + subdriver->hid_ep_out = LIBUSB_DEFAULT_HID_EP_OUT; +} + /* On success, fill in the curDevice structure and return the report * descriptor length. On failure, return -1. * Note: When callback is not NULL, the report descriptor will be @@ -795,6 +809,8 @@ static int nut_libusb_open(libusb_device_handle **udevp, /* if (if_claimed) libusb_release_interface(udev, usb_subdriver.hid_rep_index); */ libusb_close(udev); + /* reset any parameters modified by unmatched drivers back to defaults */ + nut_usb_subdriver_defaults(&usb_subdriver); } *udevp = NULL; From 959c8cb37822d091a469d394839309ee71e25755 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 7 Sep 2024 15:09:23 +0200 Subject: [PATCH 2/3] NEWS.adoc: explain USB enumeration fix [#2611] With an Arduino Leonardo compatible board attached alongside a CyberPower CP1500PFCLCD, usbhid-ups would incorrectly conclude that there was no kernel driver attached to the UPS and then fail subsequent steps accordingly. The cause is the global usb_communication_subdriver_t struct; when a subdriver (e.g. arduino-hid) sets different values during the enumeration loop, they're not set back to defaults afterwards, causing issues with other subdrivers (e.g. cps-hid). So, if a subdriver doesn't match, set a selection of fields back to their default values using the newly added nut_usb_subdriver_defaults() before the next attempt. // tofurky Signed-off-by: Jim Klimov --- NEWS.adoc | 4 ++++ docs/nut.dict | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/NEWS.adoc b/NEWS.adoc index 2932d9f9a1..b8ab0edc3c 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -175,6 +175,10 @@ https://github.com/networkupstools/nut/milestone/11 `nutdrv_atcl_usb`) and for general code to collect string readings and other data points, and for `nut-scanner`. + - USB drivers should now be more likely to succeed with iterative detection + of an UPS interface on a composite USB device or when looking at devices + with non-default interface/endpoint/config numbers. [PR #2611] + - Introduced a new driver concept for interaction with OS-reported hardware monitoring readings. Currently instantiated as `hwmon_ina219` specifically made for Texas Instruments INA219 chip as exposed in the Linux "hwmon" diff --git a/docs/nut.dict b/docs/nut.dict index 7c89cc5c82..46a395154f 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3214 utf-8 +personal_ws-1.1 en 3215 utf-8 AAC AAS ABI From 6da2fd9c3a5769c96e7d695a1ae389b01e15540f Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 7 Sep 2024 15:14:31 +0200 Subject: [PATCH 3/3] drivers/libusb{0,1}.c: rename nut_libusb_subdriver_defaults() to common style [#2611] Signed-off-by: Jim Klimov --- drivers/libusb0.c | 4 ++-- drivers/libusb1.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/libusb0.c b/drivers/libusb0.c index 472a37220d..1f86740848 100644 --- a/drivers/libusb0.c +++ b/drivers/libusb0.c @@ -186,7 +186,7 @@ static int nut_libusb_set_altinterface(usb_dev_handle *udev) return ret; } -static void nut_usb_subdriver_defaults(usb_communication_subdriver_t *subdriver) +static void nut_libusb_subdriver_defaults(usb_communication_subdriver_t *subdriver) { if (!getval("usb_config_index")) subdriver->usb_config_index = LIBUSB_DEFAULT_CONF_INDEX; @@ -726,7 +726,7 @@ static int nut_libusb_open(usb_dev_handle **udevp, usb_release_interface(udev, 0); */ usb_close(udev); /* reset any parameters modified by unmatched drivers back to defaults */ - nut_usb_subdriver_defaults(&usb_subdriver); + nut_libusb_subdriver_defaults(&usb_subdriver); } } diff --git a/drivers/libusb1.c b/drivers/libusb1.c index e4fdbbf95d..4eac05ca12 100644 --- a/drivers/libusb1.c +++ b/drivers/libusb1.c @@ -152,7 +152,7 @@ static int nut_libusb_set_altinterface(libusb_device_handle *udev) return ret; } -static void nut_usb_subdriver_defaults(usb_communication_subdriver_t *subdriver) +static void nut_libusb_subdriver_defaults(usb_communication_subdriver_t *subdriver) { if (!getval("usb_config_index")) subdriver->usb_config_index = LIBUSB_DEFAULT_CONF_INDEX; @@ -810,7 +810,7 @@ static int nut_libusb_open(libusb_device_handle **udevp, libusb_release_interface(udev, usb_subdriver.hid_rep_index); */ libusb_close(udev); /* reset any parameters modified by unmatched drivers back to defaults */ - nut_usb_subdriver_defaults(&usb_subdriver); + nut_libusb_subdriver_defaults(&usb_subdriver); } *udevp = NULL;