Re: [PATCH] hwmon: (acer-aspire14-ec) Add driver for Acer Aspire 14 52MT and 52M fan

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hwmon: architectural violation of HID layering rules
--

commit fa48d917bd5280e6dd85fd709932750623717a15
Author: Ethan Jimmy <duhxu0@gmail.com>
Subject: hwmon: (acer-aspire14-ec) Add driver for Acer Aspire 14 52MT and 52M fan

This commit introduces a new hardware monitoring driver for the Acer Aspire 14
AI series (A14-52M and A14-52MT). It reads the fan speed via an I2C HID
embedded controller that also serves as the system keyboard.

diff --git a/drivers/hwmon/acer-aspire14-ec.c b/drivers/hwmon/acer-aspire14-ec.c
new file mode 100644
index 0000000000000..9d100f2a0b807
--- /dev/null
+++ b/drivers/hwmon/acer-aspire14-ec.c

[ ... ]

> +static const struct hid_device_id acer_aspire14_id_table[] = {
> +	{ HID_I2C_DEVICE(ACER_ASPIRE14_VID, ACER_ASPIRE14_PID) },
> +	{ }
> +};
> +
> +static struct hid_driver acer_aspire14_driver = {
> +	.name = "acer-aspire14-ec",
> +	.id_table = acer_aspire14_id_table,
> +	.probe = acer_aspire14_probe,

[Severity: Medium]
Does this driver architecture bypass the HID subsystem layering?

The hardware monitoring subsystem guidelines state that if the main
functionality of a chip is not hardware monitoring (such as a system
keyboard EC), its primary driver should reside in its respective subsystem
directory (e.g., drivers/hid/). The hardware monitoring functionality should
then be implemented as an auxiliary device driver residing in drivers/hwmon/.

By defining and registering a complete hid_driver directly within the hwmon
directory, this bypasses the HID subsystem maintainer review for the transport
layer integration. Could the primary hid_driver be placed in drivers/hid/ to
spawn an auxiliary_device, allowing an auxiliary_driver in drivers/hwmon/ to
bind to the fan monitoring feature?

> +	.remove = acer_aspire14_remove,
> +	.reset_resume = acer_aspire14_reset_resume,
> +};
> +
> +static int __init acer_aspire14_init(void)

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