[PATCH v4 0/3] regcache: Avoid accessing non-initialised cache

[PATCH v4 0/3] regcache: Avoid accessing non-initialised cache

[Andy Shevchenko]

Refactor regcache flow that populates cache on initialisation phase
based on the values in HW. This makes code robust against any possible
accesses to not-fully initialised caches.

Changelog v4:
- made conditional against 'count' to make it less dependent to the previous code path

v3: 20260226135918.381979-1-andriy.shevchenko@linux.intel.com

Changelog v3:
- dropped unneeded churn in patch 3 (Mark)
- fixed code to pass kunit test cases (Mark)

v2: 20260225161659.3811671-1-andriy.shevchenko@linux.intel.com

Changelog v2:
- rebased on top of the latest regmap changes (Mark)
- added two more little cleanups (patches 4 & 5)

Andy Shevchenko (3):
  regcache: Move HW readback after cache initialisation
  regcache: Define iterator inside for-loop and align their types
  regcache: Amend printf() specifiers when printing registers

 drivers/base/regmap/internal.h |  2 +-
 drivers/base/regmap/regcache.c | 42 ++++++++++++++++++++--------------
 2 files changed, 26 insertions(+), 18 deletions(-)

-- 
2.50.1

[PATCH v4 1/3] regcache: Move HW readback after cache initialisation

[Andy Shevchenko]

Make sure that cache is initialised before calling any IO
using regmap, this makes sure that we won't access NULL or
invalid pointers in the cache which hasn't been initialised.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/regmap/regcache.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 73cfe8120669..ae60411ea8dc 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -67,6 +67,14 @@ static int regcache_hw_init(struct regmap *map, int count)
 	unsigned int reg, val;
 	void *tmp_buf;
 
+	/*
+	 * When count is zero, it means there is nothing to cache and hence
+	 * nothing to read back from HW to set up defaults, so skip this phase
+	 * without an error code returned.
+	 */
+	if (!count)
+		return 0;
+
 	map->num_reg_defaults = count;
 	map->reg_defaults = kmalloc_objs(struct reg_default, count);
 	if (!map->reg_defaults)
@@ -202,14 +210,6 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
 		count = regcache_count_cacheable_registers(map);
 		if (map->cache_bypass)
 			return 0;
-
-		/* Some devices such as PMICs don't have cache defaults,
-		 * we cope with this by reading back the HW registers and
-		 * crafting the cache defaults by hand.
-		 */
-		ret = regcache_hw_init(map, count);
-		if (ret < 0)
-			return ret;
 	}
 
 	if (!map->max_register_is_set && map->num_reg_defaults_raw) {
@@ -227,6 +227,15 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
 			goto err_free;
 	}
 
+	/*
+	 * Some devices such as PMICs don't have cache defaults,
+	 * we cope with this by reading back the HW registers and
+	 * crafting the cache defaults by hand.
+	 */
+	ret = regcache_hw_init(map, count);
+	if (ret)
+		goto err_exit;
+
 	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);
-- 
2.50.1

Re: [PATCH v4 1/3] regcache: Move HW readback after cache initialisation

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

On Mon, Mar 02, 2026 at 10:56:55AM +0100, Andy Shevchenko wrote:

> +	/*
> +	 * Some devices such as PMICs don't have cache defaults,
> +	 * we cope with this by reading back the HW registers and
> +	 * crafting the cache defaults by hand.
> +	 */
> +	ret = regcache_hw_init(map, count);
> +	if (ret)
> +		goto err_exit;
> +

We need to delete the free of reg_defaults from regcache_hw_init() when
it fails, the error handling for err_exit also frees that so we'd end up
with a double free.  Which is awfully error prone and part of what
you're trying to fix here :/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 1/3] regcache: Move HW readback after cache initialisation

[Andy Shevchenko]

On Mon, Mar 02, 2026 at 02:49:49PM +0000, Mark Brown wrote:
> On Mon, Mar 02, 2026 at 10:56:55AM +0100, Andy Shevchenko wrote:

...

> > +	/*
> > +	 * Some devices such as PMICs don't have cache defaults,
> > +	 * we cope with this by reading back the HW registers and
> > +	 * crafting the cache defaults by hand.
> > +	 */
> > +	ret = regcache_hw_init(map, count);
> > +	if (ret)
> > +		goto err_exit;
> > +
> 
> We need to delete the free of reg_defaults from regcache_hw_init() when
> it fails, the error handling for err_exit also frees that so we'd end up
> with a double free.  Which is awfully error prone and part of what
> you're trying to fix here :/

Ah, looking again at it I see your point.

Indeed, in the error path of regcache_hw_init() we free the reg_defaults,
but the same is also done at err_free label of regcache_init().

NULLifying it at regcache_hw_init() doesn't sound like a good idea as
you pointed out as error prone case. I will think more about it, thanks
for catching this (I dunno why I missed that one)!

-- 
With Best Regards,
Andy Shevchenko

[PATCH v4 2/3] regcache: Define iterator inside for-loop and align their types

[Andy Shevchenko]

Some of the iterators may be defined inside the respective for-loop
reducing the scope and potential risk of their misuse. While at it,
align their types based on the type of the upper or lower limits.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/regmap/internal.h |  2 +-
 drivers/base/regmap/regcache.c | 13 ++++++-------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 5bf993165438..c77f3a49a89e 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -162,7 +162,7 @@ struct regmap {
 	bool no_sync_defaults;
 
 	struct reg_sequence *patch;
-	int patch_regs;
+	unsigned int patch_regs;
 
 	/* if set, the regmap core can sleep */
 	bool can_sleep;
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index ae60411ea8dc..2605d0bbf550 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -44,11 +44,11 @@ EXPORT_SYMBOL_GPL(regcache_sort_defaults);
 
 static int regcache_count_cacheable_registers(struct regmap *map)
 {
-	int count;
-	int i;
+	unsigned int count;
 
 	/* calculate the size of reg_defaults */
-	for (count = 0, i = 0; i < map->num_reg_defaults_raw; i++)
+	count = 0;
+	for (unsigned int i = 0; i < map->num_reg_defaults_raw; i++)
 		if (regmap_readable(map, i * map->reg_stride) &&
 		    !regmap_volatile(map, i * map->reg_stride))
 			count++;
@@ -62,7 +62,6 @@ static int regcache_count_cacheable_registers(struct regmap *map)
 
 static int regcache_hw_init(struct regmap *map, int count)
 {
-	int i, j;
 	int ret;
 	unsigned int reg, val;
 	void *tmp_buf;
@@ -103,7 +102,7 @@ static int regcache_hw_init(struct regmap *map, int count)
 	}
 
 	/* fill the reg_defaults */
-	for (i = 0, j = 0; i < map->num_reg_defaults_raw; i++) {
+	for (unsigned int i = 0, j = 0; i < map->num_reg_defaults_raw; i++) {
 		reg = i * map->reg_stride;
 
 		if (!regmap_readable(map, reg))
@@ -849,13 +848,13 @@ static int regcache_sync_block_raw(struct regmap *map, void *block,
 			    unsigned int block_base, unsigned int start,
 			    unsigned int end)
 {
-	unsigned int i, val;
 	unsigned int regtmp = 0;
 	unsigned int base = 0;
 	const void *data = NULL;
+	unsigned int val;
 	int ret;
 
-	for (i = start; i < end; i++) {
+	for (unsigned int i = start; i < end; i++) {
 		regtmp = block_base + (i * map->reg_stride);
 
 		if (!regcache_reg_present(cache_present, i) ||
-- 
2.50.1

[PATCH v4 3/3] regcache: Amend printf() specifiers when printing registers

[Andy Shevchenko]

In one case the 0x is provided in the formatting string, while
the rest use # for that.

In a couple of more cases a decimal signed value specifier is used.

Amend them to use %#x when register is printed. Note, for the case,
when it's related to the read/write, use %x to be in align with
the similar messages in regmap core.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/regmap/regcache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 2605d0bbf550..6bec435a566b 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -120,7 +120,7 @@ static int regcache_hw_init(struct regmap *map, int count)
 			ret = regmap_read(map, reg, &val);
 			map->cache_bypass = cache_bypass;
 			if (ret != 0) {
-				dev_err(map->dev, "Failed to read %d: %d\n",
+				dev_err(map->dev, "Failed to read %x: %d\n",
 					reg, ret);
 				goto err_free;
 			}
@@ -517,7 +517,7 @@ int regcache_sync_region(struct regmap *map, unsigned int min,
 	bypass = map->cache_bypass;
 
 	name = map->cache_ops->name;
-	dev_dbg(map->dev, "Syncing %s cache from %d-%d\n", name, min, max);
+	dev_dbg(map->dev, "Syncing %s cache from %#x-%#x\n", name, min, max);
 
 	trace_regcache_sync(map, name, "start region");
 
-- 
2.50.1

Re: [PATCH v4 0/3] regcache: Avoid accessing non-initialised cache

[Mark Brown]

On Mon, 02 Mar 2026 10:56:54 +0100, Andy Shevchenko wrote:
> Refactor regcache flow that populates cache on initialisation phase
> based on the values in HW. This makes code robust against any possible
> accesses to not-fully initialised caches.
> 
> Changelog v4:
> - made conditional against 'count' to make it less dependent to the previous code path
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next

Thanks!

[1/3] regcache: Move HW readback after cache initialisation
      (no commit info)
[2/3] regcache: Define iterator inside for-loop and align their types
      commit: 8e29bc88e11928926fd97fc9acd58b8afa38de9d
[3/3] regcache: Amend printf() specifiers when printing registers
      commit: 9ab637ac5d3826606947f4e861107da958eda324

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark