Re: [PATCH 10/29] ncr5380: Fix SCSI_IRQ_NONE bugs

2014-10-03 Thread Geert Uytterhoeven
On Thu, Oct 2, 2014 at 8:56 AM, Finn Thain fth...@telegraphics.com.au wrote:
 Oak scsi doesn't use any IRQ, but it sets irq = IRQ_NONE rather than
 SCSI_IRQ_NONE. Problem is, the core NCR5380 driver expects SCSI_IRQ_NONE
 if it is to issue IDENTIFY commands that prevent target disconnection.

 Other drivers, when they can't get an IRQ or can't use one, will set
 host-irq = SCSI_IRQ_NONE (that is, 255). But when they exit they will
 attempt to free IRQ 255 which was never requested.

 Fix these bugs by using IRQ_NONE in place of SCSI_IRQ_NONE. This means
 IRQ 0 is no longer probed by ISA drivers but I don't think this matters.

IRQ_NONE is part of enum irqreturn. I guess you meant NO_IRQ?

But NO_IRQ is deprecated, and not available on all architectures.
The recommended way is to just use 0, like in if (instance-irq) 

Note that some drivers do

#ifndef NO_IRQ
#define NO_IRQ  (-1)
#endif

and others do

#ifndef NO_IRQ
#define NO_IRQ  0
#endif

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 10/29] ncr5380: Fix SCSI_IRQ_NONE bugs

2014-10-03 Thread Finn Thain

On Fri, 3 Oct 2014, Geert Uytterhoeven wrote:

 On Thu, Oct 2, 2014 at 8:56 AM, Finn Thain fth...@telegraphics.com.au 
 wrote:
  Oak scsi doesn't use any IRQ, but it sets irq = IRQ_NONE rather than
  SCSI_IRQ_NONE. Problem is, the core NCR5380 driver expects SCSI_IRQ_NONE
  if it is to issue IDENTIFY commands that prevent target disconnection.
 
  Other drivers, when they can't get an IRQ or can't use one, will set
  host-irq = SCSI_IRQ_NONE (that is, 255). But when they exit they will
  attempt to free IRQ 255 which was never requested.
 
  Fix these bugs by using IRQ_NONE in place of SCSI_IRQ_NONE. This means
  IRQ 0 is no longer probed by ISA drivers but I don't think this matters.
 
 IRQ_NONE is part of enum irqreturn. I guess you meant NO_IRQ?
 
 But NO_IRQ is deprecated, and not available on all architectures.
 The recommended way is to just use 0, like in if (instance-irq) 
 
 Note that some drivers do
 
 #ifndef NO_IRQ
 #define NO_IRQ  (-1)
 #endif
 
 and others do
 
 #ifndef NO_IRQ
 #define NO_IRQ  0
 #endif
 

Well, the question becomes, is it better to replace SCSI_IRQ_NONE with 0 
or with NO_IRQ?

I guess drivers use #ifndef in case the architecture brings its own 
definition of NO_IRQ (presumably because it can't use 0).

Since NCR5380 drivers cover a variety of architectures (ARM, m68k, ISA, 
PCI...) it seems that the more prudent option is,

#ifndef NO_IRQ
#define NO_IRQ  0
#endif

-- 
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 10/29] ncr5380: Fix SCSI_IRQ_NONE bugs

2014-10-02 Thread Finn Thain
Oak scsi doesn't use any IRQ, but it sets irq = IRQ_NONE rather than
SCSI_IRQ_NONE. Problem is, the core NCR5380 driver expects SCSI_IRQ_NONE
if it is to issue IDENTIFY commands that prevent target disconnection.

Other drivers, when they can't get an IRQ or can't use one, will set
host-irq = SCSI_IRQ_NONE (that is, 255). But when they exit they will
attempt to free IRQ 255 which was never requested.

Fix these bugs by using IRQ_NONE in place of SCSI_IRQ_NONE. This means
IRQ 0 is no longer probed by ISA drivers but I don't think this matters.

Setting IRQ = 255 for these ISA drivers is understood to mean no IRQ.
This remains supported so as to avoid breaking existing ISA setups (which
can be difficult to get working) and because existing documentation
(SANE, TLDP etc) describes this usage for the ISA NCR5380 driver options.

Signed-off-by: Finn Thain fth...@telegraphics.com.au

---

The are whitespace issues with this code; it will be cleaned up in a
later patch.

---
 drivers/scsi/NCR5380.c   |   12 ++--
 drivers/scsi/NCR5380.h   |1 -
 drivers/scsi/dmx3191d.c  |7 ---
 drivers/scsi/dtc.c   |   22 +-
 drivers/scsi/g_NCR5380.c |   18 +++---
 drivers/scsi/mac_scsi.c  |8 
 drivers/scsi/pas16.c |   20 +++-
 drivers/scsi/sun3_scsi.c |6 +++---
 drivers/scsi/t128.c  |   14 +-
 9 files changed, 61 insertions(+), 47 deletions(-)

Index: linux/drivers/scsi/dtc.c
===
--- linux.orig/drivers/scsi/dtc.c   2014-10-02 16:56:00.0 +1000
+++ linux/drivers/scsi/dtc.c2014-10-02 16:56:02.0 +1000
@@ -254,31 +254,35 @@ found:
else
instance-irq = NCR5380_probe_irq(instance, DTC_IRQS);
 
+   /* Compatibility with documented NCR5380 kernel parameters */
+   if (instance-irq == 255)
+   instance-irq = IRQ_NONE;
+
 #ifndef DONT_USE_INTR
/* With interrupts enabled, it will sometimes hang when doing 
heavy
 * reads. So better not enable them until I finger it out. */
-   if (instance-irq != SCSI_IRQ_NONE)
+   if (instance-irq != IRQ_NONE)
if (request_irq(instance-irq, dtc_intr, 0,
dtc, instance)) {
printk(KERN_ERR scsi%d : IRQ%d not free, 
interrupts disabled\n, instance-host_no, instance-irq);
-   instance-irq = SCSI_IRQ_NONE;
+   instance-irq = IRQ_NONE;
}
 
-   if (instance-irq == SCSI_IRQ_NONE) {
+   if (instance-irq == IRQ_NONE) {
printk(KERN_WARNING scsi%d : interrupts not enabled. 
for better interactive performance,\n, instance-host_no);
printk(KERN_WARNING scsi%d : please jumper the board 
for a free IRQ.\n, instance-host_no);
}
 #else
-   if (instance-irq != SCSI_IRQ_NONE)
+   if (instance-irq != IRQ_NONE)
printk(KERN_WARNING scsi%d : interrupts not used. 
Might as well not jumper it.\n, instance-host_no);
-   instance-irq = SCSI_IRQ_NONE;
+   instance-irq = IRQ_NONE;
 #endif
 #if defined(DTCDEBUG)  (DTCDEBUG  DTCDEBUG_INIT)
printk(scsi%d : irq = %d\n, instance-host_no, instance-irq);
 #endif
 
printk(KERN_INFO scsi%d : at 0x%05X, instance-host_no, (int) 
instance-base);
-   if (instance-irq == SCSI_IRQ_NONE)
+   if (instance-irq == IRQ_NONE)
printk( interrupts disabled);
else
printk( irq %d, instance-irq);
@@ -350,7 +354,7 @@ static inline int NCR5380_pread(struct S
i = 0;
NCR5380_read(RESET_PARITY_INTERRUPT_REG);
NCR5380_write(MODE_REG, MR_ENABLE_EOP_INTR | MR_DMA_MODE);
-   if (instance-irq == SCSI_IRQ_NONE)
+   if (instance-irq == IRQ_NONE)
NCR5380_write(DTC_CONTROL_REG, CSR_DIR_READ);
else
NCR5380_write(DTC_CONTROL_REG, CSR_DIR_READ | CSR_INT_BASE);
@@ -401,7 +405,7 @@ static inline int NCR5380_pwrite(struct
NCR5380_read(RESET_PARITY_INTERRUPT_REG);
NCR5380_write(MODE_REG, MR_ENABLE_EOP_INTR | MR_DMA_MODE);
/* set direction (write) */
-   if (instance-irq == SCSI_IRQ_NONE)
+   if (instance-irq == IRQ_NONE)
NCR5380_write(DTC_CONTROL_REG, 0);
else
NCR5380_write(DTC_CONTROL_REG, CSR_5380_INTR);
@@ -440,7 +444,7 @@ static int dtc_release(struct Scsi_Host
 {
NCR5380_local_declare();
NCR5380_setup(shost);
-   if (shost-irq)
+   if (shost-irq != IRQ_NONE)
free_irq(shost-irq, shost);
NCR5380_exit(shost);
if (shost-io_port  shost-n_io_port)
Index: