[U-Boot] fsl_i2c: increase I2C timeout values and make them configurable

Joakim Tjernlund joakim.tjernlund at transmode.se
Wed Sep 9 16:33:03 CEST 2009


timur.tabi at gmail.com wrote on 09/09/2009 16:24:15:
>
> On Wed, Sep 9, 2009 at 4:19 AM, Joakim
> Tjernlund<joakim.tjernlund at transmode.se> wrote:
> >
> > I wonder if this hides another problem too.
> > if the timeout hits, -1 is returned.
> >
> > Then in i2c_read()/i2c_write() you have:
> >        if (i2c_wait4bus() >= 0
> >            && i2c_write_addr(dev, I2C_WRITE_BIT, 0) != 0
> >            && __i2c_write(&a[4 - alen], alen) == alen)
> >                i = 0; /* No error so far */
> > notice how the latter part, i2c_write_addr(dev, I2C_WRITE_BIT, 0) != 0
> >  && __i2c_write(&a[4 - alen], alen) == alen)
> > is ignored(never called) when i2c_wait4bus()  returns -1
>
> Yeah, that is a bit odd.  It looks like it was supposed to be some
> short-cut way to avoid multiple if-then-else clauses.
>
> I wouldn't say my patch is *hiding* another problem -- that code looks
> broken either way.

Yes, bad wording on my part.

>
> Someone should probably fix it to look like this:
>
> if (i2c_wait4bus() < 0)
>     return -1;

I suspect that this won't work in the long run. If
i2c_wait4bus() times out, the bus is likely stuck and will
never recover. Probably best to ignore these errors and reset the controller
and try again or something ..

>
> if (!i2c_write_addr(dev, I2C_WRITE_BIT, 0))
>     return -1;
>
> if (__i2c_write(&a[4 - alen], alen) != alen)
>     return -1;
>
> and so on.
>       i = 0; /* No error so far */
>
> --
> Timur Tabi
> Linux kernel developer at Freescale
>



More information about the U-Boot mailing list