Re: [PATCH v7 RESEND] hwmon: add driver for ARCTIC Fan Controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] A lock inversion exists between the HID core's `driver_input_lock` and the driver's wait for `hwmon_device_unregister()`, causing a 1-second delay/deadlock if the device is removed while a sysfs write is pending.
--

commit 31fcf91aa0e4a85fe147c543e3c868ca27cd3001
Author: Aureo Serrano de Souza <aureo.serrano@arctic.de>

hwmon: add driver for ARCTIC Fan Controller

Adds a hwmon driver for the ARCTIC Fan Controller USB HID device. The
driver provides fan speed monitoring and PWM control via sysfs, using
out reports for configuration and waiting up to 1 second for ack reports.

> diff --git a/drivers/hwmon/arctic_fan_controller.c b/drivers/hwmon/arctic_fan_controller.c
> new file mode 100644
> index 0000000000000..dbe84cd93c083
> --- /dev/null
> +++ b/drivers/hwmon/arctic_fan_controller.c

[ ... ]

> +static void arctic_fan_remove(struct hid_device *hdev)
> +{
> +	struct arctic_fan_data *priv = hid_get_drvdata(hdev);
> +
> +	/*
> +	 * Unregister hwmon before stopping the HID transport. This removes
> +	 * the sysfs files and waits for any in-progress write() callback to
> +	 * return, so no hwmon op can call hid_hw_output_report() after
> +	 * hid_hw_stop() frees the underlying USB resources.
> +	 * Matches the pattern used by nzxt-smart2 and aquacomputer_d5next.
> +	 *
> +	 * The HID core clears hdev->io_started before invoking ->remove(),
> +	 * so hid_device_io_stop() is not called here; doing so would emit
> +	 * a spurious "io already stopped" warning.
> +	 */
> +	hwmon_device_unregister(priv->hwmon_dev);

Does calling hwmon_device_unregister() here cause a 1-second delay if the
device is removed while a sysfs write is pending?

When the device is unplugged, the hid core acquires hdev->driver_input_lock
and then calls this remove callback.

If a concurrent arctic_fan_write() is blocked on
wait_for_completion_timeout() waiting for an ack report,
hwmon_device_unregister() will block waiting for that sysfs operation to
finish.

Because driver_input_lock is already held by the remove thread, incoming
reports cannot be processed. The __hid_input_report() function will fail
its down_trylock() and return -EBUSY, dropping the ack report.

Will this cause the sysfs write to unconditionally stall for the full
1-second timeout during device removal?

> +	hid_hw_close(hdev);
> +	hid_hw_stop(hdev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508064405.38676-1-aureo.serrano@arctic.de?part=1