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

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

[Dmitry Torokhov]

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

[PATCH 2/3] platform/x86: thinkpad_acpi: convert conditional mutex locks to ACQUIRE_ERR()

[Dmitry Torokhov]

Convert conditional mutex_lock_killable() and mutex_lock_interruptible()
calls to ACQUIRE() and ACQUIRE_ERR() from linux/cleanup.h.

This eliminates explicit mutex_unlock() calls on return paths and
simplifies error handling across hotkey, brightness, volume, fan, and
dytc functions.

Assisted-by: Antigravity:gemini-3.6-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 153 ++++++++++----------
 1 file changed, 76 insertions(+), 77 deletions(-)

diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index beb85ea1103b..0d0d6fe7eecd 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -2671,8 +2671,10 @@ static ssize_t hotkey_mask_store(struct device *dev,
 	if (parse_strtoul(buf, 0xffffffffUL, &t))
 		return -EINVAL;
 
-	if (mutex_lock_killable(&hotkey_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&hotkey_mutex);
+	res = ACQUIRE_ERR(mutex_kill, &guard);
+	if (res)
+		return res;
 
 	res = hotkey_user_mask_set(t);
 
@@ -2680,8 +2682,6 @@ static ssize_t hotkey_mask_store(struct device *dev,
 	hotkey_poll_setup(true);
 #endif
 
-	mutex_unlock(&hotkey_mutex);
-
 	tpacpi_disclose_usertask("hotkey_mask", "set to 0x%08lx\n", t);
 
 	return (res) ? res : count;
@@ -2767,8 +2767,10 @@ static ssize_t hotkey_source_mask_store(struct device *dev,
 		((t & ~TPACPI_HKEY_NVRAM_KNOWN_MASK) != 0))
 		return -EINVAL;
 
-	if (mutex_lock_killable(&hotkey_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&hotkey_mutex);
+	rc = ACQUIRE_ERR(mutex_kill, &guard);
+	if (rc)
+		return rc;
 
 	HOTKEY_CONFIG_CRITICAL_START
 	hotkey_source_mask = t;
@@ -2782,8 +2784,6 @@ static ssize_t hotkey_source_mask_store(struct device *dev,
 	r_ev = hotkey_driver_mask & ~(hotkey_acpi_mask & hotkey_all_mask)
 		& ~hotkey_source_mask & TPACPI_HKEY_NVRAM_KNOWN_MASK;
 
-	mutex_unlock(&hotkey_mutex);
-
 	if (rc < 0)
 		pr_err("hotkey_source_mask: failed to update the firmware event mask!\n");
 
@@ -2811,18 +2811,19 @@ static ssize_t hotkey_poll_freq_store(struct device *dev,
 			    const char *buf, size_t count)
 {
 	unsigned long t;
+	int err;
 
 	if (parse_strtoul(buf, 25, &t))
 		return -EINVAL;
 
-	if (mutex_lock_killable(&hotkey_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&hotkey_mutex);
+	err = ACQUIRE_ERR(mutex_kill, &guard);
+	if (err)
+		return err;
 
 	hotkey_poll_set_freq(t);
 	hotkey_poll_setup(true);
 
-	mutex_unlock(&hotkey_mutex);
-
 	tpacpi_disclose_usertask("hotkey_poll_freq", "set to %lu\n", t);
 
 	return count;
@@ -3995,12 +3996,13 @@ static int hotkey_read(struct seq_file *m)
 		return 0;
 	}
 
-	if (mutex_lock_killable(&hotkey_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&hotkey_mutex);
+	res = ACQUIRE_ERR(mutex_kill, &guard);
+	if (res)
+		return res;
 	res = hotkey_status_get(&status);
 	if (!res)
 		res = hotkey_mask_get();
-	mutex_unlock(&hotkey_mutex);
 	if (res)
 		return res;
 
@@ -4033,8 +4035,10 @@ static int hotkey_write(char *buf)
 	if (!tp_features.hotkey)
 		return -ENODEV;
 
-	if (mutex_lock_killable(&hotkey_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&hotkey_mutex);
+	res = ACQUIRE_ERR(mutex_kill, &guard);
+	if (res)
+		return res;
 
 	mask = hotkey_user_mask;
 
@@ -4053,8 +4057,7 @@ static int hotkey_write(char *buf)
 		} else if (sscanf(cmd, "%x", &mask) == 1) {
 			/* mask set */
 		} else {
-			res = -EINVAL;
-			goto errexit;
+			return -EINVAL;
 		}
 	}
 
@@ -4064,8 +4067,6 @@ static int hotkey_write(char *buf)
 		res = hotkey_user_mask_set(mask);
 	}
 
-errexit:
-	mutex_unlock(&hotkey_mutex);
 	return res;
 }
 
@@ -6460,11 +6461,12 @@ static void tpacpi_brightness_checkpoint_nvram(void)
 	vdbg_printk(TPACPI_DBG_BRGHT,
 		"trying to checkpoint backlight level to NVRAM...\n");
 
-	if (mutex_lock_killable(&brightness_mutex) < 0)
+	ACQUIRE(mutex_kill, guard)(&brightness_mutex);
+	if (ACQUIRE_ERR(mutex_kill, &guard))
 		return;
 
 	if (unlikely(!acpi_ec_read(TP_EC_BACKLIGHT, &lec)))
-		goto unlock;
+		return;
 	lec &= TP_EC_BACKLIGHT_LVLMSK;
 	b_nvram = nvram_read_byte(TP_NVRAM_ADDR_BRIGHTNESS);
 
@@ -6482,9 +6484,6 @@ static void tpacpi_brightness_checkpoint_nvram(void)
 		vdbg_printk(TPACPI_DBG_BRGHT,
 			   "NVRAM backlight level already is %u (0x%02x)\n",
 			   (unsigned int) lec, (unsigned int) b_nvram);
-
-unlock:
-	mutex_unlock(&brightness_mutex);
 }
 
 
@@ -6562,8 +6561,9 @@ static int brightness_set(unsigned int value)
 	vdbg_printk(TPACPI_DBG_BRGHT,
 			"set backlight level to %d\n", value);
 
-	res = mutex_lock_killable(&brightness_mutex);
-	if (res < 0)
+	ACQUIRE(mutex_kill, guard)(&brightness_mutex);
+	res = ACQUIRE_ERR(mutex_kill, &guard);
+	if (res)
 		return res;
 
 	switch (brightness_mode) {
@@ -6578,7 +6578,6 @@ static int brightness_set(unsigned int value)
 		res = -ENXIO;
 	}
 
-	mutex_unlock(&brightness_mutex);
 	return res;
 }
 
@@ -6601,16 +6600,14 @@ static int brightness_get(struct backlight_device *bd)
 {
 	int status, res;
 
-	res = mutex_lock_killable(&brightness_mutex);
-	if (res < 0)
-		return 0;
+	ACQUIRE(mutex_kill, guard)(&brightness_mutex);
+	res = ACQUIRE_ERR(mutex_kill, &guard);
+	if (res)
+		return res;
 
 	res = tpacpi_brightness_get_raw(&status);
-
-	mutex_unlock(&brightness_mutex);
-
 	if (res < 0)
-		return 0;
+		return res;
 
 	return status & TP_EC_BACKLIGHT_LVLMSK;
 }
@@ -7073,11 +7070,12 @@ static void tpacpi_volume_checkpoint_nvram(void)
 	else
 		ec_mask = TP_EC_AUDIO_MUTESW_MSK | TP_EC_AUDIO_LVL_MSK;
 
-	if (mutex_lock_killable(&volume_mutex) < 0)
+	ACQUIRE(mutex_kill, guard)(&volume_mutex);
+	if (ACQUIRE_ERR(mutex_kill, &guard))
 		return;
 
 	if (unlikely(!acpi_ec_read(TP_EC_AUDIO, &lec)))
-		goto unlock;
+		return;
 	lec &= ec_mask;
 	b_nvram = nvram_read_byte(TP_NVRAM_ADDR_MIXER);
 
@@ -7094,9 +7092,6 @@ static void tpacpi_volume_checkpoint_nvram(void)
 			   "NVRAM mixer status already is 0x%02x (0x%02x)\n",
 			   (unsigned int) lec, (unsigned int) b_nvram);
 	}
-
-unlock:
-	mutex_unlock(&volume_mutex);
 }
 
 static int volume_get_status_ec(u8 *status)
@@ -7145,12 +7140,14 @@ static int __volume_set_mute_ec(const bool mute)
 	int rc;
 	u8 s, n;
 
-	if (mutex_lock_killable(&volume_mutex) < 0)
-		return -EINTR;
+	ACQUIRE(mutex_kill, guard)(&volume_mutex);
+	rc = ACQUIRE_ERR(mutex_kill, &guard);
+	if (rc)
+		return rc;
 
 	rc = volume_get_status_ec(&s);
 	if (rc)
-		goto unlock;
+		return rc;
 
 	n = (mute) ? s | TP_EC_AUDIO_MUTESW_MSK :
 		     s & ~TP_EC_AUDIO_MUTESW_MSK;
@@ -7161,8 +7158,6 @@ static int __volume_set_mute_ec(const bool mute)
 			rc = 1;
 	}
 
-unlock:
-	mutex_unlock(&volume_mutex);
 	return rc;
 }
 
@@ -7193,12 +7188,14 @@ static int __volume_set_volume_ec(const u8 vol)
 	if (vol > TP_EC_VOLUME_MAX)
 		return -EINVAL;
 
-	if (mutex_lock_killable(&volume_mutex) < 0)
-		return -EINTR;
+	ACQUIRE(mutex_kill, guard)(&volume_mutex);
+	rc = ACQUIRE_ERR(mutex_kill, &guard);
+	if (rc)
+		return rc;
 
 	rc = volume_get_status_ec(&s);
 	if (rc)
-		goto unlock;
+		return rc;
 
 	n = (s & ~TP_EC_AUDIO_LVL_MSK) | vol;
 
@@ -7208,8 +7205,6 @@ static int __volume_set_volume_ec(const u8 vol)
 			rc = 1;
 	}
 
-unlock:
-	mutex_unlock(&volume_mutex);
 	return rc;
 }
 
@@ -8113,13 +8108,14 @@ static int fan_get_status_safe(u8 *status)
 	int rc;
 	u8 s;
 
-	if (mutex_lock_killable(&fan_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&fan_mutex);
+	rc = ACQUIRE_ERR(mutex_kill, &guard);
+	if (rc)
+		return rc;
 	rc = fan_get_status(&s);
 	/* NS EC doesn't have register with level settings */
 	if (!rc && !fan_with_ns_addr)
 		fan_update_desired_level(s);
-	mutex_unlock(&fan_mutex);
 
 	if (rc)
 		return rc;
@@ -8312,8 +8308,10 @@ static int fan_set_level_safe(int level)
 	if (!fan_control_allowed)
 		return -EPERM;
 
-	if (mutex_lock_killable(&fan_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&fan_mutex);
+	rc = ACQUIRE_ERR(mutex_kill, &guard);
+	if (rc)
+		return rc;
 
 	if (level == TPACPI_FAN_LAST_LEVEL)
 		level = fan_control_desired_level;
@@ -8322,7 +8320,6 @@ static int fan_set_level_safe(int level)
 	if (!rc)
 		fan_update_desired_level(level);
 
-	mutex_unlock(&fan_mutex);
 	return rc;
 }
 
@@ -8334,8 +8331,10 @@ static int fan_set_enable(void)
 	if (!fan_control_allowed)
 		return -EPERM;
 
-	if (mutex_lock_killable(&fan_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&fan_mutex);
+	rc = ACQUIRE_ERR(mutex_kill, &guard);
+	if (rc)
+		return rc;
 
 	switch (fan_control_access_mode) {
 	case TPACPI_FAN_WR_ACPI_FANS:
@@ -8391,8 +8390,6 @@ static int fan_set_enable(void)
 		rc = -ENXIO;
 	}
 
-	mutex_unlock(&fan_mutex);
-
 	if (!rc)
 		vdbg_printk(TPACPI_DBG_FAN,
 			"fan control: set fan control register to 0x%02x\n",
@@ -8407,8 +8404,10 @@ static int fan_set_disable(void)
 	if (!fan_control_allowed)
 		return -EPERM;
 
-	if (mutex_lock_killable(&fan_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&fan_mutex);
+	rc = ACQUIRE_ERR(mutex_kill, &guard);
+	if (rc)
+		return rc;
 
 	rc = 0;
 	switch (fan_control_access_mode) {
@@ -8453,7 +8452,6 @@ static int fan_set_disable(void)
 		vdbg_printk(TPACPI_DBG_FAN,
 			"fan control: set fan control register to 0\n");
 
-	mutex_unlock(&fan_mutex);
 	return rc;
 }
 
@@ -8464,8 +8462,10 @@ static int fan_set_speed(int speed)
 	if (!fan_control_allowed)
 		return -EPERM;
 
-	if (mutex_lock_killable(&fan_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&fan_mutex);
+	rc = ACQUIRE_ERR(mutex_kill, &guard);
+	if (rc)
+		return rc;
 
 	rc = 0;
 	switch (fan_control_access_mode) {
@@ -8499,7 +8499,6 @@ static int fan_set_speed(int speed)
 		rc = -ENXIO;
 	}
 
-	mutex_unlock(&fan_mutex);
 	return rc;
 }
 
@@ -8659,8 +8658,10 @@ static ssize_t fan_pwm1_store(struct device *dev,
 	/* scale down from 0-255 to 0-7 */
 	newlevel = (s >> 5) & 0x07;
 
-	if (mutex_lock_killable(&fan_mutex))
-		return -ERESTARTSYS;
+	ACQUIRE(mutex_kill, guard)(&fan_mutex);
+	rc = ACQUIRE_ERR(mutex_kill, &guard);
+	if (rc)
+		return rc;
 
 	rc = fan_get_status(&status);
 	if (!rc && (status &
@@ -8674,7 +8675,6 @@ static ssize_t fan_pwm1_store(struct device *dev,
 		}
 	}
 
-	mutex_unlock(&fan_mutex);
 	return (rc) ? rc : count;
 }
 
@@ -10522,13 +10522,14 @@ static int dytc_profile_set(struct device *dev,
 	int output;
 	int err;
 
-	err = mutex_lock_interruptible(&dytc_mutex);
+	ACQUIRE(mutex_intr, guard)(&dytc_mutex);
+	err = ACQUIRE_ERR(mutex_intr, &guard);
 	if (err)
 		return err;
 
 	err = convert_profile_to_dytc(profile, &perfmode);
 	if (err)
-		goto unlock;
+		return err;
 
 	if (dytc_capabilities & BIT(DYTC_FC_MMC)) {
 		if (profile == PLATFORM_PROFILE_BALANCED) {
@@ -10540,18 +10541,18 @@ static int dytc_profile_set(struct device *dev,
 			 */
 			err = dytc_cql_command(DYTC_CMD_RESET, &output);
 			if (err)
-				goto unlock;
+				return err;
 		} else {
 			/* Determine if we are in CQL mode. This alters the commands we do */
 			err = dytc_cql_command(DYTC_SET_COMMAND(DYTC_FUNCTION_MMC, perfmode, 1),
 						&output);
 			if (err)
-				goto unlock;
+				return err;
 		}
 	} else if (dytc_capabilities & BIT(DYTC_FC_PSC)) {
 		err = dytc_command(DYTC_SET_COMMAND(DYTC_FUNCTION_PSC, perfmode, 1), &output);
 		if (err)
-			goto unlock;
+			return err;
 
 		/* system supports AMT, activate it when on balanced */
 		if (dytc_capabilities & BIT(DYTC_FC_AMT))
@@ -10559,8 +10560,6 @@ static int dytc_profile_set(struct device *dev,
 	}
 	/* Success - update current profile */
 	dytc_current_profile = profile;
-unlock:
-	mutex_unlock(&dytc_mutex);
 	return err;
 }
 
-- 
2.55.0.679.g6767b8d81c-goog

Re: [PATCH 2/3] platform/x86: thinkpad_acpi: convert conditional mutex locks to ACQUIRE_ERR()

[Mark Pearson]

Thanks Dmitry,

On Thu, Aug 6, 2026, at 2:19 AM, Dmitry Torokhov wrote:
> Convert conditional mutex_lock_killable() and mutex_lock_interruptible()
> calls to ACQUIRE() and ACQUIRE_ERR() from linux/cleanup.h.
>
> This eliminates explicit mutex_unlock() calls on return paths and
> simplifies error handling across hotkey, brightness, volume, fan, and
> dytc functions.
>
> Assisted-by: Antigravity:gemini-3.6-flash
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/platform/x86/lenovo/thinkpad_acpi.c | 153 ++++++++++----------
>  1 file changed, 76 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c 
> b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> index beb85ea1103b..0d0d6fe7eecd 100644
> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> @@ -2671,8 +2671,10 @@ static ssize_t hotkey_mask_store(struct device 
> *dev,
>  	if (parse_strtoul(buf, 0xffffffffUL, &t))
>  		return -EINVAL;
> 
> -	if (mutex_lock_killable(&hotkey_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&hotkey_mutex);
> +	res = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (res)
> +		return res;
> 
>  	res = hotkey_user_mask_set(t);
> 
> @@ -2680,8 +2682,6 @@ static ssize_t hotkey_mask_store(struct device *dev,
>  	hotkey_poll_setup(true);
>  #endif
> 
> -	mutex_unlock(&hotkey_mutex);
> -
>  	tpacpi_disclose_usertask("hotkey_mask", "set to 0x%08lx\n", t);
> 
>  	return (res) ? res : count;
> @@ -2767,8 +2767,10 @@ static ssize_t hotkey_source_mask_store(struct 
> device *dev,
>  		((t & ~TPACPI_HKEY_NVRAM_KNOWN_MASK) != 0))
>  		return -EINVAL;
> 
> -	if (mutex_lock_killable(&hotkey_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&hotkey_mutex);
> +	rc = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (rc)
> +		return rc;
> 
>  	HOTKEY_CONFIG_CRITICAL_START
>  	hotkey_source_mask = t;
> @@ -2782,8 +2784,6 @@ static ssize_t hotkey_source_mask_store(struct 
> device *dev,
>  	r_ev = hotkey_driver_mask & ~(hotkey_acpi_mask & hotkey_all_mask)
>  		& ~hotkey_source_mask & TPACPI_HKEY_NVRAM_KNOWN_MASK;
> 
> -	mutex_unlock(&hotkey_mutex);
> -
>  	if (rc < 0)
>  		pr_err("hotkey_source_mask: failed to update the firmware event mask!\n");
> 
> @@ -2811,18 +2811,19 @@ static ssize_t hotkey_poll_freq_store(struct 
> device *dev,
>  			    const char *buf, size_t count)
>  {
>  	unsigned long t;
> +	int err;
> 
>  	if (parse_strtoul(buf, 25, &t))
>  		return -EINVAL;
> 
> -	if (mutex_lock_killable(&hotkey_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&hotkey_mutex);
> +	err = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (err)
> +		return err;
> 
>  	hotkey_poll_set_freq(t);
>  	hotkey_poll_setup(true);
> 
> -	mutex_unlock(&hotkey_mutex);
> -
>  	tpacpi_disclose_usertask("hotkey_poll_freq", "set to %lu\n", t);
> 
>  	return count;
> @@ -3995,12 +3996,13 @@ static int hotkey_read(struct seq_file *m)
>  		return 0;
>  	}
> 
> -	if (mutex_lock_killable(&hotkey_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&hotkey_mutex);
> +	res = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (res)
> +		return res;
>  	res = hotkey_status_get(&status);
>  	if (!res)
>  		res = hotkey_mask_get();
> -	mutex_unlock(&hotkey_mutex);
>  	if (res)
>  		return res;
> 
> @@ -4033,8 +4035,10 @@ static int hotkey_write(char *buf)
>  	if (!tp_features.hotkey)
>  		return -ENODEV;
> 
> -	if (mutex_lock_killable(&hotkey_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&hotkey_mutex);
> +	res = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (res)
> +		return res;
> 
>  	mask = hotkey_user_mask;
> 
> @@ -4053,8 +4057,7 @@ static int hotkey_write(char *buf)
>  		} else if (sscanf(cmd, "%x", &mask) == 1) {
>  			/* mask set */
>  		} else {
> -			res = -EINVAL;
> -			goto errexit;
> +			return -EINVAL;
>  		}
>  	}
> 
> @@ -4064,8 +4067,6 @@ static int hotkey_write(char *buf)
>  		res = hotkey_user_mask_set(mask);
>  	}
> 
> -errexit:
> -	mutex_unlock(&hotkey_mutex);
>  	return res;
>  }
> 
> @@ -6460,11 +6461,12 @@ static void tpacpi_brightness_checkpoint_nvram(void)
>  	vdbg_printk(TPACPI_DBG_BRGHT,
>  		"trying to checkpoint backlight level to NVRAM...\n");
> 
> -	if (mutex_lock_killable(&brightness_mutex) < 0)
> +	ACQUIRE(mutex_kill, guard)(&brightness_mutex);
> +	if (ACQUIRE_ERR(mutex_kill, &guard))
>  		return;
> 
>  	if (unlikely(!acpi_ec_read(TP_EC_BACKLIGHT, &lec)))
> -		goto unlock;
> +		return;
>  	lec &= TP_EC_BACKLIGHT_LVLMSK;
>  	b_nvram = nvram_read_byte(TP_NVRAM_ADDR_BRIGHTNESS);
> 
> @@ -6482,9 +6484,6 @@ static void tpacpi_brightness_checkpoint_nvram(void)
>  		vdbg_printk(TPACPI_DBG_BRGHT,
>  			   "NVRAM backlight level already is %u (0x%02x)\n",
>  			   (unsigned int) lec, (unsigned int) b_nvram);
> -
> -unlock:
> -	mutex_unlock(&brightness_mutex);
>  }
> 
> 
> @@ -6562,8 +6561,9 @@ static int brightness_set(unsigned int value)
>  	vdbg_printk(TPACPI_DBG_BRGHT,
>  			"set backlight level to %d\n", value);
> 
> -	res = mutex_lock_killable(&brightness_mutex);
> -	if (res < 0)
> +	ACQUIRE(mutex_kill, guard)(&brightness_mutex);
> +	res = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (res)
>  		return res;
> 
>  	switch (brightness_mode) {
> @@ -6578,7 +6578,6 @@ static int brightness_set(unsigned int value)
>  		res = -ENXIO;
>  	}
> 
> -	mutex_unlock(&brightness_mutex);
>  	return res;
>  }
> 
> @@ -6601,16 +6600,14 @@ static int brightness_get(struct backlight_device *bd)
>  {
>  	int status, res;
> 
> -	res = mutex_lock_killable(&brightness_mutex);
> -	if (res < 0)
> -		return 0;
> +	ACQUIRE(mutex_kill, guard)(&brightness_mutex);
> +	res = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (res)
> +		return res;
> 
>  	res = tpacpi_brightness_get_raw(&status);
> -
> -	mutex_unlock(&brightness_mutex);
> -
>  	if (res < 0)
> -		return 0;
> +		return res;
> 
>  	return status & TP_EC_BACKLIGHT_LVLMSK;
>  }
> @@ -7073,11 +7070,12 @@ static void tpacpi_volume_checkpoint_nvram(void)
>  	else
>  		ec_mask = TP_EC_AUDIO_MUTESW_MSK | TP_EC_AUDIO_LVL_MSK;
> 
> -	if (mutex_lock_killable(&volume_mutex) < 0)
> +	ACQUIRE(mutex_kill, guard)(&volume_mutex);
> +	if (ACQUIRE_ERR(mutex_kill, &guard))
>  		return;
> 
>  	if (unlikely(!acpi_ec_read(TP_EC_AUDIO, &lec)))
> -		goto unlock;
> +		return;
>  	lec &= ec_mask;
>  	b_nvram = nvram_read_byte(TP_NVRAM_ADDR_MIXER);
> 
> @@ -7094,9 +7092,6 @@ static void tpacpi_volume_checkpoint_nvram(void)
>  			   "NVRAM mixer status already is 0x%02x (0x%02x)\n",
>  			   (unsigned int) lec, (unsigned int) b_nvram);
>  	}
> -
> -unlock:
> -	mutex_unlock(&volume_mutex);
>  }
> 
>  static int volume_get_status_ec(u8 *status)
> @@ -7145,12 +7140,14 @@ static int __volume_set_mute_ec(const bool mute)
>  	int rc;
>  	u8 s, n;
> 
> -	if (mutex_lock_killable(&volume_mutex) < 0)
> -		return -EINTR;
> +	ACQUIRE(mutex_kill, guard)(&volume_mutex);
> +	rc = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (rc)
> +		return rc;
> 
>  	rc = volume_get_status_ec(&s);
>  	if (rc)
> -		goto unlock;
> +		return rc;
> 
>  	n = (mute) ? s | TP_EC_AUDIO_MUTESW_MSK :
>  		     s & ~TP_EC_AUDIO_MUTESW_MSK;
> @@ -7161,8 +7158,6 @@ static int __volume_set_mute_ec(const bool mute)
>  			rc = 1;
>  	}
> 
> -unlock:
> -	mutex_unlock(&volume_mutex);
>  	return rc;
>  }
> 
> @@ -7193,12 +7188,14 @@ static int __volume_set_volume_ec(const u8 vol)
>  	if (vol > TP_EC_VOLUME_MAX)
>  		return -EINVAL;
> 
> -	if (mutex_lock_killable(&volume_mutex) < 0)
> -		return -EINTR;
> +	ACQUIRE(mutex_kill, guard)(&volume_mutex);
> +	rc = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (rc)
> +		return rc;
> 
>  	rc = volume_get_status_ec(&s);
>  	if (rc)
> -		goto unlock;
> +		return rc;
> 
>  	n = (s & ~TP_EC_AUDIO_LVL_MSK) | vol;
> 
> @@ -7208,8 +7205,6 @@ static int __volume_set_volume_ec(const u8 vol)
>  			rc = 1;
>  	}
> 
> -unlock:
> -	mutex_unlock(&volume_mutex);
>  	return rc;
>  }
> 
> @@ -8113,13 +8108,14 @@ static int fan_get_status_safe(u8 *status)
>  	int rc;
>  	u8 s;
> 
> -	if (mutex_lock_killable(&fan_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&fan_mutex);
> +	rc = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (rc)
> +		return rc;
>  	rc = fan_get_status(&s);
>  	/* NS EC doesn't have register with level settings */
>  	if (!rc && !fan_with_ns_addr)
>  		fan_update_desired_level(s);
> -	mutex_unlock(&fan_mutex);
> 
>  	if (rc)
>  		return rc;
> @@ -8312,8 +8308,10 @@ static int fan_set_level_safe(int level)
>  	if (!fan_control_allowed)
>  		return -EPERM;
> 
> -	if (mutex_lock_killable(&fan_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&fan_mutex);
> +	rc = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (rc)
> +		return rc;
> 
>  	if (level == TPACPI_FAN_LAST_LEVEL)
>  		level = fan_control_desired_level;
> @@ -8322,7 +8320,6 @@ static int fan_set_level_safe(int level)
>  	if (!rc)
>  		fan_update_desired_level(level);
> 
> -	mutex_unlock(&fan_mutex);
>  	return rc;
>  }
> 
> @@ -8334,8 +8331,10 @@ static int fan_set_enable(void)
>  	if (!fan_control_allowed)
>  		return -EPERM;
> 
> -	if (mutex_lock_killable(&fan_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&fan_mutex);
> +	rc = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (rc)
> +		return rc;
> 
>  	switch (fan_control_access_mode) {
>  	case TPACPI_FAN_WR_ACPI_FANS:
> @@ -8391,8 +8390,6 @@ static int fan_set_enable(void)
>  		rc = -ENXIO;
>  	}
> 
> -	mutex_unlock(&fan_mutex);
> -
>  	if (!rc)
>  		vdbg_printk(TPACPI_DBG_FAN,
>  			"fan control: set fan control register to 0x%02x\n",
> @@ -8407,8 +8404,10 @@ static int fan_set_disable(void)
>  	if (!fan_control_allowed)
>  		return -EPERM;
> 
> -	if (mutex_lock_killable(&fan_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&fan_mutex);
> +	rc = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (rc)
> +		return rc;
> 
>  	rc = 0;
>  	switch (fan_control_access_mode) {
> @@ -8453,7 +8452,6 @@ static int fan_set_disable(void)
>  		vdbg_printk(TPACPI_DBG_FAN,
>  			"fan control: set fan control register to 0\n");
> 
> -	mutex_unlock(&fan_mutex);
>  	return rc;
>  }
> 
> @@ -8464,8 +8462,10 @@ static int fan_set_speed(int speed)
>  	if (!fan_control_allowed)
>  		return -EPERM;
> 
> -	if (mutex_lock_killable(&fan_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&fan_mutex);
> +	rc = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (rc)
> +		return rc;
> 
>  	rc = 0;
>  	switch (fan_control_access_mode) {
> @@ -8499,7 +8499,6 @@ static int fan_set_speed(int speed)
>  		rc = -ENXIO;
>  	}
> 
> -	mutex_unlock(&fan_mutex);
>  	return rc;
>  }
> 
> @@ -8659,8 +8658,10 @@ static ssize_t fan_pwm1_store(struct device *dev,
>  	/* scale down from 0-255 to 0-7 */
>  	newlevel = (s >> 5) & 0x07;
> 
> -	if (mutex_lock_killable(&fan_mutex))
> -		return -ERESTARTSYS;
> +	ACQUIRE(mutex_kill, guard)(&fan_mutex);
> +	rc = ACQUIRE_ERR(mutex_kill, &guard);
> +	if (rc)
> +		return rc;
> 
>  	rc = fan_get_status(&status);
>  	if (!rc && (status &
> @@ -8674,7 +8675,6 @@ static ssize_t fan_pwm1_store(struct device *dev,
>  		}
>  	}
> 
> -	mutex_unlock(&fan_mutex);
>  	return (rc) ? rc : count;
>  }
> 
> @@ -10522,13 +10522,14 @@ static int dytc_profile_set(struct device *dev,
>  	int output;
>  	int err;
> 
> -	err = mutex_lock_interruptible(&dytc_mutex);
> +	ACQUIRE(mutex_intr, guard)(&dytc_mutex);
> +	err = ACQUIRE_ERR(mutex_intr, &guard);
>  	if (err)
>  		return err;
> 
>  	err = convert_profile_to_dytc(profile, &perfmode);
>  	if (err)
> -		goto unlock;
> +		return err;
> 
>  	if (dytc_capabilities & BIT(DYTC_FC_MMC)) {
>  		if (profile == PLATFORM_PROFILE_BALANCED) {
> @@ -10540,18 +10541,18 @@ static int dytc_profile_set(struct device 
> *dev,
>  			 */
>  			err = dytc_cql_command(DYTC_CMD_RESET, &output);
>  			if (err)
> -				goto unlock;
> +				return err;
>  		} else {
>  			/* Determine if we are in CQL mode. This alters the commands we do 
> */
>  			err = dytc_cql_command(DYTC_SET_COMMAND(DYTC_FUNCTION_MMC, 
> perfmode, 1),
>  						&output);
>  			if (err)
> -				goto unlock;
> +				return err;
>  		}
>  	} else if (dytc_capabilities & BIT(DYTC_FC_PSC)) {
>  		err = dytc_command(DYTC_SET_COMMAND(DYTC_FUNCTION_PSC, perfmode, 1), 
> &output);
>  		if (err)
> -			goto unlock;
> +			return err;
> 
>  		/* system supports AMT, activate it when on balanced */
>  		if (dytc_capabilities & BIT(DYTC_FC_AMT))
> @@ -10559,8 +10560,6 @@ static int dytc_profile_set(struct device *dev,
>  	}
>  	/* Success - update current profile */
>  	dytc_current_profile = profile;
> -unlock:
> -	mutex_unlock(&dytc_mutex);
>  	return err;
>  }
> 
> -- 
> 2.55.0.679.g6767b8d81c-goog

I've not come across ACQUIRE & ACQUIRE_KILL before - so this was all new to me.
I went and did some reading and all the above looks good to me, and looks like a valid clean-up.
But my reviewed-by tag does come with reduced value....

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>

Thanks for the learning experience. I will aim to go and try this out (and the other patches in the series) on some HW in the near future.

Mark

[PATCH 3/3] platform/x86: thinkpad_acpi: use __free(kfree) for automatic cleanup

[Dmitry Torokhov]

Use __free(kfree) for local pointer allocations in dispatch_proc_write(),
tpacpi_brightness_get_ecnvram(), and auxmac_init().

This ensures automatic memory cleanup when exiting function scope and
removes explicit kfree() calls on exit paths.

Assisted-by: Antigravity:gemini-3.6-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 40 +++++++--------------
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index 0d0d6fe7eecd..200e20f90a4b 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -885,7 +885,6 @@ static ssize_t dispatch_proc_write(struct file *file,
 			size_t count, loff_t *pos)
 {
 	struct ibm_struct *ibm = pde_data(file_inode(file));
-	char *kernbuf;
 	int ret;
 
 	if (!ibm || !ibm->write)
@@ -893,16 +892,15 @@ static ssize_t dispatch_proc_write(struct file *file,
 	if (count > PAGE_SIZE - 1)
 		return -EINVAL;
 
-	kernbuf = memdup_user_nul(userbuf, count);
+	char *kernbuf __free(kfree) = memdup_user_nul(userbuf, count);
 	if (IS_ERR(kernbuf))
 		return PTR_ERR(kernbuf);
-	ret = ibm->write(kernbuf);
-	if (ret == 0)
-		ret = count;
 
-	kfree(kernbuf);
+	ret = ibm->write(kernbuf);
+	if (ret)
+		return ret;
 
-	return ret;
+	return count;
 }
 
 static const struct proc_ops dispatch_proc_ops = {
@@ -6628,26 +6626,21 @@ static const struct backlight_ops ibm_backlight_data = {
 static int __init tpacpi_evaluate_bcl(struct acpi_device *adev, void *not_used)
 {
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	union acpi_object *obj;
 	acpi_status status;
-	int rc;
 
 	status = acpi_evaluate_object(adev->handle, "_BCL", NULL, &buffer);
 	if (ACPI_FAILURE(status))
 		return 0;
 
-	obj = buffer.pointer;
+	union acpi_object *obj __free(kfree) = buffer.pointer;
 	if (!obj || obj->type != ACPI_TYPE_PACKAGE) {
 		acpi_handle_info(adev->handle,
 				 "Unknown _BCL data, please report this to %s\n",
 				 TPACPI_MAIL);
-		rc = 0;
-	} else {
-		rc = obj->package.count;
+		return 0;
 	}
-	kfree(obj);
 
-	return rc;
+	return obj->package.count;
 }
 
 /*
@@ -10989,24 +10982,23 @@ static int auxmac_init(struct ibm_init_struct *iibm)
 {
 	acpi_status status;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	union acpi_object *obj;
 
-	status = acpi_evaluate_object(NULL, "\\MACA", NULL, &buffer);
+	strscpy(auxmac, "unavailable", sizeof(auxmac));
 
+	status = acpi_evaluate_object(NULL, "\\MACA", NULL, &buffer);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	obj = buffer.pointer;
-
+	union acpi_object *obj __free(kfree) = buffer.pointer;
 	if (obj->type != ACPI_TYPE_STRING || obj->string.length != AUXMAC_STRLEN) {
 		pr_info("Invalid buffer for MAC address pass-through.\n");
-		goto auxmacinvalid;
+		return 0;
 	}
 
 	if (obj->string.pointer[AUXMAC_BEGIN_MARKER] != '#' ||
 	    obj->string.pointer[AUXMAC_END_MARKER] != '#') {
 		pr_info("Invalid header for MAC address pass-through.\n");
-		goto auxmacinvalid;
+		return 0;
 	}
 
 	if (strncmp(obj->string.pointer + AUXMAC_START, "XXXXXXXXXXXX", AUXMAC_LEN) != 0)
@@ -11014,13 +11006,7 @@ static int auxmac_init(struct ibm_init_struct *iibm)
 	else
 		strscpy(auxmac, "disabled", sizeof(auxmac));
 
-free:
-	kfree(obj);
 	return 0;
-
-auxmacinvalid:
-	strscpy(auxmac, "unavailable", sizeof(auxmac));
-	goto free;
 }
 
 static struct ibm_struct auxmac_data = {
-- 
2.55.0.679.g6767b8d81c-goog

Re: [PATCH 3/3] platform/x86: thinkpad_acpi: use __free(kfree) for automatic cleanup

[Mark Pearson]

Thanks Dmitry,

On Thu, Aug 6, 2026, at 2:19 AM, Dmitry Torokhov wrote:
> Use __free(kfree) for local pointer allocations in dispatch_proc_write(),
> tpacpi_brightness_get_ecnvram(), and auxmac_init().
>
> This ensures automatic memory cleanup when exiting function scope and
> removes explicit kfree() calls on exit paths.
>
> Assisted-by: Antigravity:gemini-3.6-flash
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/platform/x86/lenovo/thinkpad_acpi.c | 40 +++++++--------------
>  1 file changed, 13 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c 
> b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> index 0d0d6fe7eecd..200e20f90a4b 100644
> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> @@ -885,7 +885,6 @@ static ssize_t dispatch_proc_write(struct file 
> *file,
>  			size_t count, loff_t *pos)
>  {
>  	struct ibm_struct *ibm = pde_data(file_inode(file));
> -	char *kernbuf;
>  	int ret;
> 
>  	if (!ibm || !ibm->write)
> @@ -893,16 +892,15 @@ static ssize_t dispatch_proc_write(struct file *file,
>  	if (count > PAGE_SIZE - 1)
>  		return -EINVAL;
> 
> -	kernbuf = memdup_user_nul(userbuf, count);
> +	char *kernbuf __free(kfree) = memdup_user_nul(userbuf, count);
>  	if (IS_ERR(kernbuf))
>  		return PTR_ERR(kernbuf);
> -	ret = ibm->write(kernbuf);
> -	if (ret == 0)
> -		ret = count;
> 
> -	kfree(kernbuf);
> +	ret = ibm->write(kernbuf);
> +	if (ret)
> +		return ret;
> 
> -	return ret;
> +	return count;
>  }
> 
>  static const struct proc_ops dispatch_proc_ops = {
> @@ -6628,26 +6626,21 @@ static const struct backlight_ops ibm_backlight_data = {
>  static int __init tpacpi_evaluate_bcl(struct acpi_device *adev, void *not_used)
>  {
>  	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> -	union acpi_object *obj;
>  	acpi_status status;
> -	int rc;
> 
>  	status = acpi_evaluate_object(adev->handle, "_BCL", NULL, &buffer);
>  	if (ACPI_FAILURE(status))
>  		return 0;
> 
> -	obj = buffer.pointer;
> +	union acpi_object *obj __free(kfree) = buffer.pointer;
>  	if (!obj || obj->type != ACPI_TYPE_PACKAGE) {
>  		acpi_handle_info(adev->handle,
>  				 "Unknown _BCL data, please report this to %s\n",
>  				 TPACPI_MAIL);
> -		rc = 0;
> -	} else {
> -		rc = obj->package.count;
> +		return 0;
>  	}
> -	kfree(obj);
> 
> -	return rc;
> +	return obj->package.count;
>  }
> 
>  /*
> @@ -10989,24 +10982,23 @@ static int auxmac_init(struct ibm_init_struct *iibm)
>  {
>  	acpi_status status;
>  	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> -	union acpi_object *obj;
> 
> -	status = acpi_evaluate_object(NULL, "\\MACA", NULL, &buffer);
> +	strscpy(auxmac, "unavailable", sizeof(auxmac));
> 
> +	status = acpi_evaluate_object(NULL, "\\MACA", NULL, &buffer);
>  	if (ACPI_FAILURE(status))
>  		return -ENODEV;
> 
> -	obj = buffer.pointer;
> -
> +	union acpi_object *obj __free(kfree) = buffer.pointer;
>  	if (obj->type != ACPI_TYPE_STRING || obj->string.length != AUXMAC_STRLEN) {
>  		pr_info("Invalid buffer for MAC address pass-through.\n");
> -		goto auxmacinvalid;
> +		return 0;
>  	}
> 
>  	if (obj->string.pointer[AUXMAC_BEGIN_MARKER] != '#' ||
>  	    obj->string.pointer[AUXMAC_END_MARKER] != '#') {
>  		pr_info("Invalid header for MAC address pass-through.\n");
> -		goto auxmacinvalid;
> +		return 0;
>  	}
> 
>  	if (strncmp(obj->string.pointer + AUXMAC_START, "XXXXXXXXXXXX", 
> AUXMAC_LEN) != 0)
> @@ -11014,13 +11006,7 @@ static int auxmac_init(struct ibm_init_struct 
> *iibm)
>  	else
>  		strscpy(auxmac, "disabled", sizeof(auxmac));
> 
> -free:
> -	kfree(obj);
>  	return 0;
> -
> -auxmacinvalid:
> -	strscpy(auxmac, "unavailable", sizeof(auxmac));
> -	goto free;
>  }
> 
>  static struct ibm_struct auxmac_data = {
> -- 
> 2.55.0.679.g6767b8d81c-goog

Another kernel implementation I didn't know about.
It all looks good to me, and gives some nice clean-up. Thanks

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Mark

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

Mark

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

[Mark Pearson]

On Wed, Aug 12, 2026, at 2:29 PM, Mark Pearson wrote:
> 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@gmail.com>
>> ---
>>  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@squebb.ca>
>
> Mark
>
Ran a build with all 3 patches on a P14s G7 and couldn't see any problems.
So, for the series:
Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca>