Re: [PATCH v13 4/4] HID: hid-msi: Add Rumble Intensity Attributes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The function `__claw_hw_output_report` violates the cleanup subsystem rules by mixing `goto`-based error handling with scope-based cleanup (`__free(kfree)`).
--

--- Patch [4]: [PATCH v13 4/4] HID: hid-msi: Add Rumble Intensity Attributes ---
commit 9f513aab0b37d6357132f5ca7a76ad690510bbc7
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-msi: Add Rumble Intensity Attributes

This commit adds intensity adjustment attributes for the left and right rumble
motors on the MSI Claw device. It introduces sysfs store and show functions
to read and write rumble profile data to the hardware.

Link: https://patch.msgid.link/20260529072111.7565-5-derekjohn.clark@gmail.com

> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 5cb85fc2f0534..3225d3bf61254 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -991,6 +1023,162 @@ static ssize_t button_mapping_options_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(button_mapping_options);
>  
> +static ssize_t rumble_intensity_left_store(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t count)
> +{
[ ... ]
> +	ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
> +				    (u8 *)&report, sizeof(report), 25);
> +	if (ret)
> +		return ret;

[Severity: Low]
Does __claw_hw_output_report() mix goto-based error handling with scope-based
cleanup?

Looking at __claw_hw_output_report(), which is called via
claw_hw_output_report() here:

	unsigned char *dmabuf __free(kfree) = NULL;
...
	ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
	if (ret < 0)
		goto err;
...
err:
	scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
		drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
		drvdata->orphan_ack_pending = false;
	}
	return ret;

The cleanup subsystem guidelines in include/linux/cleanup.h state that the
expectation is to never mix "goto" and cleanup helpers in the same function.

Is it possible to convert the error path to use scope-based cleanup entirely?

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