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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z4ppo8wV3nicOfAQ@smile.fi.intel.com>
Date: Fri, 17 Jan 2025 16:30:59 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Marek Szyprowski <m.szyprowski@...sung.com>
Cc: Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Danilo Krummrich <dakr@...nel.org>,
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
	DRI mailing list <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v3 1/1] regmap: Synchronize cache for the page selector

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@...ux.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@...ux.intel.com>
Date: Fri, 17 Jan 2025 16:29:19 +0200
Subject: [PATCH 1/1] TBD:

Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.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