Re: [PATCH v3 1/1] regmap: Synchronize cache for the page selector
From: Andy Shevchenko
Date: Fri Jan 17 2025 - 09:31:14 EST
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@xxxxxxxxxxxxxxx>
> >
> > 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):