The standard flat cache did not contain any validity info, so the cache
was always considered to be entirely valid. Multiple mechanisms exist to
initialize the cache on regmap init (defaults, raw defaults, HW init),
but not all drivers are using one of these. As a result, their
implementation might currently depend on the zero-initialized cache or
contain other workarounds.

When reading an uninitialized value from the flat cache, warn the user,
but maintain the current behavior. This will allow developers to switch
to a sparse (flat) cache independently.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
I've decided to use dev_warn() with __test_and_set_bit() to avoid log
flooding from e.g. regmap_update_bits() deciding not to write an updated
value. This way the warning is only issued once per invalid register,
which will hopefully be a bit more obvious than a single log entry from
a dev_warn_once().

Changes since v3:
- New patch: emit a warning for the flat cache on suspicious registers
---
 drivers/base/regmap/regcache-flat.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/base/regmap/regcache-flat.c b/drivers/base/regmap/regcache-flat.c
index 52ce125a74e1..8802713e0cc6 100644
--- a/drivers/base/regmap/regcache-flat.c
+++ b/drivers/base/regmap/regcache-flat.c
@@ -90,18 +90,13 @@ static int regcache_flat_read(struct regmap *map,
 	struct regcache_flat_data *cache = map->cache;
 	unsigned int index = regcache_flat_get_index(map, reg);
 
-	*value = cache->data[index];
-
-	return 0;
-}
-
-static int regcache_flat_write(struct regmap *map, unsigned int reg,
-			       unsigned int value)
-{
-	struct regcache_flat_data *cache = map->cache;
-	unsigned int index = regcache_flat_get_index(map, reg);
+	/* legacy behavior: ignore validity, but warn the user once per reg */
+	if (unlikely(!__test_and_set_bit(index, cache->valid)))
+		dev_warn(map->dev,
+			 "using zero-initialized flat cache, "
+			 "this may cause unexpected behavior");
 
-	cache->data[index] = value;
+	*value = cache->data[index];
 
 	return 0;
 }
@@ -120,7 +115,7 @@ static int regcache_flat_sparse_read(struct regmap *map,
 	return 0;
 }
 
-static int regcache_flat_sparse_write(struct regmap *map, unsigned int reg,
+static int regcache_flat_write(struct regmap *map, unsigned int reg,
 			       unsigned int value)
 {
 	struct regcache_flat_data *cache = map->cache;
@@ -159,6 +154,6 @@ struct regcache_ops regcache_flat_sparse_ops = {
 	.init = regcache_flat_init,
 	.exit = regcache_flat_exit,
 	.read = regcache_flat_sparse_read,
-	.write = regcache_flat_sparse_write,
+	.write = regcache_flat_write,
 	.drop = regcache_flat_drop,
 };
-- 
2.51.0

On Wed, Oct 22, 2025 at 10:04:08PM +0200, Sander Vanheule wrote:

> +	if (unlikely(!__test_and_set_bit(index, cache->valid)))
> +		dev_warn(map->dev,
> +			 "using zero-initialized flat cache, "
> +			 "this may cause unexpected behavior");

Please update this to have the error message on one line, it's better to
break the line length limit and have people easily able to grep for the
log message.

On Thu, 2025-10-23 at 14:00 +0100, Mark Brown wrote:
> On Wed, Oct 22, 2025 at 10:04:08PM +0200, Sander Vanheule wrote:
> 
> > +	if (unlikely(!__test_and_set_bit(index, cache->valid)))
> > +		dev_warn(map->dev,
> > +			 "using zero-initialized flat cache, "
> > +			 "this may cause unexpected behavior");
> 
> Please update this to have the error message on one line, it's better to
> break the line length limit and have people easily able to grep for the
> log message.

Okay, I'll turn it into one line.

Regarding this warning, I've noticed it confuses the regmap KUnit tests parser.
The sync tests with offset registers at 0x2000 read a lot of uncached registers,
so that results in 10's of thousands of log lines. The test itself is reported
as passing, so I only noticed this now:

[15:07:47] ================= cache_sync_marked_dirty  =================        
[15:07:47] [PASSED] flat-default @0x0                                          
[15:07:47] [PASSED] flat-default fast I/O @0x0                                 
[15:07:47] [PASSED] flat-default @0x2001                                       
[15:07:47] [PASSED] flat-default @0x2002                                       
[15:07:47] [PASSED] flat-default @0x2003                                       
[15:07:47] [PASSED] flat-default @0x2004                                       
[15:07:47] [ERROR] Test: flat-sparse-default fast I/O @0x0: Expected test number
7 but found 8
[15:07:47] [PASSED] flat-sparse-default fast I/O @0x0
[15:07:47] [ERROR] Test: flat-sparse-default @0x2001: Expected test number 8 but
found 9
...
[15:07:47] [ERROR] Test: maple-default @0x2004: Expected test number 23 but
found 24
[15:07:47] [PASSED] maple-default @0x2004
[15:07:47] ============= [PASSED] cache_sync_marked_dirty =============

If it's okay for you, I would change it back to a dev_warn_once() with a
test_bit() check.

Best,
Sander

On Thu, Oct 23, 2025 at 03:12:59PM +0200, Sander Vanheule wrote:

> If it's okay for you, I would change it back to a dev_warn_once() with a
> test_bit() check.

That sounds good.