[Patch v2 1/2] 5200/mpc: improve i2c bus error recovery

[Joakim Tjernlund]

"Albrecht Dreß" <albrecht.dress at arcor.de> wrote on 2010/02/18 16:04:16:
>
> Hi Joakim:
>
> [snip]
> > >   static void mpc_i2c_fixup(struct mpc_i2c *i2c)
> > >   {
> > > -   writeccr(i2c, 0);
> > > -   udelay(30);
> > > -   writeccr(i2c, CCR_MEN);
> > > -   udelay(30);
> > > -   writeccr(i2c, CCR_MSTA | CCR_MTX);
> > > -   udelay(30);
> > > -   writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
> > > -   udelay(30);
> > > -   writeccr(i2c, CCR_MEN);
> > > -   udelay(30);
> > > +   int k;
> > > +   u32 delay_val = 1000000 / i2c->real_clk + 1;
> > > +
> > > +   if (delay_val < 2)
> > > +      delay_val = 2;
> > > +
> > > +   for (k = 9; k; k--) {
> > > +      writeccr(i2c, 0);
> > > +      writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
> > > +      udelay(delay_val);
> > > +      writeccr(i2c, CCR_MEN);
> > > +      udelay(delay_val << 1);
> > > +   }
> > >   }
> >
> > I am curious, didn't old method work with by just wrapping
> > a for(k=9; k; k--) around it? How did the wave form look?
>
> Sure does that work!  The waveform was somewhat "streched", mainly due to the
> delays between some of the writeccr() calls which don't change the sda/scl
> lines.  Unfortunately I didn't take shots from the scope.

Yeah, the long delays has to go. So the wave form was the same but more stretched
in time? I ask because I don't understand the writeccr(i2c, CCR_MSTA | CCR_MTX);
is supposed to do.

>
> However, for *one* cycle, the old code needed (only counting the udelay's) 150
> us.  For 9 cycles, it's 1.35 ms, which isn't really nice ;-).  At 375 kHz real
> clock rate, delay_val is 3, i.e. each cycle consumes 9 us, or 81 us for the
> whole fixup procedure.  If the clock is slower, the gain is of course a lot
> smaller, and at 20.5 kHz each cycle again needs 150 us...
>
> My feeling is that the delays used in the old code are just "some" values
> which work for sure, to if you like, my change is basically optimisation...

The old code only works when the device is stuck at the last bit. To cope
with any bit (worst case is the first bit) you need 9 cycles, 8 bits + ack = 9

Just toggling the clock 9 cycles should unlock any slave stuck in a read operation.
To unlock slaves stuck in a write operation you also need to generate a START in
every cycle too.

As far as I can tell your patch does all of the above so

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>

>
> BTW, related to your earlier question, I checked the timings recorded with the
> scope at 100 and at 20 kHz against the nxp's "I2C bus specification and user
> manual", rev. 03 - everything seems to be fine.

Good, thanks.

 Jocke