hid: corsair: Driver simplification and new supported device

I tried to add support for the K40 some time ago, but the vendor specific USB protocol became over-complicated because of a lot of small differences between the K90 and the K40. Also, since I wrote the first version of this driver, I learned that USB control transfers could be done from user-space without the need to detach the kernel driver (please tell me if I am wrong).

So, I decided to move all USB related features in user-space (as far as I know, I was the only user, but if someone is looking for a replacement, I wrote a small tool available here: https://github.com/cvuchener/corsair-usb-config). This simplification only leaves the usage code remapping part and the driver no longer depends on USB and LED subsystems. This should make the driver easier to maintain or to add new supported devices.

After the removal of USB related functions in first patch, the addition of K40 support in the second patch is simply a matter of adding the device in the id list.

Clément Vuchener (2):
HID: corsair: Remove all features using the USB protocol
HID: corsair: Add K40 support

 Documentation/ABI/testing/sysfs-driver-hid-corsair |  15 -
 drivers/hid/Kconfig                                |   2 +-
 drivers/hid/hid-core.c                             |   1 +
 drivers/hid/hid-corsair.c                          | 498 +--------------------
 drivers/hid/hid-ids.h                              |   1 +
 5 files changed, 5 insertions(+), 512 deletions(-)
 delete mode 100644 Documentation/ABI/testing/sysfs-driver-hid-corsair

2.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Remove every use of USB control requests since it can be more easily done in user space. This removes the dependency on USB and LED subsystems. The simplyfied driver now only remaps Corsair usage codes.

---
 Documentation/ABI/testing/sysfs-driver-hid-corsair |  15 -
 drivers/hid/Kconfig                                |   2 +-
 drivers/hid/hid-corsair.c                          | 497 +--------------------
 3 files changed, 2 insertions(+), 512 deletions(-)
 delete mode 100644 Documentation/ABI/testing/sysfs-driver-hid-corsair

deleted file mode 100644
index b8827f0..0000000
@@ -1,15 +0,0 @@
-What:		/sys/bus/drivers/corsair/<dev>/macro_mode
-Date:		August 2015
-KernelVersion:	4.2
-Contact:	Clement Vuchener <[email protected]>
-Description:	Get/set the current playback mode. "SW" for software mode
-		where G-keys triggers their regular key codes. "HW" for
-		hardware playback mode where the G-keys play their macro
-		from the on-board memory.
-
-
-What:		/sys/bus/drivers/corsair/<dev>/current_profile
-Date:		August 2015
-KernelVersion:	4.2
-Contact:	Clement Vuchener <[email protected]>
-Description:	Get/set the current selected profile. Values are from 1 to 3.

index 4117225..43b018f 100644
@@ -173,7 +173,7 @@ config HID_CHICONY
 
 config HID_CORSAIR
 	tristate "Corsair devices"
-	depends on HID && USB && LEDS_CLASS
+	depends on HID
 	---help---
 	Support for Corsair devices that are not fully compliant with the
 	HID standard.

2.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

The Corsair K40 uses the same usage codes as the K90 for its special keys (although it has only 6 G-keys).

---
 drivers/hid/hid-core.c    | 1 +
 drivers/hid/hid-corsair.c | 1 +
 drivers/hid/hid-ids.h     | 1 +
 3 files changed, 3 insertions(+)

index bdb8cc8..73860b9 100644
@@ -1871,6 +1871,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_WIRELESS2) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_AK1D) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_ACER_SWITCH12) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K40) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K90) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL, USB_DEVICE_ID_CYGNAL_CP2112) },

index 98f40aa..85b5168 100644
@@ -151,6 +151,7 @@ static int corsair_input_mapping(struct hid_device *dev,
 }
 
 static const struct hid_device_id corsair_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K40) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K90) },
 	{}
 };

index 5c0e43e..ea9fef9 100644
@@ -256,6 +256,7 @@
 #define USB_DEVICE_ID_CODEMERCS_IOW_LAST	0x15ff
 
 #define USB_VENDOR_ID_CORSAIR		0x1b1c
+#define USB_DEVICE_ID_CORSAIR_K40	0x1b0e
 #define USB_DEVICE_ID_CORSAIR_K90	0x1b02
 
 #define USB_VENDOR_ID_CREATIVELABS	0x041e

2.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

While you are performing this move, is there anything that's actually preventing you from doing the remapping from userspace as well?

HID subsystem has for long time been providing the setkeycode() hook for remapping usages, and udev (well, more precisely, that s*****d thing) is actually shipping a lot of hw-specific remap data these days.

Thanks,

Jiri Kosina
SUSE Labs

2016-03-24 15:30 GMT+01:00 Jiri Kosina <[email protected]>:

Thanks for the tip, is it possible to ignore some usages with this method (done in the default case)?

mapping to 0xff is a bit hackish way to achieve this.

Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Clément,

I would say you can not do this this way. Even if you believe you are the only user of the API, there might be someone who uses it, and you will end up breaking his keyboard.

Jiri will correct me, but the proper way to follow is to mark the API as deprecated, make sure your driver uses the deprecated API only for the K40, and then add the K90 in the driver, without implementing the API.

After a few months (years?) with your API marked as deprecated, you then will be able to remove it. This is one of the many reasons we wrote libratbag in pure user-space, to avoid having to maintain complex API in the kernel forever.

Cheers,
Benjamin

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

You are right that this is the right way to deprecate the API.

Fortunately this one is "officially" marked as testing, so we might be a little bit more relaxed, but still we'd really need to take care not to break users left and right.

That's why I first asked whether also the remapping shouldn't be moved to userspace, to make sure that we eventuall start the depreciation of as many features as possible at the same time.

Thanks,

Jiri Kosina
SUSE Labs