Hello.

On 05/28/2014 11:44 AM, Wolfram Sang wrote:

From: Wolfram Sang <[email protected]>

The i2c core has per-adapter locks, so no need to protect again.

The core's lock is unable to protect from the IRQs. So I'm proposing to revert this patch. It's a pity I hadn't noticed this issue when the patch was posted.

Signed-off-by: Wolfram Sang <[email protected]>
---
  drivers/i2c/busses/i2c-rcar.c | 22 ----------------------
  1 file changed, 22 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index e82d64b5acef..4b088e67f2fd 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
[...]
@@ -110,7 +109,6 @@ struct rcar_i2c_priv {
        struct i2c_msg  *msg;
        struct clk *clk;

-       spinlock_t lock;

   Needed a comment (that it protects the I2C registers).

[...]
@@ -462,21 +454,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
  {
        struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
        struct device *dev = rcar_i2c_priv_to_dev(priv);
-       unsigned long flags;
        int i, ret, timeout;

        pm_runtime_get_sync(dev);

-       /*-------------- spin lock -----------------*/
-       spin_lock_irqsave(&priv->lock, flags);
-
        rcar_i2c_init(priv);
        /* start clock */
        rcar_i2c_write(priv, ICCCR, priv->icccr);

-       spin_unlock_irqrestore(&priv->lock, flags);
-       /*-------------- spin unlock -----------------*/
-

I'm afraid this unlock is misplaced, the code continues to access the registers.

        ret = rcar_i2c_bus_barrier(priv);

   Should probably unlock here instead?

        if (ret < 0)
                goto out;

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to