Re: [PATCH/RFC 1/2] 5200: improve i2c bus error recovery

["Albrecht Dreß" <albrecht.dress@arcor.de>]

[-- Attachment #1: Type: text/plain, Size: 2746 bytes --]

Am 25.01.10 05:06 schrieb(en) Ben Dooks:
> On Fri, Jan 22, 2010 at 09:17:55PM +0100, Albrecht Dreß wrote:
>> Improve the recovery of the MPC5200B's I2C bus from errors like bus hangs.
> 
> This is very sparse comapred to the large comment below the --- line, maybe some more description should be living up here.

Hmm, that was my interpretation of #15 in Documentation/SubmittingPatches... ;-)  Maybe it should read

<snip>
Improve the recovery of the MPC5200B's I2C bus from errors like bus hangs.  This includes making the bus timeout configurable, a better detection of cases where the bus has to be "fixed" after a timeout, and a more thorough fixup sequence.
</snip>

> Is thios a candidate for an -rc or should it be left to merge window?

Well, basically it was a rfc.  I apparently need it on my 5200B board, but I hoped to get some more insight from the Freescale/I2C gurus (see "open questions" in the post).  Thus merge window, IMHO...

Thanks, Albrecht.

> 
> > Signed-off-by: Albrecht Dreß <albrecht.dress@arcor.de>
> >
> > ---
> >
> > This patch introduces several improvements to the MPC5200B's I2C driver
> > as to improve the recovery from error conditions I encountered when
> > testing a custom board with several I2C devices attached (eeprom, io
> > expander, rtc, sensors).  The error conditions included cases where the
> > bus if logic of one slave apparently went south, blocking the bus
> > completely.
> >
> > My fixes include:
> > 1. make the bus timeout configurable in fsl_i2c_probe(); the default of
> >    one second is *way* too long for my use case;
> > 2. if a timeout condition occurs in mpc_xfer(), mpc_i2c_fixup() the bus
> >    if *any* of the CF, BB and RXAK flags in the MSR is 1.  I actually
> >    saw different combinations with hangs, not only all three set;
> > 3. improve the fixup procedure by calculating the timing needed from the
> >    real (configured) bus clock, calculated in mpc_i2c_setclock_52xx().
> >    Furthermore, I issue 9 instead of one cycle, as I experienced cases
> >    where the single one is not enough (found this tip in a forum).  As a
> >    side effect, the new scheme needs only 81us @375kHz bus clock instead
> >    of 150us.  I recorded waveforms for 18.4kHz, 85.9kHz and 375kHz, all
> >    looking fine, which I can provide if anyone is interested.
> >
> > Open questions:
> > - is the approach correct at all, in particular the interpretation of
> >   the flags (#2)?
> > - could this code also be used on non-5200 processors?
> >
> > --- linux-2.6.32-orig/drivers/i2c/busses/i2c-mpc.c	2009-12-03 04:51:21.000000000 +0100
> > +++ linux-2.6.32/drivers/i2c/busses/i2c-mpc.c	2010-01-22 16:05:13.000000000 +0100
[snip]

[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]