Re: [PATCH 2/5] HID: UNCTRL: add basic battery reporting for UNCTRL RAGE Mouse

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] hid: unctrl: initial battery query response may be dropped in probe
--

commit 6b35740bb80be8312a87135a9844c03a48dc1cca
Author: Bishwa Bhusan Saha <contact@bishwasaha.com>

HID: UNCTRL: add basic battery reporting for UNCTRL RAGE Mouse

This commit adds basic battery reporting for the UNCTRL RAGE Wireless Mouse
over both 2.4 GHz and wired connections. It queries the battery once during
device probe by sending an output report to the hardware.

> diff --git a/drivers/hid/hid-unctrl-m1.c b/drivers/hid/hid-unctrl-m1.c
> new file mode 100644
> index 0000000000000..59259dee7e429
> --- /dev/null
> +++ b/drivers/hid/hid-unctrl-m1.c
[ ... ]
> +static void unctrl_fetch_battery(struct hid_device *hdev)
> +{
> +	u8 *write_buf;
> +	int ret;
> +
> +	/* buf[0] is the report number, and hid_hw_output_report() sends the
> +	 * buffer verbatim.
> +	 */
> +	write_buf = kmemdup(unctrl_battery_query, sizeof(unctrl_battery_query),
> +			    GFP_KERNEL);
> +	if (!write_buf)
> +		return;
> +
> +	ret = hid_hw_output_report(hdev, write_buf,
> +				   sizeof(unctrl_battery_query));
[ ... ]
> +static int unctrl_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
[ ... ]
> +	/* usbhid polls the interrupt in endpoint only while it is open */
> +	ret = hid_hw_open(hdev);
> +	if (ret) {
> +		hid_err(hdev, "%s:open failed with error %d\n", __func__, ret);
> +		hid_hw_stop(hdev);
> +		kfree(dev);
> +		return ret;
> +	}
> +
> +	hid_set_drvdata(hdev, dev);
> +
> +	ret = unctrl_battery_register(dev);

[Severity: Low]
Since hid_device_io_start() has not been called in unctrl_probe(), will the
HID core's driver_input_lock block the incoming battery report reply?

If the device responds to the output report sent by unctrl_fetch_battery()
before the probe function returns, it appears __hid_input_report() might drop
the reply because the IO path is still gated.

Does this mean the initial battery level will remain incorrectly reported as
100% until a subsequent poll occurs?

> +	if (ret)
> +		goto err_del;
> +
> +	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-b4-unctrl-m1-battery-v1-0-4a50203b7177@bishwasaha.com?part=2