[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMet4B7oKv3wHJaXK-iFdbdtR30G5oYCz17TFQDYMLtrWH20bA@mail.gmail.com>
Date: Mon, 15 Sep 2025 15:39:04 +0530
From: Siva Reddy Kallam <siva.kallam@...adcom.com>
To: Leon Romanovsky <leonro@...dia.com>
Cc: Simon Horman <horms@...nel.org>, jgg@...dia.com, linux-rdma@...r.kernel.org,
netdev@...r.kernel.org, vikas.gupta@...adcom.com, selvin.xavier@...adcom.com,
anand.subramanian@...adcom.com, Usman Ansari <usman.ansari@...adcom.com>
Subject: Re: [PATCH 5/8] RDMA/bng_re: Add infrastructure for enabling Firmware channel
On Mon, Sep 15, 2025 at 2:31 PM Leon Romanovsky <leonro@...dia.com> wrote:
>
> 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 <horms@...nel.org> wrote:
> > >
> > > On Fri, Aug 29, 2025 at 12:30:39PM +0000, Siva Reddy Kallam wrote:
> > > > Add infrastructure for enabling Firmware channel.
> > > >
> > > > Signed-off-by: Siva Reddy Kallam <siva.kallam@...adcom.com>
> > > > Reviewed-by: Usman Ansari <usman.ansari@...adcom.com>
> > >
> > > ...
> > >
> > > > diff --git a/drivers/infiniband/hw/bng_re/bng_fw.c b/drivers/infiniband/hw/bng_re/bng_fw.c
> > >
> > > ...
> > >
> > > > +/* 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
Thanks Leon for the suggestion. Sure, We will fix it in the next
version of the patchset.
>
> >
> > >
> > > > +}
> > >
> > > ...
> > >
> > > > +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.
> >
> > >
> > > > +
> > > > + /* 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;
> > > > +}
> > >
> > > ...
> >
Powered by blists - more mailing lists