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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160324143750.GA7602@mail.corp.redhat.com>
Date:	Thu, 24 Mar 2016 15:37:51 +0100
From:	Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:	=?UTF-8?q?Cl=C3=A9ment=20Vuchener?= 
	<clement.vuchener@...il.com>
Cc:	Jiri Kosina <jikos@...nel.org>, linux-kernel@...r.kernel.org,
	linux-input@...r.kernel.org
Subject: Re: [PATCH 0/2] hid: corsair: Driver simplification and new
 supported device

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
>