[PATCH v2 -rt] ide: workaround buggy hardware issues withpreemptable hardirqs

[Anton Vorontsov]

IDE interrupt handler relies on the fact that, if necessary, hardirqs
will re-trigger on ISR exit. The assumption is valid for level sensitive
interrupts.

But some hardware (namely ULi M5228 in the ULi M1575 "Super South Brige")
behaves in a strange way: it asserts interrupts as edge sensitive. And
because preemptable IRQ handler disables PIC's interrupt, PIC will likely
miss it.

This patch fixes following issue:

ALI15X3: IDE controller (0x10b9:0x5229 rev 0xc8) at  PCI slot 0001:03:1f.0
ALI15X3: 100% native mode on irq 18
ide0: BM-DMA at 0x1120-0x1127, BIOS settings: hda:PIO, hdb:PIO
ide1: BM-DMA at 0x1128-0x112f, BIOS settings: hdc:PIO, hdd:PIO
hda: Optiarc DVD RW AD-7190A, ATAPI CD/DVD-ROM drive
hda: UDMA/66 mode selected
ide0 at 0x1100-0x1107,0x110a on irq 18
ide-cd: cmd 0x5a timed out
hda: lost interrupt
hda: ATAPI 12X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache
Uniform CD-ROM driver Revision: 3.20
ide-cd: cmd 0x3 timed out
hda: lost interrupt
ide-cd: cmd 0x3 timed out
hda: lost interrupt
...

It would be great to re-configure the ULi bridge or ULi IDE controller
to behave sanely, but no one knows how or if this is possible at all
(no available specifications).

So.. to workaround the issue IDE interrupt handler should re-check for
any pending IRQs. This isn't bulletproof solution, but it works and this
is the best one we can do.

Signed-off-by: Anton Vorontsov <avorontsov@xxxxxxxxxxxxx>
---

On Wed, Jun 25, 2008 at 04:34:31PM +0400, Anton Vorontsov wrote:
[...]
> The bug, as I see it, in the alim15x3 (ULi M5228) hardware: for some
> reason it does not hold IRQ line, but rises it for some short period
> of time (while the drive itself rises and holds it correctly -- I'm
> seeing it via oscilloscope).
> 
> So this scheme does not work:
> mask_irq()
> ...do something that will trigger IDE interrupt...
> unmask_irq()
> 
> Because at the unmask_irq() time IDE IRQ is gone already, and interrupt
> controller could not notice it (interrupts are level sensitive).
> 
> I did following test: disable RT + insert mask/unmask sequence in the
> IDE IRQ handler, and I got the same behaviour as with RT enabled.
> 
> Also, further testing showed that this issue isn't drive-specific, i.e.
> with a delay inserted before the unmask_irq(), the bug shows with any
> drive I have.
> 
> So, in summary: I think that the patch is still correct as a hw bug
> workaround (I'll need to correct its comments and description though).

Here is updated patch.

 drivers/ide/ide-io.c |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 6c1b288..19d36f0 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -1460,6 +1460,7 @@ static void unexpected_intr (int irq, ide_hwgroup_t *hwgroup)
  
 irqreturn_t ide_intr (int irq, void *dev_id)
 {
+	irqreturn_t ret = IRQ_NONE;
 	unsigned long flags;
 	ide_hwgroup_t *hwgroup = (ide_hwgroup_t *)dev_id;
 	ide_hwif_t *hwif;
@@ -1467,12 +1468,13 @@ irqreturn_t ide_intr (int irq, void *dev_id)
 	ide_handler_t *handler;
 	ide_startstop_t startstop;
 
+again:
 	spin_lock_irqsave(&ide_lock, flags);
 	hwif = hwgroup->hwif;
 
 	if (!ide_ack_intr(hwif)) {
 		spin_unlock_irqrestore(&ide_lock, flags);
-		return IRQ_NONE;
+		return ret;
 	}
 
 	if ((handler = hwgroup->handler) == NULL || hwgroup->polling) {
@@ -1510,7 +1512,7 @@ irqreturn_t ide_intr (int irq, void *dev_id)
 #endif /* CONFIG_BLK_DEV_IDEPCI */
 		}
 		spin_unlock_irqrestore(&ide_lock, flags);
-		return IRQ_NONE;
+		return ret;
 	}
 	drive = hwgroup->drive;
 	if (!drive) {
@@ -1532,7 +1534,7 @@ irqreturn_t ide_intr (int irq, void *dev_id)
 		 * enough advance overhead that the latter isn't a problem.
 		 */
 		spin_unlock_irqrestore(&ide_lock, flags);
-		return IRQ_NONE;
+		return ret;
 	}
 	if (!hwgroup->busy) {
 		hwgroup->busy = 1;	/* paranoia */
@@ -1578,7 +1580,17 @@ irqreturn_t ide_intr (int irq, void *dev_id)
 		}
 	}
 	spin_unlock_irqrestore(&ide_lock, flags);
-	return IRQ_HANDLED;
+	ret = IRQ_HANDLED;
+
+	/*
+	 * Previous handler() may have set things up for another interrupt to
+	 * occur soon... with hardirqs preemption we may lose it because of
+	 * buggy hardware that asserts edge-sensitive IRQs, so try again and
+	 * then return gracefully if no IRQs were actually pending.
+	 */
+	if (hardirq_preemption && startstop != ide_stopped)
+		goto again;
+	return ret;
 }
 
 /**
-- 
1.5.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/