linux-kernel - Re: [PATCH] HID: corsair-void: Add Corsair Void headset family driver

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: <da4ca647-6eef-4576-9882-c5ffeddd1f7b@web.de>
Date: Thu, 15 Aug 2024 09:50:51 +0200
From: Markus Elfring <Markus.Elfring@....de>
To: Stuart Hayhurst <stuart.a.hayhurst@...il.com>, linux-input@...r.kernel.org
Cc: LKML <linux-kernel@...r.kernel.org>,
 Benjamin Tissoires <bentiss@...nel.org>, Jiri Kosina <jikos@...nel.org>
Subject: Re: [PATCH] HID: corsair-void: Add Corsair Void headset family driver

…
> Should I leave it like this, or deregister the attributes, so those cases are never triggered?
>
> I'm also not sure about the block of comments at the start describing the (guessed) format of the
> packets, let me know if this should go somewhere else.
>
> Apologies if I've messed anything up, I've only sent fairly small patches until now :)

Such information should not belong to the change description.
It may be specified behind the marker line.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.11-rc3#n711


…
> +++ b/drivers/hid/hid-corsair-void.c
> @@ -0,0 +1,851 @@
…
> +static void corsair_void_process_receiver(struct corsair_void_drvdata *drvdata,
> +					  int raw_battery_capacity,
> +					  int raw_connection_status,
> +					  int raw_battery_status)
> +{
…
> +	/* Set battery status */
> +	switch (raw_battery_status) {
> +	case 1:
> +	case 2:
> +	case 3: /* Battery normal / low / critical */
…

Will any enumeration values become more helpful here?


…
> +static int corsair_void_request_status(struct hid_device *hid_dev, int id)
> +{
> +	unsigned char *send_buf;
> +	int ret;
> +
> +	send_buf = kzalloc(12, GFP_KERNEL);
…

Please improve the size determination (or explanation).


…
> +static int corsair_void_probe(struct hid_device *hid_dev,
> +			      const struct hid_device_id *hid_id)
> +{
…
> +/*failed_after_hid_start:
> +	hid_hw_stop(hid_dev);*/
> +failed_after_sysfs:
…

I guess that you would not like to preserve code which was commented out.

Regards,
Markus