On Mon, Oct 12, 2020 at 03:51:22PM -0700, James Smart wrote:
> This patch continues the libefc_sli SLI-4 library population.
>
> This patch adds routines to create mailbox commands used during
> adapter initialization and adds APIs to issue mailbox commands to the
> adapter through the bootstrap mailbox register.
>
> Co-developed-by: Ram Vegesna <ram.vegesna@xxxxxxxxxxxx>
> Signed-off-by: Ram Vegesna <ram.vegesna@xxxxxxxxxxxx>
> Signed-off-by: James Smart <james.smart@xxxxxxxxxxxx>
Two small nitpicks but the rest looks good.
Reviewed-by: Daniel Wagner <dwagner@xxxxxxx>
> +int
> +sli_cmd_read_sparm64(struct sli4 *sli4, void *buf, struct efc_dma *dma, u16 vpi)
> +{
> + struct sli4_cmd_read_sparm64 *read_sparm64 = buf;
> +
> + memset(buf, 0, SLI4_BMBX_SIZE);
> +
> + if (vpi == U16_MAX) {
> + efc_log_err(sli4, "special VPI not supported!!!\n");
> + return EFC_FAIL;
> + }
> +
> + if (!dma || !dma->phys) {
> + efc_log_err(sli4, "bad DMA buffer\n");
> + return EFC_FAIL;
> + }
> +
The memset should be after the two ifs.
> + read_sparm64->hdr.command = SLI4_MBX_CMD_READ_SPARM64;
> +
> + read_sparm64->bde_64.bde_type_buflen =
> + cpu_to_le32((SLI4_BDE_TYPE_VAL(64)) |
> + (dma->size & SLI4_BDE_LEN_MASK));
> + read_sparm64->bde_64.u.data.low =
> + cpu_to_le32(lower_32_bits(dma->phys));
> + read_sparm64->bde_64.u.data.high =
> + cpu_to_le32(upper_32_bits(dma->phys));
> +
> + read_sparm64->vpi = cpu_to_le16(vpi);
> +
> + return EFC_SUCCESS;
> +}
> +int
> +sli_cmd_reg_fcfi_mrq(struct sli4 *sli4, void *buf, u8 mode, u16 fcf_index,
> + u8 rq_selection_policy, u8 mrq_bit_mask, u16 num_mrqs,
> + struct sli4_cmd_rq_cfg rq_cfg[SLI4_CMD_REG_FCFI_NUM_RQ_CFG])
Are these arguments list not getting a little bit big? If I got it right,
sizeof(struct sli4_cmd_rq_cfg) * SLI4_CMD_REG_FCFI_NUM_RQ_CFG = 6 * 4 =
24 bytes. As it is used, passing in a pointer would do the trick as well.
> +{
> + struct sli4_cmd_reg_fcfi_mrq *reg_fcfi_mrq = buf;
> + u32 i;
> + u32 mrq_flags = 0;
> +
> + memset(buf, 0, SLI4_BMBX_SIZE);
> +
> + reg_fcfi_mrq->hdr.command = SLI4_MBX_CMD_REG_FCFI_MRQ;
> + if (mode == SLI4_CMD_REG_FCFI_SET_FCFI_MODE) {
> + reg_fcfi_mrq->fcf_index = cpu_to_le16(fcf_index);
> + goto done;
> + }
Could the memset be after the if?
