Hi Joakim:

Am 18.02.10 18:14 schrieb(en) Joakim Tjernlund:
> [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.

Afaict, this is really a no-op.  The '5200 user's manual says about MEN

<snip>
* 0 module is reset and disabled. This is the Power-ON reset. When low the 
interface is held in reset, but registers can still be accessed.
* 1 I2C module is enabled. Bit must be set before other CR bits have any effect.
</snip>

The change in the MSTA is needed -with the proper delays- as to generate the 
START and STOP conditions.

Unfortunately, the data sheet is not very clear (or my English too bad), but 
reading it *after* seeing the signals on the scope, I can at least guess what 
they mean :-/

Thus, the old code will probably produce SDA and SCL held high for ~90us, then 
a SDA/SCL low for ~30us (plus/minus the delays the hw adds internally according 
to the clock setting), and then a ~30us SDA/SCL high.  It is not possible to 
get the necessary delays from the data sheet, but as I said I empirically 
verified some cases to be safe.

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.

Yes, of course...  this was the initial motivation of the patch, as I *have* a 
slave which sometimes needs more than one cycle!

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

Signed-off-by: Joakim Tjernlund <joakim.tjernl...@transmode.se>

Thanks a lot again for your time, and your helpful comments!

Best, Albrecht.

Attachment: pgpmK1BnBMsNC.pgp
Description: PGP signature

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to