Re: [PATCH] i2c: mux: pca954x: Disable cacheing of the last channel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Nov 22, 2019 at 2:13 PM Peter Rosin <peda@xxxxxxxxxx> wrote:
>
> On 2019-11-22 05:38, Shubhrajyoti Datta wrote:
> > On Fri, Nov 22, 2019 at 6:23 AM Peter Rosin <peda@xxxxxxxxxx> wrote:
> >>
> >> On 2019-11-20 10:21, Shubhrajyoti Datta wrote:
> >>> Hi Peter ,
> >>> thanks for the review,
> >>>
> >>> On Tue, Nov 19, 2019 at 4:35 AM Peter Rosin <peda@xxxxxxxxxx> wrote:
> >>>>
> >>>> On 2019-11-14 09:17, shubhrajyoti.datta@xxxxxxxxx wrote:
> >>>>> From: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxxxxx>
> >>>>>
> >>>>> In case of multimaster configuration the last channel cached value is
> >>>>> not reliable. Basically the first processor/master does a write to the
> >>>>> mux and then to the intended slave, it caches the value.
> >>>>> Now the second processor/processor does a write to mux on another
> >>>>> channel and writes to another slave.
> >>>>> The first processor/master when it attempts to write the slave
> >>>>> skips the mux as it relies on the mux channel being the same as the
> >>>>> intended. This causes an issue.
> >>>>>
> >>>>> To fix that write always to the mux address.
> >>>>
> >>>> Thanks for your patch.
> >>>>
> >>>> However, I don't really see how this fixes anything. If you have
> >>>> multiple masters competing for the same mux, all bets are off and any
> >>>> solution not involving an out-of-band channel where the masters can
> >>>> coordinate will be racy, broken and dangerous.
> >>>> And since you need that
> >>>> extra channel anyway, it might as well also be used to coordinate when
> >>>> the cache needs to be invalidated.
> >>>>
> >>>> At the very least, all limitations needs to be carefully documented,
> >>>> but that does not mean that I will ever like it. In short, I'm extremely
> >>>> reluctant to add a glgllike this.
> >>>>
> >>>> Cheers,
> >>>> Peter
> >>>
> >>> I agree does the below patch make sense.
> >>
> >> This patch is severely white-space damaged and I have a hard time reading
> >> the details so please fix your setup. However, I gather the idea is to
> >> rely on having all masters configured to idle the mux when they don't use
> >> it. That's also racy since multiple masters can all read the zero, and
> >> deduce that the mux is free, then all of them write their thing to the
> >> mux, and proceed as if they own it. That spells disaster.
> > However since the bus is locked when the master is transacting others
> > will get bus
> > busy or an arbitration lost if they start together.
>
> Not necessarily, since a muxed transaction with some slave on the other
> side of the mux will consist of (at least) four independent transfers (with
> this patch).
>
> 1. check that the mux state is idle
> 2. set the mux to the intended child bus
> 3. do the "useful" transfer to the slave on the child bus
> 4. reset the mux to idle
>
> Two masters might very well get past 1 without noticing each other, which
> is the big fail in your patch. They might also very well get past 2 without
> running into arbitration. You cannot be sure that a master is able to put
> these four transactions on the bus back-to-back, at least not in the Linux
> case (there might e.g. be a reschedule to some totally unrelated work). And
> even if you could, two masters could in theory be completely in sync so that
> both masters think they have succeeded right until they want to set some bit
> in the mux register differently. So, it's just fragile. And even if they do
> run into each other on 1 or 2 on the I2C bus level, you have no code for
> handling that so they will probably just retry a bit later. In other words,
> the race is on, and getting more than one master past 1 before any of them
> hit 2 is enough to get into trouble.
>
> The problem is that, without coordination, the other masters do not see
> these four transactions as a unit. You *need* arbitration on a higher
> level than individual transfers.
I agree will get back once I check some arbitration mechanism like
hardware spinlock
or gpio stuff thanks.

>
> Cheers,
> Peter



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux