[i2c] Is review of AT91 patch pending?

[David Brownell]

> > My notes also say the CONFIG_I2C_CLOCKRATE seems odd ... if that's
> > actual bits-per-second, then why is the default 10K not 100K,
>
> It's a suggestion from Mark Hoffman earlier this year:
> 	http://lists.lm-sensors.org/pipermail/i2c/2007-February/000820.html
>
> I made some serious benchmarks of the patch before posting it (huge 
> amounts of data was transferred):
> 	400 kHz  Never works
> 	100 kHz  Works when cpu is idle
> 	50 kHz   Works, even when cpu is very heavily loaded
> 	30 kHz   My safety margin
> 	10 kHz   Marks safety margin

Then more info deserves to be listed in Kconfig ... minimally, say
that the driver exhibits overrun/underrun problems under load, making
clock rates "higher than X" very error prone (and list the key errors,
which I'm assuming are just Xruns) on common systems


> > why is the "highest recommended speed" 50K not 400K?  (And if those
> > recommendations are serious, why isn't there "hard" enforcement, or
> > even soft enforcement in Kconfig?)
>
> It's a design issue. The user can try his particular system for the 
> maximum speed it'll cope - given that it depends on system load and cpu 
> clockspeed. And don't forget that there are other members of the AT91 
> family than AT91rm9200. Newer members can handle higher speeds.

That's not at all clear.  You haven't tried it on those chips, and other
new silicon has the same overrun/underrun problems.  Even though it's
kind of system-dependant, it would seem to be worth a runtime warning in
some cases (maybe when the clock is faster than 50 KHz).  Otherwise
folk won't take this issue seriously enough.


> > Meanwhile, i2c-gpio hasn't had *any* problems.  ("i2cdetect -F 0"
> > says it doesn't support PEC, but that's a bug in i2cdetect:  its
> > source says it means not PEC, but _hardware_ PEC.)
>
> I believe you, however I haven't had any problems either after lowering 
> the bitclock to 50 kHz and applying my patch. Ideally I think it should 
> be up to the user. i2c-gpio is somewhat more reliable but increases 
> overhead and cpu load. i2c-at91 works well but might lose a couple of 
> bytes in *extreme* situations.

The relevant point for me is basically that this TWI module doesn't
handle repeated starts.  Accordingly, it wouldn't talk to some of
the devices I needed to use, which (reasonbly enough!) expect that
combined messages aren't split up by inappropriate STOP transitions.

The I2C code has always been kind of glitchey, as you know, and at
some point spending a little time to avoid such glitches becomes
a *very* obvious tradeoff.  You may be right that the tradeoff should
be in the hands of whoever configures the kernel... but then they
really need to know that this controller has "issues".

- Dave