diff mbox

[08/12] i2c: pxa: enable/disable irq across message xfer

Message ID 1432818224-17070-9-git-send-email-vaibhav.hiremath@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Vaibhav Hiremath May 28, 2015, 1:03 p.m. UTC 
In order to avoid "spurious irq" caused by CP polling mode,
enable irq at the entry of i2c_pxa_xfer() fn and disable it
again before exit.
Also disable it before exiting probe function.

Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
[vaibhav.hiremath@linaro.org: Split & merge patches into logical changes
and update the Changelog]
Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/i2c/busses/i2c-pxa.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Russell King - ARM Linux May 28, 2015, 1:17 p.m. UTC | #1 
On Thu, May 28, 2015 at 06:33:40PM +0530, Vaibhav Hiremath wrote:
> In order to avoid "spurious irq" caused by CP polling mode,
> enable irq at the entry of i2c_pxa_xfer() fn and disable it
> again before exit.

NAK.

It's really not nice for drivers to disable a potentially shared interrupt.
If the interrupt is shared, you disable the interrupt for other users of
that interrupt as well.  See:

commit c66dc529194be374556d166ee7ddb84a7d1d302b
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date:   Wed Feb 23 12:38:18 2011 +0100

    i2c-pxa2xx: add support for shared IRQ handler

    Sodaville has three of them on a single IRQ. IRQF_DISABLED is removed
    because it is a NOP allready and scheduled for removal.

    Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
    Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com>
    Signed-off-by: Ben Dooks <ben-linux@fluff.org>

So you're breaking Sodaville.
Vaibhav Hiremath May 28, 2015, 1:29 p.m. UTC | #2 
On Thursday 28 May 2015 06:47 PM, Russell King - ARM Linux wrote:
> On Thu, May 28, 2015 at 06:33:40PM +0530, Vaibhav Hiremath wrote:
>> In order to avoid "spurious irq" caused by CP polling mode,
>> enable irq at the entry of i2c_pxa_xfer() fn and disable it
>> again before exit.
>
> NAK.
>
> It's really not nice for drivers to disable a potentially shared interrupt.
> If the interrupt is shared, you disable the interrupt for other users of
> that interrupt as well.  See:
>
> commit c66dc529194be374556d166ee7ddb84a7d1d302b
> Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Date:   Wed Feb 23 12:38:18 2011 +0100
>
>      i2c-pxa2xx: add support for shared IRQ handler
>
>      Sodaville has three of them on a single IRQ. IRQF_DISABLED is removed
>      because it is a NOP allready and scheduled for removal.
>
>      Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>      Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com>
>      Signed-off-by: Ben Dooks <ben-linux@fluff.org>
>
> So you're breaking Sodaville.
>

Got it.

I will drop this patch from the series.

Before submitting next version, will wait for review on other patches.

Thanks,
Vaibhav
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 3c6ebb5..a3ac97c 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1163,6 +1163,7 @@  static int i2c_pxa_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num
 	struct pxa_i2c *i2c = adap->algo_data;
 	int ret, i;
 
+	enable_irq(i2c->irq);
 	for (i = adap->retries; i >= 0; i--) {
 		ret = i2c_pxa_do_xfer(i2c, msgs, num);
 		if (ret != I2C_RETRY)
@@ -1176,6 +1177,7 @@  static int i2c_pxa_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num
 	ret = -EREMOTEIO;
  out:
 	i2c_pxa_set_slave(i2c, ret);
+	disable_irq(i2c->irq);
 	return ret;
 }
 
@@ -1363,6 +1365,8 @@  static int i2c_pxa_probe(struct platform_device *dev)
 
 	i2c_pxa_reset(i2c);
 
+	disable_irq(i2c->irq);
+
 	i2c->adap.algo_data = i2c;
 	i2c->adap.dev.parent = &dev->dev;
 #ifdef CONFIG_OF