On 08/23/2014 03:54 AM, Sergei Shtylyov wrote:
From: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
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 <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
---
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
[...]
@@ -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?
After looking at the code, it looks like it was a false alarm on my side.
The I2C interrupts are disabled here and other threads should be locked out by
the I2C core locks. So it doesn't make sense to hold IRQs disabled during a
possibly long polling loop in rcar_i2c_bus_barrier(). So I'll just repost the
revert patch (if still needed).
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
