From mboxrd@z Thu Jan 1 00:00:00 1970 From: Danielle Costantino Subject: Re: [PATCH] i2c/mpc: Fix ISR return value Date: Fri, 30 Jan 2015 03:09:47 -0800 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8913768023928736773==" Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" To: Amit Tomar Cc: "linuxppc-dev@lists.ozlabs.org" , "linux-i2c@vger.kernel.org" List-Id: linux-i2c@vger.kernel.org --===============8913768023928736773== Content-Type: multipart/alternative; boundary=001a11c2fa0e884e04050ddca481 --001a11c2fa0e884e04050ddca481 Content-Type: text/plain; charset=UTF-8 I have been using the driver with this modification for the past 6 months and it has been stable in an industrial environment.I had made a few other changes that also improve reliability (using ppc in_8 and out_8 and eieio barriers to ensure in-order execution. This lets you remove the unneeded double read of the status register. I also added a more robust recovery function to handle force of bus master-ship, and clearing the arb lost interrupt that is generated. currently this can cause the isr to trigger and cause superfluous interrupts. I have not posted this patch because of the extensive changes, I will ack this patch. On Fri, Jan 30, 2015 at 2:24 AM, Amit Tomar wrote: > ISR should not return IRQ_HANDLED for not handling anything. > This patch fixes the return value of ISR for the same case. > > > Signed-off-by: Amit Singh Tomar > --- > drivers/i2c/busses/i2c-mpc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c > index 0edf630..7a3136f 100644 > --- a/drivers/i2c/busses/i2c-mpc.c > +++ b/drivers/i2c/busses/i2c-mpc.c > @@ -95,8 +95,9 @@ static irqreturn_t mpc_i2c_isr(int irq, void *dev_id) > i2c->interrupt = readb(i2c->base + MPC_I2C_SR); > writeb(0, i2c->base + MPC_I2C_SR); > wake_up(&i2c->queue); > + return IRQ_HANDLED; > } > - return IRQ_HANDLED; > + return IRQ_NONE; > } > > /* Sometimes 9th clock pulse isn't generated, and slave doesn't release > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- - Danielle Costantino --001a11c2fa0e884e04050ddca481 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
I have been using the driver with this modification for th= e past 6 months and it has been stable in an industrial environment.I had m= ade a few other changes that also improve reliability (using ppc in_8 and o= ut_8 and eieio barriers to ensure in-order execution. This lets you remove = the unneeded double read of the status register. I also added a more robust= recovery function to handle force of bus master-ship, and clearing the arb= lost interrupt that is generated. currently this can cause the isr to trig= ger and cause superfluous interrupts. I have not posted this patch because = of the extensive changes,

I will ack this patch.

On Fri, Jan 3= 0, 2015 at 2:24 AM, Amit Tomar <Amit.Tomar@freescale.com> wrote:
ISR should not return IRQ_HAN= DLED for not handling anything.
This patch fixes the return value of ISR for the same case.


Signed-off-by: Amit Singh Tomar <amit.tomar@freescale.com>
---
drivers/i2c/busses/i2c-mpc.c |=C2=A0=C2=A0=C2=A0 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c in= dex 0edf630..7a3136f 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -95,8 +95,9 @@ static irqreturn_t mpc_i2c_isr(int irq, void *dev_id)
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 i2c->interrupt =3D readb(i2c-= >base + MPC_I2C_SR);
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 writeb(0, i2c->base + MPC_I2C= _SR);
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 wake_up(&i2c->queue);
+=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 return IRQ_HANDLED;
=C2=A0=C2=A0=C2=A0 }
-=C2=A0 return IRQ_HANDLED;
+ =C2=A0return IRQ_NONE;
}

/* Sometimes 9th clock pulse isn't generated, and slave doesn't rel= ease
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c&qu= ot; in
the body of a message to major= domo@vger.kernel.org
More majordomo info at=C2=A0 http://vger.kernel.org/majordomo-info.html



--
- Danielle Costantino<= br>
--001a11c2fa0e884e04050ddca481-- --===============8913768023928736773== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KTGludXhwcGMt ZGV2IG1haWxpbmcgbGlzdApMaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZwpodHRwczovL2xp c3RzLm96bGFicy5vcmcvbGlzdGluZm8vbGludXhwcGMtZGV2 --===============8913768023928736773==-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ie0-x22b.google.com (mail-ie0-x22b.google.com [IPv6:2607:f8b0:4001:c03::22b]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 7C1DA1A0DF2 for ; Fri, 30 Jan 2015 22:09:51 +1100 (AEDT) Received: by mail-ie0-f171.google.com with SMTP id tr6so2675047ieb.2 for ; Fri, 30 Jan 2015 03:09:48 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: Date: Fri, 30 Jan 2015 03:09:47 -0800 Message-ID: Subject: Re: [PATCH] i2c/mpc: Fix ISR return value From: Danielle Costantino To: Amit Tomar Content-Type: multipart/alternative; boundary=001a11c2fa0e884e04050ddca481 Cc: "linuxppc-dev@lists.ozlabs.org" , "linux-i2c@vger.kernel.org" List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --001a11c2fa0e884e04050ddca481 Content-Type: text/plain; charset=UTF-8 I have been using the driver with this modification for the past 6 months and it has been stable in an industrial environment.I had made a few other changes that also improve reliability (using ppc in_8 and out_8 and eieio barriers to ensure in-order execution. This lets you remove the unneeded double read of the status register. I also added a more robust recovery function to handle force of bus master-ship, and clearing the arb lost interrupt that is generated. currently this can cause the isr to trigger and cause superfluous interrupts. I have not posted this patch because of the extensive changes, I will ack this patch. On Fri, Jan 30, 2015 at 2:24 AM, Amit Tomar wrote: > ISR should not return IRQ_HANDLED for not handling anything. > This patch fixes the return value of ISR for the same case. > > > Signed-off-by: Amit Singh Tomar > --- > drivers/i2c/busses/i2c-mpc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c > index 0edf630..7a3136f 100644 > --- a/drivers/i2c/busses/i2c-mpc.c > +++ b/drivers/i2c/busses/i2c-mpc.c > @@ -95,8 +95,9 @@ static irqreturn_t mpc_i2c_isr(int irq, void *dev_id) > i2c->interrupt = readb(i2c->base + MPC_I2C_SR); > writeb(0, i2c->base + MPC_I2C_SR); > wake_up(&i2c->queue); > + return IRQ_HANDLED; > } > - return IRQ_HANDLED; > + return IRQ_NONE; > } > > /* Sometimes 9th clock pulse isn't generated, and slave doesn't release > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- - Danielle Costantino --001a11c2fa0e884e04050ddca481 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
I have been using the driver with this modification for th= e past 6 months and it has been stable in an industrial environment.I had m= ade a few other changes that also improve reliability (using ppc in_8 and o= ut_8 and eieio barriers to ensure in-order execution. This lets you remove = the unneeded double read of the status register. I also added a more robust= recovery function to handle force of bus master-ship, and clearing the arb= lost interrupt that is generated. currently this can cause the isr to trig= ger and cause superfluous interrupts. I have not posted this patch because = of the extensive changes,

I will ack this patch.

On Fri, Jan 3= 0, 2015 at 2:24 AM, Amit Tomar <Amit.Tomar@freescale.com> wrote:
ISR should not return IRQ_HAN= DLED for not handling anything.
This patch fixes the return value of ISR for the same case.


Signed-off-by: Amit Singh Tomar <amit.tomar@freescale.com>
---
drivers/i2c/busses/i2c-mpc.c |=C2=A0=C2=A0=C2=A0 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c in= dex 0edf630..7a3136f 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -95,8 +95,9 @@ static irqreturn_t mpc_i2c_isr(int irq, void *dev_id)
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 i2c->interrupt =3D readb(i2c-= >base + MPC_I2C_SR);
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 writeb(0, i2c->base + MPC_I2C= _SR);
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 wake_up(&i2c->queue);
+=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 return IRQ_HANDLED;
=C2=A0=C2=A0=C2=A0 }
-=C2=A0 return IRQ_HANDLED;
+ =C2=A0return IRQ_NONE;
}

/* Sometimes 9th clock pulse isn't generated, and slave doesn't rel= ease
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c&qu= ot; in
the body of a message to major= domo@vger.kernel.org
More majordomo info at=C2=A0 http://vger.kernel.org/majordomo-info.html



--
- Danielle Costantino<= br>
--001a11c2fa0e884e04050ddca481--