Hi,
On Sat, Apr 11, 2020 at 08:32:36PM -0700, James Smart wrote:
> This patch continues the libefc_sli SLI-4 library population.
>
> This patch adds service routines to create mailbox commands
> and adds APIs to create/destroy/parse SLI-4 EQ, CQ, RQ and MQ queues.
>
> Signed-off-by: Ram Vegesna <ram.vegesna@xxxxxxxxxxxx>
> Signed-off-by: James Smart <jsmart2021@xxxxxxxxx>
>
> ---
> v3:
> Removed efc_assert define. Replaced with WARN_ON.
> Returned defined return values EFC_SUCCESS/FAIL
> ---
> drivers/scsi/elx/include/efc_common.h | 18 +
> drivers/scsi/elx/libefc_sli/sli4.c | 1514 +++++++++++++++++++++++++++++++++
> drivers/scsi/elx/libefc_sli/sli4.h | 9 +
> 3 files changed, 1541 insertions(+)
>
> diff --git a/drivers/scsi/elx/include/efc_common.h b/drivers/scsi/elx/include/efc_common.h
> index c427f75da4d5..4c7574dacb99 100644
> --- a/drivers/scsi/elx/include/efc_common.h
> +++ b/drivers/scsi/elx/include/efc_common.h
> @@ -22,4 +22,22 @@ struct efc_dma {
> struct pci_dev *pdev;
> };
>
> +#define efc_log_crit(efc, fmt, args...) \
> + dev_crit(&((efc)->pcidev)->dev, fmt, ##args)
> +
> +#define efc_log_err(efc, fmt, args...) \
> + dev_err(&((efc)->pcidev)->dev, fmt, ##args)
> +
> +#define efc_log_warn(efc, fmt, args...) \
> + dev_warn(&((efc)->pcidev)->dev, fmt, ##args)
> +
> +#define efc_log_info(efc, fmt, args...) \
> + dev_info(&((efc)->pcidev)->dev, fmt, ##args)
> +
> +#define efc_log_test(efc, fmt, args...) \
> + dev_dbg(&((efc)->pcidev)->dev, fmt, ##args)
> +
> +#define efc_log_debug(efc, fmt, args...) \
> + dev_dbg(&((efc)->pcidev)->dev, fmt, ##args)
> +
> #endif /* __EFC_COMMON_H__ */
> diff --git a/drivers/scsi/elx/libefc_sli/sli4.c b/drivers/scsi/elx/libefc_sli/sli4.c
> index 29d33becd334..224a06610c78 100644
> --- a/drivers/scsi/elx/libefc_sli/sli4.c
> +++ b/drivers/scsi/elx/libefc_sli/sli4.c
> @@ -24,3 +24,1517 @@ static struct sli4_asic_entry_t sli4_asic_table[] = {
> { SLI4_ASIC_REV_A3, SLI4_ASIC_GEN_6},
> { SLI4_ASIC_REV_A1, SLI4_ASIC_GEN_7},
> };
> +
> +/* Convert queue type enum (SLI_QTYPE_*) into a string */
> +static char *SLI_QNAME[] = {
> + "Event Queue",
> + "Completion Queue",
> + "Mailbox Queue",
> + "Work Queue",
> + "Receive Queue",
> + "Undefined"
> +};
> +
> +/*
> + * Write a SLI_CONFIG command to the provided buffer.
> + *
> + * @sli4 SLI context pointer.
> + * @buf Destination buffer for the command.
> + * @size size of the destination buffer(buf).
> + * @length Length in bytes of attached command.
> + * @dma DMA buffer for non-embedded commands.
> + *
> + */
Turn this into kerneldoc style.
> +static void *
> +sli_config_cmd_init(struct sli4 *sli4, void *buf,
> + size_t size, u32 length,
> + struct efc_dma *dma)
> +{
> + struct sli4_cmd_sli_config *config = NULL;
Not needed, config gets buf assigned.
> + u32 flags = 0;
> +
> + if (length > sizeof(config->payload.embed) && !dma) {
> + efc_log_err(sli4, "Too big for an embedded cmd with len(%d)\n",
> + length);
> + return NULL;
> + }
> +
> + config = buf;
> +
> + memset(buf, 0, size);
Maybe do the memset first and then do the config = buf assignemnt.
> +
> + config->hdr.command = MBX_CMD_SLI_CONFIG;
> + if (!dma) {
> + flags |= SLI4_SLICONF_EMB;
> + config->dw1_flags = cpu_to_le32(flags);
> + config->payload_len = cpu_to_le32(length);
> + buf += offsetof(struct sli4_cmd_sli_config, payload.embed);
> + return buf;
> + }
> +
> + flags = SLI4_SLICONF_PMDCMD_VAL_1;
> + flags &= ~SLI4_SLICONF_EMB;
> + config->dw1_flags = cpu_to_le32(flags);
> +
> + config->payload.mem.addr.low = cpu_to_le32(lower_32_bits(dma->phys));
> + config->payload.mem.addr.high = cpu_to_le32(upper_32_bits(dma->phys));
> + config->payload.mem.length =
> + cpu_to_le32(dma->size & SLI4_SLICONFIG_PMD_LEN);
> + config->payload_len = cpu_to_le32(dma->size);
> + /* save pointer to DMA for BMBX dumping purposes */
> + sli4->bmbx_non_emb_pmd = dma;
> + return dma->virt;
> +}
> +
> +/*
> + * Write a COMMON_CREATE_CQ command.
> + *
> + * This creates a Version 2 message.
> + *
> + * Returns 0 on success, or non-zero otherwise.
> + */
kerneldoc style
> +static int
> +sli_cmd_common_create_cq(struct sli4 *sli4, void *buf, size_t size,
> + struct efc_dma *qmem,
> + u16 eq_id)
> +{
> + struct sli4_rqst_cmn_create_cq_v2 *cqv2 = NULL;
> + u32 p;
> + uintptr_t addr;
> + u32 page_bytes = 0;
> + u32 num_pages = 0;
> + size_t cmd_size = 0;
> + u32 page_size = 0;
> + u32 n_cqe = 0;
> + u32 dw5_flags = 0;
> + u16 dw6w1_arm = 0;
> + __le32 len;
> +
> + /* First calculate number of pages and the mailbox cmd length */
> + n_cqe = qmem->size / SLI4_CQE_BYTES;
> + switch (n_cqe) {
> + case 256:
> + case 512:
> + case 1024:
> + case 2048:
> + page_size = 1;
> + break;
> + case 4096:
> + page_size = 2;
> + break;
> + default:
> + return EFC_FAIL;
> + }
> + page_bytes = page_size * SLI_PAGE_SIZE;
page_size is confusing variable name, multiplying page_size with
SLI_PAGE_SIZE gives bytes.
> + num_pages = sli_page_count(qmem->size, page_bytes);
> +
> + cmd_size = CFG_RQST_CMDSZ(cmn_create_cq_v2) + SZ_DMAADDR * num_pages;
> +
> + cqv2 = sli_config_cmd_init(sli4, buf, size, cmd_size, NULL);
> + if (!cqv2)
> + return EFC_FAIL;
> +
> + len = CFG_RQST_PYLD_LEN_VAR(cmn_create_cq_v2,
> + SZ_DMAADDR * num_pages);
> + sli_cmd_fill_hdr(&cqv2->hdr, CMN_CREATE_CQ, SLI4_SUBSYSTEM_COMMON,
> + CMD_V2, len);
> + cqv2->page_size = page_size;
> +
> + /* valid values for number of pages: 1, 2, 4, 8 (sec 4.4.3) */
> + cqv2->num_pages = cpu_to_le16(num_pages);
> + if (!num_pages || num_pages > SLI4_CMN_CREATE_CQ_V2_MAX_PAGES)
> + return EFC_FAIL;
> +
> + switch (num_pages) {
> + case 1:
> + dw5_flags |= CQ_CNT_VAL(256);
> + break;
> + case 2:
> + dw5_flags |= CQ_CNT_VAL(512);
> + break;
> + case 4:
> + dw5_flags |= CQ_CNT_VAL(1024);
> + break;
> + case 8:
> + dw5_flags |= CQ_CNT_VAL(LARGE);
> + cqv2->cqe_count = cpu_to_le16(n_cqe);
> + break;
> + default:
> + efc_log_err(sli4, "num_pages %d not valid\n", num_pages);
> + return EFC_FAIL;
> + }
> +
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + dw5_flags |= CREATE_CQV2_AUTOVALID;
> +
> + dw5_flags |= CREATE_CQV2_EVT;
> + dw5_flags |= CREATE_CQV2_VALID;
> +
> + cqv2->dw5_flags = cpu_to_le32(dw5_flags);
> + cqv2->dw6w1_arm = cpu_to_le16(dw6w1_arm);
> + cqv2->eq_id = cpu_to_le16(eq_id);
> +
> + for (p = 0, addr = qmem->phys; p < num_pages; p++, addr += page_bytes) {
> + cqv2->page_phys_addr[p].low = cpu_to_le32(lower_32_bits(addr));
> + cqv2->page_phys_addr[p].high = cpu_to_le32(upper_32_bits(addr));
> + }
> +
> + return EFC_SUCCESS;
> +}
> +
> +/* Write a COMMON_CREATE_EQ command */
> +static int
> +sli_cmd_common_create_eq(struct sli4 *sli4, void *buf, size_t size,
> + struct efc_dma *qmem)
> +{
> + struct sli4_rqst_cmn_create_eq *eq;
> + u32 p;
> + uintptr_t addr;
> + u16 num_pages;
> + u32 dw5_flags = 0;
> + u32 dw6_flags = 0, ver = CMD_V0;
Maybe initialize ver below when for SLI4_INTF_IF_TYPE_6 is
tested. That would keep the setting of ver in one place.
> +
> + eq = sli_config_cmd_init(sli4, buf, size,
> + SLI_CONFIG_PYLD_LENGTH(cmn_create_eq), NULL);
> + if (!eq)
> + return EFC_FAIL;
> +
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + ver = CMD_V2;
> +
> + sli_cmd_fill_hdr(&eq->hdr, CMN_CREATE_EQ, SLI4_SUBSYSTEM_COMMON,
> + ver, CFG_RQST_PYLD_LEN(cmn_create_eq));
> +
> + /* valid values for number of pages: 1, 2, 4 (sec 4.4.3) */
> + num_pages = qmem->size / SLI_PAGE_SIZE;
> + eq->num_pages = cpu_to_le16(num_pages);
> +
> + switch (num_pages) {
> + case 1:
> + dw5_flags |= SLI4_EQE_SIZE_4;
> + dw6_flags |= EQ_CNT_VAL(1024);
> + break;
> + case 2:
> + dw5_flags |= SLI4_EQE_SIZE_4;
> + dw6_flags |= EQ_CNT_VAL(2048);
> + break;
> + case 4:
> + dw5_flags |= SLI4_EQE_SIZE_4;
> + dw6_flags |= EQ_CNT_VAL(4096);
> + break;
> + default:
> + efc_log_err(sli4, "num_pages %d not valid\n", num_pages);
> + return EFC_FAIL;
> + }
> +
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + dw5_flags |= CREATE_EQ_AUTOVALID;
> +
> + dw5_flags |= CREATE_EQ_VALID;
> + dw6_flags &= (~CREATE_EQ_ARM);
> + eq->dw5_flags = cpu_to_le32(dw5_flags);
> + eq->dw6_flags = cpu_to_le32(dw6_flags);
> + eq->dw7_delaymulti = cpu_to_le32(CREATE_EQ_DELAYMULTI);
> +
> + for (p = 0, addr = qmem->phys; p < num_pages;
> + p++, addr += SLI_PAGE_SIZE) {
> + eq->page_address[p].low = cpu_to_le32(lower_32_bits(addr));
> + eq->page_address[p].high = cpu_to_le32(upper_32_bits(addr));
> + }
> +
> + return EFC_SUCCESS;
> +}
> +
> +static int
> +sli_cmd_common_create_mq_ext(struct sli4 *sli4, void *buf, size_t size,
> + struct efc_dma *qmem,
> + u16 cq_id)
> +{
> + struct sli4_rqst_cmn_create_mq_ext *mq;
> + u32 p;
> + uintptr_t addr;
> + u32 num_pages;
> + u16 dw6w1_flags = 0;
> +
> + mq = sli_config_cmd_init(sli4, buf, size,
> + SLI_CONFIG_PYLD_LENGTH(cmn_create_mq_ext),
> + NULL);
> + if (!mq)
> + return EFC_FAIL;
> +
> + sli_cmd_fill_hdr(&mq->hdr, CMN_CREATE_MQ_EXT, SLI4_SUBSYSTEM_COMMON,
> + CMD_V0, CFG_RQST_PYLD_LEN(cmn_create_mq_ext));
> +
> + /* valid values for number of pages: 1, 2, 4, 8 (sec 4.4.12) */
> + num_pages = qmem->size / SLI_PAGE_SIZE;
> + mq->num_pages = cpu_to_le16(num_pages);
> + switch (num_pages) {
> + case 1:
> + dw6w1_flags |= SLI4_MQE_SIZE_16;
> + break;
> + case 2:
> + dw6w1_flags |= SLI4_MQE_SIZE_32;
> + break;
> + case 4:
> + dw6w1_flags |= SLI4_MQE_SIZE_64;
> + break;
> + case 8:
> + dw6w1_flags |= SLI4_MQE_SIZE_128;
> + break;
> + default:
> + efc_log_info(sli4, "num_pages %d not valid\n", num_pages);
> + return EFC_FAIL;
> + }
> +
> + mq->async_event_bitmap = cpu_to_le32(SLI4_ASYNC_EVT_FC_ALL);
> +
> + if (sli4->mq_create_version) {
> + mq->cq_id_v1 = cpu_to_le16(cq_id);
> + mq->hdr.dw3_version = cpu_to_le32(CMD_V1);
> + } else {
> + dw6w1_flags |= (cq_id << CREATE_MQEXT_CQID_SHIFT);
> + }
> + mq->dw7_val = cpu_to_le32(CREATE_MQEXT_VAL);
> +
> + mq->dw6w1_flags = cpu_to_le16(dw6w1_flags);
> + for (p = 0, addr = qmem->phys; p < num_pages;
> + p++, addr += SLI_PAGE_SIZE) {
> + mq->page_phys_addr[p].low = cpu_to_le32(lower_32_bits(addr));
> + mq->page_phys_addr[p].high = cpu_to_le32(upper_32_bits(addr));
> + }
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_cmd_wq_create(struct sli4 *sli4, void *buf, size_t size,
> + struct efc_dma *qmem, u16 cq_id)
> +{
> + struct sli4_rqst_wq_create *wq;
> + u32 p;
> + uintptr_t addr;
> + u32 page_size = 0;
> + u32 page_bytes = 0;
> + u32 n_wqe = 0;
> + u16 num_pages;
> +
> + wq = sli_config_cmd_init(sli4, buf, size,
> + SLI_CONFIG_PYLD_LENGTH(wq_create), NULL);
> + if (!wq)
> + return EFC_FAIL;
> +
> + sli_cmd_fill_hdr(&wq->hdr, SLI4_OPC_WQ_CREATE, SLI4_SUBSYSTEM_FC,
> + CMD_V1, CFG_RQST_PYLD_LEN(wq_create));
> + n_wqe = qmem->size / sli4->wqe_size;
> +
> + switch (qmem->size) {
> + case 4096:
> + case 8192:
> + case 16384:
> + case 32768:
> + page_size = 1;
> + break;
> + case 65536:
> + page_size = 2;
> + break;
> + case 131072:
> + page_size = 4;
> + break;
> + case 262144:
> + page_size = 8;
> + break;
> + case 524288:
> + page_size = 10;
> + break;
> + default:
> + return EFC_FAIL;
> + }
> + page_bytes = page_size * SLI_PAGE_SIZE;
same comment as above about the page_size variable name.
> +
> + /* valid values for number of pages(num_pages): 1-8 */
> + num_pages = sli_page_count(qmem->size, page_bytes);
> + wq->num_pages = cpu_to_le16(num_pages);
> + if (!num_pages || num_pages > SLI4_WQ_CREATE_MAX_PAGES)
> + return EFC_FAIL;
> +
> + wq->cq_id = cpu_to_le16(cq_id);
> +
> + wq->page_size = page_size;
> +
> + if (sli4->wqe_size == SLI4_WQE_EXT_BYTES)
> + wq->wqe_size_byte |= SLI4_WQE_EXT_SIZE;
> + else
> + wq->wqe_size_byte |= SLI4_WQE_SIZE;
> +
> + wq->wqe_count = cpu_to_le16(n_wqe);
> +
> + for (p = 0, addr = qmem->phys; p < num_pages; p++, addr += page_bytes) {
> + wq->page_phys_addr[p].low = cpu_to_le32(lower_32_bits(addr));
> + wq->page_phys_addr[p].high = cpu_to_le32(upper_32_bits(addr));
> + }
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_cmd_rq_create(struct sli4 *sli4, void *buf, size_t size,
> + struct efc_dma *qmem,
> + u16 cq_id, u16 buffer_size)
I think sli_cmd_rq_create() should be called sli_cmd_rq_create_v0() to
match sli_cmd_rq_create_{v1|v2}() and make it more explicit in the API
which version of this function is.
> +{
> + struct sli4_rqst_rq_create *rq;
> + u32 p;
> + uintptr_t addr;
> + u16 num_pages;
> +
> + rq = sli_config_cmd_init(sli4, buf, size,
> + SLI_CONFIG_PYLD_LENGTH(rq_create), NULL);
> + if (!rq)
> + return EFC_FAIL;
> +
> + sli_cmd_fill_hdr(&rq->hdr, SLI4_OPC_RQ_CREATE, SLI4_SUBSYSTEM_FC,
> + CMD_V0, CFG_RQST_PYLD_LEN(rq_create));
> + /* valid values for number of pages: 1-8 (sec 4.5.6) */
> + num_pages = sli_page_count(qmem->size, SLI_PAGE_SIZE);
> + rq->num_pages = cpu_to_le16(num_pages);
> + if (!num_pages ||
> + num_pages > SLI4_RQ_CREATE_V0_MAX_PAGES) {
> + efc_log_info(sli4, "num_pages %d not valid\n", num_pages);
> + return EFC_FAIL;
> + }
> +
> + /*
> + * RQE count is the log base 2 of the total number of entries
> + */
> + rq->rqe_count_byte |= 31 - __builtin_clz(qmem->size / SLI4_RQE_SIZE);
> +
> + if (buffer_size < SLI4_RQ_CREATE_V0_MIN_BUF_SIZE ||
> + buffer_size > SLI4_RQ_CREATE_V0_MAX_BUF_SIZE) {
> + efc_log_err(sli4, "buffer_size %d out of range (%d-%d)\n",
> + buffer_size,
> + SLI4_RQ_CREATE_V0_MIN_BUF_SIZE,
> + SLI4_RQ_CREATE_V0_MAX_BUF_SIZE);
> + return EFC_FAIL;
> + }
> + rq->buffer_size = cpu_to_le16(buffer_size);
> +
> + rq->cq_id = cpu_to_le16(cq_id);
> +
> + for (p = 0, addr = qmem->phys; p < num_pages;
> + p++, addr += SLI_PAGE_SIZE) {
> + rq->page_phys_addr[p].low = cpu_to_le32(lower_32_bits(addr));
> + rq->page_phys_addr[p].high = cpu_to_le32(upper_32_bits(addr));
> + }
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_cmd_rq_create_v1(struct sli4 *sli4, void *buf, size_t size,
> + struct efc_dma *qmem, u16 cq_id,
> + u16 buffer_size)
> +{
> + struct sli4_rqst_rq_create_v1 *rq;
> + u32 p;
> + uintptr_t addr;
> + u32 num_pages;
> +
> + rq = sli_config_cmd_init(sli4, buf, size,
> + SLI_CONFIG_PYLD_LENGTH(rq_create_v1), NULL);
> + if (!rq)
> + return EFC_FAIL;
> +
> + sli_cmd_fill_hdr(&rq->hdr, SLI4_OPC_RQ_CREATE, SLI4_SUBSYSTEM_FC,
> + CMD_V1, CFG_RQST_PYLD_LEN(rq_create_v1));
> + /* Disable "no buffer warnings" to avoid Lancer bug */
> + rq->dim_dfd_dnb |= SLI4_RQ_CREATE_V1_DNB;
> +
> + /* valid values for number of pages: 1-8 (sec 4.5.6) */
> + num_pages = sli_page_count(qmem->size, SLI_PAGE_SIZE);
> + rq->num_pages = cpu_to_le16(num_pages);
> + if (!num_pages ||
> + num_pages > SLI4_RQ_CREATE_V1_MAX_PAGES) {
> + efc_log_info(sli4, "num_pages %d not valid, max %d\n",
> + num_pages, SLI4_RQ_CREATE_V1_MAX_PAGES);
> + return EFC_FAIL;
> + }
> +
> + /*
> + * RQE count is the total number of entries (note not lg2(# entries))
> + */
> + rq->rqe_count = cpu_to_le16(qmem->size / SLI4_RQE_SIZE);
> +
> + rq->rqe_size_byte |= SLI4_RQE_SIZE_8;
> +
> + rq->page_size = SLI4_RQ_PAGE_SIZE_4096;
> +
> + if (buffer_size < sli4->rq_min_buf_size ||
> + buffer_size > sli4->rq_max_buf_size) {
> + efc_log_err(sli4, "buffer_size %d out of range (%d-%d)\n",
> + buffer_size,
> + sli4->rq_min_buf_size,
> + sli4->rq_max_buf_size);
> + return EFC_FAIL;
> + }
> + rq->buffer_size = cpu_to_le32(buffer_size);
> +
> + rq->cq_id = cpu_to_le16(cq_id);
> +
> + for (p = 0, addr = qmem->phys;
> + p < num_pages;
> + p++, addr += SLI_PAGE_SIZE) {
> + rq->page_phys_addr[p].low = cpu_to_le32(lower_32_bits(addr));
> + rq->page_phys_addr[p].high = cpu_to_le32(upper_32_bits(addr));
> + }
> +
> + return EFC_SUCCESS;
> +}
> +
> +static int
> +sli_cmd_rq_create_v2(struct sli4 *sli4, u32 num_rqs,
> + struct sli4_queue *qs[], u32 base_cq_id,
> + u32 header_buffer_size,
> + u32 payload_buffer_size, struct efc_dma *dma)
> +{
> + struct sli4_rqst_rq_create_v2 *req = NULL;
> + u32 i, p, offset = 0;
> + u32 payload_size, page_count;
> + uintptr_t addr;
> + u32 num_pages;
> + __le32 req_len;
> +
> + page_count = sli_page_count(qs[0]->dma.size, SLI_PAGE_SIZE) * num_rqs;
> +
> + /* Payload length must accommodate both request and response */
> + payload_size = max(CFG_RQST_CMDSZ(rq_create_v2) +
> + SZ_DMAADDR * page_count,
> + sizeof(struct sli4_rsp_cmn_create_queue_set));
> +
> + dma->size = payload_size;
> + dma->virt = dma_alloc_coherent(&sli4->pcidev->dev, dma->size,
> + &dma->phys, GFP_DMA);
> + if (!dma->virt)
> + return EFC_FAIL;
> +
> + memset(dma->virt, 0, payload_size);
> +
> + req = sli_config_cmd_init(sli4, sli4->bmbx.virt, SLI4_BMBX_SIZE,
> + payload_size, dma);
> + if (!req)
> + return EFC_FAIL;
> +
> + req_len = CFG_RQST_PYLD_LEN_VAR(rq_create_v2, SZ_DMAADDR * page_count);
> + sli_cmd_fill_hdr(&req->hdr, SLI4_OPC_RQ_CREATE, SLI4_SUBSYSTEM_FC,
> + CMD_V2, req_len);
> + /* Fill Payload fields */
> + req->dim_dfd_dnb |= SLI4_RQCREATEV2_DNB;
> + num_pages = sli_page_count(qs[0]->dma.size, SLI_PAGE_SIZE);
code alignment
> + req->num_pages = cpu_to_le16(num_pages);
> + req->rqe_count = cpu_to_le16(qs[0]->dma.size / SLI4_RQE_SIZE);
> + req->rqe_size_byte |= SLI4_RQE_SIZE_8;
> + req->page_size = SLI4_RQ_PAGE_SIZE_4096;
> + req->rq_count = num_rqs;
> + req->base_cq_id = cpu_to_le16(base_cq_id);
> + req->hdr_buffer_size = cpu_to_le16(header_buffer_size);
> + req->payload_buffer_size = cpu_to_le16(payload_buffer_size);
> +
> + for (i = 0; i < num_rqs; i++) {
> + for (p = 0, addr = qs[i]->dma.phys; p < num_pages;
> + p++, addr += SLI_PAGE_SIZE) {
> + req->page_phys_addr[offset].low =
> + cpu_to_le32(lower_32_bits(addr));
> + req->page_phys_addr[offset].high =
> + cpu_to_le32(upper_32_bits(addr));
> + offset++;
> + }
> + }
> +
> + return EFC_SUCCESS;
> +}
> +
> +static void
> +__sli_queue_destroy(struct sli4 *sli4, struct sli4_queue *q)
> +{
> + if (!q->dma.size)
> + return;
> +
> + dma_free_coherent(&sli4->pcidev->dev, q->dma.size,
> + q->dma.virt, q->dma.phys);
> + memset(&q->dma, 0, sizeof(struct efc_dma));
Is this necessary to clear q->dma? Just asking if it's possible to
avoid the additional work.
> +}
> +
> +int
> +__sli_queue_init(struct sli4 *sli4, struct sli4_queue *q,
> + u32 qtype, size_t size, u32 n_entries,
> + u32 align)
> +{
> + if (!q->dma.virt || size != q->size ||
> + n_entries != q->length) {
I would put all on one line. The column limit should be reached.
And couldn't you test logical inverse and return early and avoid the
idention? And also I add the queue type to the log message so it's
not necessary to add the some log statement through the code base.
Something like:
if (q->dma.virt && size == q->size && n_entries == q->length) {
efc_log_err(sli4, "%s %s failed\n", __func__, SLI_QNAME[qtype]);
return EFC_FAIL;
}
Maybe add combine the error path here and the below together and use a
goto/label.
> + if (q->dma.size)
> + __sli_queue_destroy(sli4, q);
> +
> + memset(q, 0, sizeof(struct sli4_queue));
> +
> + q->dma.size = size * n_entries;
> + q->dma.virt = dma_alloc_coherent(&sli4->pcidev->dev,
> + q->dma.size, &q->dma.phys,
> + GFP_DMA);
> + if (!q->dma.virt) {
> + memset(&q->dma, 0, sizeof(struct efc_dma));
So if __sli_queue_destroy() keeps clearing q->dma, than this one can
go away, since if __sli_queue_init() fails __sli_queue_destroy() will
be called.
> + efc_log_err(sli4, "%s allocation failed\n",
> + SLI_QNAME[qtype]);
> + return EFC_FAIL;
> + }
> +
> + memset(q->dma.virt, 0, size * n_entries);
> +
> + spin_lock_init(&q->lock);
> +
> + q->type = qtype;
> + q->size = size;
> + q->length = n_entries;
> +
> + if (q->type == SLI_QTYPE_EQ || q->type == SLI_QTYPE_CQ) {
> + /* For prism, phase will be flipped after
> + * a sweep through eq and cq
> + */
> + q->phase = 1;
> + }
> +
> + /* Limit to hwf the queue size per interrupt */
> + q->proc_limit = n_entries / 2;
> +
> + switch (q->type) {
> + case SLI_QTYPE_EQ:
> + q->posted_limit = q->length / 2;
> + break;
> + default:
> + q->posted_limit = 64;
> + break;
> + }
> + } else {
> + efc_log_err(sli4, "%s failed\n", __func__);
> + return EFC_FAIL;
> + }
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_fc_rq_alloc(struct sli4 *sli4, struct sli4_queue *q,
> + u32 n_entries, u32 buffer_size,
> + struct sli4_queue *cq, bool is_hdr)
> +{
> + if (__sli_queue_init(sli4, q, SLI_QTYPE_RQ, SLI4_RQE_SIZE,
> + n_entries, SLI_PAGE_SIZE))
> + return EFC_FAIL;
> +
> + if (!sli_cmd_rq_create_v1(sli4, sli4->bmbx.virt, SLI4_BMBX_SIZE,
> + &q->dma, cq->id, buffer_size)) {
> + if (__sli_create_queue(sli4, q)) {
> + efc_log_info(sli4, "Create queue failed %d\n", q->id);
> + goto error;
> + }
> + if (is_hdr && q->id & 1) {
> + efc_log_info(sli4, "bad header RQ_ID %d\n", q->id);
> + goto error;
> + } else if (!is_hdr && (q->id & 1) == 0) {
> + efc_log_info(sli4, "bad data RQ_ID %d\n", q->id);
> + goto error;
> + }
> + } else {
> + goto error;
> + }
> + if (is_hdr)
> + q->u.flag.dword |= SLI4_QUEUE_FLAG_HDR;
> + else
> + q->u.flag.dword &= ~SLI4_QUEUE_FLAG_HDR;
> + return EFC_SUCCESS;
> +error:
> + __sli_queue_destroy(sli4, q);
> + return EFC_FAIL;
> +}
> +
> +int
> +sli_fc_rq_set_alloc(struct sli4 *sli4, u32 num_rq_pairs,
> + struct sli4_queue *qs[], u32 base_cq_id,
> + u32 n_entries, u32 header_buffer_size,
> + u32 payload_buffer_size)
> +{
> + u32 i;
> + struct efc_dma dma;
> + struct sli4_rsp_cmn_create_queue_set *rsp = NULL;
> + void __iomem *db_regaddr = NULL;
> + u32 num_rqs = num_rq_pairs * 2;
> +
> + for (i = 0; i < num_rqs; i++) {
> + if (__sli_queue_init(sli4, qs[i], SLI_QTYPE_RQ,
> + SLI4_RQE_SIZE, n_entries,
> + SLI_PAGE_SIZE)) {
> + goto error;
> + }
> + }
> +
> + if (sli_cmd_rq_create_v2(sli4, num_rqs, qs, base_cq_id,
> + header_buffer_size, payload_buffer_size, &dma)) {
> + goto error;
> + }
> +
> + if (sli_bmbx_command(sli4)) {
> + efc_log_err(sli4, "bootstrap mailbox write failed RQSet\n");
> + goto error;
> + }
> +
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + db_regaddr = sli4->reg[1] + SLI4_IF6_RQ_DB_REG;
> + else
> + db_regaddr = sli4->reg[0] + SLI4_RQ_DB_REG;
> +
> + rsp = dma.virt;
> + if (rsp->hdr.status) {
> + efc_log_err(sli4, "bad create RQSet status=%#x addl=%#x\n",
> + rsp->hdr.status, rsp->hdr.additional_status);
> + goto error;
> + } else {
> + for (i = 0; i < num_rqs; i++) {
> + qs[i]->id = i + le16_to_cpu(rsp->q_id);
> + if ((qs[i]->id & 1) == 0)
> + qs[i]->u.flag.dword |= SLI4_QUEUE_FLAG_HDR;
> + else
> + qs[i]->u.flag.dword &= ~SLI4_QUEUE_FLAG_HDR;
> +
> + qs[i]->db_regaddr = db_regaddr;
> + }
> + }
> +
> + dma_free_coherent(&sli4->pcidev->dev, dma.size, dma.virt, dma.phys);
> +
> + return EFC_SUCCESS;
> +
> +error:
> + for (i = 0; i < num_rqs; i++)
> + __sli_queue_destroy(sli4, qs[i]);
> +
> + if (dma.virt)
> + dma_free_coherent(&sli4->pcidev->dev, dma.size, dma.virt,
> + dma.phys);
> +
> + return EFC_FAIL;
> +}
> +
> +static int
> +sli_res_sli_config(struct sli4 *sli4, void *buf)
> +{
> + struct sli4_cmd_sli_config *sli_config = buf;
> +
> + /* sanity check */
> + if (!buf || sli_config->hdr.command !=
> + MBX_CMD_SLI_CONFIG) {
> + efc_log_err(sli4, "bad parameter buf=%p cmd=%#x\n", buf,
> + buf ? sli_config->hdr.command : -1);
> + return EFC_FAIL;
> + }
> +
> + if (le16_to_cpu(sli_config->hdr.status))
> + return le16_to_cpu(sli_config->hdr.status);
> +
> + if (le32_to_cpu(sli_config->dw1_flags) & SLI4_SLICONF_EMB)
> + return sli_config->payload.embed[4];
> +
> + efc_log_info(sli4, "external buffers not supported\n");
> + return EFC_FAIL;
> +}
> +
> +int
> +__sli_create_queue(struct sli4 *sli4, struct sli4_queue *q)
> +{
> + struct sli4_rsp_cmn_create_queue *res_q = NULL;
> +
> + if (sli_bmbx_command(sli4)) {
> + efc_log_crit(sli4, "bootstrap mailbox write fail %s\n",
> + SLI_QNAME[q->type]);
> + return EFC_FAIL;
> + }
> + if (sli_res_sli_config(sli4, sli4->bmbx.virt)) {
> + efc_log_err(sli4, "bad status create %s\n",
> + SLI_QNAME[q->type]);
> + return EFC_FAIL;
> + }
> + res_q = (void *)((u8 *)sli4->bmbx.virt +
> + offsetof(struct sli4_cmd_sli_config, payload));
> +
> + if (res_q->hdr.status) {
> + efc_log_err(sli4, "bad create %s status=%#x addl=%#x\n",
> + SLI_QNAME[q->type], res_q->hdr.status,
> + res_q->hdr.additional_status);
> + return EFC_FAIL;
> + }
> + q->id = le16_to_cpu(res_q->q_id);
> + switch (q->type) {
> + case SLI_QTYPE_EQ:
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + q->db_regaddr = sli4->reg[1] + SLI4_IF6_EQ_DB_REG;
> + else
> + q->db_regaddr = sli4->reg[0] + SLI4_EQCQ_DB_REG;
> + break;
> + case SLI_QTYPE_CQ:
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + q->db_regaddr = sli4->reg[1] + SLI4_IF6_CQ_DB_REG;
> + else
> + q->db_regaddr = sli4->reg[0] + SLI4_EQCQ_DB_REG;
> + break;
> + case SLI_QTYPE_MQ:
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + q->db_regaddr = sli4->reg[1] + SLI4_IF6_MQ_DB_REG;
> + else
> + q->db_regaddr = sli4->reg[0] + SLI4_MQ_DB_REG;
> + break;
> + case SLI_QTYPE_RQ:
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + q->db_regaddr = sli4->reg[1] + SLI4_IF6_RQ_DB_REG;
> + else
> + q->db_regaddr = sli4->reg[0] + SLI4_RQ_DB_REG;
> + break;
> + case SLI_QTYPE_WQ:
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + q->db_regaddr = sli4->reg[1] + SLI4_IF6_WQ_DB_REG;
> + else
> + q->db_regaddr = sli4->reg[0] + SLI4_IO_WQ_DB_REG;
> + break;
> + default:
> + break;
> + }
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_get_queue_entry_size(struct sli4 *sli4, u32 qtype)
> +{
> + u32 size = 0;
> +
> + switch (qtype) {
> + case SLI_QTYPE_EQ:
> + size = sizeof(u32);
> + break;
> + case SLI_QTYPE_CQ:
> + size = 16;
> + break;
> + case SLI_QTYPE_MQ:
> + size = 256;
> + break;
> + case SLI_QTYPE_WQ:
> + size = sli4->wqe_size;
> + break;
> + case SLI_QTYPE_RQ:
> + size = SLI4_RQE_SIZE;
> + break;
> + default:
> + efc_log_info(sli4, "unknown queue type %d\n", qtype);
> + return -1;
> + }
> + return size;
> +}
> +
> +int
> +sli_queue_alloc(struct sli4 *sli4, u32 qtype,
> + struct sli4_queue *q, u32 n_entries,
> + struct sli4_queue *assoc)
> +{
> + int size;
> + u32 align = 0;
> +
> + /* get queue size */
> + size = sli_get_queue_entry_size(sli4, qtype);
> + if (size < 0)
> + return EFC_FAIL;
> + align = SLI_PAGE_SIZE;
> +
> + if (__sli_queue_init(sli4, q, qtype, size, n_entries, align)) {
> + efc_log_err(sli4, "%s allocation failed\n",
> + SLI_QNAME[qtype]);
__sli_queue_init() already logs this information.
> + return EFC_FAIL;
> + }
> +
> + switch (qtype) {
> + case SLI_QTYPE_EQ:
> + if (!sli_cmd_common_create_eq(sli4, sli4->bmbx.virt,
> + SLI4_BMBX_SIZE, &q->dma)) {
> + if (__sli_create_queue(sli4, q)) {
> + efc_log_err(sli4, "create %s failed\n",
> + SLI_QNAME[qtype]);
__sli_create_queue logs this already.
> + goto error;
> + }
> + } else {
> + efc_log_err(sli4, "cannot create %s\n",
> + SLI_QNAME[qtype]);
> + goto error;
> + }
So I think you could just do
if (sli_cmd_common_create_eq(...) ||
__sli_create_queue(...)))
goto error;
or even do an early exit on success and avoid the 'goto error'
if (!sli_cmd_common_create_eq(...) &&
!__sli_create_queue(...))
return EFC_SUCCESS;
> +
> + break;
> + case SLI_QTYPE_CQ:
> + if (!sli_cmd_common_create_cq(sli4, sli4->bmbx.virt,
> + SLI4_BMBX_SIZE, &q->dma,
> + assoc ? assoc->id : 0)) {
> + if (__sli_create_queue(sli4, q)) {
> + efc_log_err(sli4, "create %s failed\n",
> + SLI_QNAME[qtype]);
> + goto error;
> + }
> + } else {
> + efc_log_err(sli4, "cannot create %s\n",
> + SLI_QNAME[qtype]);
> + goto error;
> + }
Same as above
> + break;
> + case SLI_QTYPE_MQ:
> + assoc->u.flag.dword |= SLI4_QUEUE_FLAG_MQ;
> + if (!sli_cmd_common_create_mq_ext(sli4, sli4->bmbx.virt,
> + SLI4_BMBX_SIZE, &q->dma,
> + assoc->id)) {
> + if (__sli_create_queue(sli4, q)) {
> + efc_log_err(sli4, "create %s failed\n",
> + SLI_QNAME[qtype]);
> + goto error;
> + }
> + } else {
> + efc_log_err(sli4, "cannot create %s\n",
> + SLI_QNAME[qtype]);
> + goto error;
> + }
and here
> +
> + break;
> + case SLI_QTYPE_WQ:
> + if (!sli_cmd_wq_create(sli4, sli4->bmbx.virt,
> + SLI4_BMBX_SIZE, &q->dma,
> + assoc ? assoc->id : 0)) {
> + if (__sli_create_queue(sli4, q)) {
> + efc_log_err(sli4, "create %s failed\n",
> + SLI_QNAME[qtype]);
> + goto error;
> + }
> + } else {
> + efc_log_err(sli4, "cannot create %s\n",
> + SLI_QNAME[qtype]);
> + goto error;
> + }
> + break;
> + default:
> + efc_log_info(sli4, "unknown queue type %d\n", qtype);
> + goto error;
> + }
> +
> + return EFC_SUCCESS;
> +error:
> + __sli_queue_destroy(sli4, q);
> + return EFC_FAIL;
> +}
> +
> +static int sli_cmd_cq_set_create(struct sli4 *sli4,
> + struct sli4_queue *qs[], u32 num_cqs,
> + struct sli4_queue *eqs[],
> + struct efc_dma *dma)
> +{
> + struct sli4_rqst_cmn_create_cq_set_v0 *req = NULL;
> + uintptr_t addr;
> + u32 i, offset = 0, page_bytes = 0, payload_size;
> + u32 p = 0, page_size = 0, n_cqe = 0, num_pages_cq;
> + u32 dw5_flags = 0;
> + u16 dw6w1_flags = 0;
> + __le32 req_len;
> +
> + n_cqe = qs[0]->dma.size / SLI4_CQE_BYTES;
> + switch (n_cqe) {
> + case 256:
> + case 512:
> + case 1024:
> + case 2048:
> + page_size = 1;
> + break;
> + case 4096:
> + page_size = 2;
> + break;
> + default:
> + return EFC_FAIL;
> + }
> +
> + page_bytes = page_size * SLI_PAGE_SIZE;
> + num_pages_cq = sli_page_count(qs[0]->dma.size, page_bytes);
> + payload_size = max(CFG_RQST_CMDSZ(cmn_create_cq_set_v0) +
> + (SZ_DMAADDR * num_pages_cq * num_cqs),
> + sizeof(struct sli4_rsp_cmn_create_queue_set));
> +
> + dma->size = payload_size;
> + dma->virt = dma_alloc_coherent(&sli4->pcidev->dev, dma->size,
> + &dma->phys, GFP_DMA);
> + if (!dma->virt)
> + return EFC_FAIL;
> +
> + memset(dma->virt, 0, payload_size);
> +
> + req = sli_config_cmd_init(sli4, sli4->bmbx.virt, SLI4_BMBX_SIZE,
> + payload_size, dma);
> + if (!req)
> + return EFC_FAIL;
> +
> + req_len = CFG_RQST_PYLD_LEN_VAR(cmn_create_cq_set_v0,
> + SZ_DMAADDR * num_pages_cq * num_cqs);
> + sli_cmd_fill_hdr(&req->hdr, CMN_CREATE_CQ_SET, SLI4_SUBSYSTEM_FC,
> + CMD_V0, req_len);
> + req->page_size = page_size;
> +
> + req->num_pages = cpu_to_le16(num_pages_cq);
> + switch (num_pages_cq) {
> + case 1:
> + dw5_flags |= CQ_CNT_VAL(256);
> + break;
> + case 2:
> + dw5_flags |= CQ_CNT_VAL(512);
> + break;
> + case 4:
> + dw5_flags |= CQ_CNT_VAL(1024);
> + break;
> + case 8:
> + dw5_flags |= CQ_CNT_VAL(LARGE);
> + dw6w1_flags |= (n_cqe & CREATE_CQSETV0_CQE_COUNT);
> + break;
> + default:
> + efc_log_info(sli4, "num_pages %d not valid\n", num_pages_cq);
> + return EFC_FAIL;
> + }
> +
> + dw5_flags |= CREATE_CQSETV0_EVT;
> + dw5_flags |= CREATE_CQSETV0_VALID;
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + dw5_flags |= CREATE_CQSETV0_AUTOVALID;
> +
> + dw6w1_flags &= (~CREATE_CQSETV0_ARM);
The brakets are not needed.
> +
> + req->dw5_flags = cpu_to_le32(dw5_flags);
> + req->dw6w1_flags = cpu_to_le16(dw6w1_flags);
> +
> + req->num_cq_req = cpu_to_le16(num_cqs);
> +
> + /* Fill page addresses of all the CQs. */
> + for (i = 0; i < num_cqs; i++) {
> + req->eq_id[i] = cpu_to_le16(eqs[i]->id);
> + for (p = 0, addr = qs[i]->dma.phys; p < num_pages_cq;
> + p++, addr += page_bytes) {
> + req->page_phys_addr[offset].low =
> + cpu_to_le32(lower_32_bits(addr));
> + req->page_phys_addr[offset].high =
> + cpu_to_le32(upper_32_bits(addr));
> + offset++;
> + }
> + }
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_cq_alloc_set(struct sli4 *sli4, struct sli4_queue *qs[],
> + u32 num_cqs, u32 n_entries, struct sli4_queue *eqs[])
> +{
> + u32 i;
> + struct efc_dma dma;
> + struct sli4_rsp_cmn_create_queue_set *res = NULL;
> + void __iomem *db_regaddr = NULL;
res and db_regaddr do not to be pre-initialized here
> +
> + /* Align the queue DMA memory */
> + for (i = 0; i < num_cqs; i++) {
> + if (__sli_queue_init(sli4, qs[i], SLI_QTYPE_CQ,
> + SLI4_CQE_BYTES,
> + n_entries, SLI_PAGE_SIZE)) {
> + efc_log_err(sli4, "Queue init failed.\n");
__sli_queue_init() already logs this information.
> + goto error;
> + }
> + }
> +
> + if (sli_cmd_cq_set_create(sli4, qs, num_cqs, eqs, &dma))
> + goto error;
> +
> + if (sli_bmbx_command(sli4)) {
> + efc_log_crit(sli4, "bootstrap mailbox write fail CQSet\n");
> + goto error;
> + }
Since sli_bmbx_command already logs an error, is it this necessary?
> +
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + db_regaddr = sli4->reg[1] + SLI4_IF6_CQ_DB_REG;
> + else
> + db_regaddr = sli4->reg[0] + SLI4_EQCQ_DB_REG;
> +
> + res = dma.virt;
> + if (res->hdr.status) {
> + efc_log_err(sli4, "bad create CQSet status=%#x addl=%#x\n",
> + res->hdr.status, res->hdr.additional_status);
> + goto error;
> + } else {
> + /* Check if we got all requested CQs. */
> + if (le16_to_cpu(res->num_q_allocated) != num_cqs) {
> + efc_log_crit(sli4, "Requested count CQs doesn't match.\n");
> + goto error;
> + }
> + /* Fill the resp cq ids. */
> + for (i = 0; i < num_cqs; i++) {
> + qs[i]->id = le16_to_cpu(res->q_id) + i;
> + qs[i]->db_regaddr = db_regaddr;
> + }
> + }
> +
> + dma_free_coherent(&sli4->pcidev->dev, dma.size, dma.virt, dma.phys);
> +
> + return EFC_SUCCESS;
> +
> +error:
> + for (i = 0; i < num_cqs; i++)
> + __sli_queue_destroy(sli4, qs[i]);
> +
> + if (dma.virt)
> + dma_free_coherent(&sli4->pcidev->dev, dma.size, dma.virt,
> + dma.phys);
> +
> + return EFC_FAIL;
> +}
> +
> +static int
> +sli_cmd_common_destroy_q(struct sli4 *sli4, u8 opc, u8 subsystem, u16 q_id)
> +{
> + struct sli4_rqst_cmn_destroy_q *req = NULL;
Not necessary to pre-initilize req here.
> +
> + /* Payload length must accommodate both request and response */
> + req = sli_config_cmd_init(sli4, sli4->bmbx.virt, SLI4_BMBX_SIZE,
> + SLI_CONFIG_PYLD_LENGTH(cmn_destroy_q), NULL);
> + if (!req)
> + return EFC_FAIL;
> +
> + sli_cmd_fill_hdr(&req->hdr, opc, subsystem,
> + CMD_V0, CFG_RQST_PYLD_LEN(cmn_destroy_q));
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_queue_free(struct sli4 *sli4, struct sli4_queue *q,
> + u32 destroy_queues, u32 free_memory)
> +{
> + int rc = EFC_SUCCESS;
> + u8 opcode, subsystem;
> + struct sli4_rsp_hdr *res;
> +
> + if (!q) {
> + efc_log_err(sli4, "bad parameter sli4=%p q=%p\n", sli4, q);
> + return EFC_FAIL;
> + }
> +
> + if (!destroy_queues)
> + goto free_mem;
> +
> + switch (q->type) {
> + case SLI_QTYPE_EQ:
> + opcode = CMN_DESTROY_EQ;
> + subsystem = SLI4_SUBSYSTEM_COMMON;
> + break;
> + case SLI_QTYPE_CQ:
> + opcode = CMN_DESTROY_CQ;
> + subsystem = SLI4_SUBSYSTEM_COMMON;
> + break;
> + case SLI_QTYPE_MQ:
> + opcode = CMN_DESTROY_MQ;
> + subsystem = SLI4_SUBSYSTEM_COMMON;
> + break;
> + case SLI_QTYPE_WQ:
> + opcode = SLI4_OPC_WQ_DESTROY;
> + subsystem = SLI4_SUBSYSTEM_FC;
> + break;
> + case SLI_QTYPE_RQ:
> + opcode = SLI4_OPC_RQ_DESTROY;
> + subsystem = SLI4_SUBSYSTEM_FC;
> + break;
> + default:
> + efc_log_info(sli4, "bad queue type %d\n", q->type);
> + return EFC_FAIL;
and why not 'goto free_mem'?
I think I would move the destroy queue code into a new function instead
all in this function...
> + }
> +
> + rc = sli_cmd_common_destroy_q(sli4, opcode, subsystem, q->id);
> + if (!rc)
> + goto free_mem;
> +
> + if (sli_bmbx_command(sli4)) {
> + efc_log_crit(sli4, "bootstrap mailbox fail destroy %s\n",
> + SLI_QNAME[q->type]);
...and if this is an seperate function it would be possible to use an
early return and don't have to do this else if dance.
> + } else if (sli_res_sli_config(sli4, sli4->bmbx.virt)) {
> + efc_log_err(sli4, "bad status %s\n", SLI_QNAME[q->type]);
> + } else {
> + res = (void *)((u8 *)sli4->bmbx.virt +
> + offsetof(struct sli4_cmd_sli_config, payload));
> +
> + if (res->status) {
> + efc_log_err(sli4, "destroy %s st=%#x addl=%#x\n",
> + SLI_QNAME[q->type], res->status,
> + res->additional_status);
> + } else {
> + rc = EFC_SUCCESS;
> + }
> + }
> +
> +free_mem:
> + if (free_memory)
> + __sli_queue_destroy(sli4, q);
> +
> + return rc;
> +}
> +
> +int
> +sli_queue_eq_arm(struct sli4 *sli4, struct sli4_queue *q, bool arm)
> +{
> + u32 val = 0;
No need to pre-initialize val;
> + unsigned long flags = 0;
> + u32 a = arm ? SLI4_EQCQ_ARM : SLI4_EQCQ_UNARM;
> +
> + spin_lock_irqsave(&q->lock, flags);
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + val = SLI4_IF6_EQ_DOORBELL(q->n_posted, q->id, a);
> + else
> + val = SLI4_EQ_DOORBELL(q->n_posted, q->id, a);
After seeing SLI4_*_DOORBELL in action I would prefer these are inline
functions and q and arm are passed in. All the macro magic could
happend inside there. This would avoid the "a = arm ?..." here.
> +
> + writel(val, q->db_regaddr);
> + q->n_posted = 0;
> + spin_unlock_irqrestore(&q->lock, flags);
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_queue_arm(struct sli4 *sli4, struct sli4_queue *q, bool arm)
> +{
> + u32 val = 0;
> + unsigned long flags = 0;
> + u32 a = arm ? SLI4_EQCQ_ARM : SLI4_EQCQ_UNARM;
> +
> + spin_lock_irqsave(&q->lock, flags);
> +
> + switch (q->type) {
> + case SLI_QTYPE_EQ:
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + val = SLI4_IF6_EQ_DOORBELL(q->n_posted, q->id, a);
> + else
> + val = SLI4_EQ_DOORBELL(q->n_posted, q->id, a);
> +
> + writel(val, q->db_regaddr);
> + q->n_posted = 0;
> + break;
> + case SLI_QTYPE_CQ:
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + val = SLI4_IF6_CQ_DOORBELL(q->n_posted, q->id, a);
> + else
> + val = SLI4_CQ_DOORBELL(q->n_posted, q->id, a);
> +
> + writel(val, q->db_regaddr);
> + q->n_posted = 0;
> + break;
> + default:
> + efc_log_info(sli4, "should only be used for EQ/CQ, not %s\n",
> + SLI_QNAME[q->type]);
> + }
> +
> + spin_unlock_irqrestore(&q->lock, flags);
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_wq_write(struct sli4 *sli4, struct sli4_queue *q,
> + u8 *entry)
entry still fits on the line above.
> +{
> + u8 *qe = q->dma.virt;
> + u32 qindex;
> + u32 val = 0;
code indention not consistent
> +
> + qindex = q->index;
> + qe += q->index * q->size;
> +
> + if (sli4->perf_wq_id_association)
> + sli_set_wq_id_association(entry, q->id);
> +
> + memcpy(qe, entry, q->size);
> + q->n_posted = 1;
> +
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6)
> + /* non-dpp write for iftype = 6 */
> + val = SLI4_WQ_DOORBELL(q->n_posted, 0, q->id);
> + else
> + val = SLI4_WQ_DOORBELL(q->n_posted, q->index, q->id);
> +
> + writel(val, q->db_regaddr);
> + q->index = (q->index + q->n_posted) & (q->length - 1);
> + q->n_posted = 0;
Why does this not need a lock protection as in sli_queue_arm()?
> +
> + return qindex;
> +}
> +
> +int
> +sli_mq_write(struct sli4 *sli4, struct sli4_queue *q,
> + u8 *entry)
entry also fits on the line above
> +{
> + u8 *qe = q->dma.virt;
> + u32 qindex;
> + u32 val = 0;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&q->lock, flags);
> + qindex = q->index;
> + qe += q->index * q->size;
> +
> + memcpy(qe, entry, q->size);
> + q->n_posted = 1;
Shouldn't this be q->n_posted++ ? Or is it garanteed n_posted is 0?
> +
> + val = SLI4_MQ_DOORBELL(q->n_posted, q->id);
> + writel(val, q->db_regaddr);
> + q->index = (q->index + q->n_posted) & (q->length - 1);
> + q->n_posted = 0;
> + spin_unlock_irqrestore(&q->lock, flags);
> +
> + return qindex;
> +}
> +
> +int
> +sli_rq_write(struct sli4 *sli4, struct sli4_queue *q,
> + u8 *entry)
enrry fits on the line above
> +{
> + u8 *qe = q->dma.virt;
> + u32 qindex, n_posted;
> + u32 val = 0;
> +
> + qindex = q->index;
> + qe += q->index * q->size;
> +
> + memcpy(qe, entry, q->size);
> + q->n_posted = 1;
Again why not q->n_posted++ ?
I am confused why no lock is used here and in the fuction above. A few
words on the locking desing would be highly appreciated.
> +
> + n_posted = q->n_posted;
> +
> + /*
> + * In RQ-pair, an RQ either contains the FC header
> + * (i.e. is_hdr == TRUE) or the payload.
> + *
> + * Don't ring doorbell for payload RQ
> + */
> + if (!(q->u.flag.dword & SLI4_QUEUE_FLAG_HDR))
> + goto skip;
> +
> + /*
> + * Some RQ cannot be incremented one entry at a time.
> + * Instead, the driver collects a number of entries
> + * and updates the RQ in batches.
> + */
> + if (q->u.flag.dword & SLI4_QUEUE_FLAG_RQBATCH) {
> + if (((q->index + q->n_posted) %
> + SLI4_QUEUE_RQ_BATCH)) {
> + goto skip;
> + }
> + n_posted = SLI4_QUEUE_RQ_BATCH;
> + }
> +
> + val = SLI4_RQ_DOORBELL(n_posted, q->id);
> + writel(val, q->db_regaddr);
> +skip:
> + q->index = (q->index + q->n_posted) & (q->length - 1);
> + q->n_posted = 0;
> +
> + return qindex;
> +}
> +
> +int
> +sli_eq_read(struct sli4 *sli4,
> + struct sli4_queue *q, u8 *entry)
this fits on one line
> +{
> + u8 *qe = q->dma.virt;
> + u32 *qindex = NULL;
> + unsigned long flags = 0;
> + u8 clear = false, valid = false;
> + u16 wflags = 0;
> +
> + clear = (sli4->if_type == SLI4_INTF_IF_TYPE_6) ? false : true;
> +
> + qindex = &q->index;
> +
> + spin_lock_irqsave(&q->lock, flags);
> +
> + qe += *qindex * q->size;
> +
> + /* Check if eqe is valid */
> + wflags = le16_to_cpu(((struct sli4_eqe *)qe)->dw0w0_flags);
> + valid = ((wflags & SLI4_EQE_VALID) == q->phase);
> + if (!valid) {
> + spin_unlock_irqrestore(&q->lock, flags);
> + return EFC_FAIL;
> + }
> +
> + if (valid && clear) {
> + wflags &= ~SLI4_EQE_VALID;
> + ((struct sli4_eqe *)qe)->dw0w0_flags =
> + cpu_to_le16(wflags);
> + }
> +
> + memcpy(entry, qe, q->size);
> + *qindex = (*qindex + 1) & (q->length - 1);
> + q->n_posted++;
> + /*
> + * For prism, the phase value will be used
> + * to check the validity of eq/cq entries.
> + * The value toggles after a complete sweep
> + * through the queue.
> + */
> +
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6 && *qindex == 0)
> + q->phase ^= (u16)0x1;
q->phase = !q->phase;
> +
> + spin_unlock_irqrestore(&q->lock, flags);
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_cq_read(struct sli4 *sli4,
> + struct sli4_queue *q, u8 *entry)
fits on the same line
> +{
> + u8 *qe = q->dma.virt;
> + u32 *qindex = NULL;
> + unsigned long flags = 0;
> + u8 clear = false;
> + u32 dwflags = 0;
> + bool valid = false, valid_bit_set = false;
> +
> + clear = (sli4->if_type == SLI4_INTF_IF_TYPE_6) ? false : true;
> +
> + qindex = &q->index;
> +
> + spin_lock_irqsave(&q->lock, flags);
> +
> + qe += *qindex * q->size;
> +
> + /* Check if cqe is valid */
> + dwflags = le32_to_cpu(((struct sli4_mcqe *)qe)->dw3_flags);
> + valid_bit_set = (dwflags & SLI4_MCQE_VALID) != 0;
> +
> + valid = (valid_bit_set == q->phase);
> + if (!valid) {
if (valid_bit_set == q->phase)
> + spin_unlock_irqrestore(&q->lock, flags);
> + return EFC_FAIL;
> + }
> +
> + if (valid && clear) {
valid is alway true
> + dwflags &= ~SLI4_MCQE_VALID;
> + ((struct sli4_mcqe *)qe)->dw3_flags =
> + cpu_to_le32(dwflags);
> + }
> +
> + memcpy(entry, qe, q->size);
> + *qindex = (*qindex + 1) & (q->length - 1);
> + q->n_posted++;
> + /*
> + * For prism, the phase value will be used
> + * to check the validity of eq/cq entries.
> + * The value toggles after a complete sweep
> + * through the queue.
> + */
> +
> + if (sli4->if_type == SLI4_INTF_IF_TYPE_6 && *qindex == 0)
> + q->phase ^= (u16)0x1;
q->phase = !q->phase;
> +
> + spin_unlock_irqrestore(&q->lock, flags);
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_mq_read(struct sli4 *sli4,
> + struct sli4_queue *q, u8 *entry)
fits on one line
> +{
> + u8 *qe = q->dma.virt;
> + u32 *qindex = NULL;
> + unsigned long flags = 0;
> +
> + qindex = &q->u.r_idx;
> +
> + spin_lock_irqsave(&q->lock, flags);
> +
> + qe += *qindex * q->size;
> +
> + /* Check if mqe is valid */
> + if (q->index == q->u.r_idx) {
> + spin_unlock_irqrestore(&q->lock, flags);
> + return EFC_FAIL;
> + }
> +
> + memcpy(entry, qe, q->size);
> + *qindex = (*qindex + 1) & (q->length - 1);
> +
> + spin_unlock_irqrestore(&q->lock, flags);
> +
> + return EFC_SUCCESS;
> +}
> +
> +int
> +sli_eq_parse(struct sli4 *sli4, u8 *buf, u16 *cq_id)
> +{
> + struct sli4_eqe *eqe = (void *)buf;
> + int rc = EFC_SUCCESS;
> + u16 flags = 0;
> + u16 majorcode;
> + u16 minorcode;
> +
> + if (!buf || !cq_id) {
> + efc_log_err(sli4, "bad parameters sli4=%p buf=%p cq_id=%p\n",
> + sli4, buf, cq_id);
> + return EFC_FAIL;
> + }
> +
> + flags = le16_to_cpu(eqe->dw0w0_flags);
> + majorcode = (flags & SLI4_EQE_MJCODE) >> 1;
> + minorcode = (flags & SLI4_EQE_MNCODE) >> 4;
> + switch (majorcode) {
> + case SLI4_MAJOR_CODE_STANDARD:
> + *cq_id = le16_to_cpu(eqe->resource_id);
> + break;
> + case SLI4_MAJOR_CODE_SENTINEL:
> + efc_log_info(sli4, "sentinel EQE\n");
> + rc = SLI4_EQE_STATUS_EQ_FULL;
> + break;
> + default:
> + efc_log_info(sli4, "Unsupported EQE: major %x minor %x\n",
> + majorcode, minorcode);
> + rc = EFC_FAIL;
> + }
> +
> + return rc;
> +}
> +
> +/* Parse a CQ entry to retrieve the event type and the associated queue */
> +int
> +sli_cq_parse(struct sli4 *sli4, struct sli4_queue *cq, u8 *cqe,
> + enum sli4_qentry *etype, u16 *q_id)
> +{
> + int rc = EFC_SUCCESS;
> +
> + if (!cq || !cqe || !etype) {
> + efc_log_err(sli4, "bad params sli4=%p cq=%p cqe=%p etype=%p q_id=%p\n",
> + sli4, cq, cqe, etype, q_id);
> + return -EINVAL;
> + }
> +
> + if (cq->u.flag.dword & SLI4_QUEUE_FLAG_MQ) {
> + struct sli4_mcqe *mcqe = (void *)cqe;
whitespace damage
> +
> + if (le32_to_cpu(mcqe->dw3_flags) & SLI4_MCQE_AE) {
> + *etype = SLI_QENTRY_ASYNC;
> + } else {
> + *etype = SLI_QENTRY_MQ;
> + rc = sli_cqe_mq(sli4, mcqe);
> + }
> + *q_id = -1;
> + } else {
> + rc = sli_fc_cqe_parse(sli4, cq, cqe, etype, q_id);
> + }
> +
> + return rc;
> +}
> diff --git a/drivers/scsi/elx/libefc_sli/sli4.h b/drivers/scsi/elx/libefc_sli/sli4.h
> index b360d809f144..13f5a0d8d31c 100644
> --- a/drivers/scsi/elx/libefc_sli/sli4.h
> +++ b/drivers/scsi/elx/libefc_sli/sli4.h
> @@ -3687,4 +3687,13 @@ struct sli4 {
> u32 vpd_length;
> };
>
> +static inline void
> +sli_cmd_fill_hdr(struct sli4_rqst_hdr *hdr, u8 opc, u8 sub, u32 ver, __le32 len)
line is too long
> +{
> + hdr->opcode = opc;
> + hdr->subsystem = sub;
> + hdr->dw3_version = cpu_to_le32(ver);
> + hdr->request_length = len;
> +}
> +
> #endif /* !_SLI4_H */
> --
> 2.16.4
>
>
Thanks,
Daniel
