On Fri, Apr 2, 2010 at 9:39 PM, Mark Brown
<broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
> 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");
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
