[PATCH 3/4] i2c-s3c2410: use exponential back off while polling for bus idle

Hi Mark,

On Tue, Nov 20, 2012 at 12:49 PM, Mark Brown
[off-list ref] wrote:

On Thu, Nov 15, 2012 at 05:43:32PM +0530, Naveen Krishna Chatradhi wrote:

quoted

+     iicstat = readl(i2c->regs + S3C2410_IICSTAT);
+     delay = 1;
+     while ((iicstat & S3C2410_IICSTAT_START) &&
+            ktime_us_delta(now, start) < S3C2410_IDLE_TIMEOUT) {
+             usleep_range(delay, 2 * delay);
+             if (delay < S3C2410_IDLE_TIMEOUT / 10)
+                     delay <<= 1;
+             now = ktime_get();
+             iicstat = readl(i2c->regs + S3C2410_IICSTAT);
+     }

quoted

-     /* first, try busy waiting briefly */
-     do {
-             cpu_relax();
-             iicstat = readl(i2c->regs + S3C2410_IICSTAT);
-     } while ((iicstat & S3C2410_IICSTAT_START) && --spins);

On the hardware I was using when I wrote the original code here we were
hitting 1-2 spins often enough to be interesting - starting off with a
direct busy wait was definitely useful when doing large batches of I/O,
especially compared to sleeps which might cause us to schedule.

We check the status first to avoid any sleep()/schedule() in the case,
that the CPU is slower than I2C transaction.
Remember, this loop only happens after the event_wait loop has been
woken up by the i2c irq.
Since you are talking about hitting a tiny window of time at some
arbitrary point after an irq, the CPU time to this point & I2C
finishing would have to be very precisely aligned for the 1-2 loops
(at CPU clock rate) to matter.

HTH,
-Dan

quoted

-     /* if that timed out sleep */
-     if (!spins) {
-             msleep(1);
-             iicstat = readl(i2c->regs + S3C2410_IICSTAT);
-     }

It seems like it'd be better to do the exponential backoff bit here
instead of removing the busy wait completely.