* [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex)
@ 2026-08-06 6:19 Dmitry Torokhov
2026-08-06 6:19 ` [PATCH 2/3] platform/x86: thinkpad_acpi: convert conditional mutex locks to ACQUIRE_ERR() Dmitry Torokhov
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2026-08-06 6:19 UTC (permalink / raw)
To: Mark Pearson, Derek J. Clark
Cc: Henrique de Moraes Holschuh, Hans de Goede, Ilpo Järvinen,
Nitin Joshi, platform-driver-x86, ibm-acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] platform/x86: thinkpad_acpi: convert conditional mutex locks to ACQUIRE_ERR()
2026-08-06 6:19 [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex) Dmitry Torokhov
@ 2026-08-06 6:19 ` Dmitry Torokhov
2026-08-12 18:53 ` Mark Pearson
2026-08-06 6:19 ` [PATCH 3/3] platform/x86: thinkpad_acpi: use __free(kfree) for automatic cleanup Dmitry Torokhov
2026-08-12 18:29 ` [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex) Mark Pearson
2 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2026-08-06 6:19 UTC (permalink / raw)
To: Mark Pearson, Derek J. Clark
Cc: Henrique de Moraes Holschuh, Hans de Goede, Ilpo Järvinen,
Nitin Joshi, platform-driver-x86, ibm-acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] platform/x86: thinkpad_acpi: use __free(kfree) for automatic cleanup
2026-08-06 6:19 [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex) Dmitry Torokhov
2026-08-06 6:19 ` [PATCH 2/3] platform/x86: thinkpad_acpi: convert conditional mutex locks to ACQUIRE_ERR() Dmitry Torokhov
@ 2026-08-06 6:19 ` Dmitry Torokhov
2026-08-12 19:02 ` Mark Pearson
2026-08-12 18:29 ` [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex) Mark Pearson
2 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2026-08-06 6:19 UTC (permalink / raw)
To: Mark Pearson, Derek J. Clark
Cc: Henrique de Moraes Holschuh, Hans de Goede, Ilpo Järvinen,
Nitin Joshi, platform-driver-x86, ibm-acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex)
2026-08-06 6:19 [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex) Dmitry Torokhov
2026-08-06 6:19 ` [PATCH 2/3] platform/x86: thinkpad_acpi: convert conditional mutex locks to ACQUIRE_ERR() Dmitry Torokhov
2026-08-06 6:19 ` [PATCH 3/3] platform/x86: thinkpad_acpi: use __free(kfree) for automatic cleanup Dmitry Torokhov
@ 2026-08-12 18:29 ` Mark Pearson
2026-08-12 19:55 ` [ibm-acpi-devel] " Mark Pearson
2 siblings, 1 reply; 7+ messages in thread
From: Mark Pearson @ 2026-08-12 18:29 UTC (permalink / raw)
To: Dmitry Torokhov, Derek J . Clark
Cc: Henrique de Moraes Holschuh, Hans de Goede, Ilpo Järvinen,
Nitin Joshi, platform-driver-x86, ibm-acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] platform/x86: thinkpad_acpi: convert conditional mutex locks to ACQUIRE_ERR()
2026-08-06 6:19 ` [PATCH 2/3] platform/x86: thinkpad_acpi: convert conditional mutex locks to ACQUIRE_ERR() Dmitry Torokhov
@ 2026-08-12 18:53 ` Mark Pearson
0 siblings, 0 replies; 7+ messages in thread
From: Mark Pearson @ 2026-08-12 18:53 UTC (permalink / raw)
To: Dmitry Torokhov, Derek J . Clark
Cc: Henrique de Moraes Holschuh, Hans de Goede, Ilpo Järvinen,
Nitin Joshi, platform-driver-x86, ibm-acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] platform/x86: thinkpad_acpi: use __free(kfree) for automatic cleanup
2026-08-06 6:19 ` [PATCH 3/3] platform/x86: thinkpad_acpi: use __free(kfree) for automatic cleanup Dmitry Torokhov
@ 2026-08-12 19:02 ` Mark Pearson
0 siblings, 0 replies; 7+ messages in thread
From: Mark Pearson @ 2026-08-12 19:02 UTC (permalink / raw)
To: Dmitry Torokhov, Derek J . Clark
Cc: Henrique de Moraes Holschuh, Hans de Goede, Ilpo Järvinen,
Nitin Joshi, platform-driver-x86, ibm-acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [ibm-acpi-devel] [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex)
2026-08-12 18:29 ` [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex) Mark Pearson
@ 2026-08-12 19:55 ` Mark Pearson
0 siblings, 0 replies; 7+ messages in thread
From: Mark Pearson @ 2026-08-12 19:55 UTC (permalink / raw)
To: Dmitry Torokhov, Derek J . Clark
Cc: linux-kernel, platform-driver-x86, ibm-acpi-devel,
Henrique de Moraes Holschuh, Ilpo Järvinen, Nitin Joshi,
Hans de Goede
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-12 19:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-06 6:19 [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex) Dmitry Torokhov
2026-08-06 6:19 ` [PATCH 2/3] platform/x86: thinkpad_acpi: convert conditional mutex locks to ACQUIRE_ERR() Dmitry Torokhov
2026-08-12 18:53 ` Mark Pearson
2026-08-06 6:19 ` [PATCH 3/3] platform/x86: thinkpad_acpi: use __free(kfree) for automatic cleanup Dmitry Torokhov
2026-08-12 19:02 ` Mark Pearson
2026-08-12 18:29 ` [PATCH 1/3] platform/x86: thinkpad_acpi: convert mutex_lock() to guard(mutex) Mark Pearson
2026-08-12 19:55 ` [ibm-acpi-devel] " Mark Pearson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®