lkml.org 
[lkml]   [2008]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 -rt] ide: workaround buggy hardware issues with preemptable hardirqs
    From
    Date
    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@ru.mvista.com>
    > ---
    >
    > 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;
    > }
    >
    > /**



    \
     
     \ /
      Last update: 2008-06-30 01:31    [W:5.818 / U:0.168 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site