linux-kernel - Re: [PATCH 2/2] i2c: smbus: Send alert notifications to all devices if source not found

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: <ZqdLVg6IVTjsTWb4@shikoro>
Date: Mon, 29 Jul 2024 09:57:10 +0200
From: Wolfram Sang <wsa+renesas@...g-engineering.com>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Wolfram Sang <wsa@...nel.org>, Jean Delvare <khali@...ux-fr.org>,
	linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] i2c: smbus: Send alert notifications to all devices
 if source not found

Hi Guenter,

thanks for the feedback!

> > High level question: why the retry? Did you experience address
> > collisions going away on the second try? My guess is that they would be
> > mostly persistent, so we could call smbus_do_alert_force() right away?
> > 
> 
> I honestly don't recall. I had some brute force code to trigger alerts
> on connected chips. Maybe the idea was to catch situations where another
> alert was raised after or during the first cycle.

Hmm, I'd think that SMBAlert then stays asserted and the whole alert
handling will be started right away a second time? Given that all
hardware works correctly, of course. Your setup showed that arbitration
does not work well with actual hardware. Props for finding this out!

> As for "call smbus_do_alert_force() right away", I am not sure I understand.
> Isn't that what the code is doing twice ?

It calls smbus_do_alert() twice (without '_force'). If that fails, it
calls the _force version. I am wondering now if we can't call the _force
version right after smbus_do_alert() fails once. Meaning we could remove
all the "retries" code from your patch. If there is no clear reason for
the code, not having it is easier to maintain. That's why I ask.

I hope the question is understandable now.

Happy hacking,

   Wolfram

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)