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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <Z5kJLrE6xOzOKaeb@smile.fi.intel.com>
Date: Tue, 28 Jan 2025 18:43:26 +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 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@...get:~# 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@...get:~# 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