Re: About ARECA RAID driver for Linux i386/x86-64

Re: About ARECA RAID driver for Linux i386/x86-64

[erich]

Hi,Andrew Morton

Thanks for your doing.
ARECA Linux RAID driver need to support old linux kerenel version.
There are a lot of  NAS RAID STORAGE SYSTEM development worker still use it
at old linux kernel.
Maybe I need to released one package that as cleanly as look like a Linux
driver :) modern times :( .

Best Regards
Erich Chen

----- Original Message ----- 
From: "Andrew Morton" <akpm@osdl.org>
To: "erich" <erich@areca.com.tw>
Cc: <linux-scsi@vger.kernel.org>
Sent: Thursday, February 03, 2005 10:08 AM
Subject: Re: About ARECA RAID driver for Linux i386/x86-64


> "erich" <erich@areca.com.tw> wrote:
> >
> > 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.
>
> 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.
>
>

Re: About ARECA RAID driver for Linux i386/x86-64

[Andrew Morton]

"erich" <erich@areca.com.tw> wrote:
>
> Thanks for your doing.
>  ARECA Linux RAID driver need to support old linux kerenel version.
>  There are a lot of  NAS RAID STORAGE SYSTEM development worker still use it
>  at old linux kernel.

Well that's a decision which the scsi maintainers will need to make.  Lots
of current drivers use LINUX_VERSION_CODE, even though we'd prefer they not
do so.  I don't know what the scsi policy is for new submissions.

Re: About ARECA RAID driver for Linux i386/x86-64

[James Bottomley]

On Wed, 2005-02-02 at 20:51 -0800, Andrew Morton wrote:
> Well that's a decision which the scsi maintainers will need to make.  Lots
> of current drivers use LINUX_VERSION_CODE, even though we'd prefer they not
> do so.  I don't know what the scsi policy is for new submissions.

Hey ... I have to have a policy now?

The policy generally is that new submissions need to use the latest
versions of the API.  If the driver needs to work on 2.4, it should use
the 2.4 compat layer for 2.6 drivers (and if there's a problem or
something missing, we can update 2.4).

James

Re: About ARECA RAID driver for Linux i386/x86-64

[Christoph Hellwig]

On Thu, Feb 03, 2005 at 12:33:37PM +0800, erich wrote:
> Hi,Andrew Morton
> 
> Thanks for your doing.
> ARECA Linux RAID driver need to support old linux kerenel version.
> There are a lot of  NAS RAID STORAGE SYSTEM development worker still use it
> at old linux kernel.
> Maybe I need to released one package that as cleanly as look like a Linux
> driver :) modern times :( .

If you write two proper drivers for 2.4.x and 2.6.x they're probably
less size than your current driver.

Re: About ARECA RAID driver for Linux i386/x86-64

[Andrew Morton]

"erich" <erich@areca.com.tw> wrote:
>
> Thanks for your kindness comment.
> I had modify "arcmsr" as your mention and ran it at 2.6.11 kernel.

A few points on this patch:

a) Please don't use full pathnames in the patch headers:

--- /usr/src/linux-2.6.11/drivers/scsi/Makefile.org	2005-03-11 10:20:49.000000000 +0800
+++ /usr/src/linux-2.6.11/drivers/scsi/Makefile	2005-03-11 11:32:07.000000000 +0800

   Because not everyone places their kernel tree in /usr/src/linux-2.6.11.
   See http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt

b) Please trim off all trailing whitespace.  Here's a script which you
   run against a patch to prevent it from introducing new trailing
   whitespace.  (I swear that this script has shrunk the kernel tree by a
   megabyte over the past few years)

#!/bin/sh

strip1()
{
	TMP=$(mktemp /tmp/XXXXXX)
	cp $1 $TMP
	sed -e '/^+/s/[ 	]*$//' < $TMP > $1
	rm $TMP
}

for i in $*
do
	strip1 $i
done

c) You cannot use absolute pathnames for include files.  Here's a fix:



Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 25-akpm/drivers/scsi/arcmsr/arcmsr.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff -puN drivers/scsi/arcmsr/arcmsr.c~areca-raid-linux-scsi-driver-fix drivers/scsi/arcmsr/arcmsr.c
--- 25/drivers/scsi/arcmsr/arcmsr.c~areca-raid-linux-scsi-driver-fix	Fri Mar 11 14:22:23 2005
+++ 25-akpm/drivers/scsi/arcmsr/arcmsr.c	Fri Mar 11 14:22:23 2005
@@ -107,8 +107,8 @@
 #include <linux/sched.h>
 #include <linux/init.h>
 #include <linux/spinlock.h>
-#include "/usr/src/linux/drivers/scsi/scsi.h"
-#include "/usr/src/linux/include/scsi/scsi_host.h"
+#include "../scsi.h"
+#include <scsi/scsi_host.h>
 
 #include "arcmsr.h"
 #endif
_

Re: About ARECA RAID driver for Linux i386/x86-64

[Andrew Morton]

"erich" <erich@areca.com.tw> wrote:
>
> I had modify "arcmsr" as your mention and ran it at 2.6.11 kernel.
> It looks more clean and safety in pccb memory allocation.

Oh dear.  Adrian wrote a patch which cleans up a lot of things in this
driver but you don't seem to have applied it, and none of it works any
more.

Please integrate these changes into this driver:


From: Adrian Bunk <bunk@stusta.de>

- make needlessly global functions static
- move arcmsr_scsi_host_template from arcmsr.h to arcmsr.c
  (code doesn't belong into header files)

Signed-off-by: Adrian Bunk <bunk@stusta.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 25-akpm/drivers/scsi/arcmsr/arcmsr.c |  110 +++++++++++++++++++++++------------
 25-akpm/drivers/scsi/arcmsr/arcmsr.h |   34 ----------
 2 files changed, 73 insertions(+), 71 deletions(-)

diff -puN drivers/scsi/arcmsr/arcmsr.c~drivers-scsi-arcmsr-arcmsrc-cleanups drivers/scsi/arcmsr/arcmsr.c
--- 25/drivers/scsi/arcmsr/arcmsr.c~drivers-scsi-arcmsr-arcmsrc-cleanups	2005-02-28 15:42:00.000000000 -0800
+++ 25-akpm/drivers/scsi/arcmsr/arcmsr.c	2005-02-28 15:42:00.000000000 -0800
@@ -121,15 +121,26 @@ static int arcmsr_fops_open(struct inode
 static int arcmsr_halt_notify(struct notifier_block *nb, unsigned long event,
 			      void *buf);
 static void arcmsr_free_pci_pool(PACB pACB);
-void arcmsr_pcidev_disattach(PACB pACB);
-BOOLEAN arcmsr_wait_msgint_ready(PACB pACB);
-void arcmsr_iop_init(PACB pACB);
-int arcmsr_iop_ioctlcmd(PACB pACB, int ioctl_cmd, void *arg);
-int arcmsr_initialize(PACB pACB, struct pci_dev *pPCI_DEV);
+static void arcmsr_pcidev_disattach(PACB pACB);
+static void arcmsr_iop_init(PACB pACB);
+static int arcmsr_iop_ioctlcmd(PACB pACB, int ioctl_cmd, void *arg);
+static int arcmsr_initialize(PACB pACB, struct pci_dev *pPCI_DEV);
 static irqreturn_t arcmsr_HwInterrupt(PACB pACB);
 static int arcmsr_device_probe(struct pci_dev *pPCI_DEV,
 			       const struct pci_device_id *id);
 static void arcmsr_device_remove(struct pci_dev *pPCI_DEV);
+static int arcmsr_bios_param(struct scsi_device *sdev,
+			     struct block_device *bdev,
+			     sector_t capacity, int *geom);
+static int arcmsr_bus_reset(Scsi_Cmnd * cmd);
+static int arcmsr_cmd_abort(Scsi_Cmnd * cmd);
+static const char *arcmsr_info(struct Scsi_Host *host);
+static int arcmsr_ioctl(Scsi_Device * dev, int ioctl_cmd, void *arg);
+static int arcmsr_proc_info(struct Scsi_Host *host, char *buffer, char **start,
+			    off_t offset, int length, int inout);
+static int arcmsr_queue_command(Scsi_Cmnd * cmd, void (*done) (Scsi_Cmnd *));
+static int arcmsr_release(struct Scsi_Host *host);
+
 /*
 **********************************************************************************
 **
@@ -169,13 +180,37 @@ static struct pci_device_id arcmsr_devic
 };
 
 MODULE_DEVICE_TABLE(pci, arcmsr_device_id_table);
-struct pci_driver arcmsr_pci_driver = {
+static struct pci_driver arcmsr_pci_driver = {
 	.name = "arcmsr",
 	.id_table = arcmsr_device_id_table,
 	.probe = arcmsr_device_probe,
 	.remove = arcmsr_device_remove,
 };
 
+static Scsi_Host_Template arcmsr_scsi_host_template = {
+	.module = THIS_MODULE,
+	.proc_name = "arcmsr",
+	.proc_info = arcmsr_proc_info,
+	.name = "ARCMSR ARECA SATA RAID HOST Adapter" ARCMSR_DRIVER_VERSION,	/* *name */
+	.release = arcmsr_release,
+	.info = arcmsr_info,
+	.ioctl = arcmsr_ioctl,
+	.queuecommand = arcmsr_queue_command,
+	.eh_strategy_handler = NULL,
+	.eh_abort_handler = arcmsr_cmd_abort,
+	.eh_device_reset_handler = NULL,
+	.eh_bus_reset_handler = arcmsr_bus_reset,
+	.eh_host_reset_handler = NULL,
+	.bios_param = arcmsr_bios_param,
+	.can_queue = ARCMSR_MAX_OUTSTANDING_CMD,
+	.this_id = ARCMSR_SCSI_INITIATOR_ID,
+	.sg_tablesize = ARCMSR_MAX_SG_ENTRIES,
+	.max_sectors = ARCMSR_MAX_XFER_SECTORS,
+	.cmd_per_lun = ARCMSR_MAX_CMD_PERLUN,
+	.unchecked_isa_dma = 0,
+	.use_clustering = DISABLE_CLUSTERING,
+};
+
 /*
 *********************************************************************
 *********************************************************************
@@ -213,8 +248,9 @@ static irqreturn_t arcmsr_doInterrupt(in
 *********************************************************************
 *********************************************************************
 */
-int arcmsr_bios_param(struct scsi_device *sdev, struct block_device *bdev,
-		      sector_t capacity, int *geom)
+static int arcmsr_bios_param(struct scsi_device *sdev,
+			     struct block_device *bdev,
+			     sector_t capacity, int *geom)
 {
 	int heads, sectors, cylinders, total_capacity;
 
@@ -409,7 +445,7 @@ module_exit(arcmsr_module_exit);
 **
 **********************************************************************
 */
-void arcmsr_pci_unmap_dma(PCCB pCCB)
+static void arcmsr_pci_unmap_dma(PCCB pCCB)
 {
 	PACB pACB = pCCB->pACB;
 	Scsi_Cmnd *pcmd = pCCB->pcmd;
@@ -528,7 +564,7 @@ static int arcmsr_fops_ioctl(struct inod
 **
 **********************************************************************
 */
-void arcmsr_cmd_done(Scsi_Cmnd * pcmd)
+static void arcmsr_cmd_done(Scsi_Cmnd * pcmd)
 {
 	pcmd->scsi_done(pcmd);
 	return;
@@ -540,7 +576,7 @@ void arcmsr_cmd_done(Scsi_Cmnd * pcmd)
 **
 ************************************************************************
 */
-void arcmsr_flush_adapter_cache(PACB pACB)
+static void arcmsr_flush_adapter_cache(PACB pACB)
 {
 #if ARCMSR_DEBUG0
 	printk("arcmsr_flush_adapter_cache..............\n");
@@ -557,7 +593,7 @@ void arcmsr_flush_adapter_cache(PACB pAC
 **
 **********************************************************************
 */
-void arcmsr_ccb_complete(PCCB pCCB)
+static void arcmsr_ccb_complete(PCCB pCCB)
 {
 	unsigned long flag;
 	PACB pACB = pCCB->pACB;
@@ -586,7 +622,7 @@ void arcmsr_ccb_complete(PCCB pCCB)
 **       if scsi error do auto request sense
 **********************************************************************
 */
-void arcmsr_report_SenseInfoBuffer(PCCB pCCB)
+static void arcmsr_report_SenseInfoBuffer(PCCB pCCB)
 {
 	Scsi_Cmnd *pcmd = pCCB->pcmd;
 	PSENSE_DATA psenseBuffer = (PSENSE_DATA) pcmd->sense_buffer;
@@ -611,7 +647,7 @@ void arcmsr_report_SenseInfoBuffer(PCCB 
 ** to insert pCCB into tail of pACB wait exec ccbQ
 *********************************************************************
 */
-void arcmsr_queue_wait2go_ccb(PACB pACB, PCCB pCCB)
+static void arcmsr_queue_wait2go_ccb(PACB pACB, PCCB pCCB)
 {
 	unsigned long flag;
 	int i = 0;
@@ -639,7 +675,7 @@ void arcmsr_queue_wait2go_ccb(PACB pACB,
 **
 *********************************************************************
 */
-void arcmsr_abort_allcmd(PACB pACB)
+static void arcmsr_abort_allcmd(PACB pACB)
 {
 	CHIP_REG_WRITE32(&pACB->pmu->inbound_msgaddr0,
 			 ARCMSR_INBOUND_MESG0_ABORT_CMD);
@@ -653,7 +689,7 @@ void arcmsr_abort_allcmd(PACB pACB)
 **
 **********************************************************************
 */
-BOOLEAN arcmsr_wait_msgint_ready(PACB pACB)
+static BOOLEAN arcmsr_wait_msgint_ready(PACB pACB)
 {
 	uint32_t Index;
 	uint8_t Retries = 0x00;
@@ -678,7 +714,7 @@ BOOLEAN arcmsr_wait_msgint_ready(PACB pA
 **        Return Value: Nothing.
 ****************************************************************************
 */
-void arcmsr_iop_reset(PACB pACB)
+static void arcmsr_iop_reset(PACB pACB)
 {
 	PCCB pCCB;
 	uint32_t intmask_org, mask;
@@ -760,7 +796,7 @@ void arcmsr_iop_reset(PACB pACB)
 ** PAGE_SIZE=4096 or 8192,PAGE_SHIFT=12
 **********************************************************************
 */
-void arcmsr_build_ccb(PACB pACB, PCCB pCCB, Scsi_Cmnd * pcmd)
+static void arcmsr_build_ccb(PACB pACB, PCCB pCCB, Scsi_Cmnd * pcmd)
 {
 	PARCMSR_CDB pARCMSR_CDB = (PARCMSR_CDB) & pCCB->arcmsr_cdb;
 	int8_t *psge = (int8_t *) & pARCMSR_CDB->u;
@@ -931,7 +967,7 @@ static void arcmsr_post_ccb(PACB pACB, P
 **
 **************************************************************************
 */
-void arcmsr_post_wait2go_ccb(PACB pACB)
+static void arcmsr_post_wait2go_ccb(PACB pACB)
 {
 	unsigned long flag;
 	PCCB pCCB;
@@ -965,7 +1001,7 @@ void arcmsr_post_wait2go_ccb(PACB pACB)
 **     Output:
 **********************************************************************
 */
-void arcmsr_post_Qbuffer(PACB pACB)
+static void arcmsr_post_Qbuffer(PACB pACB)
 {
 	uint8_t *pQbuffer;
 	PQBUFFER pwbuffer = (PQBUFFER) & pACB->pmu->ioctl_wbuffer;
@@ -996,7 +1032,7 @@ void arcmsr_post_Qbuffer(PACB pACB)
 **
 ************************************************************************
 */
-void arcmsr_stop_adapter_bgrb(PACB pACB)
+static void arcmsr_stop_adapter_bgrb(PACB pACB)
 {
 #if ARCMSR_DEBUG0
 	printk("arcmsr_stop_adapter_bgrb..............\n");
@@ -1271,7 +1307,7 @@ static irqreturn_t arcmsr_HwInterrupt(PA
 **
 ************************************************************************
 */
-int arcmsr_iop_ioctlcmd(PACB pACB, int ioctl_cmd, void *arg)
+static int arcmsr_iop_ioctlcmd(PACB pACB, int ioctl_cmd, void *arg)
 {
 	CMD_IOCTL_FIELD cmdioctlfld;
 	PCMD_IOCTL_FIELD pcmdioctlfld = &cmdioctlfld;
@@ -1617,7 +1653,7 @@ int arcmsr_iop_ioctlcmd(PACB pACB, int i
 **
 ************************************************************************
 */
-int arcmsr_ioctl(Scsi_Device * dev, int ioctl_cmd, void *arg)
+static int arcmsr_ioctl(Scsi_Device * dev, int ioctl_cmd, void *arg)
 {
 	PACB pACB;
 	int32_t match = 0x55AA, i;
@@ -1649,7 +1685,7 @@ int arcmsr_ioctl(Scsi_Device * dev, int 
 **
 **************************************************************************
 */
-PCCB arcmsr_get_freeccb(PACB pACB)
+static PCCB arcmsr_get_freeccb(PACB pACB)
 {
 	PCCB pCCB;
 	unsigned long flag;
@@ -1821,7 +1857,7 @@ PCCB arcmsr_get_freeccb(PACB pACB)
 **} Scsi_Pointer;
 ***********************************************************************
 */
-int arcmsr_queue_command(Scsi_Cmnd * cmd, void (*done) (Scsi_Cmnd *))
+static int arcmsr_queue_command(Scsi_Cmnd * cmd, void (*done) (Scsi_Cmnd *))
 {
 	struct Scsi_Host *host = cmd->device->host;
 	PACB pACB = (PACB) host->hostdata;
@@ -1872,7 +1908,7 @@ int arcmsr_queue_command(Scsi_Cmnd * cmd
 **
 **********************************************************************
 */
-void arcmsr_start_adapter_bgrb(PACB pACB)
+static void arcmsr_start_adapter_bgrb(PACB pACB)
 {
 #if ARCMSR_DEBUG0
 	printk
@@ -1892,7 +1928,7 @@ void arcmsr_start_adapter_bgrb(PACB pACB
 **
 **********************************************************************
 */
-void arcmsr_iop_init(PACB pACB)
+static void arcmsr_iop_init(PACB pACB)
 {
 	uint32_t intmask_org, mask, outbound_doorbell, firmware_state = 0;
 
@@ -1932,7 +1968,7 @@ void arcmsr_iop_init(PACB pACB)
 **
 ****************************************************************************
 */
-int arcmsr_bus_reset(Scsi_Cmnd * cmd)
+static int arcmsr_bus_reset(Scsi_Cmnd * cmd)
 {
 	PACB pACB;
 
@@ -1949,7 +1985,7 @@ int arcmsr_bus_reset(Scsi_Cmnd * cmd)
 **
 *****************************************************************************************
 */
-int arcmsr_seek_cmd2abort(Scsi_Cmnd * pabortcmd)
+static int arcmsr_seek_cmd2abort(Scsi_Cmnd * pabortcmd)
 {
 	PACB pACB = (PACB) pabortcmd->device->host->hostdata;
 	PCCB pCCB;
@@ -2048,7 +2084,7 @@ int arcmsr_seek_cmd2abort(Scsi_Cmnd * pa
 **
 *****************************************************************************************
 */
-int arcmsr_cmd_abort(Scsi_Cmnd * cmd)
+static int arcmsr_cmd_abort(Scsi_Cmnd * cmd)
 {
 	int error;
 
@@ -2112,7 +2148,7 @@ int arcmsr_cmd_abort(Scsi_Cmnd * cmd)
 **
 *********************************************************************
 */
-const char *arcmsr_info(struct Scsi_Host *host)
+static const char *arcmsr_info(struct Scsi_Host *host)
 {
 	static char buf[256];
 	PACB pACB;
@@ -2212,7 +2248,7 @@ const char *arcmsr_info(struct Scsi_Host
 **}
 ************************************************************************
 */
-int arcmsr_initialize(PACB pACB, struct pci_dev *pPCI_DEV)
+static int arcmsr_initialize(PACB pACB, struct pci_dev *pPCI_DEV)
 {
 	uint32_t intmask_org, page_base, page_offset, mem_base_start;
 	dma_addr_t dma_addr;
@@ -2339,7 +2375,7 @@ int arcmsr_initialize(PACB pACB, struct 
 *********************************************************************
 *********************************************************************
 */
-int arcmsr_set_info(char *buffer, int length)
+static int arcmsr_set_info(char *buffer, int length)
 {
 #if ARCMSR_DEBUG0
 	printk("arcmsr_set_info.............\n");
@@ -2352,7 +2388,7 @@ int arcmsr_set_info(char *buffer, int le
 **
 *********************************************************************
 */
-void arcmsr_pcidev_disattach(PACB pACB)
+static void arcmsr_pcidev_disattach(PACB pACB)
 {
 	PCCB pCCB;
 	uint32_t intmask_org, mask;
@@ -2489,8 +2525,8 @@ static int arcmsr_halt_notify(struct not
 if(YN) SPRINTF(" Yes ");\
 else SPRINTF(" No ")
 
-int arcmsr_proc_info(struct Scsi_Host *host, char *buffer, char **start,
-		     off_t offset, int length, int inout)
+static int arcmsr_proc_info(struct Scsi_Host *host, char *buffer, char **start,
+			    off_t offset, int length, int inout)
 {
 	uint8_t i;
 	char *pos = buffer;
@@ -2528,7 +2564,7 @@ int arcmsr_proc_info(struct Scsi_Host *h
 **
 ************************************************************************
 */
-int arcmsr_release(struct Scsi_Host *host)
+static int arcmsr_release(struct Scsi_Host *host)
 {
 	PACB pACB;
 	uint8_t match = 0xff, i;
diff -puN drivers/scsi/arcmsr/arcmsr.h~drivers-scsi-arcmsr-arcmsrc-cleanups drivers/scsi/arcmsr/arcmsr.h
--- 25/drivers/scsi/arcmsr/arcmsr.h~drivers-scsi-arcmsr-arcmsrc-cleanups	2005-02-28 15:42:00.000000000 -0800
+++ 25-akpm/drivers/scsi/arcmsr/arcmsr.h	2005-02-28 15:42:00.000000000 -0800
@@ -4802,38 +4802,4 @@ typedef struct _SENSE_DATA {
 **        (E) Checksum : checksum of length and status or data byte
 **************************************************************************
 */
-extern int arcmsr_proc_info(struct Scsi_Host *host, char *buffer, char **start,
-			    off_t offset, int length, int inout);
-extern int arcmsr_bios_param(struct scsi_device *sdev,
-			     struct block_device *bdev, sector_t capacity,
-			     int *info);
-extern int arcmsr_release(struct Scsi_Host *);
-extern int arcmsr_queue_command(Scsi_Cmnd * cmd, void (*done) (Scsi_Cmnd *));
-extern int arcmsr_cmd_abort(Scsi_Cmnd *);
-extern int arcmsr_bus_reset(Scsi_Cmnd *);
-extern int arcmsr_ioctl(Scsi_Device * dev, int ioctl_cmd, void *arg);
-extern const char *arcmsr_info(struct Scsi_Host *);
 
-static Scsi_Host_Template arcmsr_scsi_host_template = {
-	.module = THIS_MODULE,
-	.proc_name = "arcmsr",
-	.proc_info = arcmsr_proc_info,
-	.name = "ARCMSR ARECA SATA RAID HOST Adapter" ARCMSR_DRIVER_VERSION,	/* *name */
-	.release = arcmsr_release,
-	.info = arcmsr_info,
-	.ioctl = arcmsr_ioctl,
-	.queuecommand = arcmsr_queue_command,
-	.eh_strategy_handler = NULL,
-	.eh_abort_handler = arcmsr_cmd_abort,
-	.eh_device_reset_handler = NULL,
-	.eh_bus_reset_handler = arcmsr_bus_reset,
-	.eh_host_reset_handler = NULL,
-	.bios_param = arcmsr_bios_param,
-	.can_queue = ARCMSR_MAX_OUTSTANDING_CMD,
-	.this_id = ARCMSR_SCSI_INITIATOR_ID,
-	.sg_tablesize = ARCMSR_MAX_SG_ENTRIES,
-	.max_sectors = ARCMSR_MAX_XFER_SECTORS,
-	.cmd_per_lun = ARCMSR_MAX_CMD_PERLUN,
-	.unchecked_isa_dma = 0,
-	.use_clustering = DISABLE_CLUSTERING,
-};
_

Re: About ARECA RAID driver for Linux i386/x86-64

[Andrew Morton]

"erich" <erich@areca.com.tw> wrote:
>
> I had modify "arcmsr" as your mention and ran it at 2.6.11 kernel.

Your patch omitted drivers/scsi/arcmsr/Makefile, so it doesn't compile.

Re: About ARECA RAID driver for Linux i386/x86-64

[Linux Christoph Hellwig]

On Thu, Feb 17, 2005 at 04:02:25PM +0800, erich wrote:
> Hi,Andrew Morton
> 
> I had clean up "arcmsr" for linux kernel 2.6.10.
> May be it looks more cleanly  than old version.

Sorry, but I don't see that much of a difference ;-)

To get the driver into a shape where we can actually
start a review, do run the shell script scripts/Lindent
from the kernel tree over the driver, and please kill
all your SHOUTING typedefs for kernel types.  After
that compare it with the file Documentation/CodingStyle,
also in the kernel tree and try to fix up whatever seems
to not match up.

Re: About ARECA RAID driver for Linux i386/x86-64

[Andrew Morton]

Linux Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Feb 17, 2005 at 04:02:25PM +0800, erich wrote:
>  > Hi,Andrew Morton
>  > 
>  > I had clean up "arcmsr" for linux kernel 2.6.10.
>  > May be it looks more cleanly  than old version.
> 
>  Sorry, but I don't see that much of a difference ;-)

All the LINUX_VERSION_CODE tests went away.

>  To get the driver into a shape where we can actually
>  start a review, do run the shell script scripts/Lindent
>  from the kernel tree over the driver,

yup.  three seconds work.

> and please kill
>  all your SHOUTING typedefs for kernel types.

Yes, these:

+typedef int8_t CHAR, *PCHAR;
+typedef int16_t SHORT, *PSHORT;
+typedef int32_t LONG32, *PLONG32;
+typedef int64_t LONG64, *PLONG64;
+typedef uint8_t UCHAR, *PUCHAR;
+typedef uint16_t USHORT, *PUSHORT;
+typedef uint32_t ULONG32, *PULONG32;
+typedef uint64_t ULONG64, *PULONG64;
+typedef Scsi_Device SCSIDEV, *PSCSIDEV;
+typedef Scsi_Cmnd SCSICMD, *PSCSICMD;
+typedef void VOID, *PVOID;
+typedef struct Scsi_Host SH, *PSH;
+typedef struct scatterlist SGL, *PSGL;
+typedef void (*DPCFUN) (void *);
+typedef struct _MU MU, *PMU;
+typedef struct _CCB CCB, *PCCB;
+typedef struct _ACB ACB, *PACB;
+typedef struct _HCBARC HCBARC, *PHCBARC;
+typedef struct _DPC DPC, *PDPC;
+typedef struct _EVENT EVENT, *PEVENT;
+typedef struct _QBUFFER QBUFFER, *PQBUFFER;
+typedef struct _SG32ENTRY SG32ENTRY, *PSG32ENTRY;
+typedef struct _SG64ENTRY SG64ENTRY, *PSG64ENTRY;
+typedef struct _SGENTRY_UNION SGENTRY_UNION, PSGENTRY_UNION;
+typedef struct _SYSTEMTIME SYSTEMTIME, *PSYSTEMTIME;
+typedef struct _TIMERECORD TIMERECORD, *PTIMERECORD;
+typedef struct _SENSE_DATA SENSE_DATA, *PSENSE_DATA;
+typedef struct _ARCMSR_PCIINFO ARCMSR_PCIINFO, *PARCMSR_PCIINFO;
+typedef struct _ARCMSR_CDB ARCMSR_CDB, *PARCMSR_CDB;
+typedef struct _CMD_IO_CONTROL CMD_IO_CONTROL, *PCMD_IO_CONTROL;
+typedef struct _CMD_IOCTL_FIELD CMD_IOCTL_FIELD, *PCMD_IOCTL_FIELD;
+#if defined(__x86_64__)
+typedef uint64_t CPT2INT, *PCPT2INT;
+#else
+typedef uint32_t CPT2INT, *PCPT2INT;
+#endif

should be removed - just open-code the types within the .c code.

Re: About ARECA RAID driver for Linux i386/x86-64

[Andrew Morton]

"erich" <erich@areca.com.tw> wrote:
>
> 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.

>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.

Re: About ARECA RAID driver for Linux i386/x86-64

[Arjan van de Ven]

On Wed, 2005-02-02 at 18:08 -0800, Andrew Morton wrote:
> "erich" <erich@areca.com.tw> wrote:
> >
> > 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.
> 
> >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

Re: About ARECA RAID driver for Linux i386/x86-64

[Christoph Hellwig]

On Thu, Feb 03, 2005 at 11:05:32AM +0100, Arjan van de Ven wrote:
> * 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)

> * 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.