Hi Kurt,
On Fri, Feb 14, 2025, at 7:03 PM, Kurt Borja wrote:
> Let the driver core manage the lifetime of the HWMON device, by
> registering it inside tpacpi_hwmon_pdriver's probe and using
> devm_hwmon_device_register_with_groups().
>
> Signed-off-by: Kurt Borja <kuurtb@xxxxxxxxx>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 44 +++++++++++-----------------
> 1 file changed, 17 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c
> index ad9de48cc122..a7e82157bd67 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -367,7 +367,6 @@ static struct {
> u32 beep_needs_two_args:1;
> u32 mixer_no_level_control:1;
> u32 battery_force_primary:1;
> - u32 sensors_pdrv_registered:1;
> u32 hotkey_poll_active:1;
> u32 has_adaptive_kbd:1;
> u32 kbd_lang:1;
> @@ -11815,12 +11814,10 @@ static void thinkpad_acpi_module_exit(void)
> {
> tpacpi_lifecycle = TPACPI_LIFE_EXITING;
>
> - if (tpacpi_hwmon)
> - hwmon_device_unregister(tpacpi_hwmon);
> - if (tp_features.sensors_pdrv_registered)
> + if (tpacpi_sensors_pdev) {
> platform_driver_unregister(&tpacpi_hwmon_pdriver);
> - if (tpacpi_sensors_pdev)
> platform_device_unregister(tpacpi_sensors_pdev);
> + }
>
> if (tpacpi_pdev) {
> platform_driver_unregister(&tpacpi_pdriver);
> @@ -11891,6 +11888,17 @@ static int __init tpacpi_pdriver_probe(struct
> platform_device *pdev)
> return ret;
> }
>
> +static int __init tpacpi_hwmon_pdriver_probe(struct platform_device *pdev)
> +{
> + tpacpi_hwmon = devm_hwmon_device_register_with_groups(
> + &tpacpi_sensors_pdev->dev, TPACPI_NAME, NULL, tpacpi_hwmon_groups);
> +
> + if (IS_ERR(tpacpi_hwmon))
> + pr_err("unable to register hwmon device\n");
> +
> + return PTR_ERR_OR_ZERO(tpacpi_hwmon);
> +}
> +
> static int __init thinkpad_acpi_module_init(void)
> {
> const struct dmi_system_id *dmi_id;
> @@ -11964,37 +11972,19 @@ static int __init thinkpad_acpi_module_init(void)
> return ret;
> }
>
> - tpacpi_sensors_pdev = platform_device_register_simple(
> - TPACPI_HWMON_DRVR_NAME,
> - PLATFORM_DEVID_NONE, NULL, 0);
> + tpacpi_sensors_pdev = platform_create_bundle(&tpacpi_hwmon_pdriver,
> + tpacpi_hwmon_pdriver_probe,
> + NULL, 0, NULL, 0);
> if (IS_ERR(tpacpi_sensors_pdev)) {
> ret = PTR_ERR(tpacpi_sensors_pdev);
> tpacpi_sensors_pdev = NULL;
> - pr_err("unable to register hwmon platform device\n");
> + pr_err("unable to register hwmon platform device/driver bundle\n");
> thinkpad_acpi_module_exit();
> return ret;
> }
>
> tpacpi_lifecycle = TPACPI_LIFE_RUNNING;
>
> - ret = platform_driver_register(&tpacpi_hwmon_pdriver);
> - if (ret) {
> - pr_err("unable to register hwmon platform driver\n");
> - thinkpad_acpi_module_exit();
> - return ret;
> - }
> - tp_features.sensors_pdrv_registered = 1;
> -
> - tpacpi_hwmon = hwmon_device_register_with_groups(
> - &tpacpi_sensors_pdev->dev, TPACPI_NAME, NULL, tpacpi_hwmon_groups);
> - if (IS_ERR(tpacpi_hwmon)) {
> - ret = PTR_ERR(tpacpi_hwmon);
> - tpacpi_hwmon = NULL;
> - pr_err("unable to register hwmon device\n");
> - thinkpad_acpi_module_exit();
> - return ret;
> - }
> -
> return 0;
> }
>
> --
> 2.48.1
Thanks for doing this.
For the series - all looks good and I tested on a X1 Carbon 12 and confirmed the Thinkpad devices are there under /sys/devices/thinkpad_acpi and /sys/class/hwmon. Didn't find any issues.
Reviewed-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
Tested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
Mark
