Re: [U-Boot-Users] Fix empty i2c reads/writes in fsl_i2c.c

Joakim Tjernlund wrote:

This is against the freescale tree at opensource.freescale.com What is the status of this three? is it going into u-boot any time soon?

Yes.

@@ -191,15 +192,17 @@ __i2c_read(u8 *data, int length) int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length) {

• int i = 0;

• int i = -1; /* signal error */ u8 *a = (u8*)&addr;

  if (i2c_wait4bus() >= 0 && i2c_write_addr(dev, I2C_WRITE_BIT, 0) != 0

•    && __i2c_write(&a[4 - alen], alen) == alen
  

•    && i2c_write_addr(dev, I2C_READ_BIT, 1) != 0) {
  

•    && __i2c_write(&a[4 - alen], alen) == alen)
  

• i = 0; /* No error so far */
  

• if (length

•    && i2c_write_addr(dev, I2C_READ_BIT, 1) != 0)
  

  i = __i2c_read(data, length);

• }

This is a good idea, but I see a bug in the code. Say the first block succeeds, and sets i to 0. Then the "i2c_write_addr(dev, I2C_READ_BIT, 1)" fails. It will never call "__i2c_read(data, length)", and so it will never set the value of i. Then it will return, indicating success.

Frankly, I think the original code is just an abuse of short-circuit boolean operations and should be completely rewritten. Something like this:

if (i2c_wait4bus() < 0) return -1;

if (i2c_write_addr(dev, I2C_WRITE_BIT, 0) == 0)) return -1;

if (__i2c_write(&a[4 - alen], alen) != alen)) return -1;

if (length) { if (i2c_write_addr(dev, I2C_READ_BIT, 1) == 0)) return -1;

if (__i2c_read(data, length) != length) return -1; }

writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);

return 0;

This version is a lot easier to read and understand, I think.

• /* For unknow reason the controller will ACK when

Typo.