[PATCH RFC 1/4] regmap: convert lock/unlock to a scoped guard

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@xxxxxxx>

The regmap lock and unlock callbacks are invoked directly as
map->lock(map->lock_arg) / map->unlock(map->lock_arg) at every call
site. This open-coded pattern requires manual unlocking on every
return path, which spreads goto out_unlock chains and duplicated
unlock statements throughout the code and is an easy place to leak
the lock on an error path.

Define a scoped guard for the regmap lock in internal.h. The lock and
unlock callbacks are chosen at init time (mutex, spinlock, raw
spinlock, hwspinlock or none) and return void, so an unconditional
DEFINE_GUARD() is sufficient.

Convert all lock/unlock users in regmap.c to guard(regmap)() for
function-scope critical sections and scoped_guard(regmap, ...) where
work must run outside the lock (e.g. regmap_register_patch() calling
regmap_async_complete()). This drops every manual unlock and the
associated goto out_unlock labels with no functional change.

Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
---
 drivers/base/regmap/internal.h |  11 +++
 drivers/base/regmap/regmap.c   | 182 +++++++++++++----------------------------
 2 files changed, 70 insertions(+), 123 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index c73036744468..b023de9c7217 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -10,6 +10,7 @@
 #ifndef _REGMAP_INTERNAL_H
 #define _REGMAP_INTERNAL_H
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/regmap.h>
 #include <linux/fs.h>
@@ -185,6 +186,16 @@ struct regmap {
 	struct hwspinlock *hwlock;
 };
 
+/*
+ * Scoped guard for the regmap lock.  The lock/unlock callbacks are selected
+ * at init time (mutex, spinlock, raw spinlock, hwspinlock or none) and never
+ * fail, so an unconditional guard is sufficient.  Use with guard(regmap)(map)
+ * or scoped_guard(regmap, map) { ... }.
+ */
+DEFINE_GUARD(regmap, struct regmap *,
+	     _T->lock(_T->lock_arg),
+	     _T->unlock(_T->lock_arg))
+
 struct regcache_ops {
 	const char *name;
 	enum regcache_type type;
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 98184a00097a..0b40e1e7817c 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -115,9 +115,8 @@ bool regmap_cached(struct regmap *map, unsigned int reg)
 	if (map->max_register_is_set && reg > map->max_register)
 		return false;
 
-	map->lock(map->lock_arg);
-	ret = regcache_read(map, reg, &val);
-	map->unlock(map->lock_arg);
+	scoped_guard(regmap, map)
+		ret = regcache_read(map, reg, &val);
 	if (ret)
 		return false;
 
@@ -1977,18 +1976,12 @@ int _regmap_write(struct regmap *map, unsigned int reg,
  */
 int regmap_write(struct regmap *map, unsigned int reg, unsigned int val)
 {
-	int ret;
-
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
 
-	map->lock(map->lock_arg);
-
-	ret = _regmap_write(map, reg, val);
-
-	map->unlock(map->lock_arg);
+	guard(regmap)(map);
 
-	return ret;
+	return _regmap_write(map, reg, val);
 }
 EXPORT_SYMBOL_GPL(regmap_write);
 
@@ -2009,7 +2002,7 @@ int regmap_write_async(struct regmap *map, unsigned int reg, unsigned int val)
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	map->async = true;
 
@@ -2017,8 +2010,6 @@ int regmap_write_async(struct regmap *map, unsigned int reg, unsigned int val)
 
 	map->async = false;
 
-	map->unlock(map->lock_arg);
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_write_async);
@@ -2080,20 +2071,14 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
 int regmap_raw_write(struct regmap *map, unsigned int reg,
 		     const void *val, size_t val_len)
 {
-	int ret;
-
 	if (!regmap_can_raw_write(map))
 		return -EINVAL;
 	if (val_len % map->format.val_bytes)
 		return -EINVAL;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
-	ret = _regmap_raw_write(map, reg, val, val_len, false);
-
-	map->unlock(map->lock_arg);
-
-	return ret;
+	return _regmap_raw_write(map, reg, val, val_len, false);
 }
 EXPORT_SYMBOL_GPL(regmap_raw_write);
 
@@ -2211,21 +2196,17 @@ int regmap_noinc_write(struct regmap *map, unsigned int reg,
 	if (val_len == 0)
 		return -EINVAL;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
-	if (!regmap_volatile(map, reg) || !regmap_writeable_noinc(map, reg)) {
-		ret = -EINVAL;
-		goto out_unlock;
-	}
+	if (!regmap_volatile(map, reg) || !regmap_writeable_noinc(map, reg))
+		return -EINVAL;
 
 	/*
 	 * Use the accelerated operation if we can. The val drops the const
 	 * typing in order to facilitate code reuse in regmap_noinc_readwrite().
 	 */
-	if (map->bus->reg_noinc_write) {
-		ret = regmap_noinc_readwrite(map, reg, (void *)val, val_len, true);
-		goto out_unlock;
-	}
+	if (map->bus->reg_noinc_write)
+		return regmap_noinc_readwrite(map, reg, (void *)val, val_len, true);
 
 	while (val_len) {
 		if (map->max_raw_write && map->max_raw_write < val_len)
@@ -2234,14 +2215,12 @@ int regmap_noinc_write(struct regmap *map, unsigned int reg,
 			write_len = val_len;
 		ret = _regmap_raw_write(map, reg, val, write_len, true);
 		if (ret)
-			goto out_unlock;
+			return ret;
 		val = ((u8 *)val) + write_len;
 		val_len -= write_len;
 	}
 
-out_unlock:
-	map->unlock(map->lock_arg);
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(regmap_noinc_write);
 
@@ -2357,7 +2336,8 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 	 * single write operations.
 	 */
 	if (!map->write || !map->format.parse_inplace) {
-		map->lock(map->lock_arg);
+		guard(regmap)(map);
+
 		for (i = 0; i < val_count; i++) {
 			unsigned int ival;
 
@@ -2372,18 +2352,15 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 				ival = *(u32 *)(val + (i * val_bytes));
 				break;
 			default:
-				ret = -EINVAL;
-				goto out;
+				return -EINVAL;
 			}
 
 			ret = _regmap_write(map,
 					    reg + regmap_get_offset(map, i),
 					    ival);
 			if (ret != 0)
-				goto out;
+				return ret;
 		}
-out:
-		map->unlock(map->lock_arg);
 	} else {
 		void *wval;
 
@@ -2653,15 +2630,9 @@ static int _regmap_multi_reg_write(struct regmap *map,
 int regmap_multi_reg_write(struct regmap *map, const struct reg_sequence *regs,
 			   int num_regs)
 {
-	int ret;
-
-	map->lock(map->lock_arg);
-
-	ret = _regmap_multi_reg_write(map, regs, num_regs);
-
-	map->unlock(map->lock_arg);
+	guard(regmap)(map);
 
-	return ret;
+	return _regmap_multi_reg_write(map, regs, num_regs);
 }
 EXPORT_SYMBOL_GPL(regmap_multi_reg_write);
 
@@ -2690,7 +2661,7 @@ int regmap_multi_reg_write_bypassed(struct regmap *map,
 	int ret;
 	bool bypass;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	bypass = map->cache_bypass;
 	map->cache_bypass = true;
@@ -2699,8 +2670,6 @@ int regmap_multi_reg_write_bypassed(struct regmap *map,
 
 	map->cache_bypass = bypass;
 
-	map->unlock(map->lock_arg);
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_multi_reg_write_bypassed);
@@ -2737,7 +2706,7 @@ int regmap_raw_write_async(struct regmap *map, unsigned int reg,
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	map->async = true;
 
@@ -2745,8 +2714,6 @@ int regmap_raw_write_async(struct regmap *map, unsigned int reg,
 
 	map->async = false;
 
-	map->unlock(map->lock_arg);
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_raw_write_async);
@@ -2863,18 +2830,12 @@ static int _regmap_read(struct regmap *map, unsigned int reg,
  */
 int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val)
 {
-	int ret;
-
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
-	ret = _regmap_read(map, reg, val);
-
-	map->unlock(map->lock_arg);
-
-	return ret;
+	return _regmap_read(map, reg, val);
 }
 EXPORT_SYMBOL_GPL(regmap_read);
 
@@ -2897,7 +2858,7 @@ int regmap_read_bypassed(struct regmap *map, unsigned int reg, unsigned int *val
 	if (!IS_ALIGNED(reg, map->reg_stride))
 		return -EINVAL;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	bypass = map->cache_bypass;
 	cache_only = map->cache_only;
@@ -2909,8 +2870,6 @@ int regmap_read_bypassed(struct regmap *map, unsigned int reg, unsigned int *val
 	map->cache_bypass = bypass;
 	map->cache_only = cache_only;
 
-	map->unlock(map->lock_arg);
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_read_bypassed);
@@ -2941,22 +2900,18 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 	if (val_count == 0)
 		return -EINVAL;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	if (regmap_volatile_range(map, reg, val_count) || map->cache_bypass ||
 	    map->cache_type == REGCACHE_NONE) {
 		size_t chunk_count, chunk_bytes;
 		size_t chunk_regs = val_count;
 
-		if (!map->cache_bypass && map->cache_only) {
-			ret = -EBUSY;
-			goto out;
-		}
+		if (!map->cache_bypass && map->cache_only)
+			return -EBUSY;
 
-		if (!map->read) {
-			ret = -ENOTSUPP;
-			goto out;
-		}
+		if (!map->read)
+			return -ENOTSUPP;
 
 		if (map->use_single_read)
 			chunk_regs = 1;
@@ -2970,7 +2925,7 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 		for (i = 0; i < chunk_count; i++) {
 			ret = _regmap_raw_read(map, reg, val, chunk_bytes, false);
 			if (ret != 0)
-				goto out;
+				return ret;
 
 			reg += regmap_get_offset(map, chunk_regs);
 			val += chunk_bytes;
@@ -2981,7 +2936,7 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 		if (val_len) {
 			ret = _regmap_raw_read(map, reg, val, val_len, false);
 			if (ret != 0)
-				goto out;
+				return ret;
 		}
 	} else {
 		/* Otherwise go word by word for the cache; should be low
@@ -2991,16 +2946,13 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 			ret = _regmap_read(map, reg + regmap_get_offset(map, i),
 					   &v);
 			if (ret != 0)
-				goto out;
+				return ret;
 
 			map->format.format_val(val + (i * val_bytes), v, 0);
 		}
 	}
 
- out:
-	map->unlock(map->lock_arg);
-
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(regmap_raw_read);
 
@@ -3041,12 +2993,10 @@ int regmap_noinc_read(struct regmap *map, unsigned int reg,
 	if (val_len == 0)
 		return -EINVAL;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
-	if (!regmap_volatile(map, reg) || !regmap_readable_noinc(map, reg)) {
-		ret = -EINVAL;
-		goto out_unlock;
-	}
+	if (!regmap_volatile(map, reg) || !regmap_readable_noinc(map, reg))
+		return -EINVAL;
 
 	/*
 	 * We have not defined the FIFO semantics for cache, as the
@@ -3054,16 +3004,12 @@ int regmap_noinc_read(struct regmap *map, unsigned int reg,
 	 * written value? Just avoid this by always reading the FIFO
 	 * even when using cache. Cache only will not work.
 	 */
-	if (!map->cache_bypass && map->cache_only) {
-		ret = -EBUSY;
-		goto out_unlock;
-	}
+	if (!map->cache_bypass && map->cache_only)
+		return -EBUSY;
 
 	/* Use the accelerated operation if we can */
-	if (map->bus->reg_noinc_read) {
-		ret = regmap_noinc_readwrite(map, reg, val, val_len, false);
-		goto out_unlock;
-	}
+	if (map->bus->reg_noinc_read)
+		return regmap_noinc_readwrite(map, reg, val, val_len, false);
 
 	while (val_len) {
 		if (map->max_raw_read && map->max_raw_read < val_len)
@@ -3072,14 +3018,12 @@ int regmap_noinc_read(struct regmap *map, unsigned int reg,
 			read_len = val_len;
 		ret = _regmap_raw_read(map, reg, val, read_len, true);
 		if (ret)
-			goto out_unlock;
+			return ret;
 		val = ((u8 *)val) + read_len;
 		val_len -= read_len;
 	}
 
-out_unlock:
-	map->unlock(map->lock_arg);
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(regmap_noinc_read);
 
@@ -3149,22 +3093,20 @@ static int _regmap_bulk_read(struct regmap *map, unsigned int reg,
 	u8 *u8 = val;
 	int ret, i;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	for (i = 0; i < val_count; i++) {
 		unsigned int ival;
 
 		if (regs) {
-			if (!IS_ALIGNED(regs[i], map->reg_stride)) {
-				ret = -EINVAL;
-				goto out;
-			}
+			if (!IS_ALIGNED(regs[i], map->reg_stride))
+				return -EINVAL;
 			ret = _regmap_read(map, regs[i], &ival);
 		} else {
 			ret = _regmap_read(map, reg + regmap_get_offset(map, i), &ival);
 		}
 		if (ret != 0)
-			goto out;
+			return ret;
 
 		switch (map->format.val_bytes) {
 		case 4:
@@ -3177,13 +3119,11 @@ static int _regmap_bulk_read(struct regmap *map, unsigned int reg,
 			u8[i] = ival;
 			break;
 		default:
-			ret = -EINVAL;
-			goto out;
+			return -EINVAL;
 		}
 	}
-out:
-	map->unlock(map->lock_arg);
-	return ret;
+
+	return 0;
 }
 
 /**
@@ -3310,7 +3250,7 @@ int regmap_update_bits_base(struct regmap *map, unsigned int reg,
 {
 	int ret;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	map->async = async;
 
@@ -3318,8 +3258,6 @@ int regmap_update_bits_base(struct regmap *map, unsigned int reg,
 
 	map->async = false;
 
-	map->unlock(map->lock_arg);
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_update_bits_base);
@@ -3452,19 +3390,17 @@ int regmap_register_patch(struct regmap *map, const struct reg_sequence *regs,
 		return -ENOMEM;
 	}
 
-	map->lock(map->lock_arg);
+	scoped_guard(regmap, map) {
+		bypass = map->cache_bypass;
 
-	bypass = map->cache_bypass;
+		map->cache_bypass = true;
+		map->async = true;
 
-	map->cache_bypass = true;
-	map->async = true;
-
-	ret = _regmap_multi_reg_write(map, regs, num_regs);
+		ret = _regmap_multi_reg_write(map, regs, num_regs);
 
-	map->async = false;
-	map->cache_bypass = bypass;
-
-	map->unlock(map->lock_arg);
+		map->async = false;
+		map->cache_bypass = bypass;
+	}
 
 	regmap_async_complete(map);
 

-- 
2.51.0