[PATCH v4 0/2] regmap: Introduce validity info for flat cache

[PATCH v4 0/2] regmap: Introduce validity info for flat cache

[Sander Vanheule]

The flat cache behaves differently from the other caches, in that it has
no way of knowing if a cache entry is valid. Initialization has to
happen either by providing defaults, by loading defaults from hardware
(via num_reg_defaults_raw), or by performing the required register
writes. This difference in behavior may be unexpected to the user [1].

To provide feature parity between the different cache types, a new
variant of the flat cache is proposed. This allows user to migrate to
the sparse flat cache (or a different cache type) when possible.

In order to encourage migration, the second patch adds a new warning to
inform the user of their suspicious cache.

[1] https://lore.kernel.org/linux-gpio/e461ca08-ad28-44fe-85f1-afe332c1d43d@topic.nl/
---
Changes since v3:
Link: https://lore.kernel.org/lkml/20250109180256.6269-1-sander@svanheule.net/
- Split changes into two patches to provide a migration path

Changes since v2:
Link: https://lore.kernel.org/all/20250109151106.38645-1-sander@svanheule.net/
- Complete renaming of index variables so regcache-flat.c compiles again

Changes since v1:
Link: https://lore.kernel.org/all/20241231100256.194753-1-sander@svanheule.net/
- Fix off-by-one in length for bitmap_clear()
- Add REGCACHE_FLAT to the list of sparse cache tests

Sander Vanheule (2):
  regmap: add flat cache with sparse validity
  regmap: warn users about uninitialized flat cache

 drivers/base/regmap/internal.h      |  1 +
 drivers/base/regmap/regcache-flat.c | 98 +++++++++++++++++++++++++----
 drivers/base/regmap/regcache.c      |  1 +
 drivers/base/regmap/regmap-kunit.c  | 14 +++++
 include/linux/regmap.h              |  1 +
 5 files changed, 104 insertions(+), 11 deletions(-)

-- 
2.51.0

[PATCH v4 1/2] regmap: add flat cache with sparse validity

[Sander Vanheule]

The flat regcache will always assume the data in the cache is valid.
Since the cache is preferred over hardware access, this may shadow the
actual state of the device.

Add a new containing cache structure with the flat data table and a
bitmap indicating cache validity. REGCACHE_FLAT will still behave as
before, as the validity is ignored.

Define new cache type REGCACHE_FLAT_S: a flat cache with sparse
validity. The sparse validity is used to determine if a hardware access
should occur to initialize the cache on the fly, vs. at regmap init for
REGCACHE_FLAT. Contrary to REGCACHE_FLAT, this allows us to implement
regcache_ops.drop.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
Changes since v3:
- Define new flat-sparse ops instead of modifying existing ones
- Update KUnit tests
- Allocate cache as flexible array to limit performance impact
---
 drivers/base/regmap/internal.h      |   1 +
 drivers/base/regmap/regcache-flat.c | 103 +++++++++++++++++++++++++---
 drivers/base/regmap/regcache.c      |   1 +
 drivers/base/regmap/regmap-kunit.c  |  14 ++++
 include/linux/regmap.h              |   1 +
 5 files changed, 109 insertions(+), 11 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 6f31240ee4a9..8d19a1414d5b 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -288,6 +288,7 @@ enum regmap_endian regmap_get_val_endian(struct device *dev,
 					 const struct regmap_bus *bus,
 					 const struct regmap_config *config);
 
+extern struct regcache_ops regcache_flat_sparse_ops;
 extern struct regcache_ops regcache_rbtree_ops;
 extern struct regcache_ops regcache_maple_ops;
 extern struct regcache_ops regcache_flat_ops;
diff --git a/drivers/base/regmap/regcache-flat.c b/drivers/base/regmap/regcache-flat.c
index f36d3618b67c..52ce125a74e1 100644
--- a/drivers/base/regmap/regcache-flat.c
+++ b/drivers/base/regmap/regcache-flat.c
@@ -6,7 +6,11 @@
 //
 // Author: Mark Brown <broonie@opensource.wolfsonmicro.com>
 
+#include <linux/bitmap.h>
+#include <linux/bitops.h>
 #include <linux/device.h>
+#include <linux/limits.h>
+#include <linux/overflow.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 
@@ -18,34 +22,63 @@ static inline unsigned int regcache_flat_get_index(const struct regmap *map,
 	return regcache_get_index_by_order(map, reg);
 }
 
+struct regcache_flat_data {
+	unsigned long *valid;
+	unsigned int data[];
+};
+
 static int regcache_flat_init(struct regmap *map)
 {
 	int i;
-	unsigned int *cache;
+	unsigned int cache_size;
+	struct regcache_flat_data *cache = NULL;
+	unsigned long *cache_valid = NULL;
 
 	if (!map || map->reg_stride_order < 0 || !map->max_register_is_set)
 		return -EINVAL;
 
-	map->cache = kcalloc(regcache_flat_get_index(map, map->max_register)
-			     + 1, sizeof(unsigned int), map->alloc_flags);
-	if (!map->cache)
+	cache_size = regcache_flat_get_index(map, map->max_register) + 1;
+
+	size_t cache_data_size = struct_size(cache, data, cache_size);
+	if (cache_data_size == SIZE_MAX) {
+		dev_err(map->dev, "cannot allocate regmap cache");
 		return -ENOMEM;
+	}
 
-	cache = map->cache;
+	cache = kmalloc(cache_data_size, map->alloc_flags);
+	if (!cache)
+		return -ENOMEM;
+
+	cache_valid = bitmap_zalloc(cache_size, map->alloc_flags);
+	if (!cache_valid)
+		goto err_free;
+
+	cache->valid = cache_valid;
+	map->cache = cache;
 
 	for (i = 0; i < map->num_reg_defaults; i++) {
 		unsigned int reg = map->reg_defaults[i].reg;
 		unsigned int index = regcache_flat_get_index(map, reg);
 
-		cache[index] = map->reg_defaults[i].def;
+		cache->data[index] = map->reg_defaults[i].def;
+		__set_bit(index, cache->valid);
 	}
 
 	return 0;
+
+err_free:
+	kfree(cache);
+	return -ENOMEM;
 }
 
 static int regcache_flat_exit(struct regmap *map)
 {
-	kfree(map->cache);
+	struct regcache_flat_data *cache = map->cache;
+
+	if (cache)
+		bitmap_free(cache->valid);
+
+	kfree(cache);
 	map->cache = NULL;
 
 	return 0;
@@ -54,10 +87,10 @@ static int regcache_flat_exit(struct regmap *map)
 static int regcache_flat_read(struct regmap *map,
 			      unsigned int reg, unsigned int *value)
 {
-	unsigned int *cache = map->cache;
+	struct regcache_flat_data *cache = map->cache;
 	unsigned int index = regcache_flat_get_index(map, reg);
 
-	*value = cache[index];
+	*value = cache->data[index];
 
 	return 0;
 }
@@ -65,10 +98,48 @@ static int regcache_flat_read(struct regmap *map,
 static int regcache_flat_write(struct regmap *map, unsigned int reg,
 			       unsigned int value)
 {
-	unsigned int *cache = map->cache;
+	struct regcache_flat_data *cache = map->cache;
+	unsigned int index = regcache_flat_get_index(map, reg);
+
+	cache->data[index] = value;
+
+	return 0;
+}
+
+static int regcache_flat_sparse_read(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);
+
+	if (unlikely(!test_bit(index, cache->valid)))
+		return -ENOENT;
+
+	*value = cache->data[index];
+
+	return 0;
+}
+
+static int regcache_flat_sparse_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);
 
-	cache[index] = value;
+	cache->data[index] = value;
+	__set_bit(index, cache->valid);
+
+	return 0;
+}
+
+static int regcache_flat_drop(struct regmap *map, unsigned int min,
+			      unsigned int max)
+{
+	struct regcache_flat_data *cache = map->cache;
+	unsigned int bitmap_min = regcache_flat_get_index(map, min);
+	unsigned int bitmap_max = regcache_flat_get_index(map, max);
+
+	bitmap_clear(cache->valid, bitmap_min, bitmap_max + 1 - bitmap_min);
 
 	return 0;
 }
@@ -81,3 +152,13 @@ struct regcache_ops regcache_flat_ops = {
 	.read = regcache_flat_read,
 	.write = regcache_flat_write,
 };
+
+struct regcache_ops regcache_flat_sparse_ops = {
+	.type = REGCACHE_FLAT_S,
+	.name = "flat-sparse",
+	.init = regcache_flat_init,
+	.exit = regcache_flat_exit,
+	.read = regcache_flat_sparse_read,
+	.write = regcache_flat_sparse_write,
+	.drop = regcache_flat_drop,
+};
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index c7650fa434ad..0392f5525cf3 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -16,6 +16,7 @@
 #include "internal.h"
 
 static const struct regcache_ops *cache_types[] = {
+	&regcache_flat_sparse_ops,
 	&regcache_rbtree_ops,
 	&regcache_maple_ops,
 	&regcache_flat_ops,
diff --git a/drivers/base/regmap/regmap-kunit.c b/drivers/base/regmap/regmap-kunit.c
index 95c5bf2a78ee..70ecb3e46472 100644
--- a/drivers/base/regmap/regmap-kunit.c
+++ b/drivers/base/regmap/regmap-kunit.c
@@ -54,6 +54,8 @@ static const char *regcache_type_name(enum regcache_type type)
 		return "none";
 	case REGCACHE_FLAT:
 		return "flat";
+	case REGCACHE_FLAT_S:
+		return "flat-sparse";
 	case REGCACHE_RBTREE:
 		return "rbtree";
 	case REGCACHE_MAPLE:
@@ -119,6 +121,12 @@ static const struct regmap_test_param real_cache_types_list[] = {
 	{ .cache = REGCACHE_FLAT,   .from_reg = 0x2002 },
 	{ .cache = REGCACHE_FLAT,   .from_reg = 0x2003 },
 	{ .cache = REGCACHE_FLAT,   .from_reg = 0x2004 },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0 },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0, .fast_io = true },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0x2001 },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0x2002 },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0x2003 },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0x2004 },
 	{ .cache = REGCACHE_RBTREE, .from_reg = 0 },
 	{ .cache = REGCACHE_RBTREE, .from_reg = 0, .fast_io = true },
 	{ .cache = REGCACHE_RBTREE, .from_reg = 0x2001 },
@@ -136,6 +144,12 @@ static const struct regmap_test_param real_cache_types_list[] = {
 KUNIT_ARRAY_PARAM(real_cache_types, real_cache_types_list, param_to_desc);
 
 static const struct regmap_test_param sparse_cache_types_list[] = {
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0 },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0, .fast_io = true },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0x2001 },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0x2002 },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0x2003 },
+	{ .cache = REGCACHE_FLAT_S, .from_reg = 0x2004 },
 	{ .cache = REGCACHE_RBTREE, .from_reg = 0 },
 	{ .cache = REGCACHE_RBTREE, .from_reg = 0, .fast_io = true },
 	{ .cache = REGCACHE_RBTREE, .from_reg = 0x2001 },
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 4e1ac1fbcec4..a5987f1a1a76 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -67,6 +67,7 @@ enum regcache_type {
 	REGCACHE_RBTREE,
 	REGCACHE_FLAT,
 	REGCACHE_MAPLE,
+	REGCACHE_FLAT_S,
 };
 
 /**
-- 
2.51.0

[PATCH v4 2/2] regmap: warn users about uninitialized flat cache

[Sander Vanheule]

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

Re: [PATCH v4 2/2] regmap: warn users about uninitialized flat cache

[Mark Brown]

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

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.

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

Re: [PATCH v4 2/2] regmap: warn users about uninitialized flat cache

[Sander Vanheule]

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

Re: [PATCH v4 2/2] regmap: warn users about uninitialized flat cache

[Mark Brown]

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

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.

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