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?
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@xxxxxxxxxxxxxxxxxxxx>
> ---
> 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;
> }
> }
>
--
--
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
