Hi Simon,
I am testing this series on my board
and found some bugs.
On Mon, 24 Nov 2014 11:57:15 -0700
Simon Glass <s...@chromium.org> wrote:
> +
> +static bool i2c_setup_offset(struct dm_i2c_chip *chip, uint offset,
> + uint8_t offset_buf[], struct i2c_msg *msg)
> +{
> + if (!chip->offset_len)
> + return false;
> + msg->addr = chip->chip_addr;
> + msg->flags = chip->flags;
> + msg->len = chip->offset_len;
> + msg->buf = offset_buf;
> + offset_buf[0] = offset;
> + offset_buf[1] = offset >> 8;
> + offset_buf[2] = offset >> 16;
> + offset_buf[3] = offset >> 24;
> +
> + return true;
> +}
The i2c_setup_offset() function includes two bugs.
[1] Even if chip->offset_len is zero, it should not
return immediately.
struct i2c_msg *msg has not been initialized.
msg->addr and msg->len include unexpected values
and they are passed to the driver.
It results in an enexpected behavior.
[2] The endian of offset_buf[] should be big endian,
not little endian.
So, if I rewrote this function locally as follows, it is working file:
static bool i2c_setup_offset(struct dm_i2c_chip *chip, uint offset,
uint8_t offset_buf[], struct i2c_msg *msg)
{
int offset_len;
msg->addr = chip->chip_addr;
msg->flags = chip->flags;
msg->len = chip->offset_len;
msg->buf = offset_buf;
offset_len = chip->offset_len;
while(offset_len--)
*offset_buf++ = offset >> (8 * offset_len);
return true;
}
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
