Re: [PATCH rdma-next v2 RESEND 0/4] RDMA/bnxt_re: Support for FW async event handling

[Leon Romanovsky <leon@kernel.org>]

On Tue, Jan 07, 2025 at 08:15:48AM +0530, Kalesh AP wrote:
> This patch series adds support for FW async event handling
> in the bnxt_re driver.
> 
> V1->V2:
> 1. Rebased on top of the latest "for-next" tree.
> 2. Split Patch#1 into 2 - one for Ethernet driver changes and
>    another one for RDMA driver changes.
> 3. Addressed Leon's comments on Patch#1 and Patch #3.
> V1: https://lore.kernel.org/linux-rdma/1725363051-19268-1-git-send-email-selvin.xavier@broadcom.com/T/#t
> 
> Patch #1:
> 1. Removed BNXT_EN_FLAG_ULP_STOPPED state check from bnxt_ulp_async_events().
>    The ulp_ops are protected by RCU. This means that during bnxt_unregister_dev(),
>    Ethernet driver set the ulp_ops pointer to NULL and do RCU sync before return
>    to the RDMA driver.
>    So ulp_ops and the pointers in ulp_ops are always valid or NULL when the
>    Ethernet driver references ulp_ops. ULP_STOPPED is a state and should be
>    unrelated to async events. It should not affect whether async events should
>    or should not be passed to the RDMA driver.
> 2. Changed Author of Ethernet driver changes to Michael Chan.
> 3. Removed unnecessary export of function bnxt_ulp_async_events.
> 
> Patch #3:
> 1. Removed unnecessary flush_workqueue() before destroy_workqueue()
> 2. Removed unnecessary NULL assignment after free.
> 3. Changed to use "ibdev_xxx" and reduce level of couple of logs to debug.
> 
> Please review and apply.
> 
> Regards,
> Kalesh
> 
> 
> Kalesh AP (3):
>   RDMA/bnxt_re: Add Async event handling support
>   RDMA/bnxt_re: Query firmware defaults of CC params during probe
>   RDMA/bnxt_re: Add support to handle DCB_CONFIG_CHANGE event
> 
> Michael Chan (1):
>   bnxt_en: Add ULP call to notify async events
> 
>  drivers/infiniband/hw/bnxt_re/bnxt_re.h       |   3 +
>  drivers/infiniband/hw/bnxt_re/main.c          | 156 ++++++++++++++++++
>  drivers/infiniband/hw/bnxt_re/qplib_fp.h      |   1 +
>  drivers/infiniband/hw/bnxt_re/qplib_sp.c      | 113 +++++++++++++
>  drivers/infiniband/hw/bnxt_re/qplib_sp.h      |   3 +
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     |   1 +
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c |  28 ++++
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |   2 +
>  8 files changed, 307 insertions(+)

Applied with the following fix

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 1dc305689d7bb..54dee0f5dd3f5 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -1802,30 +1802,22 @@ static int bnxt_re_setup_qos(struct bnxt_re_dev *rdev)
 
 static void bnxt_re_net_unregister_async_event(struct bnxt_re_dev *rdev)
 {
-	int rc;
-
 	if (rdev->is_virtfn)
 		return;
 
 	memset(&rdev->event_bitmap, 0, sizeof(rdev->event_bitmap));
-	rc = bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap,
-					ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE);
-	if (rc)
-		ibdev_err(&rdev->ibdev, "Failed to unregister async event");
+	bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap,
+				   ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE);
 }
 
 static void bnxt_re_net_register_async_event(struct bnxt_re_dev *rdev)
 {
-	int rc;
-
 	if (rdev->is_virtfn)
 		return;
 
 	rdev->event_bitmap |= (1 << ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE);
-	rc = bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap,
-					ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE);
-	if (rc)
-		ibdev_err(&rdev->ibdev, "Failed to unregister async event");
+	bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap,
+				   ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE);
 }
 
 static void bnxt_re_query_hwrm_intf_version(struct bnxt_re_dev *rdev)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 59c280634bc5f..3e17db0a453e0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -373,9 +373,8 @@ void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl)
 	rcu_read_unlock();
 }
 
-int bnxt_register_async_events(struct bnxt_en_dev *edev,
-			       unsigned long *events_bmap,
-			       u16 max_id)
+void bnxt_register_async_events(struct bnxt_en_dev *edev,
+				unsigned long *events_bmap, u16 max_id)
 {
 	struct net_device *dev = edev->net;
 	struct bnxt *bp = netdev_priv(dev);
@@ -387,7 +386,6 @@ int bnxt_register_async_events(struct bnxt_en_dev *edev,
 	smp_wmb();
 	ulp->max_async_event_id = max_id;
 	bnxt_hwrm_func_drv_rgtr(bp, events_bmap, max_id + 1, true);
-	return 0;
 }
 EXPORT_SYMBOL(bnxt_register_async_events);
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index a21294cf197b8..ee6a5b8562c3e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -126,6 +126,6 @@ int bnxt_register_dev(struct bnxt_en_dev *edev, struct bnxt_ulp_ops *ulp_ops,
 		      void *handle);
 void bnxt_unregister_dev(struct bnxt_en_dev *edev);
 int bnxt_send_msg(struct bnxt_en_dev *edev, struct bnxt_fw_msg *fw_msg);
-int bnxt_register_async_events(struct bnxt_en_dev *edev,
-			       unsigned long *events_bmap, u16 max_id);
+void bnxt_register_async_events(struct bnxt_en_dev *edev,
+				unsigned long *events_bmap, u16 max_id);
 #endif

> 
> -- 
> 2.43.5
>