From 0785bf138d28532b047c98b4df78ba2d46c551a4 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 28 Apr 2016 15:40:03 +1000 Subject: [PATCH 1/6] hidpp20: fail for hidpp10 devices hidpp10 devices have their own driver, let's not initialize them here and just fail. If a hidpp10 device is assigned the hidpp20 driver this is a bug. Signed-off-by: Peter Hutterer --- src/driver-hidpp20.c | 10 +++++----- src/hidpp20.c | 3 +++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index 66fdce04..f267d5d1 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -808,11 +808,9 @@ hidpp20drv_probe(struct ratbag_device *device) drv_data->num_resolutions = 1; drv_data->num_buttons = 8; - if (dev->proto_major >= 2) { - rc = hidpp20drv_20_probe(device); - if (rc) - goto err; - } + rc = hidpp20drv_20_probe(device); + if (rc) + goto err; ratbag_device_init_profiles(device, drv_data->num_profiles, @@ -823,6 +821,8 @@ hidpp20drv_probe(struct ratbag_device *device) err: free(drv_data); ratbag_set_drv_data(device, NULL); + if (dev) + hidpp20_device_destroy(dev); return rc; } diff --git a/src/hidpp20.c b/src/hidpp20.c index 38b82e42..2b707a2b 100644 --- a/src/hidpp20.c +++ b/src/hidpp20.c @@ -1892,6 +1892,9 @@ hidpp20_device_new(const struct hidpp_device *base, unsigned int idx) goto err; } + if (dev->proto_major < 2) + goto err; + rc = hidpp20_feature_set_get(dev); if (rc < 0) goto err; From 40112e606164823d82a8f743f9c6a4c1cb626d51 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 28 Apr 2016 16:16:47 +1000 Subject: [PATCH 2/6] driver-hidpp20: re-use the destroy function for cleanup during init No need for two separate cleanup blocks, they're doing the same thing anyway. Just make the function more robust to uninitalized fields and we're good to go. Signed-off-by: Peter Hutterer --- src/driver-hidpp20.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index f267d5d1..054e0bda 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -773,6 +773,26 @@ hidpp20_log(void *userdata, enum hidpp_log_priority priority, const char *format log_msg_va(device->ratbag, priority, format, args); } +static void +hidpp20drv_remove(struct ratbag_device *device) +{ + struct hidpp20drv_data *drv_data = ratbag_get_drv_data(device); + struct hidpp20_device *dev = drv_data->dev; + + if (!device) + return; + + ratbag_close_hidraw(device); + + if (drv_data->profiles) + hidpp20_onboard_profiles_destroy(dev, drv_data->profiles); + free(drv_data->controls); + free(drv_data->sensors); + if (drv_data->dev) + hidpp20_device_destroy(drv_data->dev); + free(drv_data); +} + static int hidpp20drv_probe(struct ratbag_device *device) { @@ -819,29 +839,10 @@ hidpp20drv_probe(struct ratbag_device *device) return rc; err: - free(drv_data); - ratbag_set_drv_data(device, NULL); - if (dev) - hidpp20_device_destroy(dev); + hidpp20drv_remove(device); return rc; } -static void -hidpp20drv_remove(struct ratbag_device *device) -{ - struct hidpp20drv_data *drv_data = ratbag_get_drv_data(device); - struct hidpp20_device *dev = drv_data->dev; - - ratbag_close_hidraw(device); - - if (drv_data->profiles) - hidpp20_onboard_profiles_destroy(dev, drv_data->profiles); - free(drv_data->controls); - free(drv_data->sensors); - hidpp20_device_destroy(drv_data->dev); - free(drv_data); -} - struct ratbag_driver hidpp20_driver = { .name = "Logitech HID++2.0", .id = "hidpp20", From b6294d4f93e1a86088556e074b94f3519a2d643f Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 28 Apr 2016 14:51:39 +1000 Subject: [PATCH 3/6] Add a new header for PID/VIDs that we need to care about Beats having the same #define in multiple places Signed-off-by: Peter Hutterer --- src/Makefile.am | 6 ++++-- src/driver-hidpp10.c | 3 +-- src/liblur.c | 6 +++--- src/usb-ids.h | 29 +++++++++++++++++++++++++++++ 4 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 src/usb-ids.h diff --git a/src/Makefile.am b/src/Makefile.am index 56640e2a..8ad3b162 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -19,7 +19,8 @@ libratbag_la_SOURCES = \ libratbag-hidraw.h \ libratbag-private.h \ libratbag-test.c \ - libratbag-test.h + libratbag-test.h \ + usb-ids.h libratbag_la_LIBADD = \ libhidpp.la \ @@ -66,7 +67,8 @@ libhidpp_la_SOURCES = \ hidpp10.h \ hidpp10.c \ hidpp20.h \ - hidpp20.c + hidpp20.c \ + usb-ids.h libhidpp_la_LDFLAGS = -lm libutil_la_SOURCES = \ libratbag-util.c \ diff --git a/src/driver-hidpp10.c b/src/driver-hidpp10.c index 71e74590..73202eef 100644 --- a/src/driver-hidpp10.c +++ b/src/driver-hidpp10.c @@ -35,8 +35,6 @@ #include "config.h" -#define USB_VENDOR_ID_LOGITECH 0x046d - #include #include #include @@ -45,6 +43,7 @@ #include #include "hidpp10.h" +#include "usb-ids.h" #include "libratbag-private.h" #include "libratbag-hidraw.h" diff --git a/src/liblur.c b/src/liblur.c index e2149ce8..1bb5cb54 100644 --- a/src/liblur.c +++ b/src/liblur.c @@ -30,13 +30,13 @@ #include #include +#include "usb-ids.h" #include "hidpp10.h" #include "libratbag-util.h" #include "liblur.h" #define _EXPORT_ __attribute__ ((visibility("default"))) #define MAX_DEVICES 6 -#define VENDOR_ID_LOGITECH 0x046d struct lur_receiver { int refcount; @@ -113,7 +113,7 @@ lur_device_disconnect(struct lur_device *dev) _EXPORT_ int lur_is_receiver(uint16_t vid, uint16_t pid) { - return (vid == VENDOR_ID_LOGITECH && (pid == 0xc52b || pid == 0xc532)); + return (vid == USB_VENDOR_ID_LOGITECH && (pid == 0xc52b || pid == 0xc532)); } static bool @@ -232,7 +232,7 @@ lur_receiver_enumerate(struct lur_receiver *lur, lur_receiver_ref(lur); dev->refcount = 1; dev->name = strdup_safe(name); - dev->vid = VENDOR_ID_LOGITECH; + dev->vid = USB_VENDOR_ID_LOGITECH; dev->pid = wpid; dev->type = type; dev->serial = serial; diff --git a/src/usb-ids.h b/src/usb-ids.h new file mode 100644 index 00000000..f69273e3 --- /dev/null +++ b/src/usb-ids.h @@ -0,0 +1,29 @@ +/* + * Copyright © 2016 Red Hat, Inc. + * + * Permission to use, copy, modify, distribute, and sell this software and its + * documentation for any purpose is hereby granted without fee, provided that + * the above copyright notice appear in all copies and that both that copyright + * notice and this permission notice appear in supporting documentation, and + * that the name of the copyright holders not be used in advertising or + * publicity pertaining to distribution of the software without specific, + * written prior permission. The copyright holders make no representations + * about the suitability of this software for any purpose. It is provided "as + * is" without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE, + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE + * OF THIS SOFTWARE. + */ + +#ifndef USB_IDS_H +#define USB_IDS_H + +#define USB_VENDOR_ID_LOGITECH 0x046d + +#endif + From 55673442f1426dc58d1cbc974b1a2893129ba6f7 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 28 Apr 2016 15:07:13 +1000 Subject: [PATCH 4/6] Split the driver search + testing into a helper function Prep work for being able to probe multiple drivers on a device. The function that tests the actual driver simply takes a const char * with the driver name, so in the future we can loop through multiple driver names easily. Signed-off-by: Peter Hutterer --- src/libratbag.c | 52 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/src/libratbag.c b/src/libratbag.c index 65e18eea..2b0b719b 100644 --- a/src/libratbag.c +++ b/src/libratbag.c @@ -283,23 +283,15 @@ out: return rc; } -bool -ratbag_assign_driver(struct ratbag_device *device, - const struct input_id *dev_id, - struct ratbag_test_device *test_device) +static inline bool +ratbag_test_driver(struct ratbag_device *device, + const struct input_id *dev_id, + const char *driver_name, + struct ratbag_test_device *test_device) { struct ratbag *ratbag = device->ratbag; struct ratbag_driver *driver; int rc; - const char *driver_name; - - if (!test_device) { - driver_name = udev_prop_value(device->udev_device, "RATBAG_DRIVER"); - if (!driver_name) - return false; - } else { - driver_name = "test_driver"; - } list_for_each(driver, &ratbag->drivers, link) { if (streq(driver->id, driver_name)) { @@ -311,7 +303,7 @@ ratbag_assign_driver(struct ratbag_device *device, if (!device->driver) { log_error(ratbag, "%s: driver '%s' does not exist\n", device->name, driver_name); - return false; + goto error; } if (test_device) @@ -320,22 +312,46 @@ ratbag_assign_driver(struct ratbag_device *device, rc = device->driver->probe(device); if (rc == 0) { if (!ratbag_sanity_check_device(device)) { - return false; + goto error; } else { - log_debug(ratbag, "driver match found: %s\n", driver->name); + log_debug(ratbag, + "driver match found: %s\n", + device->driver->name); return true; } } - device->driver = NULL; - if (rc != -ENODEV) log_error(ratbag, "%s: error opening hidraw node (%s)\n", device->name, strerror(-rc)); +error: + device->driver = NULL; + return false; } +bool +ratbag_assign_driver(struct ratbag_device *device, + const struct input_id *dev_id, + struct ratbag_test_device *test_device) +{ + bool rc; + const char *driver_name; + + if (!test_device) { + driver_name = udev_prop_value(device->udev_device, "RATBAG_DRIVER"); + if (!driver_name) + return false; + } else { + driver_name = "test_driver"; + } + + rc = ratbag_test_driver(device, dev_id, driver_name, test_device); + + return rc; +} + static inline char* get_device_name(struct udev_device *device) { From a715679f1f2913d8cfc4bf75b5d35c7484dde082 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 28 Apr 2016 15:23:48 +1000 Subject: [PATCH 5/6] Add a fallback for Logitech's HID++ devices If a device is not specifically assigned a driver, try hidpp20 and then hidpp10 hoping that one of them will work. Signed-off-by: Peter Hutterer --- src/libratbag.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/libratbag.c b/src/libratbag.c index 2b0b719b..55067d7a 100644 --- a/src/libratbag.c +++ b/src/libratbag.c @@ -34,6 +34,7 @@ #include #include +#include "usb-ids.h" #include "libratbag-private.h" #include "libratbag-util.h" @@ -331,6 +332,22 @@ error: return false; } +static inline bool +ratbag_driver_fallback_logitech(struct ratbag_device *device, + const struct input_id *dev_id) +{ + int rc; + + if (dev_id->vendor != USB_VENDOR_ID_LOGITECH) + return false; + + rc = ratbag_test_driver(device, dev_id, "hidpp20", NULL); + if (!rc) + rc = ratbag_test_driver(device, dev_id, "hidpp10", NULL); + + return rc; +} + bool ratbag_assign_driver(struct ratbag_device *device, const struct input_id *dev_id, @@ -341,13 +358,15 @@ ratbag_assign_driver(struct ratbag_device *device, if (!test_device) { driver_name = udev_prop_value(device->udev_device, "RATBAG_DRIVER"); - if (!driver_name) - return false; } else { driver_name = "test_driver"; } - rc = ratbag_test_driver(device, dev_id, driver_name, test_device); + if (driver_name) { + rc = ratbag_test_driver(device, dev_id, driver_name, test_device); + } else { + rc = ratbag_driver_fallback_logitech(device, dev_id); + } return rc; } From e912f56c260e4786eb5cc972a6ee763ff1024df1 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 29 Apr 2016 10:14:11 +1000 Subject: [PATCH 6/6] driver-hidpp20: don't log an error if we can't init a hidpp20 device Once we start probing devices for multiple drivers we shouldn't just print error messages for every failed driver. In the hidpp20 case an error would be printed for any hidpp10 logitech device. Signed-off-by: Peter Hutterer --- src/driver-hidpp20.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index 054e0bda..a09c34de 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -812,9 +812,6 @@ hidpp20drv_probe(struct ratbag_device *device) dev = hidpp20_device_new(&base, HIDPP_RECEIVER_IDX); if (!dev) { - log_error(device->ratbag, - "Failed to get HID++2.0 device for %s\n", - device->name); rc = -ENODEV; goto err; }