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

[Guenter Roeck <linux@roeck-us.net>]

On Fri, Oct 09, 2026 at 09:13:13PM -0700, Ethan Jimmy wrote:
> The Aspire A14-52M and A14-52MT expose fan speed through an I2C HID
> embedded controller that also serves the keyboard. The same product id
> is used by other Acer keyboards, so the driver binds only when DMI
> matches these two models.
> 
> Signed-off-by: Ethan Jimmy <duhxu0@gmail.com>
> ---
>  Documentation/hwmon/acer-aspire14-ec.rst |  38 ++++
>  Documentation/hwmon/index.rst            |   1 +
>  MAINTAINERS                              |   8 +
>  drivers/hwmon/Kconfig                    |  20 ++
>  drivers/hwmon/Makefile                   |   1 +
>  drivers/hwmon/acer-aspire14-ec.c         | 270 +++++++++++++++++++++++
>  6 files changed, 338 insertions(+)
>  create mode 100644 Documentation/hwmon/acer-aspire14-ec.rst
>  create mode 100644 drivers/hwmon/acer-aspire14-ec.c
> 
> diff --git a/Documentation/hwmon/acer-aspire14-ec.rst b/Documentation/hwmon/acer-aspire14-ec.rst
> new file mode 100644
> index 000000000000..73acfb181c39
> --- /dev/null
> +++ b/Documentation/hwmon/acer-aspire14-ec.rst
> @@ -0,0 +1,38 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +Kernel driver acer-aspire14-ec
> +==============================
> +
> +Supported chips:
> +
> +  * Acer Aspire 14 AI 52MT/52M embedded controller
> +
> +    Prefix: 'aspire14_ec'
> +
> +    Addresses scanned: none
> +
> +    Datasheet: unavailable
> +
> +Author: Ethan Jimmy <duhxu0@gmail.com>
> +
> +Description
> +-----------
> +
> +This driver monitors the fan on the Acer Aspire 14 AI (Aspire A14-52M
> +and A14-52MT). The embedded controller is an I2C HID device (vendor
> +0x1025, product 0x174b) that also provides the keyboard.
> +
> +The device is autodetected on those models. The same product id is used
> +by other Acer keyboards; those machines stay with hid-generic. On the
> +Aspire A14-52M and A14-52MT this driver provides both the keyboard and
> +the fan sensor.
> +
> +Fan speed is reported in RPM through the standard hwmon attribute
> +``fan1_input``. The driver does not change fan speed.
> +
> +Sysfs entries
> +-------------
> +
> +======================= ======================================================
> +fan1_input		Fan speed in RPM
> +======================= ======================================================
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 199ca79621e9..9dd16e6ddabd 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -22,6 +22,7 @@ Hardware Monitoring Kernel Drivers
>     abituguru
>     abituguru3
>     acbel-fsg032
> +   acer-aspire14-ec
>     acpi_power_meter
>     ad7314
>     adc128d818
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5ad12c57ca5f..b944f77c7a71 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -262,6 +262,14 @@ L:	linux-gpio@vger.kernel.org
>  S:	Maintained
>  F:	drivers/gpio/gpio-pcie-idio-24.c
>  
> +ACER ASPIRE 14 AI (A14-52M, A14-52MT) EC FAN DRIVER
> +M:	Ethan Jimmy <duhxu0@gmail.com>
> +L:	linux-hwmon@vger.kernel.org
> +L:	linux-input@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/hwmon/acer-aspire14-ec.rst
> +F:	drivers/hwmon/acer-aspire14-ec.c
> +
>  ACER ASPIRE ONE TEMPERATURE AND FAN DRIVER
>  M:	Peter Kaestle <peter@piie.net>
>  L:	platform-driver-x86@vger.kernel.org
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 18c682e1a475..84e9e9c115a1 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -67,6 +67,26 @@ config SENSORS_ABITUGURU3
>  	  This driver can also be built as a module. If so, the module
>  	  will be called abituguru3.
>  
> +config SENSORS_ACER_ASPIRE14_EC
> +	tristate "Acer Aspire 14 AI EC fan sensor"
> +	depends on HID
> +	depends on X86 || COMPILE_TEST
> +	depends on DMI || COMPILE_TEST
> +	depends on I2C_HID_ACPI || I2C_HID_OF || COMPILE_TEST
> +	help
> +	  If you say yes here you get support for reading the fan
> +	  speed from the embedded controller on the Acer Aspire 14 AI
> +	  (Aspire A14-52M and A14-52MT). The controller is an I2C HID
> +	  device (vendor 0x1025, product 0x174b) that also provides the
> +	  keyboard. Fan speed is read-only.
> +
> +	  The same product id is used by other Acer keyboards. This driver
> +	  registers only on the Aspire A14-52M and A14-52MT, where it also
> +	  provides the keyboard. Other machines stay with hid-generic.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called acer-aspire14-ec.
> +
>  config SENSORS_SMPRO
>  	tristate "Ampere's Altra SMpro hardware monitoring driver"
>  	depends on MFD_SMPRO
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index d4c345ed7261..ac5cec462c81 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_SENSORS_W83791D)	+= w83791d.o
>  
>  obj-$(CONFIG_SENSORS_ABITUGURU)	+= abituguru.o
>  obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
> +obj-$(CONFIG_SENSORS_ACER_ASPIRE14_EC) += acer-aspire14-ec.o
>  obj-$(CONFIG_SENSORS_AD7314)	+= ad7314.o
>  obj-$(CONFIG_SENSORS_AD7414)	+= ad7414.o
>  obj-$(CONFIG_SENSORS_AD7418)	+= ad7418.o
> diff --git a/drivers/hwmon/acer-aspire14-ec.c b/drivers/hwmon/acer-aspire14-ec.c
> new file mode 100644
> index 000000000000..9d100f2a0b80
> --- /dev/null
> +++ b/drivers/hwmon/acer-aspire14-ec.c
> @@ -0,0 +1,270 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Hwmon driver for the Acer Aspire 14 AI embedded controller.
> + *
> + * Author: Ethan Jimmy <duhxu0@gmail.com>
> + *
> + * On the Aspire A14-52M and A14-52MT the EC is an I2C HID device
> + * (vendor 0x1025, product 0x174b) that also serves the keyboard. The same
> + * product id is used by other Acer keyboards. The HID driver is registered
> + * only when DMI matches these models, so those machines stay with hid-generic.

Doees this impact the actual keyboard functionality ?
> + * 
> + * Fan speed is read with one feature report: A0 00 A0 08 00 02 02. The
> + * reply carries a little-endian status word, command id, and RPM.
> + */
> +
> +#include <linux/dmi.h>
> +#include <linux/hid.h>
> +#include <linux/hwmon.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/string.h>
> +#include <linux/unaligned.h>
> +
> +#define ACER_ASPIRE14_VID		0x1025
> +#define ACER_ASPIRE14_PID		0x174b
> +#define ACER_ASPIRE14_REPORT_ID		0xa0
> +#define ACER_ASPIRE14_PREFIX		0xa0
> +#define ACER_ASPIRE14_CMD_MARK		0x02
> +#define ACER_ASPIRE14_RPM_OFFSET	8
> +#define ACER_ASPIRE14_MIN_REPLY		(ACER_ASPIRE14_RPM_OFFSET + sizeof(u16))
> +#define ACER_ASPIRE14_STATUS_OK		0xe000
> +#define ACER_ASPIRE14_CMD_FAN_RPM	0x0008
> +
> +struct acer_aspire14_data {
> +	struct hid_device *hdev;
> +	struct device *hwmon_dev;
> +	struct mutex lock; /* protects buf across feature report I/O */

There is no asynchronous call, meaning the lock is unnecessary
since requests are already serialised by the hwmon subsystem.

> +	u8 *buf;
> +	u32 report_len;
> +};
> +
> +static int acer_aspire14_fan_rpm(struct acer_aspire14_data *data, long *val)
> +{
> +	u16 status, command_id;
> +	int ret;
> +
> +	mutex_lock(&data->lock);
> +
> +	memset(data->buf, 0, data->report_len);
> +	data->buf[0] = ACER_ASPIRE14_REPORT_ID;
> +	data->buf[2] = ACER_ASPIRE14_PREFIX;
> +	put_unaligned_le16(ACER_ASPIRE14_CMD_FAN_RPM, &data->buf[3]);
> +	data->buf[5] = ACER_ASPIRE14_CMD_MARK;
> +	data->buf[6] = ACER_ASPIRE14_CMD_MARK;
> +
> +	ret = hid_hw_raw_request(data->hdev, ACER_ASPIRE14_REPORT_ID,
> +				 data->buf, data->report_len,
> +				 HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> +	if (ret < 0)
> +		goto unlock;
> +	if (!ret) {

Shouldn't that check for a specific length, such as data->report_len ?

> +		ret = -EIO;
> +		goto unlock;
> +	}
> +
> +	memset(data->buf, 0, data->report_len);
> +	data->buf[0] = ACER_ASPIRE14_REPORT_ID;
> +	ret = hid_hw_raw_request(data->hdev, ACER_ASPIRE14_REPORT_ID,
> +				 data->buf, data->report_len,
> +				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> +	if (ret < 0)
> +		goto unlock;
> +	if (ret < ACER_ASPIRE14_MIN_REPLY) {
> +		ret = -EIO;
> +		goto unlock;
> +	}
> +
> +	status = get_unaligned_le16(&data->buf[1]);
> +	command_id = get_unaligned_le16(&data->buf[3]);
> +	if (status != ACER_ASPIRE14_STATUS_OK ||
> +	    command_id != ACER_ASPIRE14_CMD_FAN_RPM) {
> +		ret = -EIO;
> +		goto unlock;
> +	}
> +
> +	*val = get_unaligned_le16(&data->buf[ACER_ASPIRE14_RPM_OFFSET]);
> +	ret = 0;
> +
> +unlock:
> +	mutex_unlock(&data->lock);
> +	return ret;
> +}
> +
> +static int acer_aspire14_read(struct device *dev, enum hwmon_sensor_types type,
> +			      u32 attr, int channel, long *val)
> +{
> +	struct acer_aspire14_data *data = dev_get_drvdata(dev);
> +
> +	if (type != hwmon_fan || attr != hwmon_fan_input || channel != 0)
> +		return -EOPNOTSUPP;

Unnecessary check.

> +
> +	return acer_aspire14_fan_rpm(data, val);
> +}
> +
> +static umode_t acer_aspire14_is_visible(const void *drvdata,
> +					enum hwmon_sensor_types type,
> +					u32 attr, int channel)
> +{
> +	if (type == hwmon_fan && attr == hwmon_fan_input && channel == 0)
> +		return 0444;
> +
Unnecessary check. Just return 0444.

> +	return 0;
> +}
> +
> +static const struct hwmon_channel_info *const acer_aspire14_info[] = {
> +	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
> +	NULL
> +};
> +
> +static const struct hwmon_ops acer_aspire14_ops = {
> +	.is_visible = acer_aspire14_is_visible,
> +	.read = acer_aspire14_read,
> +};
> +
> +static const struct hwmon_chip_info acer_aspire14_chip_info = {
> +	.ops = &acer_aspire14_ops,
> +	.info = acer_aspire14_info,
> +};
> +
> +static const struct dmi_system_id acer_aspire14_dmi_table[] = {
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Aspire A14-52M"),
> +		},
> +	},
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Aspire A14-52MT"),
> +		},
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(dmi, acer_aspire14_dmi_table);
> +
> +static u32 acer_aspire14_fan_report_len(struct hid_device *hdev)
> +{
> +	struct hid_report *report;
> +	u32 len;
> +
> +	report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[ACER_ASPIRE14_REPORT_ID];
> +	if (!report)
> +		return 0;
> +
> +	len = hid_report_len(report);
> +	if (len < ACER_ASPIRE14_MIN_REPLY || len > HID_MAX_BUFFER_SIZE)
> +		return 0;
> +
> +	return len;
> +}
> +
> +static int acer_aspire14_probe(struct hid_device *hdev,
> +			       const struct hid_device_id *id)
> +{
> +	struct acer_aspire14_data *data;
> +	struct device *hwmon_dev;
> +	long rpm;
> +	u32 report_len;
> +	int ret;
> +
> +	hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
> +
> +	ret = hid_parse(hdev);
> +	if (ret)
> +		return ret;
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT | HID_CONNECT_DRIVER);
> +	if (ret)
> +		return ret;
> +
> +	report_len = acer_aspire14_fan_report_len(hdev);
> +	if (!report_len)
> +		return 0;
> +
> +	data = devm_kzalloc(&hdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return 0;
> +
> +	data->buf = devm_kzalloc(&hdev->dev, report_len, GFP_KERNEL);
> +	if (!data->buf)
> +		return 0;
> +	data->report_len = report_len;
> +
> +	ret = devm_mutex_init(&hdev->dev, &data->lock);
> +	if (ret) {
> +		hid_warn(hdev, "failed to init mutex: %d\n", ret);
> +		return 0;
> +	}
> +
> +	data->hdev = hdev;
> +	hid_set_drvdata(hdev, data);
> +
> +	ret = acer_aspire14_fan_rpm(data, &rpm);
> +	if (ret)
> +		hid_dbg(hdev, "fan report failed: %d\n", ret);
> +	else
> +		hid_dbg(hdev, "initial fan speed %ld RPM\n", rpm);
> +
> +	hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "aspire14_ec",
> +						    data, &acer_aspire14_chip_info,
> +						    NULL);
> +	if (IS_ERR(hwmon_dev)) {
> +		hid_warn(hdev, "failed to register hwmon: %pe\n", hwmon_dev);
> +		return 0;

That is a problem. Something is wrong here: If the driver fails to instantiate
the hwmon device, it should return an error. Same for all other "return 0"
instances above.

> +	}
> +	data->hwmon_dev = hwmon_dev;
> +
> +	return 0;
> +}
> +
> +static void acer_aspire14_remove(struct hid_device *hdev)
> +{
> +	struct acer_aspire14_data *data = hid_get_drvdata(hdev);
> +
> +	if (data && data->hwmon_dev)
> +		hwmon_device_unregister(data->hwmon_dev);
> +	hid_hw_stop(hdev);
> +}
> +
> +static int acer_aspire14_reset_resume(struct hid_device *hdev)
> +{
> +	if (hdev->claimed & HID_CLAIMED_INPUT)
> +		hidinput_reset_resume(hdev);
> +
> +	return 0;
> +}
> +
> +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,
> +	.remove = acer_aspire14_remove,
> +	.reset_resume = acer_aspire14_reset_resume,
> +};
> +
> +static int __init acer_aspire14_init(void)
> +{
> +	if (!dmi_check_system(acer_aspire14_dmi_table))
> +		return -ENODEV;
> +
> +	return hid_register_driver(&acer_aspire14_driver);
> +}
> +
> +static void __exit acer_aspire14_exit(void)
> +{
> +	hid_unregister_driver(&acer_aspire14_driver);
> +}
> +
> +module_init(acer_aspire14_init);
> +module_exit(acer_aspire14_exit);
> +
> +MODULE_AUTHOR("Ethan Jimmy <duhxu0@gmail.com>");
> +MODULE_DESCRIPTION("Acer Aspire 14 AI EC fan driver");
> +MODULE_LICENSE("GPL");