On Mon, May 09, 2011 at 11:38:19PM +0200, Linus Walleij wrote:
> From: Virupax Sadashivpetimath <[email protected]>
> 
> It is seen that i2c-nomadik controller randomly stops generating the
> interrupts leading to a i2c timeout. As a workaround to this problem,
> add retries to the on going transfer on failure.
> 
> Signed-off-by: Virupax Sadashivpetimath 
> <[email protected]>
> Reviewed-by: Jonas ABERG <[email protected]>
> Signed-off-by: Linus Walleij <[email protected]>
> ---
>  drivers/i2c/busses/i2c-nomadik.c |   97 
> +++++++++++++++++++-------------------
>  1 files changed, 49 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-nomadik.c 
> b/drivers/i2c/busses/i2c-nomadik.c
> index f3a0af0..bf061e1 100644
> --- a/drivers/i2c/busses/i2c-nomadik.c
> +++ b/drivers/i2c/busses/i2c-nomadik.c
> @@ -255,8 +255,6 @@ static int init_hw(struct nmk_i2c_dev *dev)
>  {
>       int stat;
>  
> -     clk_enable(dev->clk);
> -
>       stat = flush_i2c_fifo(dev);
>       if (stat)
>               goto exit;
> @@ -271,8 +269,6 @@ static int init_hw(struct nmk_i2c_dev *dev)
>       dev->cli.operation = I2C_NO_OPERATION;
>  
>  exit:
> -     /* TODO: Why disable clocks after init hw? */
> -     clk_disable(dev->clk);
>       /*
>        * TODO: What is this delay for?
>        * Must be pretty pointless since the hw block
> @@ -572,6 +568,7 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
>       u32 cause;
>       struct nmk_i2c_dev *dev = i2c_get_adapdata(i2c_adap);
>       u32 i2c_sr;
> +     int j;
>  
>       dev->busy = true;
>  
> @@ -579,63 +576,67 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
>               regulator_enable(dev->regulator);
>       pm_runtime_get_sync(&dev->pdev->dev);
>  
> +     clk_enable(dev->clk);
> +
>       status = init_hw(dev);
>       if (status)
> -             goto out2;
> +             goto out;
>  
> -     clk_enable(dev->clk);
> -
> -     /* setup the i2c controller */
> -     setup_i2c_controller(dev);
> +     for (j = 0; j < 3; j++) {
> +             /* setup the i2c controller */
> +             setup_i2c_controller(dev);
>  
> -     for (i = 0; i < num_msgs; i++) {
> -             if (unlikely(msgs[i].flags & I2C_M_TEN)) {
> -                     dev_err(&dev->pdev->dev, "10 bit addressing"
> -                                     "not supported\n");
> +             for (i = 0; i < num_msgs; i++) {
> +                     if (unlikely(msgs[i].flags & I2C_M_TEN)) {
> +                             dev_err(&dev->pdev->dev, "10 bit addressing"
> +                                             "not supported\n");
>  
> -                     status = -EINVAL;
> -                     goto out;
> -             }
> -             dev->cli.slave_adr      = msgs[i].addr;
> -             dev->cli.buffer         = msgs[i].buf;
> -             dev->cli.count          = msgs[i].len;
> -             dev->stop = (i < (num_msgs - 1)) ? 0 : 1;
> -             dev->result = 0;
> -
> -             if (msgs[i].flags & I2C_M_RD) {
> -                     /* it is a read operation */
> -                     dev->cli.operation = I2C_READ;
> -                     status = read_i2c(dev);
> -             } else {
> -                     /* write operation */
> -                     dev->cli.operation = I2C_WRITE;
> -                     status = write_i2c(dev);
> -             }
> -             if (status || (dev->result)) {
> -                     i2c_sr = readl(dev->virtbase + I2C_SR);
> -                     /*
> -                      * Check if the controller I2C operation status is set
> -                      * to ABORT(11b).
> -                      */
> -                     if (((i2c_sr >> 2) & 0x3) == 0x3) {
> -                             /* get the abort cause */
> -                             cause = (i2c_sr >> 4)
> -                                     & 0x7;
> -                             dev_err(&dev->pdev->dev, "%s\n", cause >=
> -                                             ARRAY_SIZE(abort_causes) ?
> +                             status = -EINVAL;
> +                             goto out;
> +                     }
> +                     dev->cli.slave_adr      = msgs[i].addr;
> +                     dev->cli.buffer         = msgs[i].buf;
> +                     dev->cli.count          = msgs[i].len;
> +                     dev->stop = (i < (num_msgs - 1)) ? 0 : 1;
> +                     dev->result = 0;
> +
> +                     if (msgs[i].flags & I2C_M_RD) {
> +                             /* it is a read operation */
> +                             dev->cli.operation = I2C_READ;
> +                             status = read_i2c(dev);
> +                     } else {
> +                             /* write operation */
> +                             dev->cli.operation = I2C_WRITE;
> +                             status = write_i2c(dev);
> +                     }
> +                     if (status || (dev->result)) {
> +                             i2c_sr = readl(dev->virtbase + I2C_SR);
> +                             /*
> +                              * Check if the controller I2C operation status
> +                              * is set to ABORT(11b).
> +                              */
> +                             if (((i2c_sr >> 2) & 0x3) == 0x3) {
> +                                     /* get the abort cause */
> +                                     cause = (i2c_sr >> 4)
> +                                             & 0x7;
> +                                     dev_err(&dev->pdev->dev, "%s\n", cause
> +                                             >= ARRAY_SIZE(abort_causes) ?
>                                               "unknown reason" :
>                                               abort_causes[cause]);
> -                     }
> +                             }
>  
> -                     status = status ? status : dev->result;
> -                     goto out;
> +                             status = status ? status : dev->result;
> +
> +                             break;
> +                     }
> +                     udelay(I2C_DELAY);
>               }
> -             udelay(I2C_DELAY);
> +             if (status == 0)
> +                     break;

this code is getting a bit complicated, could some of it be factored
into a new sub-function?
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to