Re: [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex)

[Mark Pearson]

Thanks Dmitry,

On Thu, Aug 6, 2026, at 2:19 AM, Dmitry Torokhov wrote:
> Convert straightforward mutex_lock() and mutex_unlock() usages for
> hotkey_mutex, tpacpi_inputdev_send_mutex, kbdlight_mutex, lcdshadow_dev
> lock, and dytc_mutex to guard(mutex) and scoped_guard(mutex) helpers
> from linux/cleanup.h.
>
> This improves code readability and ensures that mutexes are
> automatically released when exiting their respective scopes.
>
> Assisted-by: Antigravity:gemini-3.6-flash
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
>  drivers/platform/x86/lenovo/thinkpad_acpi.c | 139 ++++++++------------
>  1 file changed, 57 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c 
> b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> index f8e116e8a65d..beb85ea1103b 100644
> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> @@ -2164,7 +2164,7 @@ static int tpacpi_hotkey_driver_mask_set(const 
> u32 mask)
>  		return 0;
>  	}
> 
> -	mutex_lock(&hotkey_mutex);
> +	guard(mutex)(&hotkey_mutex);
> 
>  	HOTKEY_CONFIG_CRITICAL_START
>  	hotkey_driver_mask = mask;
> @@ -2177,8 +2177,6 @@ static int tpacpi_hotkey_driver_mask_set(const u32 mask)
>  							~hotkey_source_mask);
>  	hotkey_poll_setup(true);
> 
> -	mutex_unlock(&hotkey_mutex);
> -
>  	return rc;
>  }
> 
> @@ -2202,15 +2200,12 @@ static void tpacpi_input_send_tabletsw(void)
>  {
>  	int state;
> 
> -	if (tp_features.hotkey_tablet &&
> -	    !hotkey_get_tablet_mode(&state)) {
> -		mutex_lock(&tpacpi_inputdev_send_mutex);
> +	if (tp_features.hotkey_tablet && !hotkey_get_tablet_mode(&state)) {
> +		guard(mutex)(&tpacpi_inputdev_send_mutex);
> 
>  		input_report_switch(tpacpi_inputdev,
>  				    SW_TABLET_MODE, !!state);
>  		input_sync(tpacpi_inputdev);
> -
> -		mutex_unlock(&tpacpi_inputdev_send_mutex);
>  	}
>  }
> 
> @@ -2235,7 +2230,6 @@ static int get_camera_shutter(void)
> 
>  static bool tpacpi_input_send_key(const u32 hkey, bool *send_acpi_ev)
>  {
> -	bool known_ev;
>  	u32 scancode;
> 
>  	if (tpacpi_driver_event(hkey))
> @@ -2278,11 +2272,8 @@ static bool tpacpi_input_send_key(const u32 
> hkey, bool *send_acpi_ev)
>  		scancode = hkey;
>  	}
> 
> -	mutex_lock(&tpacpi_inputdev_send_mutex);
> -	known_ev = sparse_keymap_report_event(tpacpi_inputdev, scancode, 1, true);
> -	mutex_unlock(&tpacpi_inputdev_send_mutex);
> -
> -	return known_ev;
> +	guard(mutex)(&tpacpi_inputdev_send_mutex);
> +	return sparse_keymap_report_event(tpacpi_inputdev, scancode, 1, true);
>  }
> 
>  #ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
> @@ -2572,9 +2563,8 @@ static void hotkey_poll_setup(const bool may_warn)
> 
>  static void hotkey_poll_setup_safe(const bool may_warn)
>  {
> -	mutex_lock(&hotkey_mutex);
> +	guard(mutex)(&hotkey_mutex);
>  	hotkey_poll_setup(may_warn);
> -	mutex_unlock(&hotkey_mutex);
>  }
> 
>  static void hotkey_poll_set_freq(unsigned int freq)
> @@ -3077,13 +3067,11 @@ static void tpacpi_send_radiosw_update(void)
> 
>  	/* Issue rfkill input event for WLSW switch */
>  	if (!(wlsw < 0)) {
> -		mutex_lock(&tpacpi_inputdev_send_mutex);
> +		guard(mutex)(&tpacpi_inputdev_send_mutex);
> 
>  		input_report_switch(tpacpi_inputdev,
>  				    SW_RFKILL_ALL, (wlsw > 0));
>  		input_sync(tpacpi_inputdev);
> -
> -		mutex_unlock(&tpacpi_inputdev_send_mutex);
>  	}
> 
>  	/*
> @@ -3095,7 +3083,7 @@ static void tpacpi_send_radiosw_update(void)
> 
>  static void hotkey_exit(void)
>  {
> -	mutex_lock(&hotkey_mutex);
> +	guard(mutex)(&hotkey_mutex);
>  	hotkey_poll_stop_sync();
>  	dbg_printk(TPACPI_DBG_EXIT | TPACPI_DBG_HKEY,
>  		   "restoring original HKEY status and mask\n");
> @@ -3105,8 +3093,6 @@ static void hotkey_exit(void)
>  	      hotkey_mask_set(hotkey_orig_mask)) |
>  	     hotkey_status_set(false)) != 0)
>  		pr_err("failed to restore hot key mask to BIOS defaults\n");
> -
> -	mutex_unlock(&hotkey_mutex);
>  }
> 
>  /*
> @@ -3423,11 +3409,11 @@ static int __init hotkey_init(struct 
> ibm_init_struct *iibm)
>  	if (tp_features.hotkey_mask) {
>  		/* hotkey_source_mask *must* be zero for
>  		 * the first hotkey_mask_get to return hotkey_orig_mask */
> -		mutex_lock(&hotkey_mutex);
> -		res = hotkey_mask_get();
> -		mutex_unlock(&hotkey_mutex);
> -		if (res)
> -			return res;
> +		scoped_guard(mutex, &hotkey_mutex) {
> +			res = hotkey_mask_get();
> +			if (res)
> +				return res;
> +		}
> 
>  		hotkey_orig_mask = hotkey_acpi_mask;
>  	} else {
> @@ -3526,11 +3512,11 @@ static int __init hotkey_init(struct 
> ibm_init_struct *iibm)
>  		hotkey_exit();
>  		return res;
>  	}
> -	mutex_lock(&hotkey_mutex);
> -	res = hotkey_mask_set(((hotkey_all_mask & ~hotkey_reserved_mask)
> -			       | hotkey_driver_mask)
> -			      & ~hotkey_source_mask);
> -	mutex_unlock(&hotkey_mutex);
> +	scoped_guard(mutex, &hotkey_mutex) {
> +		res = hotkey_mask_set(((hotkey_all_mask & ~hotkey_reserved_mask)
> +				       | hotkey_driver_mask)
> +				      & ~hotkey_source_mask);
> +	}
>  	if (res < 0 && res != -ENXIO) {
>  		hotkey_exit();
>  		return res;
> @@ -3977,11 +3963,11 @@ static void hotkey_resume(void)
>  {
>  	tpacpi_disable_brightness_delay();
> 
> -	mutex_lock(&hotkey_mutex);
> -	if (hotkey_status_set(true) < 0 ||
> -	    hotkey_mask_set(hotkey_acpi_mask) < 0)
> -		pr_err("error while attempting to reset the event firmware interface\n");
> -	mutex_unlock(&hotkey_mutex);
> +	scoped_guard(mutex, &hotkey_mutex) {
> +		if (hotkey_status_set(true) < 0 ||
> +		    hotkey_mask_set(hotkey_acpi_mask) < 0)
> +			pr_err("error while attempting to reset the event firmware interface\n");
> +	}
> 
>  	tpacpi_send_radiosw_update();
>  	tpacpi_input_send_tabletsw();
> @@ -5034,21 +5020,16 @@ static DEFINE_MUTEX(kbdlight_mutex);
> 
>  static int kbdlight_set_level(int level)
>  {
> -	int ret = 0;
> -
>  	if (!hkey_handle)
>  		return -ENXIO;
> 
> -	mutex_lock(&kbdlight_mutex);
> +	guard(mutex)(&kbdlight_mutex);
> 
>  	if (!acpi_evalf(hkey_handle, NULL, "MLCS", "dd", level))
> -		ret = -EIO;
> -	else
> -		kbdlight_brightness = level;
> -
> -	mutex_unlock(&kbdlight_mutex);
> +		return -EIO;
> 
> -	return ret;
> +	kbdlight_brightness = level;
> +	return 0;
>  }
> 
>  static int kbdlight_get_level(void)
> @@ -10103,9 +10084,8 @@ static void lcdshadow_resume(void)
>  	if (!lcdshadow_dev)
>  		return;
> 
> -	mutex_lock(&lcdshadow_dev->lock);
> +	guard(mutex)(&lcdshadow_dev->lock);
>  	lcdshadow_set_sw_state(lcdshadow_dev, lcdshadow_dev->sw_state);
> -	mutex_unlock(&lcdshadow_dev->lock);
>  }
> 
>  static int lcdshadow_read(struct seq_file *m)
> @@ -10137,9 +10117,8 @@ static int lcdshadow_write(char *buf)
>  	if (state >= 2 || state < 0)
>  		return -EINVAL;
> 
> -	mutex_lock(&lcdshadow_dev->lock);
> -	res = lcdshadow_set_sw_state(lcdshadow_dev, state);
> -	mutex_unlock(&lcdshadow_dev->lock);
> +	scoped_guard(mutex, &lcdshadow_dev->lock)
> +		res = lcdshadow_set_sw_state(lcdshadow_dev, state);
> 
>  	drm_privacy_screen_call_notifier_chain(lcdshadow_dev);
> 
> @@ -10603,26 +10582,26 @@ static const struct platform_profile_ops 
> dytc_profile_ops = {
>  static void dytc_profile_refresh(void)
>  {
>  	enum platform_profile_option profile;
> -	int output = 0, err = 0;
> +	int output = 0, err;
>  	int perfmode, funcmode = 0;
> 
> -	mutex_lock(&dytc_mutex);
> -	if (dytc_capabilities & BIT(DYTC_FC_MMC)) {
> -		if (dytc_mmc_get_available)
> -			err = dytc_command(DYTC_CMD_MMC_GET, &output);
> -		else
> -			err = dytc_cql_command(DYTC_CMD_GET, &output);
> -		funcmode = DYTC_FUNCTION_MMC;
> -	} else if (dytc_capabilities & BIT(DYTC_FC_PSC)) {
> -		err = dytc_command(DYTC_CMD_GET, &output);
> -		/* Check if we are PSC mode, or have AMT enabled */
> -		funcmode = (output >> DYTC_GET_FUNCTION_BIT) & 0xF;
> -	} else { /* Unknown profile mode */
> -		err = -ENODEV;
> +	scoped_guard(mutex, &dytc_mutex) {
> +		if (dytc_capabilities & BIT(DYTC_FC_MMC)) {
> +			if (dytc_mmc_get_available)
> +				err = dytc_command(DYTC_CMD_MMC_GET, &output);
> +			else
> +				err = dytc_cql_command(DYTC_CMD_GET, &output);
> +			funcmode = DYTC_FUNCTION_MMC;
> +		} else if (dytc_capabilities & BIT(DYTC_FC_PSC)) {
> +			err = dytc_command(DYTC_CMD_GET, &output);
> +			/* Check if we are PSC mode, or have AMT enabled */
> +			funcmode = (output >> DYTC_GET_FUNCTION_BIT) & 0xF;
> +		} else { /* Unknown profile mode */
> +			err = -ENODEV;
> +		}
> +		if (err)
> +			return;
>  	}
> -	mutex_unlock(&dytc_mutex);
> -	if (err)
> -		return;
> 
>  	perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
>  	err = convert_dytc_to_profile(funcmode, perfmode, &profile);
> @@ -11425,7 +11404,7 @@ static bool tpacpi_driver_event(const unsigned 
> int hkey_event)
>  		if (tp_features.kbdlight) {
>  			enum led_brightness brightness;
> 
> -			mutex_lock(&kbdlight_mutex);
> +			guard(mutex)(&kbdlight_mutex);
> 
>  			/*
>  			 * Check the brightness actually changed, setting the brightness
> @@ -11437,8 +11416,6 @@ static bool tpacpi_driver_event(const unsigned 
> int hkey_event)
>  				led_classdev_notify_brightness_hw_changed(
>  					&tpacpi_led_kbdlight.led_classdev, brightness);
>  			}
> -
> -			mutex_unlock(&kbdlight_mutex);
>  		}
>  		/* Key events are suppressed by default hotkey_user_mask */
>  		return false;
> @@ -11460,11 +11437,11 @@ static bool tpacpi_driver_event(const 
> unsigned int hkey_event)
>  			enum drm_privacy_screen_status old_hw_state;
>  			bool changed;
> 
> -			mutex_lock(&lcdshadow_dev->lock);
> -			old_hw_state = lcdshadow_dev->hw_state;
> -			lcdshadow_get_hw_state(lcdshadow_dev);
> -			changed = lcdshadow_dev->hw_state != old_hw_state;
> -			mutex_unlock(&lcdshadow_dev->lock);
> +			scoped_guard(mutex, &lcdshadow_dev->lock) {
> +				old_hw_state = lcdshadow_dev->hw_state;
> +				lcdshadow_get_hw_state(lcdshadow_dev);
> +				changed = lcdshadow_dev->hw_state != old_hw_state;
> +			}
> 
>  			if (changed)
>  				drm_privacy_screen_call_notifier_chain(lcdshadow_dev);
> @@ -11485,12 +11462,10 @@ static bool tpacpi_driver_event(const 
> unsigned int hkey_event)
>  			pr_err("Error retrieving camera shutter state after shutter 
> event\n");
>  			return true;
>  		}
> -		mutex_lock(&tpacpi_inputdev_send_mutex);
> -
> -		input_report_switch(tpacpi_inputdev, SW_CAMERA_LENS_COVER, 
> camera_shutter_state);
> -		input_sync(tpacpi_inputdev);
> -
> -		mutex_unlock(&tpacpi_inputdev_send_mutex);
> +		scoped_guard(mutex, &tpacpi_inputdev_send_mutex) {
> +			input_report_switch(tpacpi_inputdev, SW_CAMERA_LENS_COVER, 
> camera_shutter_state);
> +			input_sync(tpacpi_inputdev);
> +		}
>  		return true;
>  	case TP_HKEY_EV_DOUBLETAP_TOGGLE:
>  		/* Toggle kernel-level doubletap event filtering */
> -- 
> 2.55.0.679.g6767b8d81c-goog

Sorry, took me a while to get to this one.
Changes look good, nice cleanup.

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

Mark