Re: [PATCH] i2c: mux: pca954x: Support multiple devices on a single reset line

[Eddie James <eajames@linux.ibm.com>]

On Fri, 2021-05-07 at 00:08 +0200, Peter Rosin wrote:
> Hi!
> 
> On 2021-05-05 23:59, Eddie James wrote:
> > Some systems connect several PCA954x devices to a single reset
> > GPIO. For
> > these devices to get out of reset and probe successfully, only the
> > first
> > device probed should change the GPIO. Add this functionality by
> > checking
> > for EBUSY when getting the GPIO fails. Then, retry getting the GPIO
> > with
> > the non-exclusive flag and wait for the reset line to drop. This
> > prevents
> > the later probes from proceding while the device is still reset.
> 
> (nit: proceeding)
> 
> The patch assumes that all muxes with interconnected resets are only
> ever reset "in symphony". But there is no guarantee anywhere that
> this
> actually holds.

Thanks for your comments Peter, you are quite right, this won't do.
I've finally come back around to this and will send a new patch that
handles it differently - please let me know what you think.

Thanks!
Eddie

> 
> So, I don't see how this can be safe. Sure, it may very well work in
> the
> majority of cases, but it seems very dangerous. If one instance
> resets
> muxes controlled by other instances, any cached value is destroyed in
> those instances and anything can happen. Sure, if you have HW like
> this,
> then you have what you have. But I don't see any good way to handle
> this case in an elegant way. If this scheme is allowed the dangers of
> relying on it at minimum needs to be documented.
> 
> And what if the second instance reads the gpio just a few ns after
> the
> reset is released? The first instance waits for 1us before proceeding
> to give the chip some time to recover from the reset, but that
> respite
> may be lost to other instances.
> 
> What if the first instance does the reset but then fails the probe
> later,
> possibly because the chip isn't there, but then other instances
> manages
> to time their probe just so that the gpio is busy at the right point,
> and then proceeds without holding a reference to the gpio. Then the
> first
> instance also lets go of the gpio and you end up with a bunch of
> instances
> relying on a pin that noone holds a reference to. Or, yet another
> instance
> enters the picture and finds the gpio free and pulls a reset behind
> the
> back of the intermediate instances which have already proceeded.
> 
> Or am I reading something wrong?
> 
> Cheers,
> Peter
>