Re: [PATCH] Marvell 6440 SAS/SATA driver (draft)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.000000000 -0500
> +++ mvsas.c	2008-01-09 04:53:14.000000000 -0500
[...]
> +#define MVS_BIT(x)                         (1L << (x))
> +
> +#define PORT_TYPE_SATA                    MVS_BIT(0)
> +#define PORT_TYPE_SAS                     MVS_BIT(1)
> +
> +#define READ_PORT_CONFIG_DATA(i) \
> +	((i>3)?mr32(P4_CFG_DATA+(i-4)*8):mr32(P0_CFG_DATA+i*8))
> +#define WRITE_PORT_CONFIG_DATA(i,tmp) \
> +	{if(i>3)mw32(P4_CFG_DATA+(i-4)*8,tmp);else mw32(P0_CFG_DATA+i*8,tmp);}
> +#define WRITE_PORT_CONFIG_ADDR(i,tmp) \
> +	{if(i>3)mw32(P4_CFG_ADDR+(i-4)*8,tmp);else mw32(P0_CFG_ADDR+i*8,tmp);}
> +
> +#define READ_PORT_PHY_CONTROL(i) \
> +   ((i>3)?mr32(P4_SER_CTLSTAT+(i-4)*4):mr32(P0_SER_CTLSTAT+i*4))
> +#define WRITE_PORT_PHY_CONTROL(i,tmp) \
> +   {if(i>3)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) \
> +   ((i>3)?mr32(P4_VSR_DATA+(i-4)*8):mr32(P0_VSR_DATA+i*8))
> +#define WRITE_PORT_VSR_DATA(i,tmp) \
> +   {if(i>3)mw32(P4_VSR_DATA+(i-4)*8,tmp);else mw32(P0_VSR_DATA+i*8,tmp);}
> +#define WRITE_PORT_VSR_ADDR(i,tmp) \
> +   {if(i>3)mw32(P4_VSR_ADDR+(i-4)*8,tmp);else mw32(P0_VSR_ADDR+i*8,tmp);}
> +
> +#define READ_PORT_IRQ_STAT(i) \
> +   ((i>3)?mr32(P4_INT_STAT+(i-4)*8):mr32(P0_INT_STAT+i*8))
> +#define WRITE_PORT_IRQ_STAT(i,tmp) \
> +   {if(i>3)mw32(P4_INT_STAT+(i-4)*8,tmp);else mw32(P0_INT_STAT+i*8,tmp);}
> +#define READ_PORT_IRQ_MASK(i) \
> +   ((i>3)?mr32(P4_INT_MASK+(i-4)*8):mr32(P0_INT_MASK+i*8))
> +#define WRITE_PORT_IRQ_MASK(i,tmp) \
> +   {if(i>3)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 */
> +	MVS_P0_VSR_ADDR		= 0x1E0, /* port0 vendor specific register address */
> +	MVS_P0_VSR_DATA		= 0x1E4, /* port0 vendor specific register data */	
> +	MVS_P4_VSR_ADDR		= 0x250, /* port 4 Vendor Specific Register addr */
> +	MVS_P4_VSR_DATA		= 0x254, /* port 4 Vendor Specific Register Data */		
>  };
> 
>  enum hw_register_bits {
> @@ -140,8 +188,31 @@
> 
>  	/* MVS_GBL_PORT_TYPE */			/* shl for ports 1-3 */
>  	SATA_TARGET		= (1U << 16),	/* port0 SATA target enable */
> -	AUTO_DET		= (1U << 8),	/* port0 SAS/SATA autodetect */
> -	SAS_MODE		= (1U << 0),	/* port0 SAS(1), SATA(0) mode */
> +	MODE_AUTO_DET_PORT7 = (1U << 15),	/* port0 SAS/SATA autodetect */
> +	MODE_AUTO_DET_PORT6 = (1U << 14),
> +	MODE_AUTO_DET_PORT5 = (1U << 13),
> +	MODE_AUTO_DET_PORT4 = (1U << 12),
> +	MODE_AUTO_DET_PORT3 = (1U << 11),
> +	MODE_AUTO_DET_PORT2 = (1U << 10),
> +	MODE_AUTO_DET_PORT1 = (1U << 9),
> +	MODE_AUTO_DET_PORT0 = (1U << 8),
> +	MODE_AUTO_DET_EN    = MODE_AUTO_DET_PORT0 | MODE_AUTO_DET_PORT1 |
> +	                      MODE_AUTO_DET_PORT2 | MODE_AUTO_DET_PORT3 |
> +	                      MODE_AUTO_DET_PORT4 | MODE_AUTO_DET_PORT5 |
> +	                      MODE_AUTO_DET_PORT6 | MODE_AUTO_DET_PORT7,	
> +	MODE_SAS_PORT7_MASK = (1U << 7),  /* port0 SAS(1), SATA(0) mode */
> +	MODE_SAS_PORT6_MASK = (1U << 6),
> +	MODE_SAS_PORT5_MASK = (1U << 5),
> +	MODE_SAS_PORT4_MASK = (1U << 4),
> +	MODE_SAS_PORT3_MASK = (1U << 3),
> +	MODE_SAS_PORT2_MASK = (1U << 2),
> +	MODE_SAS_PORT1_MASK = (1U << 1),
> +	MODE_SAS_PORT0_MASK = (1U << 0),
> +	MODE_SAS_SATA		= 	MODE_SAS_PORT0_MASK | MODE_SAS_PORT1_MASK |
> +							MODE_SAS_PORT2_MASK | MODE_SAS_PORT3_MASK |
> +							MODE_SAS_PORT4_MASK | MODE_SAS_PORT5_MASK |
> +							MODE_SAS_PORT6_MASK | MODE_SAS_PORT7_MASK,
> +
>  						/* SAS_MODE value may be
>  						 * dictated (in hw) by values
>  						 * of SATA_TARGET & AUTO_DET
> @@ -167,12 +238,14 @@
>  	CINT_MEM		= (1U << 26),	/* int mem parity err */
>  	CINT_I2C_SLAVE		= (1U << 25),	/* slave I2C event */
>  	CINT_SRS		= (1U << 3),	/* SRS event */
> -	CINT_CI_STOP		= (1U << 10),	/* cmd issue stopped */
> +	CINT_CI_STOP		= (1U << 1),	/* cmd issue stopped */
>  	CINT_DONE		= (1U << 0),	/* cmd completion */
> 
>  						/* shl for ports 1-3 */
>  	CINT_PORT_STOPPED	= (1U << 16),	/* port0 stopped */
> -	CINT_PORT		= (1U << 8),	/* port0 event */
> +	CINT_PORT			= (1U << 8),	/* port0 event */
> +	CINT_PORT_MASK_OFFSET	= 8,
> +	CINT_PORT_MASK		= (0xFF << CINT_PORT_MASK_OFFSET),
> 
>  	/* TX (delivery) ring bits */
>  	TXQ_CMD_SHIFT		= 29,
> @@ -236,9 +309,14 @@
> 
>  	/* MVS_Px_SER_CTLSTAT (per-phy control) */
>  	PHY_SSP_RST		= (1U << 3),	/* reset SSP link layer */
> -	PHY_BCAST_CHG		= (1U << 2),	/* broadcast(change) notif */
> -	PHY_RST_HARD		= (1U << 1),	/* hard reset + phy reset */
> +	PHY_BCAST_CHG	= (1U << 2),	/* broadcast(change) notif */
> +	PHY_RST_HARD	= (1U << 1),	/* hard reset + phy reset */
>  	PHY_RST			= (1U << 0),	/* phy reset */
> +	PHY_MIN_SPP_PHYS_LINK_RATE_MASK = (0xF << 8),
> +	PHY_MAX_SPP_PHYS_LINK_RATE_MASK = (0xF << 12),
> +	PHY_NEG_SPP_PHYS_LINK_RATE_MASK_OFFSET = 16,
> +	PHY_NEG_SPP_PHYS_LINK_RATE_MASK = (0xF <<
> PHY_NEG_SPP_PHYS_LINK_RATE_MASK_OFFSET),
> +	PHY_READY_MASK					= (1U << 20),
> 
>  	/* MVS_Px_INT_STAT, MVS_Px_INT_MASK (per-phy events) */
>  	PHYEV_UNASSOC_FIS	= (1U << 19),	/* unassociated FIS rx'd */
> @@ -260,12 +338,14 @@
>  	PHYEV_RDY_CH		= (1U << 0),	/* phy ready changed state */
> 
>  	/* MVS_PCS */
> +	PCS_EN_PORT_XMT_START	=  (12),		/*Enable Port Transmit*/
> +	PCS_EN_PORT_XMT_START2	=  (8),			/*For 6480*/
>  	PCS_SATA_RETRY		= (1U << 8),	/* retry ctl FIS on R_ERR */
>  	PCS_RSP_RX_EN		= (1U << 7),	/* raw response rx */
>  	PCS_SELF_CLEAR		= (1U << 5),	/* self-clearing int mode */
>  	PCS_FIS_RX_EN		= (1U << 4),	/* FIS rx enable */
>  	PCS_CMD_STOP_ERR	= (1U << 3),	/* cmd stop-on-err enable */
> -	PCS_CMD_RST		= (1U << 2),	/* reset cmd issue */
> +	PCS_CMD_RST		= (1U << 1),	/* reset cmd issue */
>  	PCS_CMD_EN		= (1U << 0),	/* enable cmd issue */
>  };
> 
> @@ -288,7 +368,7 @@
>  	CMD_SAS_CTL1		= 0x128, /* SAS control register 1 */
>  	CMD_SAS_CTL2		= 0x12c, /* SAS control register 2 */
>  	CMD_SAS_CTL3		= 0x130, /* SAS control register 3 */
> -	CMD_ID_TEST		= 0x134, /* ID test register */
> +	CMD_ID_TEST			= 0x134, /* ID test register */
>  	CMD_PL_TIMER		= 0x138, /* PL timer register */
>  	CMD_WD_TIMER		= 0x13c, /* WD timer register */
>  	CMD_PORT_SEL_COUNT	= 0x140, /* port selector count register */
> @@ -345,12 +425,15 @@
> 
>  enum pci_cfg_registers {
>  	PCR_PHY_CTL		= 0x40,
> -	PCR_PHY_CTL2		= 0x90,
> +	PCR_PHY_CTL2	= 0x90,
> +	PCR_DEV_CTRL	= 0xE8,
>  };
> 
>  enum pci_cfg_register_bits {
>  	PCTL_PWR_ON		= (0xFU << 24),
>  	PCTL_OFF		= (0xFU << 12),
> +	PRD_REQ_SIZE	= (0x4000),
> +	PRD_REQ_MASK	= (0x00007000),
>  };
> 
>  enum nvram_layout_offsets {
> @@ -412,8 +495,20 @@
> 
>  struct mvs_phy {
>  	struct mvs_port		*port;
> -	struct asd_sas_phy	sas_phy;
> -
> +	struct asd_sas_phy	sas_phy;
> 	
> +	struct sas_identify identify;
> +/* from PORT_CONFIG_ADDR0-3, PhyID/device protocol/sas_addr/SIG/wide_port */
> +	__le32		DevInfo;
> +	__le64		DevSASAddr;
> +	__le32		AttDevInfo;
> +	__le64		AttDevSASAddr;
> +	u8	   		Type;
> +	u8	   		WidePortPhyMap;
> +	u8	   		Reserved0[2];
> +/* from PORT_PHY_CONTROL0-3, REG_PORT_PHY_CONTROL, linkrate, current status */
> +	__le32		PhyStatus;
> +/* from PORT_IRQ_MASK0-3 */
> +	__le32		IRQStatus;
>  	u8			frame_rcvd[24 + 1024];
>  };
> 
> @@ -447,10 +542,19 @@
> 
>  					/* further per-slot information */
>  	struct mvs_slot_info	slot_info[MVS_SLOTS];
> -	unsigned long		tags[(MVS_SLOTS / sizeof(unsigned long)) + 1];
> -
> +	unsigned long		tags[MVS_SLOTS];
>  	struct mvs_phy		phy[MVS_MAX_PHYS];
>  	struct mvs_port		port[MVS_MAX_PHYS];
> +
> +	u32					can_queue;	/* per adapter */
> +	u32					tag_out;	/*Get*/
> +	u32					tag_in;		/*Give*/
> +};
> +
> +struct mvs_queue_task {
> +	struct list_head list;
> +
> +	void   *uldd_task;
>  };
> 
>  static struct scsi_transport_template *mvs_stt;
> @@ -464,27 +568,128 @@
>  static struct scsi_host_template mvs_sht = {
>  	.module			= THIS_MODULE,
>  	.name			= DRV_NAME,
> -	.queuecommand		= sas_queuecommand,
> +	.queuecommand		= sas_queuecommand,		

This change is just adding whitespace at the end, which is unnecessary.
If you run your patch through scripts/checkpatch.pl in the linux kernel
directory, it will pick up things like this.

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

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

I didn't see any sector length limitations on the transfers in this
driver ... is 256 really the max number of sectors per command?

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


I'll look more into the rest later.

Thanks,

James


-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux