Re: [PATCH 0/3] ASoC: cs35l41/cs35l45/cs4265: sort the reg_defaults tables

[Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>]

On 8/5/26 10:52, Péter Ujfalusi wrote:
> 
> 
> On 05/08/2026 11:40, Pierre-Louis Bossart wrote:
>>
>>> reg_defaults must be sorted by ascending register address as
>>> regcache_lookup_reg() locates the entries in it with bsearch(), see commit
>>> fd80df352ba1 ("regcache: Add support for sorting defaults arrays").
>>>
>>> These three tables have entries which are out of order, so the binary search
>>> does not find part of them.  For those registers regcache_reg_needs_sync()
>>> cannot compare the cached value against the default and reports that a sync
>>> is needed, so they are written to the device on every regcache_sync() even
>>> when they were never touched.
>>>
>>> The patches only reorder the existing entries, the text of every entry is
>>> kept verbatim and no default value is changed.  Each table was verified by
>>> evaluating the register addresses and replaying lib/bsearch.c on them.
>>>
>>> Entries not reachable by the binary search, per table:
>>>
>>>   cs35l41_reg           2 (of 47)
>>>   cs35l45_defaults     36 (of 73)
>>>   cs4265_reg_defaults   3 (of 16)
>>>
>>> For cs35l45 this is nearly half of the table: the DSP1_RX*_RATE and
>>> DSP1_TX*_RATE registers sit in the middle of it while their addresses are
>>> far above everything else, which cuts the search off from the whole
>>> 0x4c40 - 0xf010 range.
>>>
>>> Found by an audit of all reg_defaults tables under sound/, the SoundWire
>>> codec drivers are fixed by a separate series.
>>
>> Wow. Would it make sense to have a regmap helper to double-check the
>> addresses are indeed in-order in those reg_default tables?
>> I am not sure how this requirement can be enforced by just inspection, a
>> warning would help detect this sort of issues on more platforms.
> 
> 
> I had this first:
> diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
> index be167ee6f57c..7d2f74494645 100644
> --- a/drivers/base/regmap/regcache.c
> +++ b/drivers/base/regmap/regcache.c
> @@ -187,6 +187,9 @@ int regcache_init(struct regmap *map, const struct
> regmap_config *config)
>  		if (!tmp_buf)
>  			return -ENOMEM;
>  		map->reg_defaults = tmp_buf;
> +
> +		/* regcache_lookup_reg() bsearch()es this array */
> +		regcache_sort_defaults(tmp_buf, map->num_reg_defaults);
>  	} else if (map->num_reg_defaults_raw) {
>  		count = regcache_count_cacheable_registers(map);
>  		if (!count)
> 
> 
> But it would run for all regmap on boot and I think that would be a big
> hit on boot time.
> I guess, a debug kernel option could enable ordering check for defaults
> in regmap and warn if it finds such?

reordering the tables could take time even on a perfectly ordered case,
I was only thinking of throwing a warning with a simple loop on the table.
Maybe also something for kernel tests? A warning would require user
feedback whereas a formal test could check for hundreds of regmap defaults.