lkml.org 
[lkml]   [2016]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 2/6] g_NCR5380: Test the IRQ before accepting it
Date
On Wednesday 02 November 2016 08:45:26 Finn Thain wrote:
> On Mon, 31 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.
>
> Once again, how does it cause a hang?

Kernel scans the bus, finds a HDD, then attempts to read MBR. modprobe process
is stuck but the system is still running. Then the transfer probably times
out and everything locks up hard, even fbcon cursor stops blinking. I guess
that kernel is trying to abort or reset.
BTW. rescan-scsi-bus also causes hang, anytime, even without IRQ.

> If the user specifies an IRQ, we should trust them. If they don't specify
> an IRQ then probe (as in patch 5/6).
>
> > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> > ---
> > drivers/scsi/g_NCR5380.c | 44
> > +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41
> > insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
> > index 09c660b..0d1f6ad 100644
> > --- a/drivers/scsi/g_NCR5380.c
> > +++ b/drivers/scsi/g_NCR5380.c
> > @@ -115,6 +115,32 @@ static int NCR5380_probe_irq(struct Scsi_Host
> > *instance) return irq;
> > }
> >
> > +static bool irq_working;
> > +
> > +static irqreturn_t test_irq(int irq, void *dev_id)
> > +{
> > + irq_working = true;
> > + return IRQ_HANDLED;
> > +}
> > +
> > +/* test if the IRQ is working */
> > +static int NCR5380_test_irq(struct Scsi_Host *instance, int irq)
> > +{
> > + struct NCR5380_hostdata *hostdata = shost_priv(instance);
> > +
> > + irq_working = false;
> > + if (request_irq(irq, test_irq, 0, "NCR5380-irqtest", NULL))
> > + return -EBUSY;
> > + NCR5380_trigger_irq(instance);
> > + NCR5380_read(RESET_PARITY_INTERRUPT_REG);
> > + free_irq(irq, NULL);
> > +
> > + if (!irq_working)
> > + return -EIO;
> > +
> > + return 0;
> > +}
> > +
>
> IMO the extra complexity is not justified by the possibility of machines
> with misconfigured BIOS or incorrect module parameters.
>
> I don't want another irq probing mechanism. We just got rid of one.
>
> But I won't mind if the SCSI maintainers who know ISA better than I do
> would like to review this.
>
> > /*
> > * Configure I/O address of 53C400A or DTC436 by writing magic numbers
> > * to ports 0x779 and 0x379.
> > @@ -323,9 +349,21 @@ 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)) {
> > - printk(KERN_WARNING "scsi%d : IRQ%d not free, interrupts disabled\n",
> > instance->host_no, instance->irq); + ret = NCR5380_test_irq(instance,
> > instance->irq);
> > + if (ret) {
> > + printk(KERN_WARNING "scsi%d : IRQ%d not %s, interrupts disabled\n",
> > + instance->host_no, instance->irq,
> > + (ret == -EBUSY) ? "free" : "working");
> > + instance->irq = NO_IRQ;
> > + }
> > + }
> > +
> > + if (instance->irq != NO_IRQ) {
> > + 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;
> > }
> > }


--
Ondrej Zary

\
 
 \ /
  Last update: 2016-11-02 20:16    [W:0.066 / U:0.432 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site