Re: [PATCH] i2c: mxs: Rework the PIO mode operation

[Torsten Fleischer <to-fleischer-zqRNUXuvxA0b1SvskN2V4Q@public.gmane.org>]

Hi Marek,

> Hi Torsten,
> 
> 
> You're right, thanks for finding this! Does the following fix look OK ? I
> added some more comments and "decompressed" the code some more.
> 
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index fb6f110..ee17b6a 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -366,7 +366,7 @@ static int mxs_i2c_pio_setup_xfer(struct i2c_adapter
> *adap, struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap);
>         uint32_t addr_data = msg->addr << 1;
>         uint32_t data = 0;
> -       int i, ret, xlen = 0;
> +       int i, ret, xlen = 0, xmit = 0;
>         uint32_t start;
> 
>         /* Mute IRQs coming from this block. */
> @@ -463,20 +463,42 @@ static int mxs_i2c_pio_setup_xfer(struct i2c_adapter
> *adap,
>                         data >>= 8;
>                         data |= (msg->buf[i] << 24);
> 
> +                       xmit = 0;
> +
> +                       /* This is the last transfer of the message. */
>                         if (i + 1 == msg->len) {
> -                               /* This is the last transfer. */
> +                               /* Add optional STOP flag. */
>                                 start |= flags;
> +                               /* Remove RETAIN_CLOCK bit. */
>                                 start &= ~MXS_I2C_CTRL0_RETAIN_CLOCK;
> -                               xlen = (i + 2) % 4;
> -                               data >>= (4 - xlen) * 8;
> -                       } else if ((i & 3) == 2) {
> -                               /* Regular transfer. */
> -                               xlen = 4;
> -                       } else {
> -                               /* Just stuff data. */
> -                               continue;
> +                               xmit = 1;
>                         }
> 
> +                       /* Four bytes are ready in the "data" variable. */
> +                       if ((i & 3) == 2)
> +                               xmit = 1;
> +
> +                       /* Nothing interesting happened, continue stuffing.
> */ +                       if (!xmit)
> +                               continue;
> +
> +                       /*
> +                        * Compute the size of the transfer and shift the
> +                        * data accordingly.
> +                        *
> +                        * i = (4k + 0) .... xlen = 2
> +                        * i = (4k + 1) .... xlen = 3
> +                        * i = (4k + 2) .... xlen = 4
> +                        * i = (4k + 3) .... xlen = 1
> +                        */
> +
> +                       if ((i % 4) == 3)
> +                               xlen = 1;
> +                       else
> +                               xlen = (i % 4) + 2;
> +
> +                       data >>= (4 - xlen) * 8;
> +
>                         dev_dbg(i2c->dev,
>                                 "PIO: len=%i pos=%i total=%i [W%s%s%s]\n",
>                                 xlen, i, msg->len,

The patch looks OK. I will test it when I'm back in the office. But this is 
not before August 12.

Best regards,
Torsten Fleischer