Re: [PATCH v2] 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:
- [High] hwmon: (acer-aspire14-ec) module_init() fails when driver is built-in
--

commit 7f4ea74f303f12662cc63822a17eaf3d1e0e84bf
Author: Ethan Jimmy <duhxu0@gmail.com>

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

This commit adds a hwmon driver for the Acer Aspire 14 AI (A14-52M and
A14-52MT) fan sensor, which is exposed via an I2C HID embedded controller.

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

[ ... ]

> +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);

[Severity: High]
Does acer_aspire14_init() fail when CONFIG_SENSORS_ACER_ASPIRE14_EC=y and
CONFIG_HID=y are built into the kernel?

In drivers/Makefile, drivers/hwmon/ is linked before drivers/hid/:

  obj-$(CONFIG_HWMON)		+= hwmon/
  ...
  obj-$(CONFIG_HID_SUPPORT)	+= hid/

Because both acer_aspire14_init() and hid_init() in drivers/hid/hid-core.c
use module_init() (which expands to device_initcall() when built-in),
acer_aspire14_init() runs before hid_init() registers hid_bus_type.

On matching DMI systems, the registration call chain:

  acer_aspire14_init()
    hid_register_driver(&acer_aspire14_driver)
      __hid_register_driver()
        driver_register()

reaches driver_register() in drivers/base/driver.c before the HID bus is
initialized:

	if (!bus_is_registered(drv->bus)) {
		pr_err("Driver '%s' was unable to register with bus_type '%s' because the bus was not initialized.\n",
			   drv->name, drv->bus->name);
		return -EINVAL;
	}

Should this use late_initcall(acer_aspire14_init) like the other HID drivers
in drivers/hwmon/ (such as corsair-psu.c, corsair-cpro.c, and nzxt-kraken2.c)
so that driver registration happens after hid_init()?

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