Skip to contentSkip to conversation

About ARECA RAID driver for Linux i386/x86-64

DISCUSSION Andrew Morton 3 participants 3 messages Feb 3, 2005 – Feb 6, 2005 1 message not in the archive SCSI

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 001001c508d9$d97a17f0$9300a8c0@erich2003

#1 lore raw
Quoted 4 lines · erich

I have contact with Andrew Morton about ARECA RAID Linux scsi driver release
issue.
I hope this package is as look like a Linux driver.

No, it doesn't look anything like a Linux driver :(

I fed the patch through scripts/Lindent. There's a copy at
http://www.zip.com.au/~akpm/linux/patches/stuff/areca-raid-linux-scsi-driver.patch.
I'll include it -mm kernels.

Quoted 1 line

From a very quick look, I'd say that the driver needs quite a bit of work:

- Remove Documentation/scsi/arcmsr.txt - none of it is relevant

- Use named structure initialisers:

	struct foo bar = {
		.name1 = value1,
		.name2 = value2,
	};

- Remove all the LINUX_VERSION_CODE tests

- The kernel thread stopping/starting stuff is old fashioned. Use the kthread API.

- I suspect it is using old-fashioned SCSI APIs too.

#2 lore raw
Quoted 6 lines · Andrew Morton
Quoted 4 lines · erich

I have contact with Andrew Morton about ARECA RAID Linux scsi driver release
issue.
I hope this package is as look like a Linux driver.

Quoted 7 lines

No, it doesn't look anything like a Linux driver :(

I fed the patch through scripts/Lindent. There's a copy at
http://www.zip.com.au/~akpm/linux/patches/stuff/areca-raid-linux-scsi-driver.patch.
I'll include it -mm kernels.

Quoted 1 line

From a very quick look, I'd say that the driver needs quite a bit of work:

It is nice to see vendors submitting drivers for their hardware; this one does need some more work though to look like a linux driver

more work items:
* needs to use list.h lists not open coded ones
* uses wrong locking for the scsi layer (eg ->done should be called without locks)
* ULONG and simliar typedefs.. does this driver come from windows ? typedef uint32_t ULONG, *PULONG;
is just not a good idea.
* lots and lots of ifdefs that are done wrongly; eg if you need to do version ifdefs.. do them right ;(
example:
arcmsr_doInterrupt()
is ifdefd needlessly/wrongly

#3 lore raw
Quoted 1 line · Arjan van de Ven

* needs to use list.h lists not open coded ones

the only list uses I saw in a quick scan were for internal queueing which should go away. (see the tmscsim driver in 2.6 on what to return when the queue is full)

Quoted 2 lines

* uses wrong locking for the scsi layer (eg ->done should be called without locks)

it's not really wrong but totally superflous, so yes, it should go away.