sim710: cleanup/remove most cli()'s

Hi,

the appended patch does the following:

-group all includes (except '#include "scsi_module.c"') together -add KERN_* to all printk's
-add error checking for call of request_region
-remove most cli() and use spin_lock instead
-move help text to Documentation/scsi/sim710.txt
-add a new debug target for debugging spinlocks
-remove the magic numbers for request_region and release_region and use
SIM710_IOREGION_SIZE instead
-same thing for the driver name
-remove assignement to driver_template.proc_name which is already done before
in sim710.h

I have not broken this into peaces because for all changes (except the first of course) the corresponding lines are grouped together so it should be clear what I've done.

A kernel with this modification has booted yesterday and was tested under heavy loads succesfully.

What has to be done is:
-remove the remaining two cli()
-port this to the new driver model

I'm already working on this but it does not boot until now, so I'll delay this until it works.

Eike

diff -Naur linux-2.5.59-vanilla/Documentation/scsi/sim710.txt linux-2.5.59-lacon/Documentation/scsi/sim710.txt
@@ -0,0 +1,90 @@
+Documentation for "sim710"
+
+1. Contributions
+2. General information
+3. Parameters
+4. Limitations
+
+
+1. Contributions
+
+ * MCA card detection code by Trent McNair.
+ * Fixes to not explicitly nul bss data from Xavier Bestel.
+ * Some multiboard fixes from Rolf Eike Beer.
+ * Auto probing of EISA config space from Trevor Hemsley.
+
+
+2. General information
+
+Various bits of code in this driver have been copied from 53c7,8xx,c,
+which is coyright Drew Eckhardt.  The scripts for the SCSI chip are
+compiled with the script compiler written by Drew.
+
+This is a simple driver for the NCR53c710.  More complex drivers
+for this chip (e.g. 53c7xx.c) require that the scsi chip be able to
+do DMA block moves between memory and on-chip registers, which can
+be a problem if those registers are in the I/O address space.  There
+can also be problems on hardware where the registers are memory
+mapped, if the design is such that memory-to-memory transfers initiated
+by the scsi chip cannot access the chip registers.
+
+This driver is designed to avoid these problems and is intended to
+work with any Intel machines using 53c710 chips, including various
+Compaq and NCR machines.  It was initially written for the Tadpole
+TP34V VME board which is 68030 based.
+
+
+3. Parameters
+
+The driver supports boot-time parameters similar to
+    sim710=addr:0x9000,irq:15
+and insmod parameters similar to
+    sim710="addr:0x9000 irq:15"
+
+Multiple controllers can also be set up by command line, provided the
+addr: parameter is specified first for each controller.  e.g.
+      sim710="addr:0x9000 irq:15 addr:0x8000 irq:14"
+
+To seperate the different options, ' ', '+', and ',' can be used, except
+that ',' can not be used in module parameters.  ' ' can be a pain, because
+it needs to be quoted, which causes problems with some installers.
+The command line above is completely equivalent to
+      sim710="addr:0x9000+irq:15+addr:0x8000+irq:14"
+
+The complete list of options are:
+
+ addr:0x9000		Specifies the base I/O port (or address) of the 53c710.
+ irq:15			Specifies the IRQ number used by the 53c710.
+ debug:0xffff		Generates lots of debug output. These are the constants
+			used to specify which type of debugging you want:
+			0x0000	Nothing
+			0x0001	Detailed trace of chip halt funtion
+			0x0002	All chip register read/writes
+			0x0004	Sync/async negotiation
+			0x0008	Phase mis-match handling
+			0x0010	General interrupt trace
+			0x0020	Selection timeouts
+			0x0040	Resume addresses for the script
+			0x0080	Commands and status returned
+			0x0100	Fixup of scsi addresses
+			0x0200	Disconnect/reselect handling
+			0x0400	spin_lock/spin_unlock debugging
+			0xffff	Any and all debug options
+ ignore:0x0a		Makes the driver ignore SCSI IDs 0 and 2. This ist just
+			the sum over ( 1 << SCSI ID )
+ nodisc:0x70		Prevents disconnects from IDs 6, 5 and 4.
+ noneg:0x10		Prevents SDTR negotiation on ID 4.
+ disabled:1		Completely disables the driver. When present, overrides
+			all other options.
+
+The driver will auto-probe chip addresses and IRQs now, so typically no
+parameters are needed.  Auto-probing of addresses is disabled if any addr:
+parameters are specified. The only option you can use if you want auto-probing
+is debug.
+
+
+4. Limitations
+
+ * Async only
+ * Severely lacking in error recovery
+ * 'debug:' should be per host really.

diff -Naur linux-2.5.59-vanilla/drivers/scsi/Kconfig linux-2.5.59-lacon/drivers/scsi/Kconfig
@@ -1383,8 +1383,8 @@
 	  avoid these problems and is intended to work with any Intel machines
 	  using 53c710 chips, including various Compaq and NCR machines.
 
-	  Please read the comments at the top of the file
-	  <file:drivers/scsi/sim710.c> for more information.
+	  Please read the file <file:Documentation/scsi/sim710.txt> for more
+	  information.
 
 	  If you want to compile this driver as a module ( = code which can be
 	  inserted in and removed from the running kernel whenever you want),

I don't think this is actually needed.

And once you're at it. please make all host_template methods static, remove the prototypes in sim710.h and initialize the driver template directly in sim710.c instead of the pre-2.4 header magic.

Actually, since you're showing an interest in this driver, I think the best thing to do would be to port it to use the 53c700 chip driver. Since the 53c700 already has all the sti/cli and DMA API issues sorted out already, the only thing that would need to be done is to convert sim710 to a template that speaks the new device model and initialises the chip fields.

This would also bring synchronous operation and tag command queueing virtually for free.

James

Hi again,

I'm now looking for the best way to port this to use 53c700.c. This will surely take a while, should I fix my fix and resend it or better leave this unfixed until I find time to port it?

For today this fix should be enough. The second one is unneeded because mca_register_driver does it.

Eike

Note: Please CC me, I'm not ...

@@ -1769,7 +1769,7 @@
        NCR_700_set_depth(SCp->device, NCR_700_get_depth(SCp->device) + 1);
 
        /* begin the command here */
-       /* no need to check for NULL, test for command_slot_cound above
+       /* no need to check for NULL, test for command_slot_count above
         * ensures a slot is free */
        slot = find_empty_slot(hostdata);
 

@@ -340,7 +340,6 @@
        .id_table = NCR_D700_id_table,
        .driver = {
                .name = "NCR_D700",
-               .bus = &mca_bus_type,
                .probe = NCR_D700_probe,
        },
 };

Do you actually have one of these (and is it EISA or MCA)?

I've got a MCA 710 in an old NCR3430 I can use. I think the conversion will only take a few days, I can try it out

It's a Compaq Prosignia with one onboard and the other one on an EISA card, using base addresses 0x8000 and 0x7000.

That would be fine, you know your code better than I ever will.

Eike

OK, the attached works for me for the 3430 MCA internal SCSI chip and an HP EISA SCSI board. Let me know how it works for the Compaq.

James

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.956 -> 1.957
# drivers/scsi/Makefile 1.34 -> 1.35
# drivers/scsi/sim710.c 1.6 -> 1.7
# drivers/scsi/Kconfig 1.9 -> 1.10
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/02/07 [email protected] 1.957
# Fix sim710 driver
#
# Make it use the 53c700 chip driver. This should give conversion

# to the DMA API, synchronous operation and tag command queueing
# --------------------------------------------
#
diff -Nru a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
--- a/drivers/scsi/Kconfig	Fri Feb  7 21:52:42 2003
+++ b/drivers/scsi/Kconfig	Fri Feb  7 21:52:42 2003
@@ -1372,24 +1372,16 @@
 # definitely looks note 64bit safe:
 config SCSI_SIM710
 	tristate "Simple 53c710 SCSI support (Compaq, NCR machines)"
-	depends on (ISA || MCA && !X86_64) && SCSI
+	depends on (EISA || MCA && !X86_64) && SCSI
 	---help---
-	  This is a simple driver for NCR53c710 based SCSI host adapters.
+	  This driver for NCR53c710 based SCSI host adapters.

-	  More complex drivers for this chip are available ("NCR53c7,8xx SCSI
-	  support", above), but they require that the scsi chip be able to do
-	  DMA block moves between memory and on-chip registers, which can
-	  cause problems under certain conditions.  This driver is designed to
-	  avoid these problems and is intended to work with any Intel machines
-	  using 53c710 chips, including various Compaq and NCR machines.
+	  It currently supports Compaq EISA cards and NCR MCA cards

- Please read the comments at the top of the file
- <file:drivers/scsi/sim710.c> for more information.
-

-	  If you want to compile this driver as a module ( = code which can be
-	  inserted in and removed from the running kernel whenever you want),
-	  say M here and read <file:Documentation/modules.txt>.  The module
-	  will be called sim710.o.

+config 53C700_IO_MAPPED
+ bool
+ depends on SCSI_SIM710
+ default y

config SCSI_SYM53C416
tristate "Symbios 53c416 SCSI support"

diff -Nru a/drivers/scsi/Makefile b/drivers/scsi/Makefile
@@ -45,7 +45,7 @@
 obj-$(CONFIG_SUN3_SCSI)		+= sun3_scsi.o  sun3_scsi_vme.o
 obj-$(CONFIG_MVME16x_SCSI)	+= mvme16x.o	53c7xx.o
 obj-$(CONFIG_BVME6000_SCSI)	+= bvme6000.o	53c7xx.o
-obj-$(CONFIG_SCSI_SIM710)	+= sim710.o
+obj-$(CONFIG_SCSI_SIM710)	+= sim710.o	53c700.o
 obj-$(CONFIG_SCSI_ADVANSYS)	+= advansys.o
 obj-$(CONFIG_SCSI_PCI2000)	+= pci2000.o
 obj-$(CONFIG_SCSI_PCI2220I)	+= pci2220i.o

Am Samstag, 8. Februar 2003 04:57 schrieben Sie:

This is from my boot.msg:

<6>EISA bus registered
<6>EISA: Probing bus...
<6>EISA: Motherboard CPQ0601 detected
<6>EISA: slot 2 : CPQ3011 detected.
<6>EISA: slot 3 : CPQ6100 detected.
<6>EISA: slot 7 : CPQ4411 detected.
<6>EISA: slot 8 : CPQ4410 detected.
<6>EISA: Detected 4 cards.

[...]

<5>IRQ_INDEX 249, 5
<4>sim710: irq nasty

This is the Compaq NCR 53c710 on an EISA board, address 0x7000, IRQ 14 (and it works fine with the original sim710).

<5>IRQ_INDEX 2, 5
<5>sim710: Compaq Integrated 32-Bit Fast-SCSI-2 Controller <5>sim710: irq = 15, clock = 50, base = 0x8000, scsi_id = 7 <5>53c700: Version 2.8 By [email protected] <5>scsi0: 53c710 rev 2
<6>scsi0 : LSI (Symbios) 710 MCA/EISA
<6>scsi0: (0:0) Synchronous at offset 8, period 100ns

<5>  Vendor: SEAGATE   Model: ST51080N          Rev: 0943
<5>  Type:   Direct-Access                      ANSI SCSI revision: 02
<6>scsi0: (1:0) Synchronous at offset 8, period 100ns
<5>  Vendor: SEAGATE   Model: ST51080N          Rev: 0943
<5>  Type:   Direct-Access                      ANSI SCSI revision: 02
<6>scsi0: (2:0) Synchronous at offset 8, period 100ns
<5>  Vendor: COMPAQ    Model: M2694ES-512       Rev: 952D
<5>  Type:   Direct-Access                      ANSI SCSI revision: 02
<6>scsi0: (3:0) Synchronous at offset 8, period 100ns
<5>  Vendor: COMPAQ    Model: M2694ES-512       Rev: 952D
<5>  Type:   Direct-Access                      ANSI SCSI revision: 02

<6>scsi0: (0:0) Enabling Tag Command Queuing

And this is the onboard NCR53c710. Works fine so far. Now we have to find a way to get the other board working, I want to use my external drive, too. I'll send some minor changes I made to the new code tomorrow or so.

Eike

Ah, OK, that's what I get for enabling something that was previously disabled. I suspected the disabled probe code had more than just the IRQ coded into it. It looks like only the three lower bits should be used to determine the IRQ. I believe it will work if you apply this patch.

James

===== drivers/scsi/sim710.c 1.8 vs edited =====

@@ -307,7 +307,7 @@
 #endif
 	} else {
 		eisa_irqs = eisa_cpq_irqs;
-		irq_index = inb(io_addr + 0xc88);
+		irq_index = inb(io_addr + 0xc88) & 0x07;
 	}
 
 	if(irq_index >= strlen(eisa_irqs)) {