Hi,

On Mon, Mar 24, 2025 at 10:31 AM Laurent Pinchart
<laurent.pinch...@ideasonboard.com> wrote:
>
> On Mon, Mar 24, 2025 at 10:17:05AM -0700, Doug Anderson wrote:
> > On Mon, Mar 24, 2025 at 9:40 AM Laurent Pinchart wrote:
> > > On Tue, Mar 18, 2025 at 10:00:31PM +0100, Wolfram Sang wrote:
> > > > Hi Laurent,
> > > >
> > > > > > Read out and check the ID registers, so we can bail out if I2C
> > > > > > communication does not work or if the device is unknown.
> > > > >
> > > > > What's the advantage of that, what are you trying to guard against ?
> > > >
> > > > That a random chip at address 0x2c will be used.
> > >
> > > Is that really a problem ? That would only occur with a broken DT, is it
> > > worth guarding against a development-time issue with a runtime check
> > > that will increase boot time for every user ?
> >
> > FWIW, this can also happen simply due to broken / damaged hardware. If
> > a board gets stressed and causes a pin to become disconnected or if a
> > regulator ages and stops providing power then we can also end up in
> > this state. Getting a nice obvious error at probe when the device
> > isn't responding at all can make problems like this much easier to
> > debug.
>
> I'm not fully convinced it's worth it, compared to an error at runtime,
> and especially given that there are way more pins than supplies or
> SCL/SDA that could suffer from a similar issue. I won't block the patch,
> but I think it's overkill.

FWIW, Wolfram's previous patch [1] did check it at runtime without a
dedicated i2c transfer. The problem was that it only handled one of
the sub-AUX-devices and left the other sub-AUX-devices dangling. Many
of the sub-AUX-devices didn't need to talk to the chip at probe time
so there wasn't a good way to make the "probe" fail for them. ...so in
this situation you'd end up with every GPIO operation or PWM operation
failing and the device would still exist. It didn't seem ideal to
me...

[1] 
http://lore.kernel.org/r/20250314224202.13128-2-wsa+rene...@sang-engineering.com

Reply via email to