* [PATCH v5 1/1] regmap: Synchronize cache for the page selector @ 2026-03-02 18:43 ` Andy Shevchenko 2026-03-02 19:03 ` Marek Szyprowski 0 siblings, 1 reply; 3+ messages in thread From: Andy Shevchenko @ 2026-03-02 18:43 UTC (permalink / raw) To: Andy Shevchenko, linux-kernel, driver-core Cc: Mark Brown, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Dmitry Baryshkov, Marek Szyprowski 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@linux.intel.com> --- 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; -- 2.50.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v5 1/1] regmap: Synchronize cache for the page selector 2026-03-02 18:43 ` [PATCH v5 1/1] regmap: Synchronize cache for the page selector Andy Shevchenko @ 2026-03-02 19:03 ` Marek Szyprowski 2026-03-11 21:51 ` Andy Shevchenko 0 siblings, 1 reply; 3+ messages in thread From: Marek Szyprowski @ 2026-03-02 19:03 UTC (permalink / raw) To: Andy Shevchenko, linux-kernel, driver-core Cc: Mark Brown, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Dmitry Baryshkov 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@linux.intel.com> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > > 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v5 1/1] regmap: Synchronize cache for the page selector 2026-03-02 19:03 ` Marek Szyprowski @ 2026-03-11 21:51 ` Andy Shevchenko 0 siblings, 0 replies; 3+ messages in thread From: Andy Shevchenko @ 2026-03-11 21:51 UTC (permalink / raw) To: Marek Szyprowski Cc: linux-kernel, driver-core, Mark Brown, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Dmitry Baryshkov On Mon, Mar 02, 2026 at 08:03:11PM +0100, Marek Szyprowski wrote: > 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@linux.intel.com> > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> Thank you! Not sure if Dmitry will be able to test this, he didn't react to this message, but at least the Marek's case is confirmed to working. Can we take this in? Dmitry, would you be able to test this change? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-11 21:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20260302184802eucas1p2704b309b3953b523d816b51e9b49b196@eucas1p2.samsung.com>
2026-03-02 18:43 ` [PATCH v5 1/1] regmap: Synchronize cache for the page selector Andy Shevchenko
2026-03-02 19:03 ` Marek Szyprowski
2026-03-11 21:51 ` Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox