On 12/20/19 11:36 PM, James Smart wrote:
> This patch continues the libefc_sli SLI-4 library population.
>
> This patch adds definitions for SLI-4 mailbox commands
> and responses.
>
> Signed-off-by: Ram Vegesna <ram.vegesna@xxxxxxxxxxxx>
> Signed-off-by: James Smart <jsmart2021@xxxxxxxxx>
> ---
> drivers/scsi/elx/libefc_sli/sli4.h | 1728 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 1727 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/elx/libefc_sli/sli4.h b/drivers/scsi/elx/libefc_sli/sli4.h
> index f86a9e72ed43..c9bd3f71b27b 100644
> --- a/drivers/scsi/elx/libefc_sli/sli4.h
> +++ b/drivers/scsi/elx/libefc_sli/sli4.h
> @@ -1995,7 +1995,7 @@ struct sli4_fc_xri_aborted_cqe {
> #define SLI4_ELS_REQUEST64_DIR_READ 0x1
>
> #define SLI4_ELS_REQUEST64_OTHER 0x0
> -#define SLI4_ELS_REQUEST64_LOGO 0x1
> +#define SLI4_ELS_REQUEST64_LOGO 0x1
> #define SLI4_ELS_REQUEST64_FDISC 0x2
> #define SLI4_ELS_REQUEST64_FLOGIN 0x3
> #define SLI4_ELS_REQUEST64_PLOGI 0x4
Shouldn't this rather be merged with the previous patch?
> @@ -2004,4 +2004,1730 @@ struct sli4_fc_xri_aborted_cqe {
> #define SLI4_ELS_REQUEST64_CMD_NON_FABRIC 0x0c
> #define SLI4_ELS_REQUEST64_CMD_FABRIC 0x0d
>
> +#define SLI_PAGE_SIZE (1 << 12) /* 4096 */
> +#define SLI_SUB_PAGE_MASK (SLI_PAGE_SIZE - 1)
> +#define SLI_ROUND_PAGE(b) (((b) + SLI_SUB_PAGE_MASK) & ~SLI_SUB_PAGE_MASK)
> +
> +#define SLI4_BMBX_TIMEOUT_MSEC 30000
> +#define SLI4_FW_READY_TIMEOUT_MSEC 30000
> +
> +#define SLI4_BMBX_DELAY_US 1000 /* 1 ms */
> +#define SLI4_INIT_PORT_DELAY_US 10000 /* 10 ms */
> +
> +static inline u32
> +sli_page_count(size_t bytes, u32 page_size)
> +{
> + if (!page_size)
> + return 0;
> +
> + return (bytes + (page_size - 1)) >> __ffs(page_size);
> +}
> +
> +/*************************************************************************
> + * SLI-4 mailbox command formats and definitions
> + */
> +
> +struct sli4_mbox_command_header {
> + u8 resvd0;
> + u8 command;
> + __le16 status; /* Port writes to indicate success/fail */
> +};
> +
> +enum {
> + MBX_CMD_CONFIG_LINK = 0x07,
> + MBX_CMD_DUMP = 0x17,
> + MBX_CMD_DOWN_LINK = 0x06,
> + MBX_CMD_INIT_LINK = 0x05,
> + MBX_CMD_INIT_VFI = 0xa3,
> + MBX_CMD_INIT_VPI = 0xa4,
> + MBX_CMD_POST_XRI = 0xa7,
> + MBX_CMD_RELEASE_XRI = 0xac,
> + MBX_CMD_READ_CONFIG = 0x0b,
> + MBX_CMD_READ_STATUS = 0x0e,
> + MBX_CMD_READ_NVPARMS = 0x02,
> + MBX_CMD_READ_REV = 0x11,
> + MBX_CMD_READ_LNK_STAT = 0x12,
> + MBX_CMD_READ_SPARM64 = 0x8d,
> + MBX_CMD_READ_TOPOLOGY = 0x95,
> + MBX_CMD_REG_FCFI = 0xa0,
> + MBX_CMD_REG_FCFI_MRQ = 0xaf,
> + MBX_CMD_REG_RPI = 0x93,
> + MBX_CMD_REG_RX_RQ = 0xa6,
> + MBX_CMD_REG_VFI = 0x9f,
> + MBX_CMD_REG_VPI = 0x96,
> + MBX_CMD_RQST_FEATURES = 0x9d,
> + MBX_CMD_SLI_CONFIG = 0x9b,
> + MBX_CMD_UNREG_FCFI = 0xa2,
> + MBX_CMD_UNREG_RPI = 0x14,
> + MBX_CMD_UNREG_VFI = 0xa1,
> + MBX_CMD_UNREG_VPI = 0x97,
> + MBX_CMD_WRITE_NVPARMS = 0x03,
> + MBX_CMD_CFG_AUTO_XFER_RDY = 0xAD,
> +
> + MBX_STATUS_SUCCESS = 0x0000,
> + MBX_STATUS_FAILURE = 0x0001,
> + MBX_STATUS_RPI_NOT_REG = 0x1400,
> +};
> +
Make this two enums, one 'enum sli4_mbx_cmd' and one 'enum sli4_mbx_status'.
> +/* CONFIG_LINK */
> +enum {
> + SLI4_CFG_LINK_BBSCN = 0xf00,
> + SLI4_CFG_LINK_CSCN = 0x1000,
> +};
> +
> +struct sli4_cmd_config_link {
> + struct sli4_mbox_command_header hdr;
> + u8 maxbbc;
> + u8 rsvd5;
> + u8 rsvd6;
> + u8 rsvd7;
> + u8 alpa;
> + __le16 n_port_id;
> + u8 rsvd11;
> + __le32 rsvd12;
> + __le32 e_d_tov;
> + __le32 lp_tov;
> + __le32 r_a_tov;
> + __le32 r_t_tov;
> + __le32 al_tov;
> + __le32 rsvd36;
> + __le32 bbscn_dword;
> +};
> +
> +enum {
> + SLI4_DUMP4_TYPE = 0xf,
> +};
Single enum should rather be converted into a #define ..
> +
> +#define SLI4_WKI_TAG_SAT_TEM 0x1040
> +
> +struct sli4_cmd_dump4 {
> + struct sli4_mbox_command_header hdr;
> + __le32 type_dword;
> + __le16 wki_selection;
> + __le16 rsvd10;
> + __le32 rsvd12;
> + __le32 returned_byte_cnt;
> + __le32 resp_data[59];
> +};
> +
> +/* INIT_LINK - initialize the link for a FC port */
> +#define FC_TOPOLOGY_FCAL 0
> +#define FC_TOPOLOGY_P2P 1
> +
> +#define SLI4_INIT_LINK_F_LOOP_BACK (1 << 0)
> +#define SLI4_INIT_LINK_F_UNFAIR (1 << 6)
> +#define SLI4_INIT_LINK_F_NO_LIRP (1 << 7)
> +#define SLI4_INIT_LINK_F_LOOP_VALID_CHK (1 << 8)
> +#define SLI4_INIT_LINK_F_NO_LISA (1 << 9)
> +#define SLI4_INIT_LINK_F_FAIL_OVER (1 << 10)
> +#define SLI4_INIT_LINK_F_NO_AUTOSPEED (1 << 11)
> +#define SLI4_INIT_LINK_F_PICK_HI_ALPA (1 << 15)
> +
> +#define SLI4_INIT_LINK_F_P2P_ONLY 1
> +#define SLI4_INIT_LINK_F_FCAL_ONLY 2
> +
> +#define SLI4_INIT_LINK_F_FCAL_FAIL_OVER 0
> +#define SLI4_INIT_LINK_F_P2P_FAIL_OVER 1
> +
> +enum {
> + SLI4_INIT_LINK_SEL_RESET_AL_PA = 0xff,
> + SLI4_INIT_LINK_FLAG_LOOPBACK = 0x1,
> + SLI4_INIT_LINK_FLAG_TOPOLOGY = 0x6,
> + SLI4_INIT_LINK_FLAG_UNFAIR = 0x40,
> + SLI4_INIT_LINK_FLAG_SKIP_LIRP_LILP = 0x80,
> + SLI4_INIT_LINK_FLAG_LOOP_VALIDITY = 0x100,
> + SLI4_INIT_LINK_FLAG_SKIP_LISA = 0x200,
> + SLI4_INIT_LINK_FLAG_EN_TOPO_FAILOVER = 0x400,
> + SLI4_INIT_LINK_FLAG_FIXED_SPEED = 0x800,
> + SLI4_INIT_LINK_FLAG_SEL_HIGHTEST_AL_PA = 0x8000,
> +};
> +
Why is this an enum, and the above SLI4_INIT_LINK_F_XXX value are defines?
Please be consistent.
And this applies throughout the remainder of the patch.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
