[PATCH 0/2] platform/x86: thinkpad_acpi: Enable devres for subdriver .init callbacks

[PATCH 0/2] platform/x86: thinkpad_acpi: Enable devres for subdriver .init callbacks

[Kurt Borja]

Hi all,

It was reported by Mark [1] that if subdrivers used devres, the
tpacpi_pdev wouldn't bind successfully to the device, thus failing to
create the various sysfs attributes that this driver exposes.

The original problem is already fixed, however a complete solution was
due.

The approach I took is to let the driver core manage the lifetimes of
the subdrivers (details in Patch [1/2]). This enables devres for
subdrivers and IMO makes the code more maintainable (because of the
lifetime gurantees).

This was compile tested only, because (unfortunately) I don't own a
thinkpad so some testing is absolutely required, as this is an extremely
intricate driver.

Based on top of the for-next branch.

~ Kurt
---
[1] https://lore.kernel.org/platform-driver-x86/20250208091438.5972-1-mpearson-lenovo@squebb.ca/#t

Kurt Borja (2):
  platform/x86: thinkpad_acpi: Move subdriver initialization to
    tpacpi_pdriver's probe.
  platform/x86: thinkpad_acpi: Move HWMON initialization to
    tpacpi_hwmon_pdriver's probe

 drivers/platform/x86/thinkpad_acpi.c | 176 ++++++++++++---------------
 1 file changed, 77 insertions(+), 99 deletions(-)


base-commit: d497c47481f8e8f13e3191c9a707ed942d3bb3d7
-- 
2.48.1

[PATCH 1/2] platform/x86: thinkpad_acpi: Move subdriver initialization to tpacpi_pdriver's probe.

[Kurt Borja]

It was reported that if subdrivers assigned devres resources inside
ibm_init_struct's .init callbacks, driver binding would fail with the
following error message:

	platform thinkpad_acpi: Resources present before probing

Let the driver core manage the lifetimes of the subdrivers and children
devices, by initializing them inside tpacpi_driver's .probe callback.
This is appropriate because these subdrivers usually expose sysfs groups
and the driver core manages this automatically to avoid races.

One immediate benefit of this, is that we are now able to use devres
inside .init subdriver callbacks.

platform_create_bundle is specifically used because it makes the
driver's probe type synchronous and returns an ERR_PTR if attachment
failed.

Additionally, to make error handling simpler, allocate the input device
using devm_input_allocate_device().

Reported-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Closes: https://lore.kernel.org/platform-driver-x86/20250208091438.5972-1-mpearson-lenovo@squebb.ca/#t
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 136 ++++++++++++---------------
 1 file changed, 62 insertions(+), 74 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index ab1cade5ef23..ad9de48cc122 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -367,8 +367,6 @@ static struct {
 	u32 beep_needs_two_args:1;
 	u32 mixer_no_level_control:1;
 	u32 battery_force_primary:1;
-	u32 input_device_registered:1;
-	u32 platform_drv_registered:1;
 	u32 sensors_pdrv_registered:1;
 	u32 hotkey_poll_active:1;
 	u32 has_adaptive_kbd:1;
@@ -11815,36 +11813,20 @@ MODULE_PARM_DESC(profile_force, "Force profile mode. -1=off, 1=MMC, 2=PSC");
 
 static void thinkpad_acpi_module_exit(void)
 {
-	struct ibm_struct *ibm, *itmp;
-
 	tpacpi_lifecycle = TPACPI_LIFE_EXITING;
 
 	if (tpacpi_hwmon)
 		hwmon_device_unregister(tpacpi_hwmon);
 	if (tp_features.sensors_pdrv_registered)
 		platform_driver_unregister(&tpacpi_hwmon_pdriver);
-	if (tp_features.platform_drv_registered)
-		platform_driver_unregister(&tpacpi_pdriver);
-
-	list_for_each_entry_safe_reverse(ibm, itmp,
-					 &tpacpi_all_drivers,
-					 all_drivers) {
-		ibm_exit(ibm);
-	}
-
-	dbg_printk(TPACPI_DBG_INIT, "finished subdriver exit path...\n");
-
-	if (tpacpi_inputdev) {
-		if (tp_features.input_device_registered)
-			input_unregister_device(tpacpi_inputdev);
-		else
-			input_free_device(tpacpi_inputdev);
-	}
-
 	if (tpacpi_sensors_pdev)
 		platform_device_unregister(tpacpi_sensors_pdev);
-	if (tpacpi_pdev)
+
+	if (tpacpi_pdev) {
+		platform_driver_unregister(&tpacpi_pdriver);
 		platform_device_unregister(tpacpi_pdev);
+	}
+
 	if (proc_dir)
 		remove_proc_entry(TPACPI_PROC_DIR, acpi_root_dir);
 	if (tpacpi_wq)
@@ -11856,11 +11838,63 @@ static void thinkpad_acpi_module_exit(void)
 	kfree(thinkpad_id.nummodel_str);
 }
 
+static void tpacpi_subdrivers_release(void *data)
+{
+	struct ibm_struct *ibm, *itmp;
+
+	list_for_each_entry_safe_reverse(ibm, itmp, &tpacpi_all_drivers, all_drivers)
+		ibm_exit(ibm);
+
+	dbg_printk(TPACPI_DBG_INIT, "finished subdriver exit path...\n");
+}
+
+static int __init tpacpi_pdriver_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	devm_mutex_init(&pdev->dev, &tpacpi_inputdev_send_mutex);
+
+	tpacpi_inputdev = devm_input_allocate_device(&pdev->dev);
+	if (!tpacpi_inputdev)
+		return -ENOMEM;
+
+	tpacpi_inputdev->name = "ThinkPad Extra Buttons";
+	tpacpi_inputdev->phys = TPACPI_DRVR_NAME "/input0";
+	tpacpi_inputdev->id.bustype = BUS_HOST;
+	tpacpi_inputdev->id.vendor = thinkpad_id.vendor;
+	tpacpi_inputdev->id.product = TPACPI_HKEY_INPUT_PRODUCT;
+	tpacpi_inputdev->id.version = TPACPI_HKEY_INPUT_VERSION;
+	tpacpi_inputdev->dev.parent = &tpacpi_pdev->dev;
+
+	/* Init subdriver dependencies */
+	tpacpi_detect_brightness_capabilities();
+
+	/* Init subdrivers */
+	for (unsigned int i = 0; i < ARRAY_SIZE(ibms_init); i++) {
+		ret = ibm_init(&ibms_init[i]);
+		if (ret >= 0 && *ibms_init[i].param)
+			ret = ibms_init[i].data->write(ibms_init[i].param);
+		if (ret < 0) {
+			tpacpi_subdrivers_release(NULL);
+			return ret;
+		}
+	}
+
+	ret = devm_add_action_or_reset(&pdev->dev, tpacpi_subdrivers_release, NULL);
+	if (ret)
+		return ret;
+
+	ret = input_register_device(tpacpi_inputdev);
+	if (ret < 0)
+		pr_err("unable to register input device\n");
+
+	return ret;
+}
 
 static int __init thinkpad_acpi_module_init(void)
 {
 	const struct dmi_system_id *dmi_id;
-	int ret, i;
+	int ret;
 	acpi_object_type obj_type;
 
 	tpacpi_lifecycle = TPACPI_LIFE_INIT;
@@ -11920,15 +11954,16 @@ static int __init thinkpad_acpi_module_init(void)
 		tp_features.quirks = dmi_id->driver_data;
 
 	/* Device initialization */
-	tpacpi_pdev = platform_device_register_simple(TPACPI_DRVR_NAME, PLATFORM_DEVID_NONE,
-							NULL, 0);
+	tpacpi_pdev = platform_create_bundle(&tpacpi_pdriver, tpacpi_pdriver_probe,
+					     NULL, 0, NULL, 0);
 	if (IS_ERR(tpacpi_pdev)) {
 		ret = PTR_ERR(tpacpi_pdev);
 		tpacpi_pdev = NULL;
-		pr_err("unable to register platform device\n");
+		pr_err("unable to register platform device/driver bundle\n");
 		thinkpad_acpi_module_exit();
 		return ret;
 	}
+
 	tpacpi_sensors_pdev = platform_device_register_simple(
 						TPACPI_HWMON_DRVR_NAME,
 						PLATFORM_DEVID_NONE, NULL, 0);
@@ -11940,46 +11975,8 @@ static int __init thinkpad_acpi_module_init(void)
 		return ret;
 	}
 
-	mutex_init(&tpacpi_inputdev_send_mutex);
-	tpacpi_inputdev = input_allocate_device();
-	if (!tpacpi_inputdev) {
-		thinkpad_acpi_module_exit();
-		return -ENOMEM;
-	} else {
-		/* Prepare input device, but don't register */
-		tpacpi_inputdev->name = "ThinkPad Extra Buttons";
-		tpacpi_inputdev->phys = TPACPI_DRVR_NAME "/input0";
-		tpacpi_inputdev->id.bustype = BUS_HOST;
-		tpacpi_inputdev->id.vendor = thinkpad_id.vendor;
-		tpacpi_inputdev->id.product = TPACPI_HKEY_INPUT_PRODUCT;
-		tpacpi_inputdev->id.version = TPACPI_HKEY_INPUT_VERSION;
-		tpacpi_inputdev->dev.parent = &tpacpi_pdev->dev;
-	}
-
-	/* Init subdriver dependencies */
-	tpacpi_detect_brightness_capabilities();
-
-	/* Init subdrivers */
-	for (i = 0; i < ARRAY_SIZE(ibms_init); i++) {
-		ret = ibm_init(&ibms_init[i]);
-		if (ret >= 0 && *ibms_init[i].param)
-			ret = ibms_init[i].data->write(ibms_init[i].param);
-		if (ret < 0) {
-			thinkpad_acpi_module_exit();
-			return ret;
-		}
-	}
-
 	tpacpi_lifecycle = TPACPI_LIFE_RUNNING;
 
-	ret = platform_driver_register(&tpacpi_pdriver);
-	if (ret) {
-		pr_err("unable to register main platform driver\n");
-		thinkpad_acpi_module_exit();
-		return ret;
-	}
-	tp_features.platform_drv_registered = 1;
-
 	ret = platform_driver_register(&tpacpi_hwmon_pdriver);
 	if (ret) {
 		pr_err("unable to register hwmon platform driver\n");
@@ -11998,15 +11995,6 @@ static int __init thinkpad_acpi_module_init(void)
 		return ret;
 	}
 
-	ret = input_register_device(tpacpi_inputdev);
-	if (ret < 0) {
-		pr_err("unable to register input device\n");
-		thinkpad_acpi_module_exit();
-		return ret;
-	} else {
-		tp_features.input_device_registered = 1;
-	}
-
 	return 0;
 }
 
-- 
2.48.1

[PATCH 2/2] platform/x86: thinkpad_acpi: Move HWMON initialization to tpacpi_hwmon_pdriver's probe

[Kurt Borja]

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@gmail.com>
---
 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

Re: [PATCH 2/2] platform/x86: thinkpad_acpi: Move HWMON initialization to tpacpi_hwmon_pdriver's probe

[Mark Pearson]

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@gmail.com>
> ---
>  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@squebb.ca>
Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca>

Mark

Re: [PATCH 2/2] platform/x86: thinkpad_acpi: Move HWMON initialization to tpacpi_hwmon_pdriver's probe

[Kurt Borja]

Hi Mark,

On Tue Feb 18, 2025 at 11:50 AM -05, Mark Pearson wrote:
> 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@gmail.com>
>> ---
>>  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.

Glad to help :)

>
> 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@squebb.ca>
> Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca>

Thank you! Making changes to this driver is a bit scary.

-- 
 ~ Kurt

>
> Mark

Re: [PATCH 0/2] platform/x86: thinkpad_acpi: Enable devres for subdriver .init callbacks

[Ilpo Järvinen]

On Fri, 14 Feb 2025 19:03:00 -0500, Kurt Borja wrote:

> It was reported by Mark [1] that if subdrivers used devres, the
> tpacpi_pdev wouldn't bind successfully to the device, thus failing to
> create the various sysfs attributes that this driver exposes.
> 
> The original problem is already fixed, however a complete solution was
> due.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/2] platform/x86: thinkpad_acpi: Move subdriver initialization to tpacpi_pdriver's probe.
      commit: 38b9ab80db31cf993a8f3ab2baf772083b62ca6f
[2/2] platform/x86: thinkpad_acpi: Move HWMON initialization to tpacpi_hwmon_pdriver's probe
      commit: 43fc63a1e8f6db7148f9cfcddda539c3437bc6c2

--
 i.