Re: [PATCH] sym53c500_cs PCMCIA SCSI driver (new)

From: Bill Davidsen
Date: Thu Apr 22 2004 - 14:38:46 EST

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

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

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 (davidsen@xxxxxxx)
"The secret to procrastination is to put things off until the
last possible moment - but no longer" -me
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at