Hi John,
On 18 August 2016 at 13:08, John Keeping john@metanate.com wrote:
The special handling of the chip address and register address must only happen before we send the data buffer, otherwise we will end up inserting both of these every 32 bytes.
Signed-off-by: John Keeping john@metanate.com
I'm not entirely sure about this; it's the smallest change that fixes the issue and I can't see another way to fix it without completely rewriting the function. I guess we could drop r_len completely since the only caller always passes zero and that would make it all a bit simpler, but I don't want to conflict with any plan to use this function elsewhere.
drivers/i2c/rk_i2c.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/rk_i2c.c b/drivers/i2c/rk_i2c.c index a4c0032..7c701cb 100644 --- a/drivers/i2c/rk_i2c.c +++ b/drivers/i2c/rk_i2c.c @@ -269,9 +269,9 @@ static int rk_i2c_write(struct rk_i2c *i2c, uchar chip, uint reg, uint r_len, if ((i * 4 + j) == bytes_xferred) break;
if (i == 0 && j == 0) {
if (i == 0 && j == 0 && pbuf == buf) { txdata |= (chip << 1);
} else if (i == 0 && j <= r_len) {
} else if (i == 0 && j <= r_len && pbuf == buf) { txdata |= (reg & (0xff << ((j - 1) * 8))) << 8; } else {-- 2.9.3.728.g30b24b4.dirty
The original code is not great. I would rather that it puts the chip and address info into txdata outside the loops.
But anyway your change looks right. I did think about using an explicit boolean like 'addr_sent', set to false at the strart of the function and true once sent. But given the existing code, we might as well go with what you have.
Acked-by: Simon Glass sjg@chromium.org
Regards, Simon