From 7a7abb1ff997def398ff7b49b28a24ce2ec78c58 Mon Sep 17 00:00:00 2001 From: Lutz Eichler Date: Sat, 4 Feb 2017 11:37:42 +0100 Subject: [PATCH 1/2] fix issue 48 SIGSEGV with RGB Keyboards in BIOS mode When switching K70RGB or K95RGB to BIOS mode, only one usb channel is provided by the KB driver. Because in usb_linux.c for all other modes the last channel is not used, zero channels have to be initialized. this brings the daemon to a SIGSEGV. While debugging this, sometimes the KB got informations which stopped the KB completely from working. Only disconnecting + connecting worked, no software-reset via the switch had been successful in this state. When connecting a KB in this state to the daemon, 0 channels were detected. These two special conditions (0 and 1 EP for an RGB device) got special treatment in the code. --- src/ckb-daemon/usb_linux.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/ckb-daemon/usb_linux.c b/src/ckb-daemon/usb_linux.c index 30c8341a86..d5b812d587 100644 --- a/src/ckb-daemon/usb_linux.c +++ b/src/ckb-daemon/usb_linux.c @@ -94,10 +94,14 @@ void* os_inputmain(void* context){ // Monitor input transfers on all endpoints for non-RGB devices // For RGB, monitor all but the last, as it's used for input/output int urbcount = IS_RGB(vendor, product) ? (kb->epcount - 1) : kb->epcount; + if (urbcount == 0) { + ckb_warn("urbcount = 0, so there is nothing to claim in os_inputmain()\n"); + return 0; + } struct usbdevfs_urb urbs[urbcount]; memset(urbs, 0, sizeof(urbs)); urbs[0].buffer_length = 8; - if(IS_RGB(vendor, product)){ + if(urbcount > 1 && IS_RGB(vendor, product)) { if(IS_MOUSE(vendor, product)) urbs[1].buffer_length = 10; else @@ -219,11 +223,15 @@ void os_closeusb(usbdevice* kb){ int usbclaim(usbdevice* kb){ int count = kb->epcount; + ckb_info("claiming %d endpoints\n", count); + for(int i = 0; i < count; i++){ struct usbdevfs_ioctl ctl = { i, USBDEVFS_DISCONNECT, 0 }; ioctl(kb->handle - 1, USBDEVFS_IOCTL, &ctl); - if(ioctl(kb->handle - 1, USBDEVFS_CLAIMINTERFACE, &i)) + if(ioctl(kb->handle - 1, USBDEVFS_CLAIMINTERFACE, &i)) { + ckb_err("Failed to claim interface %d: %s\n", i, strerror(errno)); return -1; + } } return 0; } @@ -284,13 +292,18 @@ int os_setupusb(usbdevice* kb){ // Claim the USB interfaces const char* ep_str = udev_device_get_sysattr_value(dev, "bNumInterfaces"); + ckb_info("claiming interfaces. name=%s, serial=%s, firmware=%s; Got >>%s<< as ep_str\n", name, serial, firmware, ep_str); kb->epcount = 0; if(ep_str) sscanf(ep_str, "%d", &kb->epcount); - if(kb->epcount == 0){ + if(kb->epcount < 2){ + // IF we have an RGB KB with 0 or 1 endpoints, it will be in BIOS mode. + ckb_warn("Possible unable to read endpoint count from udev, assuming %d and reading >>%s<<...\n", kb->epcount, ep_str); + return -1; + // ToDo lae. are there special versions we have to detect? // This shouldn't happen, but if it does, assume EP count based on what the device is supposed to have kb->epcount = (HAS_FEATURES(kb, FEAT_RGB) ? 4 : 3); - ckb_warn("Unable to read endpoint count from udev, assuming %d...\n", kb->epcount); + ckb_warn("Unable to read endpoint count from udev, assuming %d and reading >>%s<<...\n", kb->epcount, ep_str); } if(usbclaim(kb)){ ckb_err("Failed to claim interfaces: %s\n", strerror(errno)); @@ -306,6 +319,8 @@ int usbadd(struct udev_device* dev, short vendor, short product){ ckb_err("Failed to get device path\n"); return -1; } + //lae. + ckb_info(">>>vendor = 0x%x, product = 0x%x, path = %s, syspath = %s\n", vendor, product, path, syspath); // Find a free USB slot for(int index = 1; index < DEV_MAX; index++){ usbdevice* kb = keyboard + index; From dede7f93977b9e2106fa098b66c8dd78e64fae96 Mon Sep 17 00:00:00 2001 From: Lutz Eichler Date: Sun, 5 Feb 2017 12:49:44 +0100 Subject: [PATCH 2/2] improve the behavior when modifying modes for RGB keyboards Changing the operating modes for the RGB keyboards that provide this feature (such as the K70RGB or the K95RGB) will result in massive disruptions to the daemon and the keyboard itself. A test matrix is stored in the corresponding issue # 48. The settings here refer to the timeout before sending a USB message. The timeout was previously 10ms and has been doubled. Minor corrections and the clipping of the debug output in a #define DEBUG were also committed here. --- src/ckb-daemon/usb.c | 2 +- src/ckb-daemon/usb.h | 2 +- src/ckb-daemon/usb_linux.c | 24 +++++++++++++++--------- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/ckb-daemon/usb.c b/src/ckb-daemon/usb.c index 73c62886b7..30df2b95e0 100644 --- a/src/ckb-daemon/usb.c +++ b/src/ckb-daemon/usb.c @@ -205,7 +205,7 @@ int _usbsend(usbdevice* kb, const uchar* messages, int count, const char* file, } int _usbrecv(usbdevice* kb, const uchar* out_msg, uchar* in_msg, const char* file, int line){ - // Try a maximum of 3 times + // Try a maximum of 5 times for(int try = 0; try < 5; try++){ // Send the output message DELAY_SHORT(kb); diff --git a/src/ckb-daemon/usb.h b/src/ckb-daemon/usb.h index 288a3c1c53..6dc0e80d64 100644 --- a/src/ckb-daemon/usb.h +++ b/src/ckb-daemon/usb.h @@ -84,7 +84,7 @@ const char* product_str(short product); #define IS_MOUSE_DEV(kb) IS_MOUSE((kb)->vendor, (kb)->product) // USB delays for when the keyboards get picky about timing -#define DELAY_SHORT(kb) usleep((int)(kb)->usbdelay * 1000) // base (default: 5ms) +#define DELAY_SHORT(kb) usleep((int)(kb)->usbdelay * 2000) // base (default: 10ms for testing mode-switch of RGB keyboards) #define DELAY_MEDIUM(kb) usleep((int)(kb)->usbdelay * 10000) // x10 (default: 50ms) #define DELAY_LONG(kb) usleep(100000) // long, fixed 100ms #define USB_DELAY_DEFAULT 5 diff --git a/src/ckb-daemon/usb_linux.c b/src/ckb-daemon/usb_linux.c index d5b812d587..37e01a5033 100644 --- a/src/ckb-daemon/usb_linux.c +++ b/src/ckb-daemon/usb_linux.c @@ -6,6 +6,8 @@ #ifdef OS_LINUX +#define DEBUG + static char kbsyspath[DEV_MAX][FILENAME_MAX]; int os_usbsend(usbdevice* kb, const uchar* out_msg, int is_recv, const char* file, int line){ @@ -95,7 +97,7 @@ void* os_inputmain(void* context){ // For RGB, monitor all but the last, as it's used for input/output int urbcount = IS_RGB(vendor, product) ? (kb->epcount - 1) : kb->epcount; if (urbcount == 0) { - ckb_warn("urbcount = 0, so there is nothing to claim in os_inputmain()\n"); + ckb_err("urbcount = 0, so there is nothing to claim in os_inputmain()\n"); return 0; } struct usbdevfs_urb urbs[urbcount]; @@ -223,7 +225,9 @@ void os_closeusb(usbdevice* kb){ int usbclaim(usbdevice* kb){ int count = kb->epcount; +#ifdef DEBUG ckb_info("claiming %d endpoints\n", count); +#endif // DEBUG for(int i = 0; i < count; i++){ struct usbdevfs_ioctl ctl = { i, USBDEVFS_DISCONNECT, 0 }; @@ -288,22 +292,23 @@ int os_setupusb(usbdevice* kb){ else kb->fwversion = 0; int index = INDEX_OF(kb, keyboard); - ckb_info("Connecting %s at %s%d\n", kb->name, devpath, index); - // Claim the USB interfaces const char* ep_str = udev_device_get_sysattr_value(dev, "bNumInterfaces"); +#ifdef DEBUG + ckb_info("Connecting %s at %s%d\n", kb->name, devpath, index); ckb_info("claiming interfaces. name=%s, serial=%s, firmware=%s; Got >>%s<< as ep_str\n", name, serial, firmware, ep_str); +#endif //DEBUG kb->epcount = 0; if(ep_str) sscanf(ep_str, "%d", &kb->epcount); if(kb->epcount < 2){ // IF we have an RGB KB with 0 or 1 endpoints, it will be in BIOS mode. - ckb_warn("Possible unable to read endpoint count from udev, assuming %d and reading >>%s<<...\n", kb->epcount, ep_str); + ckb_err("Possibly unable to read endpoint count from udev, assuming %d and reading >>%s<<...\n", kb->epcount, ep_str); return -1; - // ToDo lae. are there special versions we have to detect? - // This shouldn't happen, but if it does, assume EP count based on what the device is supposed to have - kb->epcount = (HAS_FEATURES(kb, FEAT_RGB) ? 4 : 3); - ckb_warn("Unable to read endpoint count from udev, assuming %d and reading >>%s<<...\n", kb->epcount, ep_str); + // ToDo are there special versions we have to detect? If there are, that was the old code to handle it: + // This shouldn't happen, but if it does, assume EP count based onckb_warn what the device is supposed to have + // kb->epcount = (HAS_FEATURES(kb, FEAT_RGB) ? 4 : 3); + // ckb_warn("Unable to read endpoint count from udev, assuming %d and reading >>%s<<...\n", kb->epcount, ep_str); } if(usbclaim(kb)){ ckb_err("Failed to claim interfaces: %s\n", strerror(errno)); @@ -319,8 +324,9 @@ int usbadd(struct udev_device* dev, short vendor, short product){ ckb_err("Failed to get device path\n"); return -1; } - //lae. +#ifdef DEBUG ckb_info(">>>vendor = 0x%x, product = 0x%x, path = %s, syspath = %s\n", vendor, product, path, syspath); +#endif // DEDBUG // Find a free USB slot for(int index = 1; index < DEV_MAX; index++){ usbdevice* kb = keyboard + index;