On Monday 31 October 2016, Finn Thain wrote:
> On Sun, 30 Oct 2016, Ondrej Zary wrote:
> > Trigger an IRQ first with a test IRQ handler to find out if it really
> > works. Disable the IRQ if not.
> >
> > This prevents hang when incorrect IRQ was specified by user.
> >
> > Signed-off-by: Ondrej Zary <linux@xxxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/scsi/g_NCR5380.c | 32 ++++++++++++++++++++++++++++++--
> > 1 file changed, 30 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
> > index 3790ed5..261e168 100644
> > --- a/drivers/scsi/g_NCR5380.c
> > +++ b/drivers/scsi/g_NCR5380.c
> > @@ -67,6 +67,14 @@
> > MODULE_ALIAS("g_NCR5380_mmio");
> > MODULE_LICENSE("GPL");
> >
> > +static bool irq_working;
> > +
> > +static irqreturn_t test_irq(int irq, void *dev_id)
> > +{
> > + irq_working = true;
> > + return IRQ_HANDLED;
> > +}
> > +
> > /*
> > * Configure I/O address of 53C400A or DTC436 by writing magic numbers
> > * to ports 0x779 and 0x379.
> > @@ -275,10 +283,30 @@ static int generic_NCR5380_init_one(struct
> > scsi_host_template *tpnt, /* set IRQ for HP C2502 */
> > if (board == BOARD_HP_C2502)
> > magic_configure(port_idx, instance->irq, magic);
> > - if (request_irq(instance->irq, generic_NCR5380_intr,
> > - 0, "NCR5380", instance)) {
> > + /* test if the IRQ is working */
> > + irq_working = false;
> > + if (request_irq(instance->irq, test_irq,
> > + 0, "NCR5380-irqtest", NULL)) {
> > printk(KERN_WARNING "scsi%d : IRQ%d not free, interrupts disabled\n",
> > instance->host_no, instance->irq); instance->irq = NO_IRQ;
> > + } else {
> > + NCR5380_trigger_irq(instance);
> > + NCR5380_read(RESET_PARITY_INTERRUPT_REG);
> > + free_irq(instance->irq, NULL);
> > + if (irq_working) {
> > + if (request_irq(instance->irq,
> > + generic_NCR5380_intr, 0,
> > + "NCR5380", instance)) {
> > + printk(KERN_WARNING "scsi%d : IRQ%d not free, interrupts
> > disabled\n", + instance->host_no,
> > + instance->irq);
> > + instance->irq = NO_IRQ;
> > + }
> > + } else {
> > + printk(KERN_WARNING "scsi%d : IRQ%d not working, interrupts
> > disabled\n", + instance->host_no, instance->irq);
> > + instance->irq = NO_IRQ;
> > + }
> > }
> > }
>
> If the user omits to specify an irq, you can just default to IRQ_AUTO.
> This might result in NO_IRQ, which gives the same result as this patch.
Looks like a good idea.
> And when the user does specify an IRQ, we should trust them. So this
> compexity doesn't add any value AFAICT. Thanks but no thanks.
This fixes a real problem: specifying wrong IRQ hangs the machine completely.
It's really easy - if the IRQ is free but configured in BIOS as PCI IRQ (not
ISA). Everything seems fine except the IRQ will never trigger.
--
Ondrej Zary
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
