The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] regcache: Sort the local copy of an unsorted reg_defaults array
@ 2026-08-05 13:22 Peter Ujfalusi
  2026-08-05 14:07 ` Charles Keepax
  2026-08-06 12:01 ` Mark Brown
  0 siblings, 2 replies; 4+ messages in thread
From: Peter Ujfalusi @ 2026-08-05 13:22 UTC (permalink / raw)
  To: Mark Brown
  Cc: Charles Keepax, Richard Fitzgerald, Pierre-Louis Bossart,
	Peter Ujfalusi, linux-kernel

regcache_lookup_reg() bsearch()es the reg_defaults array, which requires
it to be sorted by ascending register address.  Entries following a
descending step are never found, so regcache_reg_needs_sync() reports
that they need a sync and they are written to the device on every
regcache_sync() even when they were never touched.

Detect the misordering while reg_defaults is validated against the
register stride and sort the local copy.  The check needs no new loop
and sort() only runs for the affected drivers, which are also warned
about.

Note that sort() is not stable, so for arrays with duplicated register
addresses it remains unspecified which entry is found.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
Hi,

Fixes for affected in-tree codec drivers have been posted separately, so this
is meant as a safety net for out-of-tree and future drivers rather than a
replacement for fixing them, as discussed on the Cirrus codec patches.

I'm not sure about the print level: dev_warn() makes the problem visible to
whoever boots the affected machine, but the person who can act on it is the
driver author.  Should this be dev_dbg()?

Regards,
Peter

 drivers/base/regmap/regcache.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index be167ee6f57c..aa7f6c30f232 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -123,6 +123,8 @@ static void regcache_hw_exit(struct regmap *map)
 
 int regcache_init(struct regmap *map, const struct regmap_config *config)
 {
+	bool sort_defaults = false;
+	unsigned int reg_prev = 0;
 	int count = 0;
 	int ret;
 	int i;
@@ -149,10 +151,16 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
 		return -EINVAL;
 	}
 
-	for (i = 0; i < config->num_reg_defaults; i++)
+	for (i = 0; i < config->num_reg_defaults; i++) {
 		if (config->reg_defaults[i].reg % map->reg_stride)
 			return -EINVAL;
 
+		if (reg_prev > config->reg_defaults[i].reg)
+			sort_defaults = true;
+
+		reg_prev = config->reg_defaults[i].reg;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(cache_types); i++)
 		if (cache_types[i]->type == map->cache_type)
 			break;
@@ -186,6 +194,13 @@ int regcache_init(struct regmap *map, const struct regmap_config *config)
 					sizeof(*map->reg_defaults), GFP_KERNEL);
 		if (!tmp_buf)
 			return -ENOMEM;
+
+		/* regcache_lookup_reg() bsearch()es this array */
+		if (sort_defaults) {
+			dev_warn(map->dev,
+				 "Driver needs fixing: Unsorted reg_defaults, sorting the copy\n");
+			regcache_sort_defaults(tmp_buf, map->num_reg_defaults);
+		}
 		map->reg_defaults = tmp_buf;
 	} else if (map->num_reg_defaults_raw) {
 		count = regcache_count_cacheable_registers(map);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] regcache: Sort the local copy of an unsorted reg_defaults array
  2026-08-05 13:22 [PATCH] regcache: Sort the local copy of an unsorted reg_defaults array Peter Ujfalusi
@ 2026-08-05 14:07 ` Charles Keepax
  2026-08-05 14:34   ` Mark Brown
  2026-08-06 12:01 ` Mark Brown
  1 sibling, 1 reply; 4+ messages in thread
From: Charles Keepax @ 2026-08-05 14:07 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Mark Brown, Richard Fitzgerald, Pierre-Louis Bossart,
	linux-kernel

On Wed, Aug 05, 2026 at 04:22:50PM +0300, Peter Ujfalusi wrote:
> regcache_lookup_reg() bsearch()es the reg_defaults array, which requires
> it to be sorted by ascending register address.  Entries following a
> descending step are never found, so regcache_reg_needs_sync() reports
> that they need a sync and they are written to the device on every
> regcache_sync() even when they were never touched.
> 
> Detect the misordering while reg_defaults is validated against the
> register stride and sort the local copy.  The check needs no new loop
> and sort() only runs for the affected drivers, which are also warned
> about.
> 
> Note that sort() is not stable, so for arrays with duplicated register
> addresses it remains unspecified which entry is found.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> ---
> Hi,
> 
> Fixes for affected in-tree codec drivers have been posted separately, so this
> is meant as a safety net for out-of-tree and future drivers rather than a
> replacement for fixing them, as discussed on the Cirrus codec patches.
> 
> I'm not sure about the print level: dev_warn() makes the problem visible to
> whoever boots the affected machine, but the person who can act on it is the
> driver author.  Should this be dev_dbg()?

Keep it at least as a warn, you need something annoying or no one
will take notice, it is quite likely people will apply debug on
their driver, much less likely they will apply it to the regmap
core. I still lean towards not sorting the defaults for people,
but not so strongly as to object if there is a rough concensus
forming around doing so.

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Tested-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] regcache: Sort the local copy of an unsorted reg_defaults array
  2026-08-05 14:07 ` Charles Keepax
@ 2026-08-05 14:34   ` Mark Brown
  0 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2026-08-05 14:34 UTC (permalink / raw)
  To: Charles Keepax
  Cc: Peter Ujfalusi, Richard Fitzgerald, Pierre-Louis Bossart,
	linux-kernel

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

On Wed, Aug 05, 2026 at 03:07:43PM +0100, Charles Keepax wrote:

> Keep it at least as a warn, you need something annoying or no one
> will take notice, it is quite likely people will apply debug on
> their driver, much less likely they will apply it to the regmap
> core. I still lean towards not sorting the defaults for people,
> but not so strongly as to object if there is a rough concensus
> forming around doing so.

There's far too many fixes at this point for ASoC alone, and nobody
audited any regmap users outside ASoC AFAIK.  We need to just have the
core do the right thing.  Though I see there's actually two different
comparison functions for sorting that don't always agree with each other
- I'll post a fix for that later today.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] regcache: Sort the local copy of an unsorted reg_defaults array
  2026-08-05 13:22 [PATCH] regcache: Sort the local copy of an unsorted reg_defaults array Peter Ujfalusi
  2026-08-05 14:07 ` Charles Keepax
@ 2026-08-06 12:01 ` Mark Brown
  1 sibling, 0 replies; 4+ messages in thread
From: Mark Brown @ 2026-08-06 12:01 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Charles Keepax, Richard Fitzgerald, Pierre-Louis Bossart,
	linux-kernel

On Wed, 05 Aug 2026 16:22:50 +0300, Peter Ujfalusi wrote:
> regcache: Sort the local copy of an unsorted reg_defaults array

Applied to

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

Thanks!

[1/1] regcache: Sort the local copy of an unsorted reg_defaults array
      https://git.kernel.org/broonie/regmap/c/4b05ccb17f92

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-06 13:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 13:22 [PATCH] regcache: Sort the local copy of an unsorted reg_defaults array Peter Ujfalusi
2026-08-05 14:07 ` Charles Keepax
2026-08-05 14:34   ` Mark Brown
2026-08-06 12:01 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox