Re: [PATCH v5 1/1] regmap: Synchronize cache for the page selector

[Marek Szyprowski]

On 02.03.2026 19:43, Andy Shevchenko wrote:
> If the selector register is represented in each page, its value
> according to the debugfs is stale because it gets synchronized
> only after the real page switch happens. Hence the regmap cache
> initialisation from the HW inherits outdated data in the selector
> register.
>
> Synchronize cache for the page selector just in time.
>
> Before (offset followed by hexdump, the first byte is selector):
>
>      // Real registers
>      18: 05 ff 00 00 ff 0f 00 00 f0 00 00 00
>      ...
>      // Virtual (per port)
>      40: 05 ff 00 00 e0 e0 00 00 00 00 00 1f
>      50: 00 ff 00 00 e0 e0 00 00 00 00 00 1f
>      60: 01 ff 00 00 ff ff 00 00 00 00 00 00
>      70: 02 ff 00 00 cf f3 00 00 00 00 00 0c
>      80: 03 ff 00 00 00 00 00 00 00 00 00 ff
>      90: 04 ff 00 00 ff 0f 00 00 f0 00 00 00
>
> After:
>
>      // Real registers
>      18: 05 ff 00 00 ff 0f 00 00 f0 00 00 00
>      ...
>      // Virtual (per port)
>      40: 00 ff 00 00 e0 e0 00 00 00 00 00 1f
>      50: 01 ff 00 00 e0 e0 00 00 00 00 00 1f
>      60: 02 ff 00 00 ff ff 00 00 00 00 00 00
>      70: 03 ff 00 00 cf f3 00 00 00 00 00 0c
>      80: 04 ff 00 00 00 00 00 00 00 00 00 ff
>      90: 05 ff 00 00 ff 0f 00 00 f0 00 00 00
>
> Fixes: 6863ca622759 ("regmap: Add support for register indirect addressing.")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> ---
>
> v5: applied fix to avoid circular dependency (infinite loop)
> v4: reworked the approach completely
>
> Dmitry,
> Please, test on your HW to be sure this will have no side effects
> in your case with LT9611.
>
> Marek, this is the same version I sent you earlier off the list.
> I haven't added any tag (while knowing that you tested it, so please
> confirm it by providing a formal Tested-by, thanks!
>
> FWIW, I have tested it on Intel Galileo board with Cypress CY8C9540 chip.
>
>   drivers/base/regmap/regmap.c | 30 ++++++++++++++++++++++++++----
>   1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index c299218849a1..13f994429b38 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -1509,6 +1509,7 @@ static int _regmap_select_page(struct regmap *map, unsigned int *reg,
>   			       unsigned int val_num)
>   {
>   	void *orig_work_buf;
> +	unsigned int selector_reg;
>   	unsigned int win_offset;
>   	unsigned int win_page;
>   	bool page_chg;
> @@ -1527,10 +1528,31 @@ static int _regmap_select_page(struct regmap *map, unsigned int *reg,
>   			return -EINVAL;
>   	}
>   
> -	/* It is possible to have selector register inside data window.
> -	   In that case, selector register is located on every page and
> -	   it needs no page switching, when accessed alone. */
> +	/*
> +	 * Calculate the address of the selector register in the corresponding
> +	 * data window if it is located on every page.
> +	 */
> +	page_chg = in_range(range->selector_reg, range->window_start, range->window_len);
> +	if (page_chg)
> +		selector_reg = range->range_min + win_page * range->window_len +
> +			       range->selector_reg - range->window_start;
> +
> +	/*
> +	 * It is possible to have selector register inside data window.
> +	 * In that case, selector register is located on every page and it
> +	 * needs no page switching, when accessed alone.
> +	 *
> +	 * Nevertheless we should synchronize the cache values for it.
> +	 * This can't be properly achieved if the selector register is
> +	 * the first and the only one to be read inside the data window.
> +	 * That's why we update it in that case as well.
> +	 *
> +	 * However, we specifically avoid updating it for the default page,
> +	 * when it's overlapped with the real data window, to prevent from
> +	 * infinite looping.
> +	 */
>   	if (val_num > 1 ||
> +	    (page_chg && selector_reg != range->selector_reg) ||
>   	    range->window_start + win_offset != range->selector_reg) {
>   		/* Use separate work_buf during page switching */
>   		orig_work_buf = map->work_buf;
> @@ -1539,7 +1561,7 @@ static int _regmap_select_page(struct regmap *map, unsigned int *reg,
>   		ret = _regmap_update_bits(map, range->selector_reg,
>   					  range->selector_mask,
>   					  win_page << range->selector_shift,
> -					  &page_chg, false);
> +					  NULL, false);
>   
>   		map->work_buf = orig_work_buf;
>   

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland