Re: [PATCH v9 1/4] HID: hid-msi: Add MSI Claw configuration driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Mixing goto-based error handling with scope-based cleanups (__free, guard) in claw_hw_output_report.
--

commit 279964e3a8cf0a3835903d6bad7357f0ca42f523
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-msi: Add MSI Claw configuration driver

This commit adds a configuration HID driver for the MSI Claw series of
handheld PCs. It introduces the initial driver outline and attributes for
changing gamepad mode, M-key behavior, and resetting the device. It also
adds a completion bypass when the timeout is 0 to handle USB disconnects
gracefully.

> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> new file mode 100644
> index 0000000000000..3a176f44f2c43
> --- /dev/null
> +++ b/drivers/hid/hid-msi.c

[ ... ]

> +static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
> +				 size_t len, unsigned int timeout)
> +{
> +	unsigned char *dmabuf __free(kfree) = NULL;

[ ... ]

> +	guard(mutex)(&drvdata->cfg_mutex);

[ ... ]

> +	ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
> +	if (ret < 0)
> +		goto err_clear_state;

[Severity: Medium]
Is it intended to mix scope-based cleanups with goto statements in this
function? The subsystem guidelines recommend either converting all resources
that need a goto cleanup to scope-based cleanups, or converting none of them.
Mixing __free() and guard() declarations with goto-based error handling in the
same function can create confusing ownership semantics.

> +
> +	ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
> +	if (ret)
> +		goto err_clear_state;
> +
> +	if (timeout) {
> +		ret = wait_for_completion_interruptible_timeout(&drvdata->send_cmd_complete,
> +								msecs_to_jiffies(timeout));
> +
> +		dev_dbg(&hdev->dev, "Remaining timeout: %u\n", ret);
> +		ret = ret > 0 ? drvdata->cmd_status : ret ?: -EBUSY;
> +	}
> +
> +err_clear_state:
> +	if (timeout) {
> +		guard(spinlock_irqsave)(&drvdata->cmd_lock);
> +		drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
> +		drvdata->waiting_for_ack = false;
> +	}
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525020543.519082-1-derekjohn.clark@gmail.com?part=1