Re: [PATCH 5/8] RDMA/bng_re: Add infrastructure for enabling Firmware channel

On Mon, Sep 15, 2025 at 02:14:19PM +0530, Siva Reddy Kallam wrote:

On Fri, Sep 12, 2025 at 2:09 PM Simon Horman [off-list ref] wrote:

quoted

On Fri, Aug 29, 2025 at 12:30:39PM +0000, Siva Reddy Kallam wrote:

quoted

Add infrastructure for enabling Firmware channel.

Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
Reviewed-by: Usman Ansari <redacted>

...

quoted

diff --git a/drivers/infiniband/hw/bng_re/bng_fw.c b/drivers/infiniband/hw/bng_re/bng_fw.c

...

quoted

+/* function events */
+static int bng_re_process_func_event(struct bng_re_rcfw *rcfw,
+                                  struct creq_func_event *func_event)
+{
+     int rc;
+
+     switch (func_event->event) {
+     case CREQ_FUNC_EVENT_EVENT_TX_WQE_ERROR:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_TX_DATA_ERROR:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_RX_WQE_ERROR:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_RX_DATA_ERROR:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_CQ_ERROR:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_TQM_ERROR:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_CFCQ_ERROR:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_CFCS_ERROR:
+             /* SRQ ctx error, call srq_handler??
+              * But there's no SRQ handle!
+              */
+             break;
+     case CREQ_FUNC_EVENT_EVENT_CFCC_ERROR:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_CFCM_ERROR:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_TIM_ERROR:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_VF_COMM_REQUEST:
+             break;
+     case CREQ_FUNC_EVENT_EVENT_RESOURCE_EXHAUSTED:
+             break;
+     default:
+             return -EINVAL;
+     }
+
+     return rc;

rc does not appear to be initialised in this function.

Flagged by Clang 20.1.8 with -Wuninitialized

Thank you for the review. We will fix it in the next version of the patchset.

Once you are fixing it, please squeeze you switch-case to be something
like that:

switch (func_event->event) {
     case CREQ_FUNC_EVENT_EVENT_TX_WQE_ERROR:
     case CREQ_FUNC_EVENT_EVENT_TX_DATA_ERROR:
     case CREQ_FUNC_EVENT_EVENT_RX_WQE_ERROR:
     ....
     	break;
     default:
     	return -EINVAL;
    }

Thanks

quoted

quoted

+}

...

quoted

+int bng_re_enable_fw_channel(struct bng_re_rcfw *rcfw,
+                          int msix_vector,
+                          int cp_bar_reg_off)
+{
+     struct bng_re_cmdq_ctx *cmdq;
+     struct bng_re_creq_ctx *creq;
+     int rc;
+
+     cmdq = &rcfw->cmdq;
+     creq = &rcfw->creq;

Conversely, creq is initialised here but otherwise unused in this function.

Flagged by GCC 15.1.0 and Clang 20.1.8 with -Wunused-but-set-variable

Thank you for the review. We will fix it in the next version of the patchset.

quoted

quoted

+
+     /* Assign defaults */
+     cmdq->seq_num = 0;
+     set_bit(FIRMWARE_FIRST_FLAG, &cmdq->flags);
+     init_waitqueue_head(&cmdq->waitq);
+
+     rc = bng_re_map_cmdq_mbox(rcfw);
+     if (rc)
+             return rc;
+
+     rc = bng_re_map_creq_db(rcfw, cp_bar_reg_off);
+     if (rc)
+             return rc;
+
+     rc = bng_re_rcfw_start_irq(rcfw, msix_vector, true);
+     if (rc) {
+             dev_err(&rcfw->pdev->dev,
+                     "Failed to request IRQ for CREQ rc = 0x%x\n", rc);
+             bng_re_disable_rcfw_channel(rcfw);
+             return rc;
+     }
+
+     return 0;
+}

...