CHROMIUM: i2c-s3c2410: grab adapter lock while changing i2c clock
authorDaniel Kurtz <djkurtz@chromium.org>
Sat, 11 Aug 2012 05:22:45 +0000 (13:22 +0800)
committerGerrit <chrome-bot@google.com>
Fri, 17 Aug 2012 08:22:06 +0000 (01:22 -0700)
We probably don't want to change I2C frequency while a transfer is in
progress.  The current implementation grabs a spinlock, but that only
protected the writes to IICCON when starting a message, it didn't protect
against clock changes in the middle of a transaction.

Note: The i2c-core already grabs the adapter lock before calling
s3c24xx_i2c_doxfer(), which ensures that only one caller is issuing a
xfer at a time.  Thus, i2c_claim is already protected by a lock.  Also,
this means it is not necessary to disable interrupts (spin_lock_irqsave)
when changing frequencies, since there won't be any i2c interrupts if
there is no on-going xfer.

Lastly, i2c_lock_adapter() may cause the cpufreq_transition to sleep if
if a xfer is in progress, but this is ok since cpufreq notifiers are
called in a kernel thread, and there are already cases where it could
sleep, such as when using i2c to update the output of a voltage
regulator.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
BUG=none
TEST=builds clean; i2c probes, reads and writes work during many cpufreq
     transitions.
TEST=Note: the cpufreq part of this change has no functional affect on
     exynos, where the i2c clock is independent of the cpufreq.
     But, there is a slight perfomance boost since we no longer need to
     lock/unlock an additional spinlock.

Change-Id: Ieba439ef6a4176ff83347e990e537474a1f70b65
Reviewed-on: https://gerrit.chromium.org/gerrit/29971
Reviewed-by: Olof Johansson <olofj@chromium.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Commit-Ready: Daniel Kurtz <djkurtz@chromium.org>
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
Tested-by: Daniel Kurtz <djkurtz@chromium.org>
drivers/i2c/busses/i2c-s3c2410.c

index 2788ee3..54353af 100644 (file)
@@ -69,7 +69,6 @@ enum {
 };
 
 struct s3c24xx_i2c {
-       spinlock_t              lock;
        wait_queue_head_t       wait;
        unsigned int            quirks;
        unsigned int            suspended:1;
@@ -710,12 +709,6 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
        if (i2c->suspended)
                return -EIO;
 
-       /*
-        * Claim the bus if needed.
-        *
-        * Note, this needs a lock. How come s3c24xx_i2c_set_master() below
-        * is outside the lock?
-        */
        if (s3c24xx_i2c_claim(i2c))
                return -EBUSY;
 
@@ -726,8 +719,6 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
                goto out;
        }
 
-       spin_lock_irq(&i2c->lock);
-
        i2c->msg     = msgs;
        i2c->msg_num = num;
        i2c->msg_ptr = 0;
@@ -736,7 +727,6 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
 
        s3c24xx_i2c_enable_irq(i2c);
        s3c24xx_i2c_message_start(i2c, msgs);
-       spin_unlock_irq(&i2c->lock);
 
        timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5);
 
@@ -919,7 +909,6 @@ static int s3c24xx_i2c_cpufreq_transition(struct notifier_block *nb,
                                          unsigned long val, void *data)
 {
        struct s3c24xx_i2c *i2c = freq_to_i2c(nb);
-       unsigned long flags;
        unsigned int got;
        int delta_f;
        int ret;
@@ -933,9 +922,9 @@ static int s3c24xx_i2c_cpufreq_transition(struct notifier_block *nb,
 
        if ((val == CPUFREQ_POSTCHANGE && delta_f < 0) ||
            (val == CPUFREQ_PRECHANGE && delta_f > 0)) {
-               spin_lock_irqsave(&i2c->lock, flags);
+               i2c_lock_adapter(&i2c->adap);
                ret = s3c24xx_i2c_clockrate(i2c, &got);
-               spin_unlock_irqrestore(&i2c->lock, flags);
+               i2c_unlock_adapter(&i2c->adap);
 
                if (ret < 0)
                        dev_err(i2c->dev, "cannot find frequency\n");
@@ -1209,7 +1198,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
        i2c->adap.class   = I2C_CLASS_HWMON | I2C_CLASS_SPD;
        i2c->tx_setup     = 50;
 
-       spin_lock_init(&i2c->lock);
        init_waitqueue_head(&i2c->wait);
 
        /* find the clock and enable it */