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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
