* [PATCH] i2c-rcar: fix MNR interrupt handling
@ 2014-09-01 21:15 ` Sergei Shtylyov
0 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2014-09-01 21:15 UTC (permalink / raw)
To: wsa, linux-i2c; +Cc: linux-sh
Sometimes the MNR and MST interrupts happen simultaneously (stop automatically
follows NACK, according to the manuals) and in such case the ID_NACK flag isn't
set since the MST interrupt handling precedes MNR and all interrupts are cleared
and disabled then, so that MNR interrupt is never noticed -- this causes NACK'ed
transfers to be falsely reported as successful. Exchanging MNR and MST handlers
fixes this issue, however the MNR bit somehow gets set again even after being
explicitly cleared, so I decided to completely suppress handling of all disabled
interrupts (which is a good thing anyway)...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: stable@vger.kernel.org
---
The patch is against Wolfram Sang's 'linux.git' repo.
drivers/i2c/busses/i2c-rcar.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
Index: linux/drivers/i2c/busses/i2c-rcar.c
===================================================================
--- linux.orig/drivers/i2c/busses/i2c-rcar.c
+++ linux/drivers/i2c/busses/i2c-rcar.c
@@ -367,18 +367,15 @@ static irqreturn_t rcar_i2c_irq(int irq,
msr = rcar_i2c_read(priv, ICMSR);
+ /* Only handle interrupts that are currently enabled */
+ msr &= rcar_i2c_read(priv, ICMIER);
+
/* Arbitration lost */
if (msr & MAL) {
rcar_i2c_flags_set(priv, (ID_DONE | ID_ARBLOST));
goto out;
}
- /* Stop */
- if (msr & MST) {
- rcar_i2c_flags_set(priv, ID_DONE);
- goto out;
- }
-
/* Nack */
if (msr & MNR) {
/* go to stop phase */
@@ -388,6 +385,12 @@ static irqreturn_t rcar_i2c_irq(int irq,
goto out;
}
+ /* Stop */
+ if (msr & MST) {
+ rcar_i2c_flags_set(priv, ID_DONE);
+ goto out;
+ }
+
if (rcar_i2c_is_recv(priv))
rcar_i2c_flags_set(priv, rcar_i2c_irq_recv(priv, msr));
else
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] i2c-rcar: fix MNR interrupt handling
@ 2014-09-01 21:15 ` Sergei Shtylyov
0 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2014-09-01 21:15 UTC (permalink / raw)
To: wsa, linux-i2c; +Cc: linux-sh
Sometimes the MNR and MST interrupts happen simultaneously (stop automatically
follows NACK, according to the manuals) and in such case the ID_NACK flag isn't
set since the MST interrupt handling precedes MNR and all interrupts are cleared
and disabled then, so that MNR interrupt is never noticed -- this causes NACK'ed
transfers to be falsely reported as successful. Exchanging MNR and MST handlers
fixes this issue, however the MNR bit somehow gets set again even after being
explicitly cleared, so I decided to completely suppress handling of all disabled
interrupts (which is a good thing anyway)...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: stable@vger.kernel.org
---
The patch is against Wolfram Sang's 'linux.git' repo.
drivers/i2c/busses/i2c-rcar.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
Index: linux/drivers/i2c/busses/i2c-rcar.c
=================================--- linux.orig/drivers/i2c/busses/i2c-rcar.c
+++ linux/drivers/i2c/busses/i2c-rcar.c
@@ -367,18 +367,15 @@ static irqreturn_t rcar_i2c_irq(int irq,
msr = rcar_i2c_read(priv, ICMSR);
+ /* Only handle interrupts that are currently enabled */
+ msr &= rcar_i2c_read(priv, ICMIER);
+
/* Arbitration lost */
if (msr & MAL) {
rcar_i2c_flags_set(priv, (ID_DONE | ID_ARBLOST));
goto out;
}
- /* Stop */
- if (msr & MST) {
- rcar_i2c_flags_set(priv, ID_DONE);
- goto out;
- }
-
/* Nack */
if (msr & MNR) {
/* go to stop phase */
@@ -388,6 +385,12 @@ static irqreturn_t rcar_i2c_irq(int irq,
goto out;
}
+ /* Stop */
+ if (msr & MST) {
+ rcar_i2c_flags_set(priv, ID_DONE);
+ goto out;
+ }
+
if (rcar_i2c_is_recv(priv))
rcar_i2c_flags_set(priv, rcar_i2c_irq_recv(priv, msr));
else
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] i2c-rcar: fix MNR interrupt handling
2014-09-01 21:15 ` Sergei Shtylyov
@ 2014-09-02 10:28 ` Wolfram Sang
-1 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2014-09-02 10:28 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linux-i2c, linux-sh
[-- Attachment #1: Type: text/plain, Size: 848 bytes --]
On Tue, Sep 02, 2014 at 01:15:26AM +0400, Sergei Shtylyov wrote:
> Sometimes the MNR and MST interrupts happen simultaneously (stop automatically
> follows NACK, according to the manuals) and in such case the ID_NACK flag isn't
> set since the MST interrupt handling precedes MNR and all interrupts are cleared
> and disabled then, so that MNR interrupt is never noticed -- this causes NACK'ed
> transfers to be falsely reported as successful. Exchanging MNR and MST handlers
> fixes this issue, however the MNR bit somehow gets set again even after being
> explicitly cleared, so I decided to completely suppress handling of all disabled
> interrupts (which is a good thing anyway)...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Cc: stable@vger.kernel.org
>
Applied to for-current, thanks!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] i2c-rcar: fix MNR interrupt handling
@ 2014-09-02 10:28 ` Wolfram Sang
0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2014-09-02 10:28 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linux-i2c, linux-sh
[-- Attachment #1: Type: text/plain, Size: 848 bytes --]
On Tue, Sep 02, 2014 at 01:15:26AM +0400, Sergei Shtylyov wrote:
> Sometimes the MNR and MST interrupts happen simultaneously (stop automatically
> follows NACK, according to the manuals) and in such case the ID_NACK flag isn't
> set since the MST interrupt handling precedes MNR and all interrupts are cleared
> and disabled then, so that MNR interrupt is never noticed -- this causes NACK'ed
> transfers to be falsely reported as successful. Exchanging MNR and MST handlers
> fixes this issue, however the MNR bit somehow gets set again even after being
> explicitly cleared, so I decided to completely suppress handling of all disabled
> interrupts (which is a good thing anyway)...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Cc: stable@vger.kernel.org
>
Applied to for-current, thanks!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <1843756.qQiZLxsyqD-gHKXc3Y1Z8zGSmamagVegGFoWSdPRAKMAL8bYrjMMd8@public.gmane.org>]
* Re: [PATCH] i2c-rcar: fix MNR interrupt handling
[not found] ` <1843756.qQiZLxsyqD-gHKXc3Y1Z8zGSmamagVegGFoWSdPRAKMAL8bYrjMMd8@public.gmane.org>
@ 2014-09-02 18:29 ` Sergei Shtylyov
0 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2014-09-02 18:29 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, linux-i2c-u79uwXL29TY76Z2rM5mHXA
Cc: linux-sh-u79uwXL29TY76Z2rM5mHXA
On 09/02/2014 01:15 AM, Sergei Shtylyov wrote:
> Sometimes the MNR and MST interrupts happen simultaneously (stop automatically
> follows NACK, according to the manuals) and in such case the ID_NACK flag isn't
> set since the MST interrupt handling precedes MNR and all interrupts are cleared
> and disabled then, so that MNR interrupt is never noticed -- this causes NACK'ed
> transfers to be falsely reported as successful. Exchanging MNR and MST handlers
> fixes this issue, however the MNR bit somehow gets set again even after being
> explicitly cleared, so I decided to completely suppress handling of all disabled
> interrupts (which is a good thing anyway)...
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> The patch is against Wolfram Sang's 'linux.git' repo.
> drivers/i2c/busses/i2c-rcar.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> Index: linux/drivers/i2c/busses/i2c-rcar.c
> ===================================================================
> --- linux.orig/drivers/i2c/busses/i2c-rcar.c
> +++ linux/drivers/i2c/busses/i2c-rcar.c
> @@ -367,18 +367,15 @@ static irqreturn_t rcar_i2c_irq(int irq,
>
> msr = rcar_i2c_read(priv, ICMSR);
>
> + /* Only handle interrupts that are currently enabled */
> + msr &= rcar_i2c_read(priv, ICMIER);
> +
I think it makes sense to check 'msr' for 0 and return IRQ_NONE in this
case. Would you apply such patch (probably to the -next branch)?
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] i2c-rcar: fix MNR interrupt handling
@ 2014-09-02 18:29 ` Sergei Shtylyov
0 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2014-09-02 18:29 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, linux-i2c-u79uwXL29TY76Z2rM5mHXA
Cc: linux-sh-u79uwXL29TY76Z2rM5mHXA
On 09/02/2014 01:15 AM, Sergei Shtylyov wrote:
> Sometimes the MNR and MST interrupts happen simultaneously (stop automatically
> follows NACK, according to the manuals) and in such case the ID_NACK flag isn't
> set since the MST interrupt handling precedes MNR and all interrupts are cleared
> and disabled then, so that MNR interrupt is never noticed -- this causes NACK'ed
> transfers to be falsely reported as successful. Exchanging MNR and MST handlers
> fixes this issue, however the MNR bit somehow gets set again even after being
> explicitly cleared, so I decided to completely suppress handling of all disabled
> interrupts (which is a good thing anyway)...
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Cc: stable@vger.kernel.org
> ---
> The patch is against Wolfram Sang's 'linux.git' repo.
> drivers/i2c/busses/i2c-rcar.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> Index: linux/drivers/i2c/busses/i2c-rcar.c
> =================================> --- linux.orig/drivers/i2c/busses/i2c-rcar.c
> +++ linux/drivers/i2c/busses/i2c-rcar.c
> @@ -367,18 +367,15 @@ static irqreturn_t rcar_i2c_irq(int irq,
>
> msr = rcar_i2c_read(priv, ICMSR);
>
> + /* Only handle interrupts that are currently enabled */
> + msr &= rcar_i2c_read(priv, ICMIER);
> +
I think it makes sense to check 'msr' for 0 and return IRQ_NONE in this
case. Would you apply such patch (probably to the -next branch)?
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-02 18:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-01 21:15 [PATCH] i2c-rcar: fix MNR interrupt handling Sergei Shtylyov
2014-09-01 21:15 ` Sergei Shtylyov
2014-09-02 10:28 ` Wolfram Sang
2014-09-02 10:28 ` Wolfram Sang
[not found] ` <1843756.qQiZLxsyqD-gHKXc3Y1Z8zGSmamagVegGFoWSdPRAKMAL8bYrjMMd8@public.gmane.org>
2014-09-02 18:29 ` Sergei Shtylyov
2014-09-02 18:29 ` Sergei Shtylyov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.