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. Cheers, Peter
