[PATCH 0/2] regmap: kunit: Add test for cache sync interaction with range selectors

[PATCH 0/2] regmap: kunit: Add test for cache sync interaction with range selectors

[Mark Brown]

Add a test for a bug reported by Hector Martin, plus a fix for a thinko
in the tests that I spotted while adding that test case.  I've not
included a fix since I believe Hector either has or is working on
something and I don't want to invalidate his work.

Hector also reports that there is a similar interaction around cache
initialisation from device defaults.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
Mark Brown (2):
      regmap: kunit: Fix marking of the range window as volatile
      regmap: kunit: Add test for cache sync interaction with ranges

 drivers/base/regmap/regmap-kunit.c | 68 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 65 insertions(+), 3 deletions(-)
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20231023-regmap-test-window-cache-c3a2b3d0a769

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

[PATCH 1/2] regmap: kunit: Fix marking of the range window as volatile

[Mark Brown]

For some reason the regmap used for testing ranges was not including the
end of the range of paged registers as volatile since it found the end by
counting from the selector register rather than the base of the window.

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

diff --git a/drivers/base/regmap/regmap-kunit.c b/drivers/base/regmap/regmap-kunit.c
index 264d29b3fced..f79fd5ec187e 100644
--- a/drivers/base/regmap/regmap-kunit.c
+++ b/drivers/base/regmap/regmap-kunit.c
@@ -445,7 +445,7 @@ static struct regmap_range_cfg test_range = {
 static bool test_range_volatile(struct device *dev, unsigned int reg)
 {
 	if (reg >= test_range.window_start &&
-	    reg <= test_range.selector_reg + test_range.window_len)
+	    reg <= test_range.window_start + test_range.window_len)
 		return true;
 
 	if (reg >= test_range.range_min && reg <= test_range.range_max)

-- 
2.30.2

[PATCH 2/2] regmap: kunit: Add test for cache sync interaction with ranges

[Mark Brown]

Hector Martin reports that since when doing a cache sync we enable cache
bypass if the selector register for a range is cached then we might leave
the physical selector register pointing to a different value to that which
we have in the cache. If we then try to write to the page that our cache
tells us is selected we will not update the selector register and write to
the wrong page. Add a test case covering this.

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

diff --git a/drivers/base/regmap/regmap-kunit.c b/drivers/base/regmap/regmap-kunit.c
index f79fd5ec187e..e14cc03a17f6 100644
--- a/drivers/base/regmap/regmap-kunit.c
+++ b/drivers/base/regmap/regmap-kunit.c
@@ -442,12 +442,20 @@ static struct regmap_range_cfg test_range = {
 	.range_max = 40,
 };
 
-static bool test_range_volatile(struct device *dev, unsigned int reg)
+static bool test_range_window_volatile(struct device *dev, unsigned int reg)
 {
 	if (reg >= test_range.window_start &&
 	    reg <= test_range.window_start + test_range.window_len)
 		return true;
 
+	return false;
+}
+
+static bool test_range_all_volatile(struct device *dev, unsigned int reg)
+{
+	if (test_range_window_volatile(dev, reg))
+		return true;
+
 	if (reg >= test_range.range_min && reg <= test_range.range_max)
 		return true;
 
@@ -465,7 +473,7 @@ static void basic_ranges(struct kunit *test)
 
 	config = test_regmap_config;
 	config.cache_type = t->type;
-	config.volatile_reg = test_range_volatile;
+	config.volatile_reg = test_range_all_volatile;
 	config.ranges = &test_range;
 	config.num_ranges = 1;
 	config.max_register = test_range.range_max;
@@ -875,6 +883,59 @@ static void cache_present(struct kunit *test)
 	regmap_exit(map);
 }
 
+/* Check that caching the window register works with sync */
+static void cache_range_window_reg(struct kunit *test)
+{
+	struct regcache_types *t = (struct regcache_types *)test->param_value;
+	struct regmap *map;
+	struct regmap_config config;
+	struct regmap_ram_data *data;
+	unsigned int val;
+	int i;
+
+	config = test_regmap_config;
+	config.cache_type = t->type;
+	config.volatile_reg = test_range_window_volatile;
+	config.ranges = &test_range;
+	config.num_ranges = 1;
+	config.max_register = test_range.range_max;
+
+	map = gen_regmap(&config, &data);
+	KUNIT_ASSERT_FALSE(test, IS_ERR(map));
+	if (IS_ERR(map))
+		return;
+
+	/* Write new values to the entire range */
+	for (i = test_range.range_min; i <= test_range.range_max; i++)
+		KUNIT_ASSERT_EQ(test, 0, regmap_write(map, i, 0));
+
+	val = data->vals[test_range.selector_reg] & test_range.selector_mask;
+	KUNIT_ASSERT_EQ(test, val, 2);
+
+	/* Write to the first register in the range to reset the page */
+	KUNIT_ASSERT_EQ(test, 0, regmap_write(map, test_range.range_min, 0));
+	val = data->vals[test_range.selector_reg] & test_range.selector_mask;
+	KUNIT_ASSERT_EQ(test, val, 0);
+
+	/* Trigger a cache sync */
+	regcache_mark_dirty(map);
+	KUNIT_ASSERT_EQ(test, 0, regcache_sync(map));
+
+	/* Write to the first register again, the page should be reset */
+	KUNIT_ASSERT_EQ(test, 0, regmap_write(map, test_range.range_min, 0));
+	val = data->vals[test_range.selector_reg] & test_range.selector_mask;
+	KUNIT_ASSERT_EQ(test, val, 0);
+
+	/* Trigger another cache sync */
+	regcache_mark_dirty(map);
+	KUNIT_ASSERT_EQ(test, 0, regcache_sync(map));
+
+	/* Write to the last register again, the page should be reset */
+	KUNIT_ASSERT_EQ(test, 0, regmap_write(map, test_range.range_max, 0));
+	val = data->vals[test_range.selector_reg] & test_range.selector_mask;
+	KUNIT_ASSERT_EQ(test, val, 2);
+}
+
 struct raw_test_types {
 	const char *name;
 
@@ -1217,6 +1278,7 @@ static struct kunit_case regmap_test_cases[] = {
 	KUNIT_CASE_PARAM(cache_sync_patch, real_cache_types_gen_params),
 	KUNIT_CASE_PARAM(cache_drop, sparse_cache_types_gen_params),
 	KUNIT_CASE_PARAM(cache_present, sparse_cache_types_gen_params),
+	KUNIT_CASE_PARAM(cache_range_window_reg, real_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),

-- 
2.30.2

Re: [PATCH 0/2] regmap: kunit: Add test for cache sync interaction with range selectors

[Mark Brown]

On Mon, 23 Oct 2023 18:19:10 +0100, Mark Brown wrote:
> Add a test for a bug reported by Hector Martin, plus a fix for a thinko
> in the tests that I spotted while adding that test case.  I've not
> included a fix since I believe Hector either has or is working on
> something and I don't want to invalidate his work.
> 
> Hector also reports that there is a similar interaction around cache
> initialisation from device defaults.
> 
> [...]

Applied to

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

Thanks!

[1/2] regmap: kunit: Fix marking of the range window as volatile
      commit: fabe32cc1eca7857837abcb56b242bc4845b7067
[2/2] regmap: kunit: Add test for cache sync interaction with ranges
      commit: 6a2e332c2cbddd17d7dcb8f334953593f1324c8e

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