sym53c500_cs PCMCIA SCSI driver (new)

The attached patch set implements a PCMCIA SCSI driver for host adapters based on the Symbios 53c500 chip. The original driver for this chip was written by Thomas Corner and available only as an add-on to the pcmcia-cs package. I've been maintaining the add-on driver on an infrequent basis for the past several years, and the release of the 2.6 kernel "forced" a long overdue update.

The only host adapter I'm aware of that uses the sym53c500 controller chip is the "new" version of the New Media Bus Toaster (circa 1996), and the attached driver has been tested using this particular adapter on a 2.6.4 kernel. The patch set applies cleanly to 2.6.4 and 2.6.5.

Comments / feedback / cheers / jeers accepted...

-----------------------------------------------------------------------
Bob Tracy                   WTO + WIPO = DMCA? http://www.anti-dmca.org
[email protected]
-----------------------------------------------------------------------

@@ -69,4 +69,14 @@
 	  To compile this driver as a module, choose M here: the
 	  module will be called qlogic_cs.
 
+config PCMCIA_SYM53C500
+	tristate "Symbios 53c500 PCMCIA support"
+	depends on m
+	help
+	  Say Y here if you have a New Media Bus Toaster or other PCMCIA
+	  SCSI adapter based on the Symbios 53c500 controller.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called sym53c500_cs.
+
 endmenu

@@ -6,7 +6,9 @@
 obj-$(CONFIG_PCMCIA_FDOMAIN)	+= fdomain_cs.o
 obj-$(CONFIG_PCMCIA_AHA152X)	+= aha152x_cs.o
 obj-$(CONFIG_PCMCIA_NINJA_SCSI)	+= nsp_cs.o
+obj-$(CONFIG_PCMCIA_SYM53C500)	+= sym53c500_cs.o
 
 aha152x_cs-objs	:= aha152x_stub.o aha152x_core.o
 fdomain_cs-objs	:= fdomain_stub.o fdomain_core.o
 qlogic_cs-objs	:= qlogic_stub.o qlogic_core.o
+sym53c500_cs-objs	:= sym53c500.o SYM53C500.o

@@ -0,0 +1,16 @@
+The sym53c500_cs driver originated as an add-on to David Hinds' pcmcia-cs
+package, and was written by Tom Corner ([email protected]).  This rewrite
+addresses the following concerns:
+
+	(1) extensive kernel changes between 2.4 and 2.6.
+	(2) deprecated PCMCIA support outside the kernel.
+
+The Symbios Logic 53c500 chip was used in the "new" (circa 1997) version
+of the New Media Bus Toaster PCMCIA SCSI controller.  I am unaware of any
+other products using this chip.
+
+Through the years, there have been a number of downloads of the pcmcia-cs
+version of this driver, and I guess it worked for those users.  It worked
+for Tom Corner, and it works for me.  Your mileage will probably vary.
+
+--Bob Tracy ([email protected])

@@ -62,6 +62,8 @@
 	- info on API between SCSI layer and low level drivers
 st.txt
 	- info on scsi tape driver
+sym53c500_cs.txt
+	- info on PCMCIA driver for Symbios Logic 53c500 based adapters
 sym53c8xx_2.txt
 	- info on second generation driver for sym53c8xx based adapters
 tmscsim.txt

I've given it a short spin and here's a bunch of comments:

- the split into three source files is supserflous, one file should do it
- please don't use host.h or scsi.h from drivers/scsi/. The defintions not present in include/scsi/ are deprecated and shall not be used (the most prominent example in your driver are the Scsi_<Foo> typedefs that have been replaced by struct scsi_foo
- the driver doesn't even try to deal with multiple HBAs
- your detection logic could be streamlined a little, e.g. the request/release resource mess

Thank you for taking the time and trouble to review it.

Given that the driver currently supports only PCMCIA implementations, I agree. My thinking was if someone comes up with a host adapter that isn't PCMCIA, the SYM53C500.c file is to the sym53c500_cs driver what the qlogicfas.c file is to the qlogic_cs driver, that is, core functions that could support multiple types of host adapters. The logic to handle the different types of adapters isn't there, and I don't know that it ever will be (else, it's probable that someone would have written the Linux driver long before now). However, after baring my ignorance to the world and saying I was unaware of non-PCMCIA implementations, I found a FreeBSD driver for the NCR 53c500. Never say "never," I guess... Your opinion counts for much, but you're the only person I've heard from. Is there a consensus I should forget about the non-PCMCIA cases?

I caught that in the coding style guidelines (and in the mentioned include files), and will fix for the next submission.

Guilty as charged. Functionally, there's nothing in the driver I submitted that wasn't in the original. Suggestions welcome... Which of the existing PCMCIA SCSI drivers do a proper job of handling multiple host adapters in your opinion? I'll try to adapt that code to fit this driver. If I have to "roll my own" from scratch, I'm probably in over my head.

I'll see what I can do.

Although I touched on it above, by way of apology/explanation, the goal for the initial port was to replicate the functionality I already had in older kernel versions. It appears I faithfully replicated the deficiencies of the old driver as well :-). Again, thank you for the feedback.

Anyone else have input before I act on the recommendations I've been given? Unless I hear otherwise, I'll start work on the code consolidation and removal of dependencies on deprecated include files. The detection logic and handling multiple HBAs will take a bit more effort...

-----------------------------------------------------------------------
Bob Tracy                   WTO + WIPO = DMCA? http://www.anti-dmca.org
[email protected]
-----------------------------------------------------------------------

I'd suggest to keep it as simple as you can for the time beeing. If we ever find a user with a ISA or whatever variant he can split it out. And such a split would work a little different from what you did now.

It looks like nsp_cs at least tries to :-)

Hey, you don't need to apologize. Anyt 2.6 driver is better than none and
your looks quite okay from the functional standpoint. We just need to have a little higher bars for new drivers as we already have lots of maintaince overhead for old and sloppy written drivers.

(Summary -- new version of sym53c500_cs driver attached.)

I'm not sure that nsp_cs does much more than assign a value to the "unique_id" member of the scsi_host structure (base port), and I probably missed what I was looking for, but I can certainly do as much :-). Is there any way to test whether the driver code can support multiple HBAs short of having more than one?

Thank you for the explanation: clearly I don't want to increase the maintenance burden on myself and/or anyone else.

Attached is the second attempt at a 2.6 PCMCIA driver for SCSI cards based on the Symbios 53c500 controller. The patch set was tested with (and applies cleanly to) 2.6.5. As requested, the driver source has been consolidated into a single file. All remnants of code that might have supported non-PCMCIA host adapters have been ripped out, which allowed for the requested streamlining of the detection logic: Card Services is doing most of the heavy lifting in that area anyway :-).

-----------------------------------------------------------------------
Bob Tracy                   WTO + WIPO = DMCA? http://www.anti-dmca.org
[email protected]
-----------------------------------------------------------------------

@@ -69,4 +69,14 @@
 	  To compile this driver as a module, choose M here: the
 	  module will be called qlogic_cs.
 
+config PCMCIA_SYM53C500
+	tristate "Symbios 53c500 PCMCIA support"
+	depends on m
+	help
+	  Say Y here if you have a New Media Bus Toaster or other PCMCIA
+	  SCSI adapter based on the Symbios 53c500 controller.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called sym53c500_cs.
+
 endmenu

@@ -6,6 +6,7 @@
 obj-$(CONFIG_PCMCIA_FDOMAIN)	+= fdomain_cs.o
 obj-$(CONFIG_PCMCIA_AHA152X)	+= aha152x_cs.o
 obj-$(CONFIG_PCMCIA_NINJA_SCSI)	+= nsp_cs.o
+obj-$(CONFIG_PCMCIA_SYM53C500)	+= sym53c500_cs.o
 
 aha152x_cs-objs	:= aha152x_stub.o aha152x_core.o
 fdomain_cs-objs	:= fdomain_stub.o fdomain_core.o

@@ -0,0 +1,17 @@
+The sym53c500_cs driver originated as an add-on to David Hinds' pcmcia-cs
+package, and was written by Tom Corner ([email protected]).  This rewrite
+addresses the following concerns:
+
+	(1) extensive kernel changes between 2.4 and 2.6.
+	(2) deprecated PCMCIA support outside the kernel.
+
+The Symbios Logic 53c500 chip was used in the "new" (circa 1997) version
+of the New Media Bus Toaster PCMCIA SCSI controller.  Presumably there are
+other products using this chip, but I've never laid eyes (much less hands)
+on one.
+
+Through the years, there have been a number of downloads of the pcmcia-cs
+version of this driver, and I guess it worked for those users.  It worked
+for Tom Corner, and it works for me.  Your mileage will probably vary.
+
+--Bob Tracy ([email protected])

@@ -62,6 +62,8 @@
 	- info on API between SCSI layer and low level drivers
 st.txt
 	- info on scsi tape driver
+sym53c500_cs.txt
+	- info on PCMCIA driver for Symbios Logic 53c500 based adapters
 sym53c8xx_2.txt
 	- info on second generation driver for sym53c8xx based adapters
 tmscsim.txt

There's not real test. But a good indication is that you have all almost no global variables (except some debug switches) and all I/O address / state / etc in per-host instance data.

A proper pcmcia driver shouldn't need this one.

Well, here's the latest attempt at a 2.6 driver for PCMCIA SCSI cards using the Symbios 53c500 controller. This includes (at least) "newer" versions of the New Media Bus Toaster (circa 1997). The attached patch set applies cleanly to 2.6.5.

In addition to all the concerns Christoph raised in response to the previous two submissions, there was an issue with the code for handling a CS_EVENT_CARD_RESET event: the event handler was calling the bus_reset function with a NULL scsi_cmnd argument. The potential problem was masked by the fact my bus_reset function was a trivial reimplementation of the default behavior in the absence of a bus_reset function. Folks using the qlogic_cs driver are hereby warned: if the event handler tries to process a CS_EVENT_CARD_RESET event (probably following a "resume"), qlogicfas_bus_reset() will be called with a NULL scsi_cmnd argument, and the next thing you'll see will be the "oops" resulting from a null pointer dereference. For what it's worth, someone besides me has noted this can happen: see the comment block above qlogicfas_bus_reset() in linux/drivers/scsi/qlogicfas.c.

Major differences between this version and prior ones:

(1) Support for multiple HBAs should now be in place. The only global host variable affects the PIO speed, and that's affected by a compile- time define.

(2) Although proc_info is officially obsolete in 2.6, I didn't delete the associated code: it can be enabled by defining PROC_INFO at compile time (for people like me who find it useful).

(3) Cleanup/consolidation of multiple functions, including deletion of code rendered superfluous by new 2.6 ways of doing things.

Barring any remaining issues (Christoph?), I think we're finally ready for prime time :-).

-----------------------------------------------------------------------
Bob Tracy                   WTO + WIPO = DMCA? http://www.anti-dmca.org
[email protected]
-----------------------------------------------------------------------

@@ -69,4 +69,14 @@
 	  To compile this driver as a module, choose M here: the
 	  module will be called qlogic_cs.
 
+config PCMCIA_SYM53C500
+	tristate "Symbios 53c500 PCMCIA support"
+	depends on m
+	help
+	  Say Y here if you have a New Media Bus Toaster or other PCMCIA
+	  SCSI adapter based on the Symbios 53c500 controller.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called sym53c500_cs.
+
 endmenu

@@ -6,6 +6,7 @@
 obj-$(CONFIG_PCMCIA_FDOMAIN)	+= fdomain_cs.o
 obj-$(CONFIG_PCMCIA_AHA152X)	+= aha152x_cs.o
 obj-$(CONFIG_PCMCIA_NINJA_SCSI)	+= nsp_cs.o
+obj-$(CONFIG_PCMCIA_SYM53C500)	+= sym53c500_cs.o
 
 aha152x_cs-objs	:= aha152x_stub.o aha152x_core.o
 fdomain_cs-objs	:= fdomain_stub.o fdomain_core.o

@@ -0,0 +1,23 @@
+The sym53c500_cs driver originated as an add-on to David Hinds' pcmcia-cs
+package, and was written by Tom Corner ([email protected]).  A rewrite was
+long overdue, and the current version addresses the following concerns:
+
+	(1) extensive kernel changes between 2.4 and 2.6.
+	(2) deprecated PCMCIA support outside the kernel.
+
+All the USE_BIOS code has been ripped out.  It was never used, and could
+not have worked anyway.  The USE_DMA code is likewise gone.  Many thanks
+to YOKOTA Hiroshi (nsp_cs driver) and David Hinds (qlogic_cs driver) for
+the code fragments I shamelessly adapted for this work.  Thanks also to
+Christoph Hellwig for his patient tutelage while I stumbled about.
+
+The Symbios Logic 53c500 chip was used in the "newer" (circa 1997) version
+of the New Media Bus Toaster PCMCIA SCSI controller.  Presumably there are
+other products using this chip, but I've never laid eyes (much less hands)
+on one.
+
+Through the years, there have been a number of downloads of the pcmcia-cs
+version of this driver, and I guess it worked for those users.  It worked
+for Tom Corner, and it works for me.  Your mileage will probably vary.
+
+--Bob Tracy ([email protected])

@@ -62,6 +62,8 @@
 	- info on API between SCSI layer and low level drivers
 st.txt
 	- info on scsi tape driver
+sym53c500_cs.txt
+	- info on PCMCIA driver for Symbios Logic 53c500 based adapters
 sym53c8xx_2.txt
 	- info on second generation driver for sym53c8xx based adapters
 tmscsim.txt

Hmm, so what happens if you're in the middle of a transaction, and you receive a CS_EVENT_CARD_RESET. What happens to the command in progress ?

Sure, the hardware is reset to a sane state, but what about the software state in the driver?

Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Candidly, I don't know. A fair question to ask in return is, under what circumstances might a PCMCIA driver see a CS_EVENT_CARD_RESET?

None of the existing PCMCIA SCSI drivers I saw do anything other than reset the hardware: evidently the assumption is there's no command in progress at that point, or we don't care. The nsp_cs driver toggles a stop flag in the per-instance data to indicate the host is accepting I/O: the flag is set to block I/O upon receipt of a suspend, physical reset, or card removal event. The card reset code in the nsp_cs driver, as in mine, is a subset of (fall-through case for) the resume logic.

Given the above, I'm tempted to believe the mid and/or upper driver layers are handling the "command in progress" issue, but I haven't delved into that code deeply enough to know.

-----------------------------------------------------------------------
Bob Tracy                   WTO + WIPO = DMCA? http://www.anti-dmca.org
[email protected]
-----------------------------------------------------------------------

When the user issues "cardctl reset"

will go gaga if someone ever invoked "cardctl reset".

Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

(Driver attempt 4 posted to linux-scsi a few minutes ago.)

Would it then be fair to characterize this action as either a self- inflicted gunshot wound or a desperate attempt to restore things to a sane state? I can see someone trying a reset if things got wedged in the middle of a critical transaction, and I guess the actions that could be taken by the low-level driver would properly depend on what can be assumed with respect to the mid and upper layers. In the case of the self-inflicted wound, the task at hand would be roughly equivalent to ensuring the integrity of a floppy diskette when the user ejects it from the drive in the middle of a write.

Ouch! I hate to be like the doctor that suggests if something hurts when you do it, then don't do it, but it sounds like the user is already hurting if he's reaching for the "card reset" hammer. For a sequential access device (magnetic tape and such), the user would probably end up having to restart the job anyway (read or write). For random access devices, I think the results would be minimally bad in the case of an interrupted read. If the user is in the middle of a write operation, it's probably fsck time (or enjoy your new coaster in the case of one-shot writable media). Mitigating factors? If things are truly wedged, the attached devices are probably in a pretty much quiescent state anyway, implying any damage caused by a reset would be minimal compared to what has already occurred.

Has this been discussed before? If so, I don't recall. Any ideas as far as what *should* be done (by the drivers at any level)? Resetting the host hardware is an obviously correct thing to do if there's anything to be done in response to the card reset event. The software state is a trickier matter: doing nothing might well be the best option.

-----------------------------------------------------------------------
Bob Tracy                   WTO + WIPO = DMCA? http://www.anti-dmca.org
[email protected]
-----------------------------------------------------------------------

there's still a bunch of minor issues. If you don't have them cleaned up until James' next push to Linux I'd say include the current variant and fix it up later, else I'd say wait for the next revision.

I don't think you need this one at all.

use linux/bitops.h instead.

you don't need to initialize this to NULL, the compiler will do for you.

Fourth attempt at a 2.6 driver for PCMCIA SCSI cards using the Symbios 53c500 controller is attached.

I compromised... PROC_INFO is set to 0 for submission. I intended that to be the default, but obviously *I* still use the stuff under /proc.

Gone.

Done.

Done. Two reasons it wasn't accomplished sooner: (1) initially, I didn't see a way to avoid at least one forward declaration/prototype; and (2) beauty being in the eye of the beholder, I frankly thought having the host template declaration in the middle of the code was uglier than using the prototypes to place the template declaration where it was. Item (2) became a non-issue for me when I came to the realization the template placement is more aesthetically pleasing *without* the prototypes than it was *with* the prototypes :-).

<Homer_Simpson>Doh!</Homer_Simpson> Done.

Nod. Also eliminated the tpnt->can_queue assignment statements. In the case of irq_level > 0, the template initializer has already set that member to 1. Setting it to zero in the case of irq_level == 0 was a waste of object code bytes because we were on the error return path at that point anyway.

Sorry... Missed that the last go-round.

Done.

Anything else? There's a dangling "unknown" with respect to what the low-level driver should do upon receipt of CS_EVENT_CARD_RESET: Russell King asked what happens to the command in progress. Based on what I think I saw in other PCMCIA SCSI drivers, should I care? Right now, I do pretty much what the other drivers do -- reset the host instance.

-----------------------------------------------------------------------
Bob Tracy                   WTO + WIPO = DMCA? http://www.anti-dmca.org
[email protected]
-----------------------------------------------------------------------

@@ -69,4 +69,14 @@
 	  To compile this driver as a module, choose M here: the
 	  module will be called qlogic_cs.
 
+config PCMCIA_SYM53C500
+	tristate "Symbios 53c500 PCMCIA support"
+	depends on m
+	help
+	  Say Y here if you have a New Media Bus Toaster or other PCMCIA
+	  SCSI adapter based on the Symbios 53c500 controller.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called sym53c500_cs.
+
 endmenu

@@ -6,6 +6,7 @@
 obj-$(CONFIG_PCMCIA_FDOMAIN)	+= fdomain_cs.o
 obj-$(CONFIG_PCMCIA_AHA152X)	+= aha152x_cs.o
 obj-$(CONFIG_PCMCIA_NINJA_SCSI)	+= nsp_cs.o
+obj-$(CONFIG_PCMCIA_SYM53C500)	+= sym53c500_cs.o
 
 aha152x_cs-objs	:= aha152x_stub.o aha152x_core.o
 fdomain_cs-objs	:= fdomain_stub.o fdomain_core.o

@@ -0,0 +1,23 @@
+The sym53c500_cs driver originated as an add-on to David Hinds' pcmcia-cs
+package, and was written by Tom Corner ([email protected]).  A rewrite was
+long overdue, and the current version addresses the following concerns:
+
+	(1) extensive kernel changes between 2.4 and 2.6.
+	(2) deprecated PCMCIA support outside the kernel.
+
+All the USE_BIOS code has been ripped out.  It was never used, and could
+not have worked anyway.  The USE_DMA code is likewise gone.  Many thanks
+to YOKOTA Hiroshi (nsp_cs driver) and David Hinds (qlogic_cs driver) for
+the code fragments I shamelessly adapted for this work.  Thanks also to
+Christoph Hellwig for his patient tutelage while I stumbled about.
+
+The Symbios Logic 53c500 chip was used in the "newer" (circa 1997) version
+of the New Media Bus Toaster PCMCIA SCSI controller.  Presumably there are
+other products using this chip, but I've never laid eyes (much less hands)
+on one.
+
+Through the years, there have been a number of downloads of the pcmcia-cs
+version of this driver, and I guess it worked for those users.  It worked
+for Tom Corner, and it works for me.  Your mileage will probably vary.
+
+--Bob Tracy ([email protected])

@@ -62,6 +62,8 @@
 	- info on API between SCSI layer and low level drivers
 st.txt
 	- info on scsi tape driver
+sym53c500_cs.txt
+	- info on PCMCIA driver for Symbios Logic 53c500 based adapters
 sym53c8xx_2.txt
 	- info on second generation driver for sym53c8xx based adapters
 tmscsim.txt

WRT the split of code... if there is some reason why there will never be another type of card then the split is unnessessary. But otherwise, you've done the work, and it matches the way other drivers were split, so why scrap it?

As you guessed I don't feel strongly one way or the other, just thought you could use a little support for having made the effort to design for the future.

   -bill davidsen ([email protected])
"The secret to procrastination is to put things off until the
 last possible moment - but no longer"  -me

You give me too much credit :-). I inherited the split design from the original author and simply preserved it. As far as maintaining the split design, the only evidence I've seen of non-PCMCIA cards using the Symbios/NCR 53c500 controller is the existence of a driver in the FreeBSD tree. In any event, I don't have the hardware to test anything other than the PCMCIA case, and if there's someone out there who needs support for the non-PCMCIA case, I expect the Linux driver would have been written before now. Christoph suggested in an earlier posting that the split to support other hardware would probably be accomplished differently these days anyway, so we (the community) probably haven't lost all that much by concentrating on producing a well-written PCMCIA-only driver.

I saved my first attempt at a 2.6 driver (in case someone wants to revisit the issue). Otherwise, although I appreciate your support, it's OBE. I'm waiting to hear back from Christoph on a few design issues (questions asked off-line) before submitting a third 2.6 driver.

The main issue I'm wrestling with at the moment is support for the improbable case of multiple HBAs... I agree in principle with the objective as long as I don't do something stupid that kills performance for the normal case of a single HBA. As I see it, there are essentially two approaches:

(1) Calculate all the needed hardware register offsets up front at HBA init time, and carry them as part of the per-instance data. This approach has a few things going for it -- the in-core memory requirement is essentially identical for the single HBA case, and dereferencing a structure pointer to get a register address is barely worse than accessing the equivalent global variable in the driver.

(2) Calculate a particular register offset each time that register is accessed, using something like shost->io_port + offset, which, it is claimed, is normal Linux driver practice. Although this approach is extremely attractive from the standpoint of code simplicity, I'm concerned about the arithmetic overhead. If it turns out most of the hardware register accesses occur during initialization, my concerns are pretty well meaningless. Obviously I need to look at this more closely.

You may safely assume I'm learning more about this than I originally anticipated :-).

-----------------------------------------------------------------------
Bob Tracy                   WTO + WIPO = DMCA? http://www.anti-dmca.org
[email protected]
-----------------------------------------------------------------------