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

From: Markus Elfring
Date: Thu Aug 15 2024 - 03:51:24 EST



> 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