Re: [PATCH 02/17] i2c: omap: decrease indentation level on data handling

Shilimkar, Santosh
2012-06-14T10:41:14Z

On Thu, Jun 14, 2012 at 3:50 PM, Felipe Balbi <balbi@ti.com> wrote:
> trivial patch, no functional changes.
>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  drivers/i2c/busses/i2c-omap.c |   63 ++++++++++++++++++++---------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 9b532cd..39d5583 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -856,22 +856,7 @@ complete:
>                                                        >> 8) & 0x3F;
>                        }
>                        while (num_bytes--) {
> -                               w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG);
> -                               if (dev->buf_len) {
> -                                       *dev->buf++ = w;
> -                                       dev->buf_len--;
> -                                       /*
> -                                        * Data reg in 2430, omap3 and
> -                                        * omap4 is 8 bit wide
> -                                        */
> -                                       if (dev->flags &
> -                                                OMAP_I2C_FLAG_16BIT_DATA_REG) {
> -                                               if (dev->buf_len) {
> -                                                       *dev->buf++ = w >> 8;
> -                                                       dev->buf_len--;
> -                                               }
> -                                       }
> -                               } else {
> +                               if (!dev->buf_len) {
>                                        if (stat & OMAP_I2C_STAT_RRDY)
>                                                dev_err(dev->dev,
>                                                        "RRDY IRQ while no data"
> @@ -882,6 +867,21 @@ complete:
>                                                                " requested\n");
>                                        break;
>                                }
> +
> +                               w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG);
> +                               *dev->buf++ = w;
> +                               dev->buf_len--;
> +                               /*
> +                                * Data reg in 2430, omap3 and
> +                                * omap4 is 8 bit wide
> +                                */
> +                               if (dev->flags &
> +                                               OMAP_I2C_FLAG_16BIT_DATA_REG) {
> +                                       if (dev->buf_len) {
> +                                               *dev->buf++ = w >> 8;
> +                                               dev->buf_len--;
> +                                       }
> +                               }
>                        }
>                        omap_i2c_ack_stat(dev,
>                                stat & (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR));
> @@ -898,22 +898,7 @@ complete:
>                                                        & 0x3F;
>                        }
>                        while (num_bytes--) {
> -                               w = 0;
> -                               if (dev->buf_len) {
> -                                       w = *dev->buf++;
> -                                       dev->buf_len--;
> -                                       /*
> -                                        * Data reg in 2430, omap3 and
> -                                        * omap4 is 8 bit wide
> -                                        */
> -                                       if (dev->flags &
> -                                                OMAP_I2C_FLAG_16BIT_DATA_REG) {
> -                                               if (dev->buf_len) {
> -                                                       w |= *dev->buf++ << 8;
> -                                                       dev->buf_len--;
> -                                               }
> -                                       }
> -                               } else {
> +                               if (!dev->buf_len) {
>                                        if (stat & OMAP_I2C_STAT_XRDY)
>                                                dev_err(dev->dev,
>                                                        "XRDY IRQ while no "
> @@ -925,6 +910,20 @@ complete:
>                                        break;
>                                }
>
> +                               w = *dev->buf++;
> +                               dev->buf_len--;
> +                               /*
> +                                * Data reg in 2430, omap3 and
> +                                * omap4 is 8 bit wide
> +                                */
> +                               if (dev->flags &
> +                                               OMAP_I2C_FLAG_16BIT_DATA_REG) {
> +                                       if (dev->buf_len) {
> +                                               w |= *dev->buf++ << 8;
> +                                               dev->buf_len--;
> +                                       }
> +                               }
> +
>                                if ((dev->errata & I2C_OMAP3_1P153) &&
>                                    errata_omap3_1p153(dev, &stat, &err))
>                                        goto complete;

Avoids couple of if check and kills the un-necessary if/else inside the loop.
Did I miss anything else in the patch ?

Looks good to me
Reviewed-by : Santosh Shilimkar <santosh.shilimkar@ti.com>