Re: [PATCH] platform/x86: lenovo: convert mutex_init() users to devm_mutex_init()

[Mark Pearson]

Hi Rajat,

On Fri, Oct 30, 2026, at 10:03 AM, Ilpo Järvinen wrote:
> On Fri, 30 Oct 2026, surirajat@xxxxxxxxx wrote:
>
>> From: Rajat Suri <rajat.suri@xxxxxxxx>
>> 
>> Convert the mutex_init() calls under drivers/platform/x86/lenovo/
>> to devm_mutex_init() so that the mutex lifecycle is managed by devres.
>> 
>> This removes the need for manual mutex_destroy() calls in wmi-camera and
>> ideapad-laptop, and fixes thinkpad_acpi, which never destroyed the
>> hotkey, brightness, volume and fan mutexes on its teardown path.
>> 
>> Details per driver:
>> 
>>  - wmi-camera: bind notify_lock to the WMI device and drop the
>>    mutex_destroy() from remove().
>> 
>>  - ideapad-laptop: allocate the DYTC private data with devm_kzalloc() and
>>    bind its mutex to the platform device. Calling mutex_destroy() on a
>>    manually freed object is not possible with devres, so the kfree() and
>>    ideapad_dytc_profile_exit() helper are removed as well. The memory now
>>    outlives the platform profile registration, which is also devm managed
>>    and therefore released in reverse order.
>
> Lets do this per driver.
>
> Clearly you when beyond the scope of devm_mutex_init() here (the summary 
> in the shortlog on Subject). It's easier to write a focused changelog when 
> you don't try to combine many things into one.
>
> Whenever you need to start to use bullet lists like this to describe 
> variations, you almost always want to split things into multiple patches 
> instead.
>
>>  - thinkpad_acpi: bind the hotkey, hotkey thread data, brightness, volume
>>    and fan mutexes to tpacpi_pdev, which is registered before any
>>    subdriver init runs, and propagate a possible failure from
>>    devm_mutex_init().
>> 
>> No functional change intended.
>
> Not true, in thinkpad_acpi case you wanted to add the missing 
> mutex_destroy() which surely is a function change.
>
>> Signed-off-by: Rajat Suri <rajat.suri@xxxxxxxx>
>> ---
>>  drivers/platform/x86/lenovo/ideapad-laptop.c | 23 ++++----------------
>>  drivers/platform/x86/lenovo/thinkpad_acpi.c  | 21 +++++++++++++-----
>>  drivers/platform/x86/lenovo/wmi-camera.c     |  6 +----
>>  3 files changed, 21 insertions(+), 29 deletions(-)
>> 
>> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
>> index fcb5ed7035f5..ee8e00bdaf6b 100644
>> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
>> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
>> @@ -1172,11 +1172,13 @@ static int ideapad_dytc_profile_init(struct ideapad_private *priv)
>>  		return -ENODEV;
>>  	}
>>  
>> -	priv->dytc = kzalloc_obj(*priv->dytc);
>> +	priv->dytc = devm_kzalloc(&priv->platform_device->dev, sizeof(*priv->dytc), GFP_KERNEL);
>>  	if (!priv->dytc)
>>  		return -ENOMEM;
>>  
>> -	mutex_init(&priv->dytc->mutex);
>> +	err = devm_mutex_init(&priv->platform_device->dev, &priv->dytc->mutex);
>> +	if (err)
>> +		goto pp_reg_failed;
>>  
>>  	priv->dytc->priv = priv;
>>  
>> @@ -1195,24 +1197,11 @@ static int ideapad_dytc_profile_init(struct ideapad_private *priv)
>>  	return 0;
>>  
>>  pp_reg_failed:
>> -	mutex_destroy(&priv->dytc->mutex);
>> -	kfree(priv->dytc);
>>  	priv->dytc = NULL;
>>  
>>  	return err;
>>  }
>>  
>> -static void ideapad_dytc_profile_exit(struct ideapad_private *priv)
>> -{
>> -	if (!priv->dytc)
>> -		return;
>> -
>> -	mutex_destroy(&priv->dytc->mutex);
>> -	kfree(priv->dytc);
>> -
>> -	priv->dytc = NULL;
>> -}
>> -
>>  /*
>>   * Rfkill
>>   */
>> @@ -2487,8 +2476,6 @@ static int ideapad_acpi_add(struct platform_device *pdev)
>>  	ideapad_backlight_exit(priv);
>>  
>>  backlight_failed:
>> -	ideapad_dytc_profile_exit(priv);
>> -
>>  	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
>>  		ideapad_unregister_rfkill(priv, i);
>>  
>> @@ -2516,8 +2503,6 @@ static void ideapad_acpi_remove(struct platform_device *pdev)
>>  				   ideapad_acpi_notify);
>>  
>>  	ideapad_backlight_exit(priv);
>> -	ideapad_dytc_profile_exit(priv);
>> -
>>  	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
>>  		ideapad_unregister_rfkill(priv, i);
>>  
>> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> index 4f81855ad721..8c7d6f281ae3 100644
>> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> @@ -3381,10 +3381,14 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
>>  	       tpacpi_inputdev->close != NULL);
>>  
>>  	TPACPI_ACPIHANDLE_INIT(hkey);
>> -	mutex_init(&hotkey_mutex);
>> +	res = devm_mutex_init(&tpacpi_pdev->dev, &hotkey_mutex);
>> +	if (res)
>> +		return res;
>>  
>>  #ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
>> -	mutex_init(&hotkey_thread_data_mutex);
>> +	res = devm_mutex_init(&tpacpi_pdev->dev, &hotkey_thread_data_mutex);
>> +	if (res)
>> +		return res;
>>  #endif
>>  
>>  	/* hotkey not supported on 570 */
>> @@ -6834,7 +6838,9 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
>>  
>>  	vdbg_printk(TPACPI_DBG_INIT, "initializing brightness subdriver\n");
>>  
>> -	mutex_init(&brightness_mutex);
>> +	b = devm_mutex_init(&tpacpi_pdev->dev, &brightness_mutex);
>> +	if (b)
>
> b ?
>
>> +		return b;
>>  
>>  	quirks = tpacpi_check_quirks(brightness_quirk_table,
>>  				ARRAY_SIZE(brightness_quirk_table));
>> @@ -7575,7 +7581,9 @@ static int __init volume_init(struct ibm_init_struct *iibm)
>>  
>>  	vdbg_printk(TPACPI_DBG_INIT, "initializing volume subdriver\n");
>>  
>> -	mutex_init(&volume_mutex);
>> +	rc = devm_mutex_init(&tpacpi_pdev->dev, &volume_mutex);
>> +	if (rc)
>> +		return rc;
>>  
>>  	/*
>>  	 * Check for module parameter bogosity, note that we
>> @@ -8913,11 +8921,14 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
>>  static int __init fan_init(struct ibm_init_struct *iibm)
>>  {
>>  	unsigned long quirks;
>> +	int rc;
>>  
>>  	vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
>>  			"initializing fan subdriver\n");
>>  
>> -	mutex_init(&fan_mutex);
>> +	rc = devm_mutex_init(&tpacpi_pdev->dev, &fan_mutex);
>> +	if (rc)
>> +		return rc;
>>  	fan_status_access_mode = TPACPI_FAN_NONE;
>>  	fan_control_access_mode = TPACPI_FAN_WR_NONE;
>>  	fan_control_commands = 0;
>> diff --git a/drivers/platform/x86/lenovo/wmi-camera.c b/drivers/platform/x86/lenovo/wmi-camera.c
>> index 89ecbce60bf4..de4596686d2a 100644
>> --- a/drivers/platform/x86/lenovo/wmi-camera.c
>> +++ b/drivers/platform/x86/lenovo/wmi-camera.c
>> @@ -107,9 +107,7 @@ static int lenovo_wmi_probe(struct wmi_device *wdev, const void *context)
>>  
>>  	dev_set_drvdata(&wdev->dev, priv);
>>  
>> -	mutex_init(&priv->notify_lock);
>> -
>> -	return 0;
>> +	return devm_mutex_init(&wdev->dev, &priv->notify_lock);
>>  }
>>  
>>  static void lenovo_wmi_remove(struct wmi_device *wdev)
>> @@ -118,8 +116,6 @@ static void lenovo_wmi_remove(struct wmi_device *wdev)
>>  
>>  	if (priv->idev)
>>  		input_unregister_device(priv->idev);
>> -
>> -	mutex_destroy(&priv->notify_lock);
>>  }
>>  
>>  static const struct wmi_device_id lenovo_wmi_id_table[] = {
>> 
>
> -- 
>  i.

I'm slightly hesitant as we've had a few of these cleanup changes go in recently that then broke things by changing behaviour unintentionally; but the changes look fine to me.

I'm assuming with the Dell email address that you've not tested on any Lenovo hardware :) 
I did run a quick test on an X1 Carbon G14 and didn't see any issues (covers thinkpad_acpi).
I'll confirm on some other HW when the next version comes out but I'm not able to cover the ideapad or wmi-camera changes unfortunately as I don't have that HW.

Tested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>

Mark