Re: [PATCH 2/2] omap i2c: add a timeout to the busy waiting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Alexander Shishkin said the following on 12/17/2009 07:01 PM:
On Thu, Dec 17, 2009 at 08:38:39 +0530, Menon, Nishanth wrote:
Alexander Shishkin said the following on 12/16/2009 07:32 PM:
The errata 1.153 workaround is busy waiting on XUDF bit in interrupt
context, which may lead to kernel hangs. The problem can be reproduced
by running the bus with wrong (too high) speed.

Signed-off-by: Alexander Shishkin <virtuoso@xxxxxxxxx>
CC: linux-i2c@xxxxxxxxxxxxxxx
CC: linux-omap@xxxxxxxxxxxxxxx
---
drivers/i2c/busses/i2c-omap.c |    7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index ad8242a..b474c20 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -678,7 +678,9 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
 */
static int omap3430_workaround(struct omap_i2c_dev *dev, u16 *stat, int *err)
{
-	while (!(*stat & OMAP_I2C_STAT_XUDF)) {
+	unsigned long timeout = 10000;
+
+	while (!(*stat & OMAP_I2C_STAT_XUDF && --timeout)) {
a) timeout without using an actual delay is not a good idea -
consider each OPP - we can go upto 1ghz on 3630,
the actual time for 10000 iterations will depend on the MPU speed.

Well, I could calculate the timeout value based on current operating speed,
I guess. Or a delay. Perhaps OMAP_I2C_TIMEOUT can be used here?
it might be an overkill trying to generate counter based on opp speeds. wondering if this delay is required in the first place..

Moiz Sonasath / Vikram P, (commit cd086d3a) could probably comment?
Ref: http://www.mail-archive.com/linux-omap@xxxxxxxxxxxxxxx/msg15317.html

b) how did you arrive at the 10k iteration limit?

It was random, but then it seemed ok considering the l4 latency.
I had guessed this is an emperical value. It will be good to know the exact rationale on why a timeout will happen, else we will have all kind of questions pending trying to figure out if a timeout happened.

		if (*stat & (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
			omap_i2c_ack_stat(dev, *stat & (OMAP_I2C_STAT_XRDY |
							OMAP_I2C_STAT_XDR));
@@ -689,6 +691,9 @@ static int omap3430_workaround(struct omap_i2c_dev *dev, u16 *stat, int *err)
		*stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
	}
+	if (!timeout)
+		dev_err(dev->dev, "timeout waiting on XUDF bit\n");
+
	return 0;
}
Regards,
Nishanth Menon

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux