* [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-06 6:19 ` [PATCH 3/3] platform/x86: thinkpad_acpi: use __free(kfree) for automatic cleanup Dmitry Torokhov
1 sibling, 0 replies; 3+ 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 related [flat|nested] 3+ 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
1 sibling, 0 replies; 3+ 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 related [flat|nested] 3+ messages in thread