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

["Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>]

On 05/08/2026 12:38, Richard Fitzgerald wrote:
> On 05/08/2026 10:25 am, Charles Keepax wrote:
>> On Wed, Aug 05, 2026 at 12:10:10PM +0300, Péter Ujfalusi wrote:
>>> On 05/08/2026 12:00, Richard Fitzgerald wrote:
>>>>>> 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.
>>>>>
>>>> It does seem probable that anything that relies on people just
>>>> remembering to keep a large table sorted is prone to breaking,
>>>> especially if the addresses are provided by named constant instead of
>>>> a list of hardcoded numbers.
>>>>
>>>> Should regmap check the table when the regmap is first created?
>>>> As it has to search the table during normal use anyway, one extra walk
>>>> when the regmap is created probably isn't a serious overhead.
>>>
>>> But it will be done for _all_ devices which uses regmap on boot, small
>>> things do add up, see my reply to Pierre-Louis.
>>
>> Indeed, if we were to add some sort of auto-checker it should
>> be guarded behind something like perhaps a Kconfig option or the
>> DEBUG define.
>>
>> Thanks,
>> Charles
> 
> regcache_init() already walks the defaults table checking the stride
> 
>     for (i = 0; i < config->num_reg_defaults; i++)
>         if (config->reg_defaults[i].reg % map->reg_stride)
>             return -EINVAL;
> 
> so checking that each entry is larger than previous is trivial extra
> overhead. I think most regmaps are cached.

Right, I think this should work:

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index aa8f2efed779..ce4b11d8a758 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -123,6 +123,7 @@ static void regcache_hw_exit(struct regmap *map)
 
 int regcache_init(struct regmap *map, const struct regmap_config *config)
 {
+	bool sort_defaults = false;
 	int count = 0;
 	int ret;
 	int i;
@@ -149,10 +150,15 @@ 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 (i && config->reg_defaults[i - 1].reg >
+		    config->reg_defaults[i].reg)
+			sort_defaults = true;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(cache_types); i++)
 		if (cache_types[i]->type == map->cache_type)
 			break;
@@ -186,6 +192,16 @@ 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, sort the local
+		 * copy if it is not ordered
+		 */
+		if (sort_defaults) {
+			dev_dbg(map->dev,
+				"Register defaults are not ordered, sorting the local 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);


-- 
Péter