Re: [PATCH 0/2] hid: corsair: Driver simplification and new supported device

[Jiri Kosina]

On Thu, 24 Mar 2016, Clément VUCHENER wrote:

> 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

Re: [PATCH 0/2] hid: corsair: Driver simplification and new supported device

[Jiri Kosina]

On Thu, 24 Mar 2016, Clément VUCHENER wrote:

> 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

Re: [PATCH 0/2] hid: corsair: Driver simplification and new supported device

[Clément VUCHENER]

2016-03-24 15:30 GMT+01:00 Jiri Kosina :
> On Wed, 23 Mar 2016, =?UTF-8?q?Cl=C3=A9ment=20Vuchener?= wrote:
>
>> 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.
>
> 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 for the tip, is it possible to ignore some usages with this
method (done in the default case)?

>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

Re: [PATCH 0/2] hid: corsair: Driver simplification and new supported device

[Clément VUCHENER]

2016-03-24 15:30 GMT+01:00 Jiri Kosina :
> On Wed, 23 Mar 2016, =?UTF-8?q?Cl=C3=A9ment=20Vuchener?= wrote:
>
>> 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.
>
> 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 for the tip, is it possible to ignore some usages with this
method (done in the default case)?

>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

Re: [PATCH 0/2] hid: corsair: Driver simplification and new supported device

[Jiri Kosina]

On Thu, 24 Mar 2016, Benjamin Tissoires wrote:

> 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.

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

Re: [PATCH 0/2] hid: corsair: Driver simplification and new supported device

[Jiri Kosina]

On Thu, 24 Mar 2016, Benjamin Tissoires wrote:

> 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.

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

Re: [PATCH 0/2] hid: corsair: Driver simplification and new supported device

[Benjamin Tissoires]

Hi Clément,

On Mar 23 2016 or thereabouts, =?UTF-8?q?Cl=C3=A9ment=20Vuchener?= wrote:
> 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.

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

> 
> 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
> 

Re: [PATCH 0/2] hid: corsair: Driver simplification and new supported device

[Benjamin Tissoires]

Hi Clément,

On Mar 23 2016 or thereabouts, =?UTF-8?q?Cl=C3=A9ment=20Vuchener?= wrote:
> 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.

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

> 
> 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
> 

Re: [PATCH 0/2] hid: corsair: Driver simplification and new supported device

[Jiri Kosina]

On Wed, 23 Mar 2016, =?UTF-8?q?Cl=C3=A9ment=20Vuchener?= wrote:

> 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.

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

Re: [PATCH 0/2] hid: corsair: Driver simplification and new supported device

[Jiri Kosina]

On Wed, 23 Mar 2016, =?UTF-8?q?Cl=C3=A9ment=20Vuchener?= wrote:

> 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.

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

[PATCH 0/2] hid: corsair: Driver simplification and new supported device

[=?UTF-8?q?Cl=C3=A9ment=20Vuchener?=]

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

[PATCH 0/2] hid: corsair: Driver simplification and new supported device

[=?UTF-8?q?Cl=C3=A9ment=20Vuchener?=]

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