Messages in this thread | | | Date | Tue, 12 Jun 2012 09:54:22 +0200 | From | Wolfram Sang <> | Subject | Re: [PATCH 1/4] i2c: tegra: make sure register writes completes |
| |
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 <ldewangan@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/ | [unhandled content-type:application/pgp-signature] | |