From mboxrd@z Thu Jan  1 00:00:00 1970
From: jassisinghbrar@gmail.com (jassi brar)
Date: Fri, 2 Apr 2010 22:00:52 +0900
Subject: [PATCH] i2c-s3c2410: Remove unconditional 1ms delay on each
	transfer
In-Reply-To: <1270211946-25103-1-git-send-email-broonie@opensource.wolfsonmicro.com>
References: <1270211946-25103-1-git-send-email-broonie@opensource.wolfsonmicro.com>
Message-ID: <p2q1b68c6791004020600g283db571gacd0f8b7e4ddaf6@mail.gmail.com>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

On Fri, Apr 2, 2010 at 9:39 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> The S3C I2C controller indicates completion of I2C transfers before
> the bus has a stop condition on it. In order to ensure that we do not
> attempt to start a new transfer before the bus is idle the driver
> currently inserts a 1ms delay. This is vastly larger than is generally
> required and has a visible effect on performance under load, such as
> when bringing up audio CODECs or reading back status information with
> non-bulk I2C reads.
>
> Replace the sleep with a spin on the IIC status register for up to 1ms.
> This will busy wait but testing on my SMDK6410 system indicates that
> the overwhelming majority of transactions complete on the first spin,
> with maximum latencies of less than 10 spins so the absolute overhead
> of busy waiting should be at worst comprable to msleep(), and the
> overall system performance is dramatically improved.
>
> The main risk is poor interaction with multimaster systems where
> we may miss the bus going idle before the next transaction. Defend
> against this by falling back to the original 1ms delay after 20 spins.
>
> The overall effect in my testing is an approximately 20% improvement
> in kernel startup time.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> ?drivers/i2c/busses/i2c-s3c2410.c | ? 20 ++++++++++++++++++--
> ?1 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index d27072b..ba52543 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -482,7 +482,8 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c)
> ?static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct i2c_msg *msgs, int num)
> ?{
> - ? ? ? unsigned long timeout;
> + ? ? ? unsigned long iicstat, timeout;
> + ? ? ? int spins = 20;
> ? ? ? ?int ret;
>
> ? ? ? ?if (i2c->suspended)
> @@ -521,7 +522,22 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
>
> ? ? ? ?/* ensure the stop has been through the bus */
>
> - ? ? ? msleep(1);
> + ? ? ? dev_dbg(i2c->dev, "waiting for bus idle\n");
> +
> + ? ? ? /* first, try busy waiting briefly */
> + ? ? ? spins = 0;
          ^^^^^^^^^^^^ ?
> + ? ? ? do {
> + ? ? ? ? ? ? ? iicstat = readl(i2c->regs + S3C2410_IICSTAT);
> + ? ? ? } while ((iicstat & S3C2410_IICSTAT_START) && --spins);
> +
> + ? ? ? /* if that timed out sleep */
> + ? ? ? if (!spins) {
> + ? ? ? ? ? ? ? msleep(1);
> + ? ? ? ? ? ? ? iicstat = readl(i2c->regs + S3C2410_IICSTAT);
> + ? ? ? }
> +
> + ? ? ? if (iicstat & S3C2410_IICSTAT_START)
> + ? ? ? ? ? ? ? dev_warn(i2c->dev, "timeout waiting for bus idle\n");