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]
-----------------------------------------------------------------------
On Fri, Apr 09, 2004 at 09:17:03PM -0500, Bob Tracy wrote:
> 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...
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
Christoph Hellwig wrote:
> I've given it a short spin and here's a bunch of comments:
Thank you for taking the time and trouble to review it.
> - the split into three source files is supserflous, one file should do 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?
> - 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
I caught that in the coding style guidelines (and in the mentioned
include files), and will fix for the next submission.
> - the driver doesn't even try to deal with multiple HBAs
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.
> - your detection logic could be streamlined a little, e.g. the request/release
> resource mess
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]
-----------------------------------------------------------------------
<crap>
Its me!
I'm the second person in our solar system that has such a card!
I do have the New Media *Basics SCSI* card! I remember back in the kernel 2.2
days that I had found that code, hacked it and used it.. Since then, I hadn't
tried to use it again. Actually, I lost (hd failure) the system together with
the patch I had for that code..
Anyway, I believe that support for such hw is another Linux success story and
reserves a sticker at major distros saying "Linux: Supports all hardware;
even New Media's SCSI cards!".
</crap>
I cleanly compiled and run your module. Looks OK. I haven't yet attached any
peripheral, though. It's of no use to me, but I will be glad to help you by
testing that code (w. devices as well).
Thanks for resurrecting the dead!
P. Christeas wrote:
> I'm the second person in our solar system that has such a card!
>
> I cleanly compiled and run your module. Looks OK. I haven't yet attached any
> peripheral, though. It's of no use to me, but I will be glad to help you by
> testing that code (w. devices as well).
> Thanks for resurrecting the dead!
Bravo! You've made my day :-). What kind of devices do you have
available? When I repair the #$%@! capstan on my Archive cartridge
tape unit, I need to test with that... The need for tape backup was
the reason I bought my card in the first place. At the moment, the
only device I've got that's even halfway convenient to use in testing
is an Olympus optical disk unit (removable media). The driver works
reliably with that, but I've got a sneaking suspicion that a tape unit
would stress things a bit more (error handling and such).
Per Christoph's critique, I've successfully combined the three driver
source files into a single source file, and eliminated all the
deprecated includes as well as the structure typedefs. Still to do:
add support for multiple HBAs, and eliminate distasteful hacks in
detection code. Dunno how I'm going to test the multiple HBA code...
--
-----------------------------------------------------------------------
Bob Tracy WTO + WIPO = DMCA? http://www.anti-dmca.org
[email protected]
-----------------------------------------------------------------------
On Fri, Apr 16, 2004 at 09:17:20AM -0500, Bob Tracy wrote:
> 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'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.
> > - the driver doesn't even try to deal with multiple HBAs
>
> 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.
It looks like nsp_cs at least tries to :-)
> > - your detection logic could be streamlined a little, e.g. the request/release
> > resource mess
>
> 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.
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.)
Christoph Hellwig wrote:
> - the driver doesn't even try to deal with multiple HBAs
>
> It looks like nsp_cs at least tries to :-)
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?
> 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.
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]
-----------------------------------------------------------------------
On Tue, Apr 20, 2004 at 11:11:11AM -0500, Bob Tracy wrote:
> 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?
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.
> +#include <linux/module.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/ioport.h>
> +#include <linux/proc_fs.h>
> +#include <linux/stat.h>
> +#include <asm/io.h>
> +#include <asm/dma.h>
> +#include <asm/bitops.h>
> +#include <asm/irq.h>
> +#include <linux/major.h>
> +#include <linux/blkdev.h>
> +#include <linux/spinlock.h>
Please try to include all <asm/*> headers after <linux/*>.
You should't need major.h and proc_fs.h at least.
> +#include <pcmcia/version.h>
A proper pcmcia driver shouldn't need this one.
> +/* A useful macro... */
> +#define MIN(a, b) ((a) > (b) ? (b) : (a))
The kernel already has min(), and it even does type-checking.
> +#ifndef NULL
> +#define NULL 0
> +#endif
Shouldn't be needed.
> +/* Function prototypes. */
> +const char *SYM53C500_info(struct Scsi_Host *);
> +int SYM53C500_queue(struct scsi_cmnd *, void (*done)(struct scsi_cmnd *));
> +int SYM53C500_abort(struct scsi_cmnd *);
> +int SYM53C500_device_reset(struct scsi_cmnd *);
> +int SYM53C500_bus_reset(struct scsi_cmnd *);
> +int SYM53C500_host_reset(struct scsi_cmnd *);
> +int SYM53C500_biosparm(struct scsi_device *, struct block_device *, sector_t, int *);
> +int SYM53C500_proc_info(struct Scsi_Host *, char *, char **, off_t, int, int);
Everything in a scsi driver should be static. Also try to avoid prototypes
for static functions whenever possible by ordering functions by their calling
chain.
> +#ifdef PCMCIA_DEBUG
> +static int pc_debug = PCMCIA_DEBUG;
> +MODULE_PARM(pc_debug, "i");
In 2.6 please use the type-chcking whizz-bang module_param instead of
MODULE_PARAM.
> +/*
> +* the following will set the monitor border color
> +* (useful to find where things crash or get stuck)
> +*
> +* 1 = blue
> +* 2 = green
> +* 3 = cyan
> +* 4 = red
> +* 5 = magenta
> +* 6 = yellow
> +* 7 = white
> +*/
> +
> +#if SYM53C500_DEBUG
> +#define rtrc(i) {inb(0x3da);outb(0x31,0x3c0);outb((i),0x3c0);}
> +#else
> +#define rtrc(i) {}
> +#endif
Do you actually use this debug code? It's rather pc-specific..
> +typedef struct scsi_info_t {
> + dev_link_t link;
> + dev_node_t node;
> + struct Scsi_Host *host;
> + unsigned short manf_id;
> +} scsi_info_t;
Try to avoid typedefs if possible according to the Linux Kernel coding
style. And yes, the pcmcia code isn't how it should be done, I know
you only copied it ;-)
> +static int port_base = 0;
> +static int irq_level = -1; /* 0 is 'no irq', so use -1 for 'uninitialized'*/
> +
> +static int fast_pio = USE_FAST_PIO;
> +
> +static struct scsi_cmnd *current_SC;
> +static char info_msg[256];
This is what I mean. To support more than one card these variables and
some more would have to be in a per-host struct.
> + scsi_add_host(host, NULL); /* XXX handle failure */
So handle the failure case :)
> + struct Scsi_Host *shost = info->host;
> +
> + DEBUG(0, "SYM53C500_release(0x%p)\n", link);
> +
> + /*
> + * Interrupts getting hosed on card removal. Try
> + * the following code, mostly from qlogicfas.c.
> + */
> + if (shost->irq)
> + free_irq(shost->irq, shost);
> + if (shost->dma_channel != 0xff)
> + free_dma(shost->dma_channel);
> + if (shost->io_port && shost->n_io_port)
> + release_region(shost->io_port, shost->n_io_port);
> +
> + scsi_remove_host(shost);
You need to call scsi_remove_host as the first thing in this function.
> +/*
> +* SYM53C500_proc_info is basically a stub function for now. It
> +* wouldn't exist except for the fact there were /proc entries for
> +* this driver under 2.4 and earlier kernels, and the author (sole
> +* user?) expects to see them.
Well, please just kill it. ->proc_info is deprecated in 2.6.
> +int
> +SYM53C500_device_reset(struct scsi_cmnd *SCpnt)
> +{
> + return FAILED;
> +}
> +
> +int
> +SYM53C500_bus_reset(struct scsi_cmnd *SCpnt)
> +{
> + return FAILED;
> +}
You don't need to implement these. If an EH method isn't implemented
FAILED is the default return value.
> +static irqreturn_t
> +do_SYM53C500_intr(int irq, void *dev_id, struct pt_regs *regs)
> +{
> + unsigned long flags;
> + struct Scsi_Host *dev = dev_id;
> +
> + spin_lock_irqsave(dev->host_lock, flags);
> + SYM53C500_intr(irq, dev_id, regs);
> + spin_unlock_irqrestore(dev->host_lock, flags);
> + return IRQ_HANDLED;
> +}
You should probably merged SYM53C500_intr with this one.
> + if (pio_status & 0x80) {
> + printk("SYM53C500: Warning: PIO error!\n");
> + current_SC->SCp.phase = idle;
> + current_SC->result = DID_ERROR << 16;
> + current_SC->scsi_done(current_SC);
> + return;
> + }
> +
> + if (status & 0x20) { /* Parity error */
> + printk("SYM53C500: Warning: parity error!\n");
> + current_SC->SCp.phase = idle;
> + current_SC->result = DID_PARITY << 16;
> + current_SC->scsi_done(current_SC);
> + return;
> + }
This screams for a goto error and the error handler in one place :)
> + /* Control Register Set 0 */
> + TC_LSB = (port_base + 0x00);
> + TC_MSB = (port_base + 0x01);
> + SCSI_FIFO = (port_base + 0x02);
These variables are _really_ strange. In a normal Linux driver
the names that are variables here (TC_LSB, etc..) would be defines
to the numberical values you add to port_base and when you're actually
using them you'd use shost->io_port + TC_LSB or whatever as the actual
address.
> +static void __exit
> +exit_sym53c500_cs(void)
> +{
> + pcmcia_unregister_driver(&sym53c500_cs_driver);
> +
> + /* XXX: this really needs to move into generic code... */
> + while (dev_list != NULL)
> + SYM53C500_detach(dev_list);
It's actually superflous for scsi drivers these days.
Bob Tracy wrote:
> Christoph Hellwig wrote:
>
>>I've given it a short spin and here's a bunch of comments:
>
>
> Thank you for taking the time and trouble to review it.
>
>
>> - the split into three source files is supserflous, one file should do 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?
>
>
>> - 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
>
>
> I caught that in the coding style guidelines (and in the mentioned
> include files), and will fix for the next submission.
>
>
>> - the driver doesn't even try to deal with multiple HBAs
>
>
> 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.
>
>
>> - your detection logic could be streamlined a little, e.g. the request/release
>> resource mess
>
>
> 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...
>
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
Bill Davidsen wrote:
> 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.
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]
-----------------------------------------------------------------------
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]
-----------------------------------------------------------------------
On Sat, Apr 24, 2004 at 10:01:54PM -0500, Bob Tracy wrote:
> 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.
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
Russell King wrote:
> 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 ?
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]
-----------------------------------------------------------------------
On Sun, Apr 25, 2004 at 04:26:34PM -0500, Bob Tracy wrote:
> Russell King wrote:
> > 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 ?
>
> 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?
When the user issues "cardctl 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.
>From the brief look I had, it didn't look like it - I suspect things
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.)
Russell King wrote:
> On Sun, Apr 25, 2004 at 04:26:34PM -0500, Bob Tracy wrote:
> > Russell King wrote:
> > > 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 ?
> >
> > 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?
>
> When the user issues "cardctl reset"
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.
> From the brief look I had, it didn't look like (the mid and upper
> driver layers were handling the "command in progress" issue) - I
> suspect things will go gaga if someone ever invoked "cardctl reset".
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]
-----------------------------------------------------------------------