* [PATCH] sata_rcar: fix interrupt handling
@ 2013-05-31 22:38 Sergei Shtylyov
2013-05-31 23:06 ` Sergei Shtylyov
0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2013-05-31 22:38 UTC (permalink / raw)
To: linux-ide, tj, vladimir.barinov
The driver's interrupt handling code is too picky in deciding whether it should
handle an interrupt or not which causes completely unneeded spurious interrupts.
Thus make sata_rcar_{ata|serr}_interrupt() *void*; add ATA status register read
to sata_rcar_ata_interrupt() to clear an unexpected ATA interrupt -- it doesn't
get cleared by writing to the SATAINTSTAT register in the interrupt mode we use.
Also, in sata_rcar_ata_interrupt() we should check SATAINTSTAT register only for
enabled interrupts and we should clear only those interrupts that we have read
as active first time around, because else we have a race and risk clearing an
interrupt that can occur between read and write of the SATAINTSTAT register
and never registering it...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
This patch is against 'for-3.10-fixes' branch of Tejun Heo's 'libata.git' repo.
It will cause some rejects when merged to 'for-3.11' branch...
drivers/ata/sata_rcar.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
Index: libata/drivers/ata/sata_rcar.c
===================================================================
--- libata.orig/drivers/ata/sata_rcar.c
+++ libata/drivers/ata/sata_rcar.c
@@ -619,17 +619,16 @@ static struct ata_port_operations sata_r
.bmdma_status = sata_rcar_bmdma_status,
};
-static int sata_rcar_serr_interrupt(struct ata_port *ap)
+static void sata_rcar_serr_interrupt(struct ata_port *ap)
{
struct sata_rcar_priv *priv = ap->host->private_data;
struct ata_eh_info *ehi = &ap->link.eh_info;
int freeze = 0;
- int handled = 0;
u32 serror;
serror = ioread32(priv->base + SCRSERR_REG);
if (!serror)
- return 0;
+ return;
DPRINTK("SError @host_intr: 0x%x\n", serror);
@@ -642,7 +641,6 @@ static int sata_rcar_serr_interrupt(stru
ata_ehi_push_desc(ehi, "%s", "hotplug");
freeze = serror & SERR_COMM_WAKE ? 0 : 1;
- handled = 1;
}
/* freeze or abort */
@@ -650,11 +648,9 @@ static int sata_rcar_serr_interrupt(stru
ata_port_freeze(ap);
else
ata_port_abort(ap);
-
- return handled;
}
-static int sata_rcar_ata_interrupt(struct ata_port *ap)
+static void sata_rcar_ata_interrupt(struct ata_port *ap)
{
struct ata_queued_cmd *qc;
int handled = 0;
@@ -663,7 +659,9 @@ static int sata_rcar_ata_interrupt(struc
if (qc)
handled |= ata_bmdma_port_intr(ap, qc);
- return handled;
+ /* be sure to clear ATA interrupt */
+ if (!handled)
+ sata_rcar_check_status(ap);
}
static irqreturn_t sata_rcar_interrupt(int irq, void *dev_instance)
@@ -678,20 +676,21 @@ static irqreturn_t sata_rcar_interrupt(i
spin_lock_irqsave(&host->lock, flags);
sataintstat = ioread32(priv->base + SATAINTSTAT_REG);
+ sataintstat &= SATA_RCAR_INT_MASK;
if (!sataintstat)
goto done;
/* ack */
- iowrite32(sataintstat & ~SATA_RCAR_INT_MASK,
- priv->base + SATAINTSTAT_REG);
+ iowrite32(~sataintstat & 0x7ff, priv->base + SATAINTSTAT_REG);
ap = host->ports[0];
if (sataintstat & SATAINTSTAT_ATA)
- handled |= sata_rcar_ata_interrupt(ap);
+ sata_rcar_ata_interrupt(ap);
if (sataintstat & SATAINTSTAT_SERR)
- handled |= sata_rcar_serr_interrupt(ap);
+ sata_rcar_serr_interrupt(ap);
+ handled = 1;
done:
spin_unlock_irqrestore(&host->lock, flags);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sata_rcar: fix interrupt handling
2013-05-31 22:38 [PATCH] sata_rcar: fix interrupt handling Sergei Shtylyov
@ 2013-05-31 23:06 ` Sergei Shtylyov
2013-06-02 7:58 ` Tejun Heo
0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2013-05-31 23:06 UTC (permalink / raw)
To: linux-ide, tj; +Cc: vladimir.barinov
Hello.
On 06/01/2013 02:38 AM, Sergei Shtylyov wrote:
> The driver's interrupt handling code is too picky in deciding whether it should
> handle an interrupt or not which causes completely unneeded spurious interrupts.
> Thus make sata_rcar_{ata|serr}_interrupt() *void*; add ATA status register read
> to sata_rcar_ata_interrupt() to clear an unexpected ATA interrupt -- it doesn't
> get cleared by writing to the SATAINTSTAT register in the interrupt mode we use.
>
> Also, in sata_rcar_ata_interrupt() we should check SATAINTSTAT register only for
> enabled interrupts and we should clear only those interrupts that we have read
> as active first time around, because else we have a race and risk clearing an
> interrupt that can occur between read and write of the SATAINTSTAT register
> and never registering it...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Darn, I forgot to Cc: stable@vger.kernel.org again.
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sata_rcar: fix interrupt handling
2013-05-31 23:06 ` Sergei Shtylyov
@ 2013-06-02 7:58 ` Tejun Heo
2013-06-02 11:39 ` Sergei Shtylyov
0 siblings, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2013-06-02 7:58 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linux-ide, vladimir.barinov
On Sat, Jun 01, 2013 at 03:06:24AM +0400, Sergei Shtylyov wrote:
> Hello.
>
> On 06/01/2013 02:38 AM, Sergei Shtylyov wrote:
>
> >The driver's interrupt handling code is too picky in deciding whether it should
> >handle an interrupt or not which causes completely unneeded spurious interrupts.
> >Thus make sata_rcar_{ata|serr}_interrupt() *void*; add ATA status register read
> >to sata_rcar_ata_interrupt() to clear an unexpected ATA interrupt -- it doesn't
> >get cleared by writing to the SATAINTSTAT register in the interrupt mode we use.
> >
> >Also, in sata_rcar_ata_interrupt() we should check SATAINTSTAT register only for
> >enabled interrupts and we should clear only those interrupts that we have read
> >as active first time around, because else we have a race and risk clearing an
> >interrupt that can occur between read and write of the SATAINTSTAT register
> >and never registering it...
> >
> >Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> Darn, I forgot to Cc: stable@vger.kernel.org again.
Applied to libata/for-3.10-fixes w/ stable cc'd. BTW, the driver
generates the following compile warning.
CC drivers/ata/sata_rcar.o
drivers/ata/sata_rcar.c: In function ‘sata_rcar_thaw’:
drivers/ata/sata_rcar.c:183:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
Care to fix it?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sata_rcar: fix interrupt handling
2013-06-02 7:58 ` Tejun Heo
@ 2013-06-02 11:39 ` Sergei Shtylyov
2013-06-02 17:00 ` Sergei Shtylyov
0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2013-06-02 11:39 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-ide, vladimir.barinov
Hello.
On 02-06-2013 11:58, Tejun Heo wrote:
>>> The driver's interrupt handling code is too picky in deciding whether it should
>>> handle an interrupt or not which causes completely unneeded spurious interrupts.
>>> Thus make sata_rcar_{ata|serr}_interrupt() *void*; add ATA status register read
>>> to sata_rcar_ata_interrupt() to clear an unexpected ATA interrupt -- it doesn't
>>> get cleared by writing to the SATAINTSTAT register in the interrupt mode we use.
>>> Also, in sata_rcar_ata_interrupt() we should check SATAINTSTAT register only for
>>> enabled interrupts and we should clear only those interrupts that we have read
>>> as active first time around, because else we have a race and risk clearing an
>>> interrupt that can occur between read and write of the SATAINTSTAT register
>>> and never registering it...
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>> Darn, I forgot to Cc: stable@vger.kernel.org again.
> Applied to libata/for-3.10-fixes w/ stable cc'd. BTW, the driver
> generates the following compile warning.
> CC drivers/ata/sata_rcar.o
> drivers/ata/sata_rcar.c: In function ‘sata_rcar_thaw’:
> drivers/ata/sata_rcar.c:183:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
It compiles without warnings for me, IIRC. What version of gcc are
you using?
> Care to fix it?
I'll look into it...
> Thanks.
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sata_rcar: fix interrupt handling
2013-06-02 11:39 ` Sergei Shtylyov
@ 2013-06-02 17:00 ` Sergei Shtylyov
2013-06-03 3:49 ` Tejun Heo
0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2013-06-02 17:00 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-ide, vladimir.barinov
Hello.
On 06/02/2013 03:39 PM, Sergei Shtylyov wrote:
>
>>> Darn, I forgot to Cc: stable@vger.kernel.org again.
>
>> Applied to libata/for-3.10-fixes w/ stable cc'd. BTW, the driver
>> generates the following compile warning.
>
>> CC drivers/ata/sata_rcar.o
>> drivers/ata/sata_rcar.c: In function ‘sata_rcar_thaw’:
>> drivers/ata/sata_rcar.c:183:2: warning: large integer implicitly
>> truncated to unsigned type [-Woverflow]
>
> It compiles without warnings for me, IIRC. What version of gcc are
> you using?
>> Care to fix it?
>
> I'll look into it...
I tried 4.2.0, 4.3.3 (that one broke with segmentation fault), and
4.7.3 and no warning...
So, please tell me what version you have for the changelog.
>> Thanks.
>
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sata_rcar: fix interrupt handling
2013-06-02 17:00 ` Sergei Shtylyov
@ 2013-06-03 3:49 ` Tejun Heo
0 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2013-06-03 3:49 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linux-ide, vladimir.barinov
Hey,
On Sun, Jun 02, 2013 at 09:00:11PM +0400, Sergei Shtylyov wrote:
> >> CC drivers/ata/sata_rcar.o
> >> drivers/ata/sata_rcar.c: In function ‘sata_rcar_thaw’:
> >> drivers/ata/sata_rcar.c:183:2: warning: large integer
> >>implicitly truncated to unsigned type [-Woverflow]
> >
> > It compiles without warnings for me, IIRC. What version of gcc
> >are you using?
>
> >>Care to fix it?
> >
> > I'll look into it...
>
> I tried 4.2.0, 4.3.3 (that one broke with segmentation fault),
> and 4.7.3 and no warning...
> So, please tell me what version you have for the changelog.
I'm on fedora 19 beta x86-64.
$ gcc --version
gcc (GCC) 4.8.0 20130526 (Red Hat 4.8.0-8)
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
# make drivers/ata/sata_rcar.o
...
CC drivers/ata/sata_rcar.o
drivers/ata/sata_rcar.c: In function ‘sata_rcar_thaw’:
drivers/ata/sata_rcar.c:183:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
iowrite32(~SATA_RCAR_INT_MASK, priv->base + SATAINTSTAT_REG);
^
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-03 3:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-31 22:38 [PATCH] sata_rcar: fix interrupt handling Sergei Shtylyov
2013-05-31 23:06 ` Sergei Shtylyov
2013-06-02 7:58 ` Tejun Heo
2013-06-02 11:39 ` Sergei Shtylyov
2013-06-02 17:00 ` Sergei Shtylyov
2013-06-03 3:49 ` Tejun Heo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox