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

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

[Andy Shevchenko]

If the selector register is represented in each page, its value
in accordance to the debugfs is stale because it gets synchronized
only after the real page switch happens. Synchronize cache for
the page selector.

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>
---
v3: rebased on the top of latest vanilla
 drivers/base/regmap/regmap.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index f2843f814675..197c79b66828 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1557,24 +1557,40 @@ 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. */
+	/*
+	 * 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.
+	 */
 	if (val_num > 1 ||
 	    range->window_start + win_offset != range->selector_reg) {
+		unsigned int page_off = win_page * range->window_len;
+		unsigned int sel_offset = range->selector_reg - range->window_start;
+		unsigned int sel_register = range->range_min + page_off + sel_offset;
+		unsigned int val = win_page << range->selector_shift;
+		unsigned int mask = range->selector_mask;
+
 		/* Use separate work_buf during page switching */
 		orig_work_buf = map->work_buf;
 		map->work_buf = map->selector_work_buf;
 
-		ret = _regmap_update_bits(map, range->selector_reg,
-					  range->selector_mask,
-					  win_page << range->selector_shift,
+		ret = _regmap_update_bits(map, range->selector_reg, mask, val,
 					  &page_chg, false);
 
 		map->work_buf = orig_work_buf;
 
 		if (ret != 0)
 			return ret;
+
+		/*
+		 * If selector register has been just updated, update the respective
+		 * virtual copy as well.
+		 */
+		if (page_chg &&
+		    in_range(range->selector_reg, range->window_start, range->window_len))
+			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
 	}
 
 	*reg = range->window_start + win_offset;
-- 
2.43.0.rc1.1336.g36b5255a03ac

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

[Mark Brown]

On Thu, 16 Jan 2025 14:42:48 +0200, Andy Shevchenko wrote:
> If the selector register is represented in each page, its value
> in accordance to the debugfs is stale because it gets synchronized
> only after the real page switch happens. Synchronize cache for
> the page selector.
> 
> Before (offset followed by hexdump, the first byte is selector):
> 
> [...]

Applied to

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

Thanks!

[1/1] regmap: Synchronize cache for the page selector
      commit: 1fd60ed1700cf25d7ee233580c08fda755f9a61b

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

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

[Marek Szyprowski]

Dear All,

On 16.01.2025 13:42, Andy Shevchenko wrote:
> If the selector register is represented in each page, its value
> in accordance to the debugfs is stale because it gets synchronized
> only after the real page switch happens. Synchronize cache for
> the page selector.
>
> 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>

This patch landed in linux-next some time ago as commit 1fd60ed1700c 
("regmap: Synchronize cache for the page selector"). Today I've noticed 
that it causes a regression for Lontium LT9611UXC HDMI bridge driver. 
With today's linux-next I got the following messages on QCom RB5 board:

# dmesg | grep  lt9611uxc
[   13.737346] lt9611uxc 5-002b: LT9611 revision: 0x00.00.00
[   13.804190] lt9611uxc 5-002b: LT9611 version: 0x00
[   13.870564] lt9611uxc 5-002b: FW version 0, enforcing firmware update
[   13.877437] lt9611uxc 5-002b: Direct firmware load for 
lt9611uxc_fw.bin failed with error -2
[   13.887517] lt9611uxc 5-002b: probe with driver lt9611uxc failed with 
error -2

after reverting the $subject patch, the driver probes fine on that board.

I'm not sure if this is really a bug caused by this change or simply the 
driver already was aligned to old regmap behavior. Dmitry, could you 
check the regamp usage and review the changes introduced by this patch? 
Let me know if there is anything to check on the real hardware to help 
resolving this issue.

> ---
> v3: rebased on the top of latest vanilla
>   drivers/base/regmap/regmap.c | 28 ++++++++++++++++++++++------
>   1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index f2843f814675..197c79b66828 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -1557,24 +1557,40 @@ 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. */
> +	/*
> +	 * 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.
> +	 */
>   	if (val_num > 1 ||
>   	    range->window_start + win_offset != range->selector_reg) {
> +		unsigned int page_off = win_page * range->window_len;
> +		unsigned int sel_offset = range->selector_reg - range->window_start;
> +		unsigned int sel_register = range->range_min + page_off + sel_offset;
> +		unsigned int val = win_page << range->selector_shift;
> +		unsigned int mask = range->selector_mask;
> +
>   		/* Use separate work_buf during page switching */
>   		orig_work_buf = map->work_buf;
>   		map->work_buf = map->selector_work_buf;
>   
> -		ret = _regmap_update_bits(map, range->selector_reg,
> -					  range->selector_mask,
> -					  win_page << range->selector_shift,
> +		ret = _regmap_update_bits(map, range->selector_reg, mask, val,
>   					  &page_chg, false);
>   
>   		map->work_buf = orig_work_buf;
>   
>   		if (ret != 0)
>   			return ret;
> +
> +		/*
> +		 * If selector register has been just updated, update the respective
> +		 * virtual copy as well.
> +		 */
> +		if (page_chg &&
> +		    in_range(range->selector_reg, range->window_start, range->window_len))
> +			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
>   	}
>   
>   	*reg = range->window_start + win_offset;

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

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

[Andy Shevchenko]

On Fri, Jan 17, 2025 at 02:57:52PM +0100, Marek Szyprowski wrote:
> On 16.01.2025 13:42, Andy Shevchenko wrote:
> > If the selector register is represented in each page, its value
> > in accordance to the debugfs is stale because it gets synchronized
> > only after the real page switch happens. Synchronize cache for
> > the page selector.
> >
> > 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>
> 
> This patch landed in linux-next some time ago as commit 1fd60ed1700c 
> ("regmap: Synchronize cache for the page selector"). Today I've noticed 
> that it causes a regression for Lontium LT9611UXC HDMI bridge driver. 

Is there any datasheet link to the HW in question?

(FWIW, I have tested this with the CY8C9540 GPIO I²C expander on Intel Galileo
 Gen 1 board.)

> With today's linux-next I got the following messages on QCom RB5 board:
> 
> # dmesg | grep  lt9611uxc
> [   13.737346] lt9611uxc 5-002b: LT9611 revision: 0x00.00.00
> [   13.804190] lt9611uxc 5-002b: LT9611 version: 0x00
> [   13.870564] lt9611uxc 5-002b: FW version 0, enforcing firmware update
> [   13.877437] lt9611uxc 5-002b: Direct firmware load for 
> lt9611uxc_fw.bin failed with error -2
> [   13.887517] lt9611uxc 5-002b: probe with driver lt9611uxc failed with 
> error -2
> 
> after reverting the $subject patch, the driver probes fine on that board.
> 
> I'm not sure if this is really a bug caused by this change or simply the 
> driver already was aligned to old regmap behavior. Dmitry, could you 
> check the regamp usage and review the changes introduced by this patch? 
> Let me know if there is anything to check on the real hardware to help 
> resolving this issue.

Yes, see below. And thank you for your report!

...

> > +		/*
> > +		 * If selector register has been just updated, update the respective
> > +		 * virtual copy as well.
> > +		 */
> > +		if (page_chg &&
> > +		    in_range(range->selector_reg, range->window_start, range->window_len))
> > +			_regmap_update_bits(map, sel_register, mask, val, NULL, false);

Can you add a test printk() here to show

page_chg
range->selector_reg, range->window_start, range->window_len
sel_register, mask, val

?

And would commenting these three lines make it work again?

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Fri, Jan 17, 2025 at 04:09:58PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 17, 2025 at 02:57:52PM +0100, Marek Szyprowski wrote:
> > On 16.01.2025 13:42, Andy Shevchenko wrote:
> > > If the selector register is represented in each page, its value
> > > in accordance to the debugfs is stale because it gets synchronized
> > > only after the real page switch happens. Synchronize cache for
> > > the page selector.
> > >
> > > 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>
> > 
> > This patch landed in linux-next some time ago as commit 1fd60ed1700c 
> > ("regmap: Synchronize cache for the page selector"). Today I've noticed 
> > that it causes a regression for Lontium LT9611UXC HDMI bridge driver. 
> 
> Is there any datasheet link to the HW in question?
> 
> (FWIW, I have tested this with the CY8C9540 GPIO I²C expander on Intel Galileo
>  Gen 1 board.)
> 
> > With today's linux-next I got the following messages on QCom RB5 board:
> > 
> > # dmesg | grep  lt9611uxc
> > [   13.737346] lt9611uxc 5-002b: LT9611 revision: 0x00.00.00
> > [   13.804190] lt9611uxc 5-002b: LT9611 version: 0x00
> > [   13.870564] lt9611uxc 5-002b: FW version 0, enforcing firmware update
> > [   13.877437] lt9611uxc 5-002b: Direct firmware load for 
> > lt9611uxc_fw.bin failed with error -2
> > [   13.887517] lt9611uxc 5-002b: probe with driver lt9611uxc failed with 
> > error -2
> > 
> > after reverting the $subject patch, the driver probes fine on that board.
> > 
> > I'm not sure if this is really a bug caused by this change or simply the 
> > driver already was aligned to old regmap behavior. Dmitry, could you 
> > check the regamp usage and review the changes introduced by this patch? 
> > Let me know if there is anything to check on the real hardware to help 
> > resolving this issue.
> 
> Yes, see below. And thank you for your report!
> 
> ...
> 
> > > +		/*
> > > +		 * If selector register has been just updated, update the respective
> > > +		 * virtual copy as well.
> > > +		 */
> > > +		if (page_chg &&
> > > +		    in_range(range->selector_reg, range->window_start, range->window_len))
> > > +			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
> 
> Can you add a test printk() here to show
> 
> page_chg
> range->selector_reg, range->window_start, range->window_len
> sel_register, mask, val
> 
> ?
> 
> And would commenting these three lines make it work again?

Also try to apply this patch (while having the above lines not commented):

From 0fe5fe51d8b86305a4ca1ae44ede34a24fe2f9d7 Mon Sep 17 00:00:00 2001
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Fri, 17 Jan 2025 16:29:19 +0200
Subject: [PATCH 1/1] TBD:

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index f4c3ff1fdc69..35a1dd568bbb 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -69,7 +69,7 @@ struct lt9611uxc {
 static const struct regmap_range_cfg lt9611uxc_ranges[] = {
 	{
 		.name = "register_range",
-		.range_min =  0,
+		.range_min = 0x0100,
 		.range_max = 0xd0ff,
 		.selector_reg = LT9611_PAGE_CONTROL,
 		.selector_mask = 0xff,
-- 
2.43.0.rc1.1336.g36b5255a03ac

-- 
With Best Regards,
Andy Shevchenko

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

[Mark Brown]

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

On Fri, Jan 17, 2025 at 04:30:59PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 17, 2025 at 04:09:58PM +0200, Andy Shevchenko wrote:

> @@ -69,7 +69,7 @@ struct lt9611uxc {
>  static const struct regmap_range_cfg lt9611uxc_ranges[] = {
>  	{
>  		.name = "register_range",
> -		.range_min =  0,
> +		.range_min = 0x0100,
>  		.range_max = 0xd0ff,
>  		.selector_reg = LT9611_PAGE_CONTROL,
>  		.selector_mask = 0xff,

There's a bunch of other (generally broken) drivers with their ranges
overlapping their windows, we probably have to drop this for now.

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

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

[Marek Szyprowski]

On 17.01.2025 15:09, Andy Shevchenko wrote:

> On Fri, Jan 17, 2025 at 02:57:52PM +0100, Marek Szyprowski wrote:
>> On 16.01.2025 13:42, Andy Shevchenko wrote:
>>> If the selector register is represented in each page, its value
>>> in accordance to the debugfs is stale because it gets synchronized
>>> only after the real page switch happens. Synchronize cache for
>>> the page selector.
>>>
>>> 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>
>> This patch landed in linux-next some time ago as commit 1fd60ed1700c
>> ("regmap: Synchronize cache for the page selector"). Today I've noticed
>> that it causes a regression for Lontium LT9611UXC HDMI bridge driver.
> Is there any datasheet link to the HW in question?
>
> (FWIW, I have tested this with the CY8C9540 GPIO I²C expander on Intel Galileo
>   Gen 1 board.)

I'm not aware of it, but I'm not related to lt9611uxc driver development 
at all...

>> With today's linux-next I got the following messages on QCom RB5 board:
>>
>> # dmesg | grep  lt9611uxc
>> [   13.737346] lt9611uxc 5-002b: LT9611 revision: 0x00.00.00
>> [   13.804190] lt9611uxc 5-002b: LT9611 version: 0x00
>> [   13.870564] lt9611uxc 5-002b: FW version 0, enforcing firmware update
>> [   13.877437] lt9611uxc 5-002b: Direct firmware load for
>> lt9611uxc_fw.bin failed with error -2
>> [   13.887517] lt9611uxc 5-002b: probe with driver lt9611uxc failed with
>> error -2
>>
>> after reverting the $subject patch, the driver probes fine on that board.
>>
>> I'm not sure if this is really a bug caused by this change or simply the
>> driver already was aligned to old regmap behavior. Dmitry, could you
>> check the regamp usage and review the changes introduced by this patch?
>> Let me know if there is anything to check on the real hardware to help
>> resolving this issue.
> Yes, see below. And thank you for your report!
>
> ...
>
>>> +		/*
>>> +		 * If selector register has been just updated, update the respective
>>> +		 * virtual copy as well.
>>> +		 */
>>> +		if (page_chg &&
>>> +		    in_range(range->selector_reg, range->window_start, range->window_len))
>>> +			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
> Can you add a test printk() here to show
>
> page_chg
> range->selector_reg, range->window_start, range->window_len
> sel_register, mask, val
>
> ?
>
> And would commenting these three lines make it work again?

I've added the following debug message before that check:

printk("page_chg %d range->selector_reg %x range->window_start %x 
range->window_len %x sel_register %x mask %x val %x\n", page_chg, 
range->selector_reg, range->window_start, range->window_len, 
sel_register, mask, val);

Here is the result:

root@target:~# modprobe lontium_lt9611uxc
[   29.892962] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   29.907419] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   29.926712] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   29.958301] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   29.979951] lt9611uxc 5-002b: LT9611 revision: 0x00.00.00
[   29.987002] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   30.052370] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   30.066808] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register b0ff mask ff val b0
[   30.079859] lt9611uxc 5-002b: LT9611 version: 0x00
[   30.085990] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   30.151735] lt9611uxc 5-002b: FW version 0, enforcing firmware update
[   30.158830] lt9611uxc 5-002b: Direct firmware load for 
lt9611uxc_fw.bin failed with error -2
[   30.168653] lt9611uxc 5-002b: probe with driver lt9611uxc failed with 
error -2

after disabling the above mentioned 3 lines:

root@target:~# modprobe lontium_lt9611uxc
[   44.584893] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   44.597589] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   44.609936] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   44.622277] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 81ff mask ff val 81
[   44.634579] lt9611uxc 5-002b: LT9611 revision: 0x17.04.93
[   44.641444] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   44.710694] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   44.724579] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register b0ff mask ff val b0
[   44.736853] lt9611uxc 5-002b: LT9611 version: 0x40
[   44.743253] page_chg 1 range->selector_reg ff range->window_start 0 
range->window_len 100 sel_register 80ff mask ff val 80
[   44.816433] msm_dpu ae01000.display-controller: bound ae94000.dsi 
(ops dsi_ops [msm])
[   44.843944] msm_dpu ae01000.display-controller: bound 
ae90000.displayport-controller (ops msm_dp_display_comp_ops [msm])
[   44.862886] adreno 3d00000.gpu: supply vdd not found, using dummy 
regulator
[   44.870974] adreno 3d00000.gpu: supply vddcx not found, using dummy 
regulator
[   44.897116] platform 3d6a000.gmu: Adding to iommu group 27
[   44.938871] msm_dpu ae01000.display-controller: bound 3d00000.gpu 
(ops a3xx_ops [msm])
[   44.952435] [drm:dpu_kms_hw_init:1156] dpu hardware revision:0x60000000
[   44.976496] [drm] Initialized msm 1.12.0 for 
ae01000.display-controller on minor 0
...

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

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

[Marek Szyprowski]

On 17.01.2025 15:30, Andy Shevchenko wrote:
> On Fri, Jan 17, 2025 at 04:09:58PM +0200, Andy Shevchenko wrote:
>> On Fri, Jan 17, 2025 at 02:57:52PM +0100, Marek Szyprowski wrote:
>>> On 16.01.2025 13:42, Andy Shevchenko wrote:
>>>> If the selector register is represented in each page, its value
>>>> in accordance to the debugfs is stale because it gets synchronized
>>>> only after the real page switch happens. Synchronize cache for
>>>> the page selector.
>>>>
>>>> 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>
>>> This patch landed in linux-next some time ago as commit 1fd60ed1700c
>>> ("regmap: Synchronize cache for the page selector"). Today I've noticed
>>> that it causes a regression for Lontium LT9611UXC HDMI bridge driver.
>> Is there any datasheet link to the HW in question?
>>
>> (FWIW, I have tested this with the CY8C9540 GPIO I²C expander on Intel Galileo
>>   Gen 1 board.)
>>
>>> With today's linux-next I got the following messages on QCom RB5 board:
>>>
>>> # dmesg | grep  lt9611uxc
>>> [   13.737346] lt9611uxc 5-002b: LT9611 revision: 0x00.00.00
>>> [   13.804190] lt9611uxc 5-002b: LT9611 version: 0x00
>>> [   13.870564] lt9611uxc 5-002b: FW version 0, enforcing firmware update
>>> [   13.877437] lt9611uxc 5-002b: Direct firmware load for
>>> lt9611uxc_fw.bin failed with error -2
>>> [   13.887517] lt9611uxc 5-002b: probe with driver lt9611uxc failed with
>>> error -2
>>>
>>> after reverting the $subject patch, the driver probes fine on that board.
>>>
>>> I'm not sure if this is really a bug caused by this change or simply the
>>> driver already was aligned to old regmap behavior. Dmitry, could you
>>> check the regamp usage and review the changes introduced by this patch?
>>> Let me know if there is anything to check on the real hardware to help
>>> resolving this issue.
>> Yes, see below. And thank you for your report!
>>
>> ...
>>
>>>> +		/*
>>>> +		 * If selector register has been just updated, update the respective
>>>> +		 * virtual copy as well.
>>>> +		 */
>>>> +		if (page_chg &&
>>>> +		    in_range(range->selector_reg, range->window_start, range->window_len))
>>>> +			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
>> Can you add a test printk() here to show
>>
>> page_chg
>> range->selector_reg, range->window_start, range->window_len
>> sel_register, mask, val
>>
>> ?
>>
>> And would commenting these three lines make it work again?
> Also try to apply this patch (while having the above lines not commented):

This patch applied alone doesn't change anything. Probe still fails.

However, with the mentioned 3 lines in the regmap code commented AND 
this patch applied, lt9611uxc driver probe also fails. Does it mean that 
there is really a bug in the driver?

>  From 0fe5fe51d8b86305a4ca1ae44ede34a24fe2f9d7 Mon Sep 17 00:00:00 2001
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date: Fri, 17 Jan 2025 16:29:19 +0200
> Subject: [PATCH 1/1] TBD:
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> index f4c3ff1fdc69..35a1dd568bbb 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> @@ -69,7 +69,7 @@ struct lt9611uxc {
>   static const struct regmap_range_cfg lt9611uxc_ranges[] = {
>   	{
>   		.name = "register_range",
> -		.range_min =  0,
> +		.range_min = 0x0100,
>   		.range_max = 0xd0ff,
>   		.selector_reg = LT9611_PAGE_CONTROL,
>   		.selector_mask = 0xff,

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

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

[Andy Shevchenko]

On Fri, Jan 17, 2025 at 05:05:42PM +0100, Marek Szyprowski wrote:
> On 17.01.2025 15:30, Andy Shevchenko wrote:
> > On Fri, Jan 17, 2025 at 04:09:58PM +0200, Andy Shevchenko wrote:
> >> On Fri, Jan 17, 2025 at 02:57:52PM +0100, Marek Szyprowski wrote:
> >>> On 16.01.2025 13:42, Andy Shevchenko wrote:
> >>>> If the selector register is represented in each page, its value
> >>>> in accordance to the debugfs is stale because it gets synchronized
> >>>> only after the real page switch happens. Synchronize cache for
> >>>> the page selector.
> >>>>
> >>>> 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>
> >>> This patch landed in linux-next some time ago as commit 1fd60ed1700c
> >>> ("regmap: Synchronize cache for the page selector"). Today I've noticed
> >>> that it causes a regression for Lontium LT9611UXC HDMI bridge driver.
> >> Is there any datasheet link to the HW in question?
> >>
> >> (FWIW, I have tested this with the CY8C9540 GPIO I²C expander on Intel Galileo
> >>   Gen 1 board.)
> >>
> >>> With today's linux-next I got the following messages on QCom RB5 board:
> >>>
> >>> # dmesg | grep  lt9611uxc
> >>> [   13.737346] lt9611uxc 5-002b: LT9611 revision: 0x00.00.00
> >>> [   13.804190] lt9611uxc 5-002b: LT9611 version: 0x00
> >>> [   13.870564] lt9611uxc 5-002b: FW version 0, enforcing firmware update
> >>> [   13.877437] lt9611uxc 5-002b: Direct firmware load for
> >>> lt9611uxc_fw.bin failed with error -2
> >>> [   13.887517] lt9611uxc 5-002b: probe with driver lt9611uxc failed with
> >>> error -2
> >>>
> >>> after reverting the $subject patch, the driver probes fine on that board.
> >>>
> >>> I'm not sure if this is really a bug caused by this change or simply the
> >>> driver already was aligned to old regmap behavior. Dmitry, could you
> >>> check the regamp usage and review the changes introduced by this patch?
> >>> Let me know if there is anything to check on the real hardware to help
> >>> resolving this issue.
> >> Yes, see below. And thank you for your report!

...

> >>>> +		/*
> >>>> +		 * If selector register has been just updated, update the respective
> >>>> +		 * virtual copy as well.
> >>>> +		 */
> >>>> +		if (page_chg &&
> >>>> +		    in_range(range->selector_reg, range->window_start, range->window_len))
> >>>> +			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
> >> Can you add a test printk() here to show
> >>
> >> page_chg
> >> range->selector_reg, range->window_start, range->window_len
> >> sel_register, mask, val
> >>
> >> ?
> >>
> >> And would commenting these three lines make it work again?
> > Also try to apply this patch (while having the above lines not commented):
> 
> This patch applied alone doesn't change anything. Probe still fails.
> 
> However, with the mentioned 3 lines in the regmap code commented AND 
> this patch applied, lt9611uxc driver probe also fails.

Does it fail in the same way?

> Does it mean that there is really a bug in the driver?

Without looking at the datasheet it's hard to say. At least what I found so far
is one page of the I²C traffic dump on Windows as an example how to use their
evaluation board and software, but it doesn't unveil the bigger picture. At
least what I think is going on here is that the programming is not so easy as
just paging. Something is more complicated there.

But at least (and as Mark said) the most of the regmap based drivers got
the ranges wrong (so, at least there is one bug in the driver).

> > --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> > +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> > @@ -69,7 +69,7 @@ struct lt9611uxc {
> >   static const struct regmap_range_cfg lt9611uxc_ranges[] = {
> >   	{
> >   		.name = "register_range",
> > -		.range_min =  0,
> > +		.range_min = 0x0100,
> >   		.range_max = 0xd0ff,
> >   		.selector_reg = LT9611_PAGE_CONTROL,
> >   		.selector_mask = 0xff,

-- 
With Best Regards,
Andy Shevchenko

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

[Marek Szyprowski]

On 17.01.2025 18:28, Andy Shevchenko wrote:
> On Fri, Jan 17, 2025 at 05:05:42PM +0100, Marek Szyprowski wrote:
>> On 17.01.2025 15:30, Andy Shevchenko wrote:
>>> On Fri, Jan 17, 2025 at 04:09:58PM +0200, Andy Shevchenko wrote:
>>>> On Fri, Jan 17, 2025 at 02:57:52PM +0100, Marek Szyprowski wrote:
>>>>> On 16.01.2025 13:42, Andy Shevchenko wrote:
>>>>>> If the selector register is represented in each page, its value
>>>>>> in accordance to the debugfs is stale because it gets synchronized
>>>>>> only after the real page switch happens. Synchronize cache for
>>>>>> the page selector.
>>>>>>
>>>>>> 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>
>>>>> This patch landed in linux-next some time ago as commit 1fd60ed1700c
>>>>> ("regmap: Synchronize cache for the page selector"). Today I've noticed
>>>>> that it causes a regression for Lontium LT9611UXC HDMI bridge driver.
>>>> Is there any datasheet link to the HW in question?
>>>>
>>>> (FWIW, I have tested this with the CY8C9540 GPIO I²C expander on Intel Galileo
>>>>    Gen 1 board.)
>>>>
>>>>> With today's linux-next I got the following messages on QCom RB5 board:
>>>>>
>>>>> # dmesg | grep  lt9611uxc
>>>>> [   13.737346] lt9611uxc 5-002b: LT9611 revision: 0x00.00.00
>>>>> [   13.804190] lt9611uxc 5-002b: LT9611 version: 0x00
>>>>> [   13.870564] lt9611uxc 5-002b: FW version 0, enforcing firmware update
>>>>> [   13.877437] lt9611uxc 5-002b: Direct firmware load for
>>>>> lt9611uxc_fw.bin failed with error -2
>>>>> [   13.887517] lt9611uxc 5-002b: probe with driver lt9611uxc failed with
>>>>> error -2
>>>>>
>>>>> after reverting the $subject patch, the driver probes fine on that board.
>>>>>
>>>>> I'm not sure if this is really a bug caused by this change or simply the
>>>>> driver already was aligned to old regmap behavior. Dmitry, could you
>>>>> check the regamp usage and review the changes introduced by this patch?
>>>>> Let me know if there is anything to check on the real hardware to help
>>>>> resolving this issue.
>>>> Yes, see below. And thank you for your report!
> ...
>
>>>>>> +		/*
>>>>>> +		 * If selector register has been just updated, update the respective
>>>>>> +		 * virtual copy as well.
>>>>>> +		 */
>>>>>> +		if (page_chg &&
>>>>>> +		    in_range(range->selector_reg, range->window_start, range->window_len))
>>>>>> +			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
>>>> Can you add a test printk() here to show
>>>>
>>>> page_chg
>>>> range->selector_reg, range->window_start, range->window_len
>>>> sel_register, mask, val
>>>>
>>>> ?
>>>>
>>>> And would commenting these three lines make it work again?
>>> Also try to apply this patch (while having the above lines not commented):
>> This patch applied alone doesn't change anything. Probe still fails.
>>
>> However, with the mentioned 3 lines in the regmap code commented AND
>> this patch applied, lt9611uxc driver probe also fails.
> Does it fail in the same way?

Yes, the hw revision is reported as zero in this case: LT9611 revision: 
0x00.00.00

>> Does it mean that there is really a bug in the driver?
> Without looking at the datasheet it's hard to say. At least what I found so far
> is one page of the I²C traffic dump on Windows as an example how to use their
> evaluation board and software, but it doesn't unveil the bigger picture. At
> least what I think is going on here is that the programming is not so easy as
> just paging. Something is more complicated there.
>
> But at least (and as Mark said) the most of the regmap based drivers got
> the ranges wrong (so, at least there is one bug in the driver).

I can do more experiments if this helps. Do you need a dump of all 
regmap accesses or i2c traffic from this driver?

>>> --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
>>> +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
>>> @@ -69,7 +69,7 @@ struct lt9611uxc {
>>>    static const struct regmap_range_cfg lt9611uxc_ranges[] = {
>>>    	{
>>>    		.name = "register_range",
>>> -		.range_min =  0,
>>> +		.range_min = 0x0100,
>>>    		.range_max = 0xd0ff,
>>>    		.selector_reg = LT9611_PAGE_CONTROL,
>>>    		.selector_mask = 0xff,

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

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

[Andy Shevchenko]

On Tue, Jan 21, 2025 at 08:33:09AM +0100, Marek Szyprowski wrote:
> On 17.01.2025 18:28, Andy Shevchenko wrote:
> > On Fri, Jan 17, 2025 at 05:05:42PM +0100, Marek Szyprowski wrote:

> > Does it fail in the same way?
> 
> Yes, the hw revision is reported as zero in this case: LT9611 revision: 
> 0x00.00.00

Hmm... This is very interesting! It means that the page selector is a bit
magical there. Dmitry, can you chime in and perhaps shed some light on this?

> >> Does it mean that there is really a bug in the driver?
> > Without looking at the datasheet it's hard to say. At least what I found so far
> > is one page of the I²C traffic dump on Windows as an example how to use their
> > evaluation board and software, but it doesn't unveil the bigger picture. At
> > least what I think is going on here is that the programming is not so easy as
> > just paging. Something is more complicated there.
> >
> > But at least (and as Mark said) the most of the regmap based drivers got
> > the ranges wrong (so, at least there is one bug in the driver).
> 
> I can do more experiments if this helps. Do you need a dump of all 
> regmap accesses or i2c traffic from this driver?

It would be helpful! Traces from the failed and non-failed cases
till the firmware revision and chip ID reading would be enough to
start with.

-- 
With Best Regards,
Andy Shevchenko

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

[Marek Szyprowski]

Hi Andy,

On 21.01.2025 14:29, Andy Shevchenko wrote:
> On Tue, Jan 21, 2025 at 08:33:09AM +0100, Marek Szyprowski wrote:
>> On 17.01.2025 18:28, Andy Shevchenko wrote:
>>> On Fri, Jan 17, 2025 at 05:05:42PM +0100, Marek Szyprowski wrote:
>>>
>>> Does it fail in the same way?
>> Yes, the hw revision is reported as zero in this case: LT9611 revision:
>> 0x00.00.00
> Hmm... This is very interesting! It means that the page selector is a bit
> magical there. Dmitry, can you chime in and perhaps shed some light on this?
>
>>>> Does it mean that there is really a bug in the driver?
>>> Without looking at the datasheet it's hard to say. At least what I found so far
>>> is one page of the I²C traffic dump on Windows as an example how to use their
>>> evaluation board and software, but it doesn't unveil the bigger picture. At
>>> least what I think is going on here is that the programming is not so easy as
>>> just paging. Something is more complicated there.
>>>
>>> But at least (and as Mark said) the most of the regmap based drivers got
>>> the ranges wrong (so, at least there is one bug in the driver).
>> I can do more experiments if this helps. Do you need a dump of all
>> regmap accesses or i2c traffic from this driver?
> It would be helpful! Traces from the failed and non-failed cases
> till the firmware revision and chip ID reading would be enough to
> start with.

I'm sorry for the delay, I was a bit busy with other stuff.

Here are logs (all values are in hex):

next-20250128 (probe broken):
root@target:~# dmesg | grep regmap
[   14.817604] regmap_write reg 80ee <- 1
[   14.823036] regmap_read reg 8100 -> 0
[   14.827631] regmap_read reg 8101 -> 0
[   14.832130] regmap_read reg 8102 -> 0
[   14.841477] regmap_write reg 80ee <- 0
[   14.901796] regmap_write reg 80ee <- 1
[   14.909895] regmap_read reg b021 -> 0
[   14.927409] regmap_write reg 80ee <- 0

next-20250128 + 1fd60ed1700c reverted (probe okay):
root@target:~# dmesg | grep regmap
[   13.565920] regmap_write reg 80ee <- 1
[   13.567509] regmap_read reg 8100 -> 17
[   13.568219] regmap_read reg 8101 -> 4
[   13.568909] regmap_read reg 8102 -> 93
[   13.568927] regmap_write reg 80ee <- 0
[   13.625844] regmap_write reg 80ee <- 1
[   13.630963] regmap_read reg b021 -> 40
[   13.645522] regmap_write reg 80ee <- 0
[   13.880192] regmap_write reg 80ee <- 1
[   13.884921] regmap_read reg b023 -> 0
[   13.892159] regmap_write reg 80ee <- 0
[   13.954508] regmap_write reg 80ee <- 1
[   13.959441] regmap_read reg b023 -> 0
[   13.963269] regmap_write reg 80ee <- 0
[   14.029818] regmap_write reg 80ee <- 1
[   14.034644] regmap_read reg b023 -> 0
[   14.038491] regmap_write reg 80ee <- 0
[   16.278387] regmap_write reg 80ee <- 1
[   16.283902] regmap_read reg b023 -> 0
[   16.287944] regmap_write reg 80ee <- 0
[   16.347570] regmap_write reg 80ee <- 1
[   16.355894] regmap_read reg b023 -> 0
[   16.359748] regmap_write reg 80ee <- 0
[   16.452472] regmap_write reg 80ee <- 1
[   16.454387] regmap_write reg d00d <- 4
[   16.456228] regmap_write reg d00e <- 65
[   16.462096] regmap_write reg d00f <- 4
[   16.462971] regmap_write reg d010 <- 38
[   16.463741] regmap_write reg d011 <- 8
[   16.464741] regmap_write reg d012 <- 98
[   16.465250] regmap_write reg d013 <- 7
[   16.465764] regmap_write reg d014 <- 80
[   16.466481] regmap_write reg d015 <- 5
[   16.467156] regmap_update_bits reg d016 <- 0/f
[   16.467817] regmap_write reg d017 <- 2c
[   16.468325] regmap_update_bits reg d018 <- 0/f
[   16.469101] regmap_write reg d019 <- 4
[   16.469749] regmap_update_bits reg d01a <- 0/f
[   16.470509] regmap_write reg d01b <- 58
[   16.471041] regmap_write reg 80ee <- 0


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

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

[Andy Shevchenko]

On Tue, Jan 28, 2025 at 05:08:08PM +0100, Marek Szyprowski wrote:
> On 21.01.2025 14:29, Andy Shevchenko wrote:
> > On Tue, Jan 21, 2025 at 08:33:09AM +0100, Marek Szyprowski wrote:
> >> On 17.01.2025 18:28, Andy Shevchenko wrote:
> >>> On Fri, Jan 17, 2025 at 05:05:42PM +0100, Marek Szyprowski wrote:
> >>>
> >>> Does it fail in the same way?
> >> Yes, the hw revision is reported as zero in this case: LT9611 revision:
> >> 0x00.00.00
> > Hmm... This is very interesting! It means that the page selector is a bit
> > magical there. Dmitry, can you chime in and perhaps shed some light on this?
> >
> >>>> Does it mean that there is really a bug in the driver?
> >>> Without looking at the datasheet it's hard to say. At least what I found so far
> >>> is one page of the I²C traffic dump on Windows as an example how to use their
> >>> evaluation board and software, but it doesn't unveil the bigger picture. At
> >>> least what I think is going on here is that the programming is not so easy as
> >>> just paging. Something is more complicated there.
> >>>
> >>> But at least (and as Mark said) the most of the regmap based drivers got
> >>> the ranges wrong (so, at least there is one bug in the driver).
> >> I can do more experiments if this helps. Do you need a dump of all
> >> regmap accesses or i2c traffic from this driver?
> > It would be helpful! Traces from the failed and non-failed cases
> > till the firmware revision and chip ID reading would be enough to
> > start with.
> 
> I'm sorry for the delay, I was a bit busy with other stuff.

No problem and thanks for sharing.

> Here are logs (all values are in hex):
> 
> next-20250128 (probe broken):
> root@target:~# dmesg | grep regmap
> [   14.817604] regmap_write reg 80ee <- 1
> [   14.823036] regmap_read reg 8100 -> 0
> [   14.827631] regmap_read reg 8101 -> 0
> [   14.832130] regmap_read reg 8102 -> 0



> next-20250128 + 1fd60ed1700c reverted (probe okay):
> root@target:~# dmesg | grep regmap
> [   13.565920] regmap_write reg 80ee <- 1
> [   13.567509] regmap_read reg 8100 -> 17
> [   13.568219] regmap_read reg 8101 -> 4
> [   13.568909] regmap_read reg 8102 -> 93

Something is missing here. Like we have an identical start and an immediate
failure. If you did it via printk() approach, it's probably wrong as my patch
uses internal regmap function. Most likely you need to enable trace events
for regmap and collect those for let's say 2 seconds:

	echo 1 > ...trace events...
	modprobe ...
	sleep 2
	echo 0 > ...trace events...

and dump the buffer to a file. It might have though more than needed
as some other devices might also use regmap at the same time. I don't remember
if the trace events for regmap have a device instance name field which can be
used as a filter.

Alternatively you may also try to add a printk() into regmap core, but I don't
think it's more practical than trace events.

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Tue, Jan 28, 2025 at 06:43:26PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 28, 2025 at 05:08:08PM +0100, Marek Szyprowski wrote:
> > On 21.01.2025 14:29, Andy Shevchenko wrote:
> > > On Tue, Jan 21, 2025 at 08:33:09AM +0100, Marek Szyprowski wrote:
> > >> On 17.01.2025 18:28, Andy Shevchenko wrote:
> > >>> On Fri, Jan 17, 2025 at 05:05:42PM +0100, Marek Szyprowski wrote:
> > >>>
> > >>> Does it fail in the same way?
> > >> Yes, the hw revision is reported as zero in this case: LT9611 revision:
> > >> 0x00.00.00
> > > Hmm... This is very interesting! It means that the page selector is a bit
> > > magical there. Dmitry, can you chime in and perhaps shed some light on this?
> > >
> > >>>> Does it mean that there is really a bug in the driver?
> > >>> Without looking at the datasheet it's hard to say. At least what I found so far
> > >>> is one page of the I²C traffic dump on Windows as an example how to use their
> > >>> evaluation board and software, but it doesn't unveil the bigger picture. At
> > >>> least what I think is going on here is that the programming is not so easy as
> > >>> just paging. Something is more complicated there.
> > >>>
> > >>> But at least (and as Mark said) the most of the regmap based drivers got
> > >>> the ranges wrong (so, at least there is one bug in the driver).
> > >> I can do more experiments if this helps. Do you need a dump of all
> > >> regmap accesses or i2c traffic from this driver?
> > > It would be helpful! Traces from the failed and non-failed cases
> > > till the firmware revision and chip ID reading would be enough to
> > > start with.
> > 
> > I'm sorry for the delay, I was a bit busy with other stuff.
> 
> No problem and thanks for sharing.
> 
> > Here are logs (all values are in hex):
> > 
> > next-20250128 (probe broken):
> > root@target:~# dmesg | grep regmap
> > [   14.817604] regmap_write reg 80ee <- 1
> > [   14.823036] regmap_read reg 8100 -> 0
> > [   14.827631] regmap_read reg 8101 -> 0
> > [   14.832130] regmap_read reg 8102 -> 0
> 
> 
> 
> > next-20250128 + 1fd60ed1700c reverted (probe okay):
> > root@target:~# dmesg | grep regmap
> > [   13.565920] regmap_write reg 80ee <- 1
> > [   13.567509] regmap_read reg 8100 -> 17
> > [   13.568219] regmap_read reg 8101 -> 4
> > [   13.568909] regmap_read reg 8102 -> 93
> 
> Something is missing here. Like we have an identical start and an immediate
> failure. If you did it via printk() approach, it's probably wrong as my patch
> uses internal regmap function. Most likely you need to enable trace events
> for regmap and collect those for let's say 2 seconds:
> 
> 	echo 1 > ...trace events...
> 	modprobe ...
> 	sleep 2
> 	echo 0 > ...trace events...
> 
> and dump the buffer to a file. It might have though more than needed
> as some other devices might also use regmap at the same time. I don't remember
> if the trace events for regmap have a device instance name field which can be
> used as a filter.
> 
> Alternatively you may also try to add a printk() into regmap core, but I don't
> think it's more practical than trace events.

Meanwhile, can you test this patch (on top of non-working case)?

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 2314744201b4..f799a7a80231 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1553,8 +1553,19 @@ static int _regmap_select_page(struct regmap *map, unsigned int *reg,
 		 * virtual copy as well.
 		 */
 		if (page_chg &&
-		    in_range(range->selector_reg, range->window_start, range->window_len))
+		    in_range(range->selector_reg, range->window_start, range->window_len)) {
+			bool bypass, cache_only;
+
+			bypass = map->cache_bypass;
+			cache_only = map->cache_only;
+			map->cache_bypass = false;
+			map->cache_only = true;
+
 			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
+
+			map->cache_bypass = bypass;
+			map->cache_only = cache_only;
+		}
 	}
 
 	*reg = range->window_start + win_offset;

If I understood the case, the affected driver doesn't use case and we actually
write to the selector register twice which most likely messes up the things.
But this is only a theory (since we don't have the traces yet).

-- 
With Best Regards,
Andy Shevchenko

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

[Marek Szyprowski]

On 29.01.2025 16:07, Andy Shevchenko wrote:
>> [...]
> Meanwhile, can you test this patch (on top of non-working case)?
>
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index 2314744201b4..f799a7a80231 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -1553,8 +1553,19 @@ static int _regmap_select_page(struct regmap *map, unsigned int *reg,
>   		 * virtual copy as well.
>   		 */
>   		if (page_chg &&
> -		    in_range(range->selector_reg, range->window_start, range->window_len))
> +		    in_range(range->selector_reg, range->window_start, range->window_len)) {
> +			bool bypass, cache_only;
> +
> +			bypass = map->cache_bypass;
> +			cache_only = map->cache_only;
> +			map->cache_bypass = false;
> +			map->cache_only = true;
> +
>   			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
> +
> +			map->cache_bypass = bypass;
> +			map->cache_only = cache_only;
> +		}
>   	}
>   
>   	*reg = range->window_start + win_offset;
>
> If I understood the case, the affected driver doesn't use case and we actually
> write to the selector register twice which most likely messes up the things.
> But this is only a theory (since we don't have the traces yet).

Bingo! This patch (on top of current linux-next) fixed the probe issue. 
Feel free to add:

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

(although I'm not sure if this is a fix for the generic case or just 
this driver)

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

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

[Mark Brown]

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

On Wed, Jan 29, 2025 at 06:50:57PM +0100, Marek Szyprowski wrote:

> Bingo! This patch (on top of current linux-next) fixed the probe issue. 
> Feel free to add:

> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> (although I'm not sure if this is a fix for the generic case or just 
> this driver)

It's a fix for a broader class of issue, if there's other issues or not
is a separate question.

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

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

[Andy Shevchenko]

On Wed, Jan 29, 2025 at 06:50:57PM +0100, Marek Szyprowski wrote:
> On 29.01.2025 16:07, Andy Shevchenko wrote:
> >> [...]
> > Meanwhile, can you test this patch (on top of non-working case)?
> >
> > If I understood the case, the affected driver doesn't use case and we actually
> > write to the selector register twice which most likely messes up the things.
> > But this is only a theory (since we don't have the traces yet).
> 
> Bingo! This patch (on top of current linux-next) fixed the probe issue. 
> Feel free to add:
> 
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Thank you! I will test it on my HW as well to confirm that there no regressions
and will submit a formal fix.

> (although I'm not sure if this is a fix for the generic case or just 
> this driver)

This is for all cases where cache is not in use.

-- 
With Best Regards,
Andy Shevchenko

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

[Dmitry Baryshkov]

On Wed, Jan 29, 2025 at 05:07:52PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 28, 2025 at 06:43:26PM +0200, Andy Shevchenko wrote:
> > On Tue, Jan 28, 2025 at 05:08:08PM +0100, Marek Szyprowski wrote:
> > > On 21.01.2025 14:29, Andy Shevchenko wrote:
> > > > On Tue, Jan 21, 2025 at 08:33:09AM +0100, Marek Szyprowski wrote:
> > > >> On 17.01.2025 18:28, Andy Shevchenko wrote:
> > > >>> On Fri, Jan 17, 2025 at 05:05:42PM +0100, Marek Szyprowski wrote:
> > > >>>
> > > >>> Does it fail in the same way?
> > > >> Yes, the hw revision is reported as zero in this case: LT9611 revision:
> > > >> 0x00.00.00
> > > > Hmm... This is very interesting! It means that the page selector is a bit
> > > > magical there. Dmitry, can you chime in and perhaps shed some light on this?
> > > >
> > > >>>> Does it mean that there is really a bug in the driver?
> > > >>> Without looking at the datasheet it's hard to say. At least what I found so far
> > > >>> is one page of the I²C traffic dump on Windows as an example how to use their
> > > >>> evaluation board and software, but it doesn't unveil the bigger picture. At
> > > >>> least what I think is going on here is that the programming is not so easy as
> > > >>> just paging. Something is more complicated there.
> > > >>>
> > > >>> But at least (and as Mark said) the most of the regmap based drivers got
> > > >>> the ranges wrong (so, at least there is one bug in the driver).
> > > >> I can do more experiments if this helps. Do you need a dump of all
> > > >> regmap accesses or i2c traffic from this driver?
> > > > It would be helpful! Traces from the failed and non-failed cases
> > > > till the firmware revision and chip ID reading would be enough to
> > > > start with.
> > > 
> > > I'm sorry for the delay, I was a bit busy with other stuff.
> > 
> > No problem and thanks for sharing.
> > 
> > > Here are logs (all values are in hex):
> > > 
> > > next-20250128 (probe broken):
> > > root@target:~# dmesg | grep regmap
> > > [   14.817604] regmap_write reg 80ee <- 1
> > > [   14.823036] regmap_read reg 8100 -> 0
> > > [   14.827631] regmap_read reg 8101 -> 0
> > > [   14.832130] regmap_read reg 8102 -> 0
> > 
> > 
> > 
> > > next-20250128 + 1fd60ed1700c reverted (probe okay):
> > > root@target:~# dmesg | grep regmap
> > > [   13.565920] regmap_write reg 80ee <- 1
> > > [   13.567509] regmap_read reg 8100 -> 17
> > > [   13.568219] regmap_read reg 8101 -> 4
> > > [   13.568909] regmap_read reg 8102 -> 93
> > 
> > Something is missing here. Like we have an identical start and an immediate
> > failure. If you did it via printk() approach, it's probably wrong as my patch
> > uses internal regmap function. Most likely you need to enable trace events
> > for regmap and collect those for let's say 2 seconds:
> > 
> > 	echo 1 > ...trace events...
> > 	modprobe ...
> > 	sleep 2
> > 	echo 0 > ...trace events...
> > 
> > and dump the buffer to a file. It might have though more than needed
> > as some other devices might also use regmap at the same time. I don't remember
> > if the trace events for regmap have a device instance name field which can be
> > used as a filter.
> > 
> > Alternatively you may also try to add a printk() into regmap core, but I don't
> > think it's more practical than trace events.
> 
> Meanwhile, can you test this patch (on top of non-working case)?
> 
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index 2314744201b4..f799a7a80231 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -1553,8 +1553,19 @@ static int _regmap_select_page(struct regmap *map, unsigned int *reg,
>  		 * virtual copy as well.
>  		 */
>  		if (page_chg &&
> -		    in_range(range->selector_reg, range->window_start, range->window_len))
> +		    in_range(range->selector_reg, range->window_start, range->window_len)) {
> +			bool bypass, cache_only;
> +
> +			bypass = map->cache_bypass;
> +			cache_only = map->cache_only;
> +			map->cache_bypass = false;
> +			map->cache_only = true;
> +
>  			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
> +
> +			map->cache_bypass = bypass;
> +			map->cache_only = cache_only;
> +		}
>  	}
>  
>  	*reg = range->window_start + win_offset;
> 
> If I understood the case, the affected driver doesn't use case and we actually
> write to the selector register twice which most likely messes up the things.

Unfortunately I can not comment regarding the LT9611UXC itself, the
datasheet that I have here is pretty cryptic, incomplete and partially
written in Mandarin.

This patch though fixes an issue for me too, So:

Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> # Qualcomm RB1

> But this is only a theory (since we don't have the traces yet).
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

-- 
With best wishes
Dmitry

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

[Andy Shevchenko]

On Sat, Feb 01, 2025 at 07:18:56PM +0200, Dmitry Baryshkov wrote:
> On Wed, Jan 29, 2025 at 05:07:52PM +0200, Andy Shevchenko wrote:
> > On Tue, Jan 28, 2025 at 06:43:26PM +0200, Andy Shevchenko wrote:
> > > On Tue, Jan 28, 2025 at 05:08:08PM +0100, Marek Szyprowski wrote:
> > > > On 21.01.2025 14:29, Andy Shevchenko wrote:
> > > > > On Tue, Jan 21, 2025 at 08:33:09AM +0100, Marek Szyprowski wrote:
> > > > >> On 17.01.2025 18:28, Andy Shevchenko wrote:
> > > > >>> On Fri, Jan 17, 2025 at 05:05:42PM +0100, Marek Szyprowski wrote:
> > > > >>>
> > > > >>> Does it fail in the same way?
> > > > >> Yes, the hw revision is reported as zero in this case: LT9611 revision:
> > > > >> 0x00.00.00
> > > > > Hmm... This is very interesting! It means that the page selector is a bit
> > > > > magical there. Dmitry, can you chime in and perhaps shed some light on this?
> > > > >
> > > > >>>> Does it mean that there is really a bug in the driver?
> > > > >>> Without looking at the datasheet it's hard to say. At least what I found so far
> > > > >>> is one page of the I²C traffic dump on Windows as an example how to use their
> > > > >>> evaluation board and software, but it doesn't unveil the bigger picture. At
> > > > >>> least what I think is going on here is that the programming is not so easy as
> > > > >>> just paging. Something is more complicated there.
> > > > >>>
> > > > >>> But at least (and as Mark said) the most of the regmap based drivers got
> > > > >>> the ranges wrong (so, at least there is one bug in the driver).
> > > > >> I can do more experiments if this helps. Do you need a dump of all
> > > > >> regmap accesses or i2c traffic from this driver?
> > > > > It would be helpful! Traces from the failed and non-failed cases
> > > > > till the firmware revision and chip ID reading would be enough to
> > > > > start with.
> > > > 
> > > > I'm sorry for the delay, I was a bit busy with other stuff.
> > > 
> > > No problem and thanks for sharing.
> > > 
> > > > Here are logs (all values are in hex):
> > > > 
> > > > next-20250128 (probe broken):
> > > > root@target:~# dmesg | grep regmap
> > > > [   14.817604] regmap_write reg 80ee <- 1
> > > > [   14.823036] regmap_read reg 8100 -> 0
> > > > [   14.827631] regmap_read reg 8101 -> 0
> > > > [   14.832130] regmap_read reg 8102 -> 0
> > > 
> > > 
> > > 
> > > > next-20250128 + 1fd60ed1700c reverted (probe okay):
> > > > root@target:~# dmesg | grep regmap
> > > > [   13.565920] regmap_write reg 80ee <- 1
> > > > [   13.567509] regmap_read reg 8100 -> 17
> > > > [   13.568219] regmap_read reg 8101 -> 4
> > > > [   13.568909] regmap_read reg 8102 -> 93
> > > 
> > > Something is missing here. Like we have an identical start and an immediate
> > > failure. If you did it via printk() approach, it's probably wrong as my patch
> > > uses internal regmap function. Most likely you need to enable trace events
> > > for regmap and collect those for let's say 2 seconds:
> > > 
> > > 	echo 1 > ...trace events...
> > > 	modprobe ...
> > > 	sleep 2
> > > 	echo 0 > ...trace events...
> > > 
> > > and dump the buffer to a file. It might have though more than needed
> > > as some other devices might also use regmap at the same time. I don't remember
> > > if the trace events for regmap have a device instance name field which can be
> > > used as a filter.
> > > 
> > > Alternatively you may also try to add a printk() into regmap core, but I don't
> > > think it's more practical than trace events.
> > 
> > Meanwhile, can you test this patch (on top of non-working case)?
> > 
> > diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> > index 2314744201b4..f799a7a80231 100644
> > --- a/drivers/base/regmap/regmap.c
> > +++ b/drivers/base/regmap/regmap.c
> > @@ -1553,8 +1553,19 @@ static int _regmap_select_page(struct regmap *map, unsigned int *reg,
> >  		 * virtual copy as well.
> >  		 */
> >  		if (page_chg &&
> > -		    in_range(range->selector_reg, range->window_start, range->window_len))
> > +		    in_range(range->selector_reg, range->window_start, range->window_len)) {
> > +			bool bypass, cache_only;
> > +
> > +			bypass = map->cache_bypass;
> > +			cache_only = map->cache_only;
> > +			map->cache_bypass = false;
> > +			map->cache_only = true;
> > +
> >  			_regmap_update_bits(map, sel_register, mask, val, NULL, false);
> > +
> > +			map->cache_bypass = bypass;
> > +			map->cache_only = cache_only;
> > +		}
> >  	}
> >  
> >  	*reg = range->window_start + win_offset;
> > 
> > If I understood the case, the affected driver doesn't use case and we actually
> > write to the selector register twice which most likely messes up the things.
> 
> Unfortunately I can not comment regarding the LT9611UXC itself, the
> datasheet that I have here is pretty cryptic, incomplete and partially
> written in Mandarin.
> 
> This patch though fixes an issue for me too, So:
> 
> Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> # Qualcomm RB1

Thank you, guys, for reporting an testing, but it seems the simple problem
to solve requires a lot of changes to be done without regressions
(this fix on fix makes a regression to those who have cache enabled), which
means that for now I propose to revert it (or drop) if possible, Mark,
what is your preference?

> > But this is only a theory (since we don't have the traces yet).

-- 
With Best Regards,
Andy Shevchenko

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

[Mark Brown]

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

On Mon, Feb 03, 2025 at 11:19:22AM +0200, Andy Shevchenko wrote:

> Thank you, guys, for reporting an testing, but it seems the simple problem
> to solve requires a lot of changes to be done without regressions
> (this fix on fix makes a regression to those who have cache enabled), which
> means that for now I propose to revert it (or drop) if possible, Mark,
> what is your preference?

I dropped it a while ago, it's not in mainline.

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

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

[Andy Shevchenko]

On Mon, Feb 03, 2025 at 12:45:27PM +0000, Mark Brown wrote:
> On Mon, Feb 03, 2025 at 11:19:22AM +0200, Andy Shevchenko wrote:
> 
> > Thank you, guys, for reporting an testing, but it seems the simple problem
> > to solve requires a lot of changes to be done without regressions
> > (this fix on fix makes a regression to those who have cache enabled), which
> > means that for now I propose to revert it (or drop) if possible, Mark,
> > what is your preference?
> 
> I dropped it a while ago, it's not in mainline.

Okay, thanks!

-- 
With Best Regards,
Andy Shevchenko