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 sjg@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