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
