Re: [PATCH 1/4] i2c: tegra: make sure register writes completes

[Wolfram Sang]

On Tue, Jun 05, 2012 at 06:39:57PM +0530, Laxman Dewangan wrote:
> The Tegra PPSB (an peripheral bus) queues writes transactions.
> In order to guarantee that writes have completed before a
> certain time, a read transaction to a register on the same
> bus must be executed.
> This is necessary in situations such as when clearing an
> interrupt status or enable, so that when returning from an
> interrupt handler, the HW has already de-asserted its
> interrupt status output, which will avoid spurious interrupts.
> 
> Signed-off-by: Laxman Dewangan <ldewan...@nvidia.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 8b2e555..fa92396 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -430,6 +430,13 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
>       if (i2c_dev->is_dvc)
>               dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
>  
> +     /*
> +      * Register write get queued in the PPSB bus and write can
> +      * happen later. Read back register to make sure that register
> +      * write is completed.
> +      */
> +     i2c_readl(i2c_dev, I2C_INT_STATUS);

Does it make sense to put the read into i2c_writel?

> +
>       if (status & I2C_INT_PACKET_XFER_COMPLETE) {
>               BUG_ON(i2c_dev->msg_buf_remaining);
>               complete(&i2c_dev->msg_complete);
> @@ -444,6 +451,9 @@ err:
>       if (i2c_dev->is_dvc)
>               dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
>  
> +     /* Read back register to make sure that register writes completed */
> +     i2c_readl(i2c_dev, I2C_INT_STATUS);
> +
>       complete(&i2c_dev->msg_complete);
>       return IRQ_HANDLED;
>  }
> @@ -505,6 +515,9 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev 
> *i2c_dev,
>       ret = wait_for_completion_timeout(&i2c_dev->msg_complete, 
> TEGRA_I2C_TIMEOUT);
>       tegra_i2c_mask_irq(i2c_dev, int_mask);
>  
> +     /* Read back register to make sure that register writes completed */
> +     i2c_readl(i2c_dev, I2C_INT_MASK);
> +

It definately makes sense to put this read into tegra_i2c_mask_irq()?

>       if (WARN_ON(ret == 0)) {
>               dev_err(i2c_dev->dev, "i2c transfer timed out\n");

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

signature.asc
Description: Digital signature