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..bde60255ddbb 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 num_reg_defaults_raw is unset, it means there is nothing
+	 * to read back from HW to set up defaults in the cache, so skip
+	 * this phase without an error code returned.
+	 */
+	if (!map->num_reg_defaults_raw)
+		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

On Thu, Feb 26, 2026 at 02:57:09PM +0100, Andy Shevchenko wrote:

> 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.

> @@ -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;

This is in the case where num_reg_defaults_raw != 0 (and we didn't have
any explicit defaults!), it's the only place where count gets set...

>  
> +	/*
> +	 * 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;
> +

...and we now pass count off to regcache_hw_init() which will attempt to
allocate a zero length array and presumably faceplant if that happens.
I don't *think* we should ever hit that case (at least not for a
sensible regmap), but I'm having to think far too hard about the whole
thing to convince myself it's safe.  I think we should keep the counting
of registers to allocate and the decision to call regcache_hw_init()
more joined up.

On Thu, Feb 26, 2026 at 10:14:55PM +0000, Mark Brown wrote:
> On Thu, Feb 26, 2026 at 02:57:09PM +0100, Andy Shevchenko wrote:
> > 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.

...

> > @@ -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;
> 
> This is in the case where num_reg_defaults_raw != 0 (and we didn't have
> any explicit defaults!), it's the only place where count gets set...

Actually the check in the regmap_hw_init() should be done against count,
indeed.

> > +	/*
> > +	 * 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;
> 
> ...and we now pass count off to regcache_hw_init() which will attempt to
> allocate a zero length array and presumably faceplant if that happens.
> I don't *think* we should ever hit that case (at least not for a
> sensible regmap), but I'm having to think far too hard about the whole
> thing to convince myself it's safe.  I think we should keep the counting
> of registers to allocate and the decision to call regcache_hw_init()
> more joined up.

Is it possible to allocate something, then count, then reallocate
in the existing caches and in the potential future ones?

I think it's the patter that we first count the size of the allocation,
do the allocation, and only then fill up the allocated area with data.

Otherwise we came back to the chicken-egg issue this patch tries to solve.

-- 
With Best Regards,
Andy Shevchenko

On Fri, Feb 27, 2026 at 07:43:59AM +0200, Andy Shevchenko wrote:

> Is it possible to allocate something, then count, then reallocate
> in the existing caches and in the potential future ones?

> I think it's the patter that we first count the size of the allocation,
> do the allocation, and only then fill up the allocated area with data.

> Otherwise we came back to the chicken-egg issue this patch tries to solve.

The main worry there would be transiently consuming huge amounts of
memory if there's a very sparse regsiter map, though we'd also be
burning lots of CPU time checking all those registers so perhaps it's
not a real concern, and the whole thing is kind of an edge case anyway.
Though now I look again we are careful to bypass the cache during the
hw_init() so we should be safe anyway I think.

On Sat, Feb 28, 2026 at 01:09:09PM +0000, Mark Brown wrote:
> On Fri, Feb 27, 2026 at 07:43:59AM +0200, Andy Shevchenko wrote:
> 
> > Is it possible to allocate something, then count, then reallocate
> > in the existing caches and in the potential future ones?
> 
> > I think it's the patter that we first count the size of the allocation,
> > do the allocation, and only then fill up the allocated area with data.
> 
> > Otherwise we came back to the chicken-egg issue this patch tries to solve.
> 
> The main worry there would be transiently consuming huge amounts of
> memory if there's a very sparse regsiter map, though we'd also be
> burning lots of CPU time checking all those registers so perhaps it's
> not a real concern, and the whole thing is kind of an edge case anyway.
> Though now I look again we are careful to bypass the cache during the
> hw_init() so we should be safe anyway I think.

Yes, but amending check to be against 'count' needs to be done for clarity.
'count' represent the amount of cacheable registers, it is independent number
on how sparse the map is.

I will send a v4 shortly with the above amendment.

-- 
With Best Regards,
Andy Shevchenko