On Sat, 2008-06-28 at 04:54 +0400, Anton Vorontsov wrote:
> 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.
Don't we replay edge IRQs that happen while soft-disabled ? Could be a
bug in your PIC code not to do so...
Ben.
> 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;
> }
>
> /**
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
