[PATCH RFC v2 0/4] regmap: convert map->lock/unlock users to a scoped guard

[PATCH RFC v2 0/4] regmap: convert map->lock/unlock users to a scoped guard

[Peng Fan (OSS)]

The regmap lock is taken by calling the lock and unlock callbacks
directly at every call site:

	map->lock(map->lock_arg);
	...
	map->unlock(map->lock_arg);

The open-coded pattern forces every error path to unlock by hand,
which spreads goto out_unlock chains and duplicated unlock statements
throughout regmap.c and regcache.c and makes it easy to leak the lock
on a newly added return path.

Define a scoped guard for the regmap lock (DEFINE_GUARD) in internal.h
and convert the users to it. The lock and unlock callbacks are chosen
at init time (mutex, spinlock, raw spinlock, hwspinlock or none) and
return void, so an unconditional guard is sufficient. Function-scope
critical sections use guard(regmap)(); sites that must run work after
the lock is dropped - for example regmap_register_patch() calling
regmap_async_complete() and the debugfs cache_only handler calling
regcache_sync() (which takes the lock itself) - use
scoped_guard(regmap, ...) so the trailing work stays outside the
guarded region.

regcache_sync() and regcache_sync_region() are deliberately left
unconverted: they already use a single goto out unlock path, so a
guard would save nothing while forcing either a goto inside a
scoped_guard scope or a control-flow rewrite, neither of which is an
improvement.

This patchset removes many manual unlock statements together with the
associated goto out_unlock labels.

The series is split to keep the regcache: and regmap: changes on their
own commits and to preserve independent revertibility:

  1. define the guard and convert regmap.c
  2. convert regcache.c (except the sync helpers, see above)
  3. convert the regcache rbtree debugfs dump
  4. convert the regmap debugfs write handlers

No functional change.

Tested with the regmap KUnit suite (drivers/base/regmap/regmap-kunit.c)
under ARCH=um: 551/551 tests pass, and 551/551 again with lockdep
(PROVE_LOCKING, DEBUG_LOCK_ALLOC, DEBUG_ATOMIC_SLEEP) enabled with no
splats. Built clean with sparse (C=1) showing no lock-context
imbalance warnings.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v2:
- Drop the regcache_sync() and regcache_sync_region() conversions.
  They used a goto out unlock path that a scoped_guard cannot express
  cleanly; leaving them as map->lock()/unlock() avoids mixing goto with
  a guard scope (per Mark's review). No other functional change.
- Link to v1: https://lore.kernel.org/r/20260921-regmap-lock-guard-v1-0-cdbd97b46074@nxp.com

---
Peng Fan (4):
      regmap: convert lock/unlock to a scoped guard
      regcache: use the regmap scoped lock guard
      regcache: rbtree: use the regmap scoped lock guard
      regmap: debugfs: use the regmap scoped lock guard

 drivers/base/regmap/internal.h        |  11 ++
 drivers/base/regmap/regcache-rbtree.c |   4 +-
 drivers/base/regmap/regcache.c        |  43 +++-----
 drivers/base/regmap/regmap-debugfs.c  |  24 ++---
 drivers/base/regmap/regmap.c          | 182 +++++++++++-----------------------
 5 files changed, 95 insertions(+), 169 deletions(-)
---
base-commit: 0d9d0dbf2fddcff5859d623e90ca73c4054276e1
change-id: 20260921-regmap-lock-guard-939bbf9dbeee

Best regards,
--  
Peng Fan <peng.fan@nxp.com>

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

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

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@nxp.com>
---
 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..ea375cc8ae87 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

[PATCH RFC v2 2/4] regcache: use the regmap scoped lock guard

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Convert the open-coded map->lock()/map->unlock() users in regcache.c
to the regmap scoped guard introduced for regmap.c. Use
scoped_guard(regmap, ...) in regcache_init() and regcache_exit(),
where the locked region is a subsection of the function, and
guard(regmap)() for the function-scope critical sections.

regcache_sync() and regcache_sync_region() are left as-is: they
already use a single goto out unlock path, so converting them would
require either mixing a goto with a scoped_guard scope or restructuring
their control flow, neither of which is an improvement.

regcache_init() keeps its goto err_* cleanup ladder alongside the
scoped_guard(). This does not conflict with the cleanup.h guidance
against mixing goto with cleanup helpers: that rule concerns a single
resource whose lifetime is managed by a goto. Here the goto ladder
only unwinds the allocation / hw-init path (kfree(map->reg_defaults),
regcache_hw_exit(), ->exit()), while the scoped_guard() covers a
different resource - the transient map lock around each callback - that
is acquired and released within its one-line scope and is never held
across a goto. The two mechanisms manage independent resources and no
goto ever crosses a live guard.

No functional change.

Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/base/regmap/regcache.c | 43 ++++++++++++++----------------------------
 1 file changed, 14 insertions(+), 29 deletions(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 0d58d900a2ca..d030fcbe11d9 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -225,9 +225,8 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
 	if (map->cache_ops->init) {
 		dev_dbg(map->dev, "Initializing %s cache\n",
 			map->cache_ops->name);
-		map->lock(map->lock_arg);
-		ret = map->cache_ops->init(map);
-		map->unlock(map->lock_arg);
+		scoped_guard(regmap, map)
+			ret = map->cache_ops->init(map);
 		if (ret)
 			goto err_free_reg_defaults;
 	}
@@ -246,9 +245,8 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
 	if (map->cache_ops->populate &&
 	    (map->num_reg_defaults || map->reg_default_cb)) {
 		dev_dbg(map->dev, "Populating %s cache\n", map->cache_ops->name);
-		map->lock(map->lock_arg);
-		ret = map->cache_ops->populate(map);
-		map->unlock(map->lock_arg);
+		scoped_guard(regmap, map)
+			ret = map->cache_ops->populate(map);
 		if (ret)
 			goto err_free;
 	}
@@ -259,9 +257,8 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
 err_exit:
 	if (map->cache_ops->exit) {
 		dev_dbg(map->dev, "Destroying %s cache\n", map->cache_ops->name);
-		map->lock(map->lock_arg);
-		map->cache_ops->exit(map);
-		map->unlock(map->lock_arg);
+		scoped_guard(regmap, map)
+			map->cache_ops->exit(map);
 	}
 err_free_reg_defaults:
 	kfree(map->reg_defaults);
@@ -281,9 +278,8 @@ void regcache_exit(struct regmap *map)
 	if (map->cache_ops->exit) {
 		dev_dbg(map->dev, "Destroying %s cache\n",
 			map->cache_ops->name);
-		map->lock(map->lock_arg);
-		map->cache_ops->exit(map);
-		map->unlock(map->lock_arg);
+		scoped_guard(regmap, map)
+			map->cache_ops->exit(map);
 	}
 
 	kfree(map->reg_defaults);
@@ -584,20 +580,14 @@ EXPORT_SYMBOL_GPL(regcache_sync_region);
 int regcache_drop_region(struct regmap *map, unsigned int min,
 			 unsigned int max)
 {
-	int ret = 0;
-
 	if (!map->cache_ops || !map->cache_ops->drop)
 		return -EINVAL;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	trace_regcache_drop_region(map, min, max);
 
-	ret = map->cache_ops->drop(map, min, max);
-
-	map->unlock(map->lock_arg);
-
-	return ret;
+	return map->cache_ops->drop(map, min, max);
 }
 EXPORT_SYMBOL_GPL(regcache_drop_region);
 
@@ -615,12 +605,11 @@ EXPORT_SYMBOL_GPL(regcache_drop_region);
  */
 void regcache_cache_only(struct regmap *map, bool enable)
 {
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 	WARN_ON(map->cache_type != REGCACHE_NONE &&
 		map->cache_bypass && enable);
 	map->cache_only = enable;
 	trace_regmap_cache_only(map, enable);
-	map->unlock(map->lock_arg);
 }
 EXPORT_SYMBOL_GPL(regcache_cache_only);
 
@@ -639,10 +628,9 @@ EXPORT_SYMBOL_GPL(regcache_cache_only);
  */
 void regcache_mark_dirty(struct regmap *map)
 {
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 	map->cache_dirty = true;
 	map->no_sync_defaults = true;
-	map->unlock(map->lock_arg);
 }
 EXPORT_SYMBOL_GPL(regcache_mark_dirty);
 
@@ -659,11 +647,10 @@ EXPORT_SYMBOL_GPL(regcache_mark_dirty);
  */
 void regcache_cache_bypass(struct regmap *map, bool enable)
 {
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 	WARN_ON(map->cache_only && enable);
 	map->cache_bypass = enable;
 	trace_regmap_cache_bypass(map, enable);
-	map->unlock(map->lock_arg);
 }
 EXPORT_SYMBOL_GPL(regcache_cache_bypass);
 
@@ -680,12 +667,10 @@ bool regcache_reg_cached(struct regmap *map, unsigned int reg)
 	unsigned int val;
 	int ret;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	ret = regcache_read(map, reg, &val);
 
-	map->unlock(map->lock_arg);
-
 	return ret == 0;
 }
 EXPORT_SYMBOL_GPL(regcache_reg_cached);

-- 
2.51.0

Re: [PATCH RFC v2 2/4] regcache: use the regmap scoped lock guard

[Peng Fan]

On Tue, Sep 22, 2026 at 07:10:56PM +0800, Peng Fan (OSS) wrote:
>From: Peng Fan <peng.fan@nxp.com>
>
...
>diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
>index 0d58d900a2ca..d030fcbe11d9 100644
>--- a/drivers/base/regmap/regcache.c
>+++ b/drivers/base/regmap/regcache.c
>@@ -225,9 +225,8 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
> 	if (map->cache_ops->init) {
> 		dev_dbg(map->dev, "Initializing %s cache\n",
> 			map->cache_ops->name);
>-		map->lock(map->lock_arg);
>-		ret = map->cache_ops->init(map);
>-		map->unlock(map->lock_arg);
>+		scoped_guard(regmap, map)
>+			ret = map->cache_ops->init(map);
> 		if (ret)
> 			goto err_free_reg_defaults;

Sashiko reported that goto and scoped_guard should not be used in one
function per cleanup.h.

Actually goto does not break scoped_guard. Anyway, to follow cleanup.h
rule, I may need to use a helper saying:

int regmap_map_cache_ops_init(regmap)
{
	guard(regmap)(map);

	return regmap->cache_ops->init(map);
}

I will look into make the helper to support other cache_ops in V3. and
avoid mix goto and scoped_guard in one function.

Thanks
Peng


> 	}
>@@ -246,9 +245,8 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
> 	if (map->cache_ops->populate &&
> 	    (map->num_reg_defaults || map->reg_default_cb)) {
> 		dev_dbg(map->dev, "Populating %s cache\n", map->cache_ops->name);
>-		map->lock(map->lock_arg);
>-		ret = map->cache_ops->populate(map);
>-		map->unlock(map->lock_arg);
>+		scoped_guard(regmap, map)
>+			ret = map->cache_ops->populate(map);
> 		if (ret)
> 			goto err_free;
> 	}
>@@ -259,9 +257,8 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
> err_exit:
> 	if (map->cache_ops->exit) {
> 		dev_dbg(map->dev, "Destroying %s cache\n", map->cache_ops->name);
>-		map->lock(map->lock_arg);
>-		map->cache_ops->exit(map);
>-		map->unlock(map->lock_arg);
>+		scoped_guard(regmap, map)
>+			map->cache_ops->exit(map);
> 	}
> err_free_reg_defaults:
> 	kfree(map->reg_defaults);
>@@ -281,9 +278,8 @@ void regcache_exit(struct regmap *map)
> 	if (map->cache_ops->exit) {
> 		dev_dbg(map->dev, "Destroying %s cache\n",
> 			map->cache_ops->name);
>-		map->lock(map->lock_arg);
>-		map->cache_ops->exit(map);
>-		map->unlock(map->lock_arg);
>+		scoped_guard(regmap, map)
>+			map->cache_ops->exit(map);
> 	}
> 
> 	kfree(map->reg_defaults);
>@@ -584,20 +580,14 @@ EXPORT_SYMBOL_GPL(regcache_sync_region);
> int regcache_drop_region(struct regmap *map, unsigned int min,
> 			 unsigned int max)
> {
>-	int ret = 0;
>-
> 	if (!map->cache_ops || !map->cache_ops->drop)
> 		return -EINVAL;
> 
>-	map->lock(map->lock_arg);
>+	guard(regmap)(map);
> 
> 	trace_regcache_drop_region(map, min, max);
> 
>-	ret = map->cache_ops->drop(map, min, max);
>-
>-	map->unlock(map->lock_arg);
>-
>-	return ret;
>+	return map->cache_ops->drop(map, min, max);
> }
> EXPORT_SYMBOL_GPL(regcache_drop_region);
> 
>@@ -615,12 +605,11 @@ EXPORT_SYMBOL_GPL(regcache_drop_region);
>  */
> void regcache_cache_only(struct regmap *map, bool enable)
> {
>-	map->lock(map->lock_arg);
>+	guard(regmap)(map);
> 	WARN_ON(map->cache_type != REGCACHE_NONE &&
> 		map->cache_bypass && enable);
> 	map->cache_only = enable;
> 	trace_regmap_cache_only(map, enable);
>-	map->unlock(map->lock_arg);
> }
> EXPORT_SYMBOL_GPL(regcache_cache_only);
> 
>@@ -639,10 +628,9 @@ EXPORT_SYMBOL_GPL(regcache_cache_only);
>  */
> void regcache_mark_dirty(struct regmap *map)
> {
>-	map->lock(map->lock_arg);
>+	guard(regmap)(map);
> 	map->cache_dirty = true;
> 	map->no_sync_defaults = true;
>-	map->unlock(map->lock_arg);
> }
> EXPORT_SYMBOL_GPL(regcache_mark_dirty);
> 
>@@ -659,11 +647,10 @@ EXPORT_SYMBOL_GPL(regcache_mark_dirty);
>  */
> void regcache_cache_bypass(struct regmap *map, bool enable)
> {
>-	map->lock(map->lock_arg);
>+	guard(regmap)(map);
> 	WARN_ON(map->cache_only && enable);
> 	map->cache_bypass = enable;
> 	trace_regmap_cache_bypass(map, enable);
>-	map->unlock(map->lock_arg);
> }
> EXPORT_SYMBOL_GPL(regcache_cache_bypass);
> 
>@@ -680,12 +667,10 @@ bool regcache_reg_cached(struct regmap *map, unsigned int reg)
> 	unsigned int val;
> 	int ret;
> 
>-	map->lock(map->lock_arg);
>+	guard(regmap)(map);
> 
> 	ret = regcache_read(map, reg, &val);
> 
>-	map->unlock(map->lock_arg);
>-
> 	return ret == 0;
> }
> EXPORT_SYMBOL_GPL(regcache_reg_cached);
>
>-- 
>2.51.0
>
>

[PATCH RFC v2 3/4] regcache: rbtree: use the regmap scoped lock guard

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Convert the open-coded map->lock()/map->unlock() pair in rbtree_show()
to guard(regmap)(). The locked region spans the whole function body up
to the single return, so a function-scope guard drops the manual
unlock with no functional change.

Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/base/regmap/regcache-rbtree.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
index 520d5f8ba3cd..e2feed6dbecf 100644
--- a/drivers/base/regmap/regcache-rbtree.c
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -141,7 +141,7 @@ static int rbtree_show(struct seq_file *s, void *ignored)
 	int registers = 0;
 	int this_registers, average;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	mem_size = sizeof(*rbtree_ctx);
 
@@ -168,8 +168,6 @@ static int rbtree_show(struct seq_file *s, void *ignored)
 	seq_printf(s, "%d nodes, %d registers, average %d registers, used %zu bytes\n",
 		   nodes, registers, average, mem_size);
 
-	map->unlock(map->lock_arg);
-
 	return 0;
 }
 

-- 
2.51.0

[PATCH RFC v2 4/4] regmap: debugfs: use the regmap scoped lock guard

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Convert the open-coded map->lock()/map->unlock() users in the debugfs
cache_only and cache_bypass write handlers to the regmap scoped guard.

regmap_cache_bypass_write_file() uses guard(regmap)() since the locked
region runs to the return. regmap_cache_only_write_file() uses
scoped_guard(regmap, ...) because the subsequent regcache_sync() must
run with the lock released - it takes the regmap lock itself - so it
stays outside the guarded scope exactly as before. No functional
change.

Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/base/regmap/regmap-debugfs.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 18f1c60749fe..ec207548a6bb 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -471,18 +471,16 @@ static ssize_t regmap_cache_only_write_file(struct file *file,
 	if (err)
 		return count;
 
-	map->lock(map->lock_arg);
-
-	if (new_val && !map->cache_only) {
-		dev_warn(map->dev, "debugfs cache_only=Y forced\n");
-		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
-	} else if (!new_val && map->cache_only) {
-		dev_warn(map->dev, "debugfs cache_only=N forced: syncing cache\n");
-		require_sync = true;
+	scoped_guard(regmap, map) {
+		if (new_val && !map->cache_only) {
+			dev_warn(map->dev, "debugfs cache_only=Y forced\n");
+			add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+		} else if (!new_val && map->cache_only) {
+			dev_warn(map->dev, "debugfs cache_only=N forced: syncing cache\n");
+			require_sync = true;
+		}
+		map->cache_only = new_val;
 	}
-	map->cache_only = new_val;
-
-	map->unlock(map->lock_arg);
 
 	if (require_sync) {
 		err = regcache_sync(map);
@@ -513,7 +511,7 @@ static ssize_t regmap_cache_bypass_write_file(struct file *file,
 	if (err)
 		return count;
 
-	map->lock(map->lock_arg);
+	guard(regmap)(map);
 
 	if (new_val && !map->cache_bypass) {
 		dev_warn(map->dev, "debugfs cache_bypass=Y forced\n");
@@ -523,8 +521,6 @@ static ssize_t regmap_cache_bypass_write_file(struct file *file,
 	}
 	map->cache_bypass = new_val;
 
-	map->unlock(map->lock_arg);
-
 	return count;
 }
 

-- 
2.51.0