From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762623AbcJaIgG (ORCPT ); Mon, 31 Oct 2016 04:36:06 -0400 Received: from smtp-1b.atlantis.sk ([80.94.52.26]:37101 "EHLO smtp-1b.atlantis.sk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1762059AbcJaIgE (ORCPT ); Mon, 31 Oct 2016 04:36:04 -0400 From: Ondrej Zary To: Finn Thain Subject: Re: [PATCH 2/3] g_NCR5380: Test the IRQ before accepting it Date: Mon, 31 Oct 2016 09:35:58 +0100 User-Agent: KMail/1.9.10 (enterprise35 0.20100827.1168748) Cc: Christoph Hellwig , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org References: <1477867203-7480-1-git-send-email-linux@rainbow-software.org> <1477867203-7480-2-git-send-email-linux@rainbow-software.org> In-Reply-To: X-KMail-QuotePrefix: > MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <201610310935.58892.linux@rainbow-software.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > --- > > 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