[U-Boot] [PATCH 3/3] rockchip: i2c: fix >32 byte writes

Simon Glass sjg at chromium.org
Tue Sep 6 03:03:12 CEST 2016


Hi John,

On 18 August 2016 at 13:08, John Keeping <john at 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 at 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 at chromium.org>

Regards,
Simon


More information about the U-Boot mailing list