On Mon, 2005-05-16 at 02:59 -0400, Bagalkote, Sreenivas wrote: > + * This program is free software; you can redistribute it > and/or your mailer corrupts patches > + > +/** > + * MegaRAID SAS supported controllers > + */ > +#define PCI_DEVICE_LSI_1064 0x0411 > + > +#define PCI_DEVICE_ID_DELL_PERC5 0x0015 > +#define PCI_DEVICE_ID_DELL_SAS5 0x0054 > + > +#define PCI_SUBSYSTEM_DELL_PERC5E 0x1F01 > +#define PCI_SUBSYSTEM_DELL_PERC5I 0x1F02 > +#define PCI_SUBSYSTEM_DELL_PERC5I_INTEGRATED 0x1F03 > +#define PCI_SUBSYSTEM_DELL_SAS5I 0x1F05 > +#define PCI_SUBSYSTEM_DELL_SAS5I_INTEGRATED 0x1F06 > + > +#define PCI_SUB_DEVICEID_FSC 0x1081 > +#define PCI_SUB_VENDORID_FSC 0x1734 these really need to go into the generic pci id header > +/* > + * SAS controller information > + */ > +struct megasas_ctrl_info { > + > + /* > + * PCI device information > + */ > + struct { > + > + u16 vendor_id; > + u16 device_id; > + u16 sub_vendor_id; > + u16 sub_device_id; > + u8 reserved[24]; > + > + } __attribute__ ((packed)) pci; I hope you're not going to store pci config space into this; you really need to use the pci-> versions of these. Not doing so doesn't allow the kernel to compensate for quirks (like chipsets byteswapping config space ;) > + u32 current_fw_time; what is this for ? > + * All MFI register set macros accept megasas_register_set* > + */ > +#define RD_OB_MSG_0(regs) readl((void*)(&(regs)->outbound_msg_0)) > +#define WR_IN_MSG_0(v, regs) writel((v),(void*)(&(regs)->inbound_msg_0)) > +#define WR_IN_DOORBELL(v, regs) > writel((v),(void*)(&(regs)->inbound_doorbell)) > +#define WR_IN_QPORT(v, regs) > writel((v),(void*)(&(regs)->inbound_queue_port)) > + > +#define RD_OB_INTR_STATUS(regs) > readl((void*)(&(regs)->outbound_intr_status)) > +#define WR_OB_INTR_STATUS(v, regs) > writel((v),(&(regs)->outbound_intr_status)) why these (wrapped) abstractions ? > +#define MFI_ENABLE_INTR(regs) > writel(1,(void*)(&(regs)->outbound_intr_mask)) > + > +#define MFI_DISABLE_INTR(regs) \ > +{ \ > + u32 disable = ~0x00000001; \ > + u32 mask = readl((void*)(&(regs)->outbound_intr_mask)); \ > + mask &= disable; \ > + writel(mask, (void*)(&(regs)->outbound_intr_mask)); \ > +} you most likely want a PCI posting flush in this macro. Also I would actually suggest you make this an (inline) function.. much nicer. It's 4 lines already after all. > + > + > +struct megasas_register_set { > + > + u32 reserved_0[4]; /*0000h*/ > + > + u32 inbound_msg_0; /*0010h*/ > + u32 inbound_msg_1; /*0014h*/ > + u32 outbound_msg_0; /*0018h*/ > + u32 outbound_msg_1; /*001Ch*/ > + > + u32 inbound_doorbell; /*0020h*/ > + u32 inbound_intr_status; /*0024h*/ > + u32 inbound_intr_mask; /*0028h*/ > + > + u32 outbound_doorbell; /*002Ch*/ > + u32 outbound_intr_status; /*0030h*/ > + u32 outbound_intr_mask; /*0034h*/ > + > + u32 reserved_1[2]; /*0038h*/ > + > + u32 inbound_queue_port; /*0040h*/ > + u32 outbound_queue_port; /*0044h*/ > + > + u32 reserved_2; /*004Ch*/ > + > + u32 index_registers[1004]; /*0050h*/ > + > +} __attribute__ ((packed)); I guess these are shared with the hardware... isn't it better to use le32/be32 types then to have them have declared endianness ? > + > +struct megasas_sge32 { > + > + u32 phys_addr; > + u32 length; > + > +} __attribute__ ((packed)); same here and for all other hardware-shared structs > + > +/* > + * Various conversion macros from SCSI command > + */ ehhhh why? to make the driver unreadable ? > +#define SCP2HOST(scp) (scp)->device->host // to host > +#define SCP2HOSTDATA(scp) SCP2HOST(scp)->hostdata // to soft state > +#define SCP2CHANNEL(scp) (scp)->device->channel // to channel > +#define SCP2TARGET(scp) (scp)->device->id // to target > +#define SCP2LUN(scp) (scp)->device->lun // to LUN > + > +#define SCSIHOST2ADAP(host) (((caddr_t *)(host->hostdata))[0]) please don't use caddr_t in kerenl mode > +#define SCP2ADAPTER(scp) (struct > megasas_instance*)SCSIHOST2ADAP(SCP2HOST(scp)) please if you think you need casts consider an inline function which supports proper typing of it's argument. Casting is a *great* way to hide bugs otherwise - : 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