Re: [PATCH] Marvell 6440 SAS/SATA driver (draft)
Hello James: I read your comments. .target_alloc = sas_target_alloc, .slave_configure= sas_slave_configure, .slave_destroy = sas_slave_destroy, .change_queue_depth = sas_change_queue_depth, .change_queue_type = sas_change_queue_type, .bios_param = sas_bios_param, - .can_queue = 1, + .can_queue = 30, I don't think you want to do this. It starts sending multiple commands at once. The design use of libsas is to start off at 1 and then up the limit in slave configure once we know what type of device we're dealing with. The current sas_slave_configure has a limitation in that the depth is hard coded to 32, so if you need it smaller, we'll have to find a way of allowing this? Is 30 your max queue depth? Yes , I may not do this. But I need to set sas_ha_struct.lldd_queue_size to suitable value. Because sas_queue sends multiple commands according to lldd_queue_size before calling lldd_execute_task function which supports queue depth , 30 is suitable. .cmd_per_lun= 1, .this_id= -1, - .sg_tablesize = SG_ALL, - .max_sectors= SCSI_DEFAULT_MAX_SECTORS, - .use_clustering = ENABLE_CLUSTERING, + .sg_tablesize = 32, 32 looks a little small. My reading of the code is that the PRD table has to fit with the command header, OAF area and status area into an 8k segment of memory, so at 16 bytes per PRD entry, it looks like a page worth at least (256) isn't unreasonable. You should probably have some type of macro for this though. + .max_sectors= (128*1024)9, Yes , It's demo code . And I need to test device to find a good value according to performance and reliability. + .use_clustering = DISABLE_CLUSTERING, I think this is wrong ... there should be no modern device that disables clustering (otherwise they fall over badly on iommu platforms). .eh_device_reset_handler= sas_eh_device_reset_handler, .eh_bus_reset_handler = sas_eh_bus_reset_handler, - .slave_alloc= sas_slave_alloc, Please don't remove this ... it's a standard callback, and it's required for the day you support SATA. You are right. I forgot to recover these codes when I debuged. And Driver has support for SATA devices . I will commit the patches in this weeks. Thank you for your help. Ke Wei. On Jan 19, 2008 2:53 AM, James Bottomley [EMAIL PROTECTED] wrote: On Thu, 2008-01-10 at 14:53 +0800, Ke Wei wrote: The 88SE6440 driver : The driver is based on bare code from Jeff Garzik. And it can work under linux kernel 2.6.23. By far, Can discover and find SAS HDD, but SATA is currently unsupported. Command queue depth can be above 1. Most error handling, and some phy handling code is notably missing. contains the following updates: --- mvsas_orig.c 2007-12-06 19:21:32.0 -0500 +++ mvsas.c 2008-01-09 04:53:14.0 -0500 [...] +#define MVS_BIT(x) (1L (x)) + +#define PORT_TYPE_SATAMVS_BIT(0) +#define PORT_TYPE_SAS MVS_BIT(1) + +#define READ_PORT_CONFIG_DATA(i) \ + ((i3)?mr32(P4_CFG_DATA+(i-4)*8):mr32(P0_CFG_DATA+i*8)) +#define WRITE_PORT_CONFIG_DATA(i,tmp) \ + {if(i3)mw32(P4_CFG_DATA+(i-4)*8,tmp);else mw32(P0_CFG_DATA+i*8,tmp);} +#define WRITE_PORT_CONFIG_ADDR(i,tmp) \ + {if(i3)mw32(P4_CFG_ADDR+(i-4)*8,tmp);else mw32(P0_CFG_ADDR+i*8,tmp);} + +#define READ_PORT_PHY_CONTROL(i) \ + ((i3)?mr32(P4_SER_CTLSTAT+(i-4)*4):mr32(P0_SER_CTLSTAT+i*4)) +#define WRITE_PORT_PHY_CONTROL(i,tmp) \ + {if(i3)mw32(P4_SER_CTLSTAT+(i-4)*4,tmp);else mw32(P0_SER_CTLSTAT+i*4,tmp);} This is an example of where you mailer has broken a line (which causes the patch not to apply). + +#define READ_PORT_VSR_DATA(i) \ + ((i3)?mr32(P4_VSR_DATA+(i-4)*8):mr32(P0_VSR_DATA+i*8)) +#define WRITE_PORT_VSR_DATA(i,tmp) \ + {if(i3)mw32(P4_VSR_DATA+(i-4)*8,tmp);else mw32(P0_VSR_DATA+i*8,tmp);} +#define WRITE_PORT_VSR_ADDR(i,tmp) \ + {if(i3)mw32(P4_VSR_ADDR+(i-4)*8,tmp);else mw32(P0_VSR_ADDR+i*8,tmp);} + +#define READ_PORT_IRQ_STAT(i) \ + ((i3)?mr32(P4_INT_STAT+(i-4)*8):mr32(P0_INT_STAT+i*8)) +#define WRITE_PORT_IRQ_STAT(i,tmp) \ + {if(i3)mw32(P4_INT_STAT+(i-4)*8,tmp);else mw32(P0_INT_STAT+i*8,tmp);} +#define READ_PORT_IRQ_MASK(i) \ + ((i3)?mr32(P4_INT_MASK+(i-4)*8):mr32(P0_INT_MASK+i*8)) +#define WRITE_PORT_IRQ_MASK(i,tmp) \ + {if(i3)mw32(P4_INT_MASK+(i-4)*8,tmp);else mw32(P0_INT_MASK+i*8,tmp);} + /* driver compile-time configuration */ enum driver_configuration { - MVS_TX_RING_SZ = 1024, /* TX ring size (12-bit) */ + MVS_TX_RING_SZ = 512, /* TX ring size (12-bit) */ MVS_RX_RING_SZ = 1024, /* RX ring size
Re: [PATCH] Marvell 6440 SAS/SATA driver (draft)
On Thu, 2008-01-10 at 02:21 -0500, Jeff Garzik wrote: Jeff Garzik wrote: 1) To make it easier for people to review and test the driver, I would suggest posting a diff against 2.6.24-rc7 (or 2.6.23), ignoring my original code. Thus, it would result in a patch Er, that sentence was incomplete. Continuing... Thus it would result in a patch that adds a new file drivers/scsi/mvsas.c to the 2.6.24-rc7 kernel, and modifies drivers/scsi/Makefile and drivers/scsi/Kconfig to enable this new driver. That is the format that developers and users are most familiar with, when reviewing (and testing) a new driver. But of course this is a draft, so these guidelines are certainly loose... OK, in order to try to expedite this, I've created a mvsas branch in git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi-misc-2.6.git I think the first patch (the infrastructure change) is safe to go in immediately. Unfortunately, I can't put the marvell update in because the emailed patch is corrupt (it looks like the mailer has added line breaks). Ke, If you can't get your email tool to insert plain text (as a lot of microsoft based one's can't), you can add the patch as an attachment; I can apply it from that (Although in line plain text is by far the preferred method for review if you can do it, we have a bunch of other driver writers who have problematic email tools, so we're reasonably used to this). Thanks, James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Marvell 6440 SAS/SATA driver (draft)
On Thu, 2008-01-10 at 14:53 +0800, Ke Wei wrote: The 88SE6440 driver : The driver is based on bare code from Jeff Garzik. And it can work under linux kernel 2.6.23. By far, Can discover and find SAS HDD, but SATA is currently unsupported. Command queue depth can be above 1. Most error handling, and some phy handling code is notably missing. contains the following updates: --- mvsas_orig.c 2007-12-06 19:21:32.0 -0500 +++ mvsas.c 2008-01-09 04:53:14.0 -0500 [...] +#define MVS_BIT(x) (1L (x)) + +#define PORT_TYPE_SATAMVS_BIT(0) +#define PORT_TYPE_SAS MVS_BIT(1) + +#define READ_PORT_CONFIG_DATA(i) \ + ((i3)?mr32(P4_CFG_DATA+(i-4)*8):mr32(P0_CFG_DATA+i*8)) +#define WRITE_PORT_CONFIG_DATA(i,tmp) \ + {if(i3)mw32(P4_CFG_DATA+(i-4)*8,tmp);else mw32(P0_CFG_DATA+i*8,tmp);} +#define WRITE_PORT_CONFIG_ADDR(i,tmp) \ + {if(i3)mw32(P4_CFG_ADDR+(i-4)*8,tmp);else mw32(P0_CFG_ADDR+i*8,tmp);} + +#define READ_PORT_PHY_CONTROL(i) \ + ((i3)?mr32(P4_SER_CTLSTAT+(i-4)*4):mr32(P0_SER_CTLSTAT+i*4)) +#define WRITE_PORT_PHY_CONTROL(i,tmp) \ + {if(i3)mw32(P4_SER_CTLSTAT+(i-4)*4,tmp);else mw32(P0_SER_CTLSTAT+i*4,tmp);} This is an example of where you mailer has broken a line (which causes the patch not to apply). + +#define READ_PORT_VSR_DATA(i) \ + ((i3)?mr32(P4_VSR_DATA+(i-4)*8):mr32(P0_VSR_DATA+i*8)) +#define WRITE_PORT_VSR_DATA(i,tmp) \ + {if(i3)mw32(P4_VSR_DATA+(i-4)*8,tmp);else mw32(P0_VSR_DATA+i*8,tmp);} +#define WRITE_PORT_VSR_ADDR(i,tmp) \ + {if(i3)mw32(P4_VSR_ADDR+(i-4)*8,tmp);else mw32(P0_VSR_ADDR+i*8,tmp);} + +#define READ_PORT_IRQ_STAT(i) \ + ((i3)?mr32(P4_INT_STAT+(i-4)*8):mr32(P0_INT_STAT+i*8)) +#define WRITE_PORT_IRQ_STAT(i,tmp) \ + {if(i3)mw32(P4_INT_STAT+(i-4)*8,tmp);else mw32(P0_INT_STAT+i*8,tmp);} +#define READ_PORT_IRQ_MASK(i) \ + ((i3)?mr32(P4_INT_MASK+(i-4)*8):mr32(P0_INT_MASK+i*8)) +#define WRITE_PORT_IRQ_MASK(i,tmp) \ + {if(i3)mw32(P4_INT_MASK+(i-4)*8,tmp);else mw32(P0_INT_MASK+i*8,tmp);} + /* driver compile-time configuration */ enum driver_configuration { - MVS_TX_RING_SZ = 1024, /* TX ring size (12-bit) */ + MVS_TX_RING_SZ = 512, /* TX ring size (12-bit) */ MVS_RX_RING_SZ = 1024, /* RX ring size (12-bit) */ /* software requires power-of-2 ring size */ @@ -89,7 +125,7 @@ MVS_GBL_CTL = 0x04, /* global control */ MVS_GBL_INT_STAT= 0x08, /* global irq status */ MVS_GBL_PI = 0x0C, /* ports implemented bitmask */ - MVS_GBL_PORT_TYPE = 0x00, /* port type */ + MVS_GBL_PORT_TYPE = 0xa0, /* port type */ MVS_CTL = 0x100, /* SAS/SATA port configuration */ MVS_PCS = 0x104, /* SAS/SATA port control/status */ @@ -102,11 +138,12 @@ MVS_TX_LO = 0x124, /* TX (delivery) ring addr */ MVS_TX_HI = 0x128, - MVS_RX_PROD_IDX = 0x12C, /* RX producer pointer */ - MVS_RX_CONS_IDX = 0x130, /* RX consumer pointer (RO) */ + MVS_TX_PROD_IDX = 0x12C, /* TX producer pointer */ + MVS_TX_CONS_IDX = 0x130, /* TX consumer pointer (RO) */ MVS_RX_CFG = 0x134, /* RX configuration */ MVS_RX_LO = 0x138, /* RX (completion) ring addr */ MVS_RX_HI = 0x13C, + MVS_RX_CONS_IDX = 0x140, /* RX consumer pointer (RO) */ MVS_INT_COAL= 0x148, /* Int coalescing config */ MVS_INT_COAL_TMOUT = 0x14C, /* Int coalescing timeout */ @@ -117,9 +154,12 @@ /* ports 1-3 follow after this */ MVS_P0_INT_STAT = 0x160, /* port0 interrupt status */ MVS_P0_INT_MASK = 0x164, /* port0 interrupt mask */ + MVS_P4_INT_STAT = 0x200, /* Port 4 interrupt status */ + MVS_P4_INT_MASK = 0x204, /* Port 4 interrupt enable/disable mask */ /* ports 1-3 follow after this */ MVS_P0_SER_CTLSTAT = 0x180, /* port0 serial control/status */ + MVS_P4_SER_CTLSTAT = 0x220, /* port4 serial control/status */ MVS_CMD_ADDR= 0x1B8, /* Command register port (addr) */ MVS_CMD_DATA= 0x1BC, /* Command register port (data) */ @@ -127,6 +167,14 @@ /* ports 1-3 follow after this */ MVS_P0_CFG_ADDR = 0x1C0, /* port0 phy register address */ MVS_P0_CFG_DATA = 0x1C4, /* port0 phy register data */ + MVS_P4_CFG_ADDR = 0x230, /* Port 4 config address */ + MVS_P4_CFG_DATA = 0x234, /* Port 4 config data */ + + /* ports 1-3 follow after this */ +
[PATCH] Marvell 6440 SAS/SATA driver (draft)
The 88SE6440 driver : The driver is based on bare code from Jeff Garzik. And it can work under linux kernel 2.6.23. By far, Can discover and find SAS HDD, but SATA is currently unsupported. Command queue depth can be above 1. Most error handling, and some phy handling code is notably missing. contains the following updates: --- mvsas_orig.c2007-12-06 19:21:32.0 -0500 +++ mvsas.c 2008-01-09 04:53:14.0 -0500 @@ -2,6 +2,7 @@ mvsas.c - Marvell 88SE6440 SAS/SATA support Copyright 2007 Red Hat, Inc. + Copyright 2008 Marvell. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as @@ -37,8 +38,10 @@ #include scsi/libsas.h #include asm/io.h -#define DRV_NAME mvsas -#define DRV_VERSION 0.1 +#define DRV_NAME mvsas +#define DRV_VERSION0.2 +#define _MV_DUMP 0 +#define MVS_PRINTK(_x_,...)printk(KERN_NOTICE DRV_NAME : _x_ , ## __VA_ARGS__) #define mr32(reg) readl(regs + MVS_##reg) #define mw32(reg,val) writel((val), regs + MVS_##reg) @@ -47,9 +50,42 @@ readl(regs + MVS_##reg);\ } while (0) +#define MVS_BIT(x) (1L (x)) + +#define PORT_TYPE_SATAMVS_BIT(0) +#define PORT_TYPE_SAS MVS_BIT(1) + +#define READ_PORT_CONFIG_DATA(i) \ + ((i3)?mr32(P4_CFG_DATA+(i-4)*8):mr32(P0_CFG_DATA+i*8)) +#define WRITE_PORT_CONFIG_DATA(i,tmp) \ + {if(i3)mw32(P4_CFG_DATA+(i-4)*8,tmp);else mw32(P0_CFG_DATA+i*8,tmp);} +#define WRITE_PORT_CONFIG_ADDR(i,tmp) \ + {if(i3)mw32(P4_CFG_ADDR+(i-4)*8,tmp);else mw32(P0_CFG_ADDR+i*8,tmp);} + +#define READ_PORT_PHY_CONTROL(i) \ + ((i3)?mr32(P4_SER_CTLSTAT+(i-4)*4):mr32(P0_SER_CTLSTAT+i*4)) +#define WRITE_PORT_PHY_CONTROL(i,tmp) \ + {if(i3)mw32(P4_SER_CTLSTAT+(i-4)*4,tmp);else mw32(P0_SER_CTLSTAT+i*4,tmp);} + +#define READ_PORT_VSR_DATA(i) \ + ((i3)?mr32(P4_VSR_DATA+(i-4)*8):mr32(P0_VSR_DATA+i*8)) +#define WRITE_PORT_VSR_DATA(i,tmp) \ + {if(i3)mw32(P4_VSR_DATA+(i-4)*8,tmp);else mw32(P0_VSR_DATA+i*8,tmp);} +#define WRITE_PORT_VSR_ADDR(i,tmp) \ + {if(i3)mw32(P4_VSR_ADDR+(i-4)*8,tmp);else mw32(P0_VSR_ADDR+i*8,tmp);} + +#define READ_PORT_IRQ_STAT(i) \ + ((i3)?mr32(P4_INT_STAT+(i-4)*8):mr32(P0_INT_STAT+i*8)) +#define WRITE_PORT_IRQ_STAT(i,tmp) \ + {if(i3)mw32(P4_INT_STAT+(i-4)*8,tmp);else mw32(P0_INT_STAT+i*8,tmp);} +#define READ_PORT_IRQ_MASK(i) \ + ((i3)?mr32(P4_INT_MASK+(i-4)*8):mr32(P0_INT_MASK+i*8)) +#define WRITE_PORT_IRQ_MASK(i,tmp) \ + {if(i3)mw32(P4_INT_MASK+(i-4)*8,tmp);else mw32(P0_INT_MASK+i*8,tmp);} + /* driver compile-time configuration */ enum driver_configuration { - MVS_TX_RING_SZ = 1024, /* TX ring size (12-bit) */ + MVS_TX_RING_SZ = 512, /* TX ring size (12-bit) */ MVS_RX_RING_SZ = 1024, /* RX ring size (12-bit) */ /* software requires power-of-2 ring size */ @@ -89,7 +125,7 @@ MVS_GBL_CTL = 0x04, /* global control */ MVS_GBL_INT_STAT= 0x08, /* global irq status */ MVS_GBL_PI = 0x0C, /* ports implemented bitmask */ - MVS_GBL_PORT_TYPE = 0x00, /* port type */ + MVS_GBL_PORT_TYPE = 0xa0, /* port type */ MVS_CTL = 0x100, /* SAS/SATA port configuration */ MVS_PCS = 0x104, /* SAS/SATA port control/status */ @@ -102,11 +138,12 @@ MVS_TX_LO = 0x124, /* TX (delivery) ring addr */ MVS_TX_HI = 0x128, - MVS_RX_PROD_IDX = 0x12C, /* RX producer pointer */ - MVS_RX_CONS_IDX = 0x130, /* RX consumer pointer (RO) */ + MVS_TX_PROD_IDX = 0x12C, /* TX producer pointer */ + MVS_TX_CONS_IDX = 0x130, /* TX consumer pointer (RO) */ MVS_RX_CFG = 0x134, /* RX configuration */ MVS_RX_LO = 0x138, /* RX (completion) ring addr */ MVS_RX_HI = 0x13C, + MVS_RX_CONS_IDX = 0x140, /* RX consumer pointer (RO) */ MVS_INT_COAL= 0x148, /* Int coalescing config */ MVS_INT_COAL_TMOUT = 0x14C, /* Int coalescing timeout */ @@ -117,9 +154,12 @@ /* ports 1-3 follow after this */ MVS_P0_INT_STAT = 0x160, /* port0 interrupt status */ MVS_P0_INT_MASK = 0x164, /* port0 interrupt mask */ + MVS_P4_INT_STAT = 0x200, /* Port 4 interrupt status */ + MVS_P4_INT_MASK = 0x204, /* Port 4 interrupt enable/disable mask */ /* ports 1-3 follow after this */ MVS_P0_SER_CTLSTAT = 0x180, /* port0 serial control/status */ + MVS_P4_SER_CTLSTAT = 0x220, /* port4 serial control/status */ MVS_CMD_ADDR= 0x1B8,
Re: [PATCH] Marvell 6440 SAS/SATA driver (draft)
Ke Wei wrote: The 88SE6440 driver : The driver is based on bare code from Jeff Garzik. And it can work under linux kernel 2.6.23. By far, Can discover and find SAS HDD, but SATA is currently unsupported. Command queue depth can be above 1. Most error handling, and some phy handling code is notably missing. contains the following updates: --- mvsas_orig.c2007-12-06 19:21:32.0 -0500 +++ mvsas.c 2008-01-09 04:53:14.0 -0500 Fantastic! I'm delighted to see this is moving, and thanks much for getting the driver to that critical it works milestone. We should note for the linux-scsi audience and potential testers that the base driver upon which this is based is available on the sas branch of git://git.kernel.org:/pub/scm/linux/kernel/git/jgarzik/misc-2.6.git I will give some substantive comments tomorrow (just got back from long trip). Two quick suggestions: 1) To make it easier for people to review and test the driver, I would suggest posting a diff against 2.6.24-rc7 (or 2.6.23), ignoring my original code. Thus, it would result in a patch 2) In future patches, include a Signed-off-by: line when you are ready for your patch to be included in the kernel. Thanks! Jeff - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Marvell 6440 SAS/SATA driver (draft)
Jeff Garzik wrote: 1) To make it easier for people to review and test the driver, I would suggest posting a diff against 2.6.24-rc7 (or 2.6.23), ignoring my original code. Thus, it would result in a patch Er, that sentence was incomplete. Continuing... Thus it would result in a patch that adds a new file drivers/scsi/mvsas.c to the 2.6.24-rc7 kernel, and modifies drivers/scsi/Makefile and drivers/scsi/Kconfig to enable this new driver. That is the format that developers and users are most familiar with, when reviewing (and testing) a new driver. But of course this is a draft, so these guidelines are certainly loose... Jeff - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html