[PATCH 0/2] regmap: Provide basic test coverage for raw I/O

[PATCH 0/2] regmap: Provide basic test coverage for raw I/O

[Mark Brown]

Our existing coverage only deals with buses that provide single register
read and write operations, extend it to cover raw buses using a similar
approach with a RAM backed register map that the tests can inspect to
check operations.  This coverage could be more complete but provides a
good start.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
Mark Brown (2):
      regmap: Provide a ram backed regmap with raw support
      regmap: Provide basic KUnit coverage for the raw register I/O

 drivers/base/regmap/Makefile         |   2 +-
 drivers/base/regmap/internal.h       |   8 +
 drivers/base/regmap/regmap-kunit.c   | 327 +++++++++++++++++++++++++++++++++++
 drivers/base/regmap/regmap-raw-ram.c | 133 ++++++++++++++
 4 files changed, 469 insertions(+), 1 deletion(-)
---
base-commit: 9561de3a55bed6bdd44a12820ba81ec416e705a7
change-id: 20230610-regcache-raw-kunit-3d866fb82485

Best regards,
-- 
Mark Brown <broonie@kernel.org>

[PATCH 1/2] regmap: Provide a ram backed regmap with raw support

[Mark Brown]

Provide a simple, 16 bit only, RAM backed regmap which supports raw I/O for
use in testing.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/Makefile         |   2 +-
 drivers/base/regmap/internal.h       |   8 +++
 drivers/base/regmap/regmap-raw-ram.c | 133 +++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 1 deletion(-)

diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index f6c6cb017200..5fdd0845b45e 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_DEBUG_FS) += regmap-debugfs.o
 obj-$(CONFIG_REGMAP_KUNIT) += regmap-kunit.o
 obj-$(CONFIG_REGMAP_AC97) += regmap-ac97.o
 obj-$(CONFIG_REGMAP_I2C) += regmap-i2c.o
-obj-$(CONFIG_REGMAP_RAM) += regmap-ram.o
+obj-$(CONFIG_REGMAP_RAM) += regmap-ram.o regmap-raw-ram.o
 obj-$(CONFIG_REGMAP_SLIMBUS) += regmap-slimbus.o
 obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
 obj-$(CONFIG_REGMAP_SPMI) += regmap-spmi.o
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 9bd0dfd1e259..d987ce182d22 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -312,6 +312,7 @@ struct regmap_ram_data {
 	unsigned int *vals;  /* Allocatd by caller */
 	bool *read;
 	bool *written;
+	enum regmap_endian reg_endian;
 };
 
 /*
@@ -326,5 +327,12 @@ struct regmap *__regmap_init_ram(const struct regmap_config *config,
 #define regmap_init_ram(config, data)					\
 	__regmap_lockdep_wrapper(__regmap_init_ram, #config, config, data)
 
+struct regmap *__regmap_init_raw_ram(const struct regmap_config *config,
+				     struct regmap_ram_data *data,
+				     struct lock_class_key *lock_key,
+				     const char *lock_name);
+
+#define regmap_init_raw_ram(config, data)				\
+	__regmap_lockdep_wrapper(__regmap_init_raw_ram, #config, config, data)
 
 #endif
diff --git a/drivers/base/regmap/regmap-raw-ram.c b/drivers/base/regmap/regmap-raw-ram.c
new file mode 100644
index 000000000000..c9b800885f3b
--- /dev/null
+++ b/drivers/base/regmap/regmap-raw-ram.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Register map access API - Memory region with raw access
+//
+// This is intended for testing only
+//
+// Copyright (c) 2023, Arm Ltd
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/swab.h>
+
+#include "internal.h"
+
+static unsigned int decode_reg(enum regmap_endian endian, const void *reg)
+{
+	const u16 *r = reg;
+
+	if (endian == REGMAP_ENDIAN_BIG)
+		return be16_to_cpu(*r);
+	else
+		return le16_to_cpu(*r);
+}
+
+static int regmap_raw_ram_gather_write(void *context,
+				       const void *reg, size_t reg_len,
+				       const void *val, size_t val_len)
+{
+	struct regmap_ram_data *data = context;
+	unsigned int r;
+	u16 *our_buf = (u16 *)data->vals;
+	int i;
+
+	if (reg_len != 2)
+		return -EINVAL;
+	if (val_len % 2)
+		return -EINVAL;
+
+	r = decode_reg(data->reg_endian, reg);
+	memcpy(&our_buf[r], val, val_len);
+
+	for (i = 0; i < val_len / 2; i++)
+		data->written[r + i] = true;
+	
+	return 0;
+}
+
+static int regmap_raw_ram_write(void *context, const void *data, size_t count)
+{
+	return regmap_raw_ram_gather_write(context, data, 2,
+					   data + 2, count - 2);
+}
+
+static int regmap_raw_ram_read(void *context,
+			       const void *reg, size_t reg_len,
+			       void *val, size_t val_len)
+{
+	struct regmap_ram_data *data = context;
+	unsigned int r;
+	u16 *our_buf = (u16 *)data->vals;
+	int i;
+
+	if (reg_len != 2)
+		return -EINVAL;
+	if (val_len % 2)
+		return -EINVAL;
+
+	r = decode_reg(data->reg_endian, reg);
+	memcpy(val, &our_buf[r], val_len);
+
+	for (i = 0; i < val_len / 2; i++)
+		data->read[r + i] = true;
+
+	return 0;
+}
+
+static void regmap_raw_ram_free_context(void *context)
+{
+	struct regmap_ram_data *data = context;
+
+	kfree(data->vals);
+	kfree(data->read);
+	kfree(data->written);
+	kfree(data);
+}
+
+static const struct regmap_bus regmap_raw_ram = {
+	.fast_io = true,
+	.write = regmap_raw_ram_write,
+	.gather_write = regmap_raw_ram_gather_write,
+	.read = regmap_raw_ram_read,
+	.free_context = regmap_raw_ram_free_context,
+};
+
+struct regmap *__regmap_init_raw_ram(const struct regmap_config *config,
+				     struct regmap_ram_data *data,
+				     struct lock_class_key *lock_key,
+				     const char *lock_name)
+{
+	struct regmap *map;
+
+	if (config->reg_bits != 16)
+		return ERR_PTR(-EINVAL);
+
+	if (!config->max_register) {
+		pr_crit("No max_register specified for RAM regmap\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	data->read = kcalloc(sizeof(bool), config->max_register + 1,
+			     GFP_KERNEL);
+	if (!data->read)
+		return ERR_PTR(-ENOMEM);
+
+	data->written = kcalloc(sizeof(bool), config->max_register + 1,
+				GFP_KERNEL);
+	if (!data->written)
+		return ERR_PTR(-ENOMEM);
+
+	data->reg_endian = config->reg_format_endian;
+
+	map = __regmap_init(NULL, &regmap_raw_ram, data, config,
+			    lock_key, lock_name);
+
+	return map;
+}
+EXPORT_SYMBOL_GPL(__regmap_init_raw_ram);
+
+MODULE_LICENSE("GPL v2");

-- 
2.30.2

[PATCH 2/2] regmap: Provide basic KUnit coverage for the raw register I/O

[Mark Brown]

Simple tests that cover basic raw I/O, plus basic coverage of cache sync
since the caches generate bulk I/O with raw register maps. This could be
more comprehensive but it is good for testing generic code.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/regmap-kunit.c | 327 +++++++++++++++++++++++++++++++++++++
 1 file changed, 327 insertions(+)

diff --git a/drivers/base/regmap/regmap-kunit.c b/drivers/base/regmap/regmap-kunit.c
index f76d41688134..4ec5cbfc0ca0 100644
--- a/drivers/base/regmap/regmap-kunit.c
+++ b/drivers/base/regmap/regmap-kunit.c
@@ -712,6 +712,327 @@ static void cache_drop(struct kunit *test)
 	regmap_exit(map);
 }
 
+struct raw_test_types {
+	const char *name;
+
+	enum regcache_type cache_type;
+	enum regmap_endian val_endian;
+};
+
+static void raw_to_desc(const struct raw_test_types *t, char *desc)
+{
+	strcpy(desc, t->name);
+}
+
+static const struct raw_test_types raw_types_list[] = {
+	{ "none-little",   REGCACHE_NONE,   REGMAP_ENDIAN_LITTLE },
+	{ "none-big",      REGCACHE_NONE,   REGMAP_ENDIAN_BIG },
+	{ "flat-little",   REGCACHE_FLAT,   REGMAP_ENDIAN_LITTLE },
+	{ "flat-big",      REGCACHE_FLAT,   REGMAP_ENDIAN_BIG },
+	{ "rbtree-little", REGCACHE_RBTREE, REGMAP_ENDIAN_LITTLE },
+	{ "rbtree-big",    REGCACHE_RBTREE, REGMAP_ENDIAN_BIG },
+	{ "maple-little",  REGCACHE_MAPLE,  REGMAP_ENDIAN_LITTLE },
+	{ "maple-big",     REGCACHE_MAPLE,  REGMAP_ENDIAN_BIG },
+};
+
+KUNIT_ARRAY_PARAM(raw_test_types, raw_types_list, raw_to_desc);
+
+static const struct raw_test_types raw_cache_types_list[] = {
+	{ "flat-little",   REGCACHE_FLAT,   REGMAP_ENDIAN_LITTLE },
+	{ "flat-big",      REGCACHE_FLAT,   REGMAP_ENDIAN_BIG },
+	{ "rbtree-little", REGCACHE_RBTREE, REGMAP_ENDIAN_LITTLE },
+	{ "rbtree-big",    REGCACHE_RBTREE, REGMAP_ENDIAN_BIG },
+	{ "maple-little",  REGCACHE_MAPLE,  REGMAP_ENDIAN_LITTLE },
+	{ "maple-big",     REGCACHE_MAPLE,  REGMAP_ENDIAN_BIG },
+};
+
+KUNIT_ARRAY_PARAM(raw_test_cache_types, raw_cache_types_list, raw_to_desc);
+
+static const struct regmap_config raw_regmap_config = {
+	.max_register = BLOCK_TEST_SIZE,
+
+	.reg_format_endian = REGMAP_ENDIAN_LITTLE,
+	.reg_bits = 16,
+	.val_bits = 16,
+};
+
+static struct regmap *gen_raw_regmap(struct regmap_config *config,
+				     struct raw_test_types *test_type,
+				     struct regmap_ram_data **data)
+{
+	u16 *buf;
+	struct regmap *ret;
+	size_t size = (config->max_register + 1) * config->reg_bits / 8;
+	int i;
+	struct reg_default *defaults;
+
+	config->cache_type = test_type->cache_type;
+	config->val_format_endian = test_type->val_endian;
+
+	buf = kmalloc(size, GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
+	get_random_bytes(buf, size);
+
+	*data = kzalloc(sizeof(**data), GFP_KERNEL);
+	if (!(*data))
+		return ERR_PTR(-ENOMEM);
+	(*data)->vals = (void *)buf;
+
+	config->num_reg_defaults = config->max_register + 1;
+	defaults = kcalloc(config->num_reg_defaults,
+			   sizeof(struct reg_default),
+			   GFP_KERNEL);
+	if (!defaults)
+		return ERR_PTR(-ENOMEM);
+	config->reg_defaults = defaults;
+
+	for (i = 0; i < config->num_reg_defaults; i++) {
+		defaults[i].reg = i;
+		switch (test_type->val_endian) {
+		case REGMAP_ENDIAN_LITTLE:
+			defaults[i].def = le16_to_cpu(buf[i]);
+			break;
+		case REGMAP_ENDIAN_BIG:
+			defaults[i].def = be16_to_cpu(buf[i]);
+			break;
+		default:
+			return ERR_PTR(-EINVAL);
+		}
+	}
+
+	/*
+	 * We use the defaults in the tests but they don't make sense
+	 * to the core if there's no cache.
+	 */
+	if (config->cache_type == REGCACHE_NONE)
+		config->num_reg_defaults = 0;
+
+	ret = regmap_init_raw_ram(config, *data);
+	if (IS_ERR(ret)) {
+		kfree(buf);
+		kfree(*data);
+	}
+
+	return ret;
+}
+
+static void raw_read_defaults_single(struct kunit *test)
+{
+	struct raw_test_types *t = (struct raw_test_types *)test->param_value;
+	struct regmap *map;
+	struct regmap_config config;
+	struct regmap_ram_data *data;
+	unsigned int rval;
+	int i;
+
+	config = raw_regmap_config;
+
+	map = gen_raw_regmap(&config, t, &data);
+	KUNIT_ASSERT_FALSE(test, IS_ERR(map));
+	if (IS_ERR(map))
+		return;
+
+	/* Check that we can read the defaults via the API */
+	for (i = 0; i < config.max_register + 1; i++) {
+		KUNIT_EXPECT_EQ(test, 0, regmap_read(map, i, &rval));
+		KUNIT_EXPECT_EQ(test, config.reg_defaults[i].def, rval);
+	}
+
+	regmap_exit(map);
+}
+
+static void raw_read_defaults(struct kunit *test)
+{
+	struct raw_test_types *t = (struct raw_test_types *)test->param_value;
+	struct regmap *map;
+	struct regmap_config config;
+	struct regmap_ram_data *data;
+	u16 *rval;
+	u16 def;
+	size_t val_len;
+	int i;
+
+	config = raw_regmap_config;
+
+	map = gen_raw_regmap(&config, t, &data);
+	KUNIT_ASSERT_FALSE(test, IS_ERR(map));
+	if (IS_ERR(map))
+		return;
+
+	val_len = sizeof(*rval) * (config.max_register + 1);
+	rval = kmalloc(val_len, GFP_KERNEL);
+	KUNIT_ASSERT_TRUE(test, rval != NULL);
+	if (!rval)
+		return;
+	
+	/* Check that we can read the defaults via the API */
+	KUNIT_EXPECT_EQ(test, 0, regmap_raw_read(map, 0, rval, val_len));
+	for (i = 0; i < config.max_register + 1; i++) {
+		def = config.reg_defaults[i].def;
+		if (config.val_format_endian == REGMAP_ENDIAN_BIG) {
+			KUNIT_EXPECT_EQ(test, def, be16_to_cpu(rval[i]));
+		} else {
+			KUNIT_EXPECT_EQ(test, def, le16_to_cpu(rval[i]));
+		}
+	}
+	
+	kfree(rval);
+	regmap_exit(map);
+}
+
+static void raw_write_read_single(struct kunit *test)
+{
+	struct raw_test_types *t = (struct raw_test_types *)test->param_value;
+	struct regmap *map;
+	struct regmap_config config;
+	struct regmap_ram_data *data;
+	u16 val;
+	unsigned int rval;
+
+	config = raw_regmap_config;
+
+	map = gen_raw_regmap(&config, t, &data);
+	KUNIT_ASSERT_FALSE(test, IS_ERR(map));
+	if (IS_ERR(map))
+		return;
+
+	get_random_bytes(&val, sizeof(val));
+
+	/* If we write a value to a register we can read it back */
+	KUNIT_EXPECT_EQ(test, 0, regmap_write(map, 0, val));
+	KUNIT_EXPECT_EQ(test, 0, regmap_read(map, 0, &rval));
+	KUNIT_EXPECT_EQ(test, val, rval);
+
+	regmap_exit(map);
+}
+
+static void raw_write(struct kunit *test)
+{
+	struct raw_test_types *t = (struct raw_test_types *)test->param_value;
+	struct regmap *map;
+	struct regmap_config config;
+	struct regmap_ram_data *data;
+	u16 *hw_buf;
+	u16 val[2];
+	unsigned int rval;
+	int i;
+
+	config = raw_regmap_config;
+
+	map = gen_raw_regmap(&config, t, &data);
+	KUNIT_ASSERT_FALSE(test, IS_ERR(map));
+	if (IS_ERR(map))
+		return;
+
+	hw_buf = (u16 *)data->vals;
+
+	get_random_bytes(&val, sizeof(val));
+
+	/* Do a raw write */
+	KUNIT_EXPECT_EQ(test, 0, regmap_raw_write(map, 2, val, sizeof(val)));
+
+	/* We should read back the new values, and defaults for the rest */
+	for (i = 0; i < config.max_register + 1; i++) {
+		KUNIT_EXPECT_EQ(test, 0, regmap_read(map, i, &rval));
+
+		switch (i) {
+		case 2:
+		case 3:
+			if (config.val_format_endian == REGMAP_ENDIAN_BIG) {
+				KUNIT_EXPECT_EQ(test, rval,
+						be16_to_cpu(val[i % 2]));
+			} else {
+				KUNIT_EXPECT_EQ(test, rval,
+						le16_to_cpu(val[i % 2]));
+			}
+			break;
+		default:
+			KUNIT_EXPECT_EQ(test, config.reg_defaults[i].def, rval);
+			break;
+		}
+	}
+
+	/* The values should appear in the "hardware" */
+	KUNIT_EXPECT_MEMEQ(test, &hw_buf[2], val, sizeof(val));
+
+	regmap_exit(map);
+}
+
+static void raw_sync(struct kunit *test)
+{
+	struct raw_test_types *t = (struct raw_test_types *)test->param_value;
+	struct regmap *map;
+	struct regmap_config config;
+	struct regmap_ram_data *data;
+	u16 val[2];
+	u16 *hw_buf;
+	unsigned int rval;
+	int i;
+
+	config = raw_regmap_config;
+
+	map = gen_raw_regmap(&config, t, &data);
+	KUNIT_ASSERT_FALSE(test, IS_ERR(map));
+	if (IS_ERR(map))
+		return;
+
+	hw_buf = (u16 *)data->vals;
+
+	get_random_bytes(&val, sizeof(val));
+
+	/* Do a regular write and a raw write in cache only mode */
+	regcache_cache_only(map, true);
+	KUNIT_EXPECT_EQ(test, 0, regmap_raw_write(map, 2, val, sizeof(val)));
+	if (config.val_format_endian == REGMAP_ENDIAN_BIG)
+		KUNIT_EXPECT_EQ(test, 0, regmap_write(map, 6,
+						      be16_to_cpu(val[0])));
+	else
+		KUNIT_EXPECT_EQ(test, 0, regmap_write(map, 6,
+						      le16_to_cpu(val[0])));
+
+	/* We should read back the new values, and defaults for the rest */
+	for (i = 0; i < config.max_register + 1; i++) {
+		KUNIT_EXPECT_EQ(test, 0, regmap_read(map, i, &rval));
+
+		switch (i) {
+		case 2:
+		case 3:
+		case 6:
+			if (config.val_format_endian == REGMAP_ENDIAN_BIG) {
+				KUNIT_EXPECT_EQ(test, rval,
+						be16_to_cpu(val[i % 2]));
+			} else {
+				KUNIT_EXPECT_EQ(test, rval,
+						le16_to_cpu(val[i % 2]));
+			}
+			break;
+		default:
+			KUNIT_EXPECT_EQ(test, config.reg_defaults[i].def, rval);
+			break;
+		}
+	}
+	
+	/* The values should not appear in the "hardware" */
+	KUNIT_EXPECT_MEMNEQ(test, &hw_buf[2], val, sizeof(val));
+	KUNIT_EXPECT_MEMNEQ(test, &hw_buf[6], val, sizeof(u16));
+
+	for (i = 0; i < config.max_register + 1; i++)
+		data->written[i] = false;
+
+	/* Do the sync */
+	regcache_cache_only(map, false);
+	regcache_mark_dirty(map);
+	KUNIT_EXPECT_EQ(test, 0, regcache_sync(map));
+
+	/* The values should now appear in the "hardware" */
+	KUNIT_EXPECT_MEMEQ(test, &hw_buf[2], val, sizeof(val));
+	KUNIT_EXPECT_MEMEQ(test, &hw_buf[6], val, sizeof(u16));
+
+	regmap_exit(map);
+}
+
 static struct kunit_case regmap_test_cases[] = {
 	KUNIT_CASE_PARAM(basic_read_write, regcache_types_gen_params),
 	KUNIT_CASE_PARAM(bulk_write, regcache_types_gen_params),
@@ -727,6 +1048,12 @@ static struct kunit_case regmap_test_cases[] = {
 	KUNIT_CASE_PARAM(cache_sync_defaults, real_cache_types_gen_params),
 	KUNIT_CASE_PARAM(cache_sync_patch, real_cache_types_gen_params),
 	KUNIT_CASE_PARAM(cache_drop, sparse_cache_types_gen_params),
+
+	KUNIT_CASE_PARAM(raw_read_defaults_single, raw_test_types_gen_params),
+	KUNIT_CASE_PARAM(raw_read_defaults, raw_test_types_gen_params),
+	KUNIT_CASE_PARAM(raw_write_read_single, raw_test_types_gen_params),
+	KUNIT_CASE_PARAM(raw_write, raw_test_types_gen_params),
+	KUNIT_CASE_PARAM(raw_sync, raw_test_cache_types_gen_params),
 	{}
 };
 

-- 
2.30.2

Re: [PATCH 0/2] regmap: Provide basic test coverage for raw I/O

[Mark Brown]

On Sun, 11 Jun 2023 14:25:01 +0100, Mark Brown wrote:
> Our existing coverage only deals with buses that provide single register
> read and write operations, extend it to cover raw buses using a similar
> approach with a RAM backed register map that the tests can inspect to
> check operations.  This coverage could be more complete but provides a
> good start.
> 
> 
> [...]

Applied to

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

Thanks!

[1/2] regmap: Provide a ram backed regmap with raw support
      commit: 65dd2f671875b1d97b6fa9bcf7677f5e1c55f776
[2/2] regmap: Provide basic KUnit coverage for the raw register I/O
      commit: 155a6bd6375b584c8bdbf963b8ddef672ff9aca3

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