[PATCH v4 0/6] Add Auxiliary driver support

[PATCH v4 0/6] Add Auxiliary driver support

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]

Add auxiliary device driver for Broadcom devices.
The bnxt_en driver will register and initialize an aux device
if RDMA is enabled in the underlying device.
The bnxt_re driver will then probe and initialize the
RoCE interfaces with the infiniband stack.

We got rid of the bnxt_en_ops which the bnxt_re driver used to
communicate with bnxt_en.
Similarly  We have tried to clean up most of the bnxt_ulp_ops.
In most of the cases we used the functions and entry points provided
by the auxiliary bus driver framework.
And now these are the minimal functions needed to support the functionality.

We will try to work on getting rid of the remaining if we find any
other viable option in future.

v1->v2:
- Incorporated review comments including usage of ulp_id &
  complex function indirections.
- Used function calls provided by the auxiliary bus interface
  instead of proprietary calls.
- Refactor code to remove ROCE driver's access to bnxt structure.

v2->v3:
- Addressed review comments including cleanup of some unnecessary wrappers
- Fixed warnings seen during cross compilation

v3->v4:
- Cleaned up bnxt_ulp.c and bnxt_ulp.h further
- Removed some more dead code
- Sending the patchset as a standalone series

Please apply. Thanks.

Ajit Khaparde (5):
  bnxt_en: Add auxiliary driver support
  RDMA/bnxt_re: Use auxiliary driver interface
  bnxt_en: Remove usage of ulp_id
  bnxt_en: Use direct API instead of indirection
  bnxt_en: Use auxiliary bus calls over proprietary calls

Hongguang Gao (1):
  bnxt_en: Remove struct bnxt access from RoCE driver

 drivers/infiniband/hw/bnxt_re/bnxt_re.h       |   9 +-
 drivers/infiniband/hw/bnxt_re/main.c          | 578 +++++++-----------
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  10 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   8 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 388 ++++++------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  52 +-
 6 files changed, 463 insertions(+), 582 deletions(-)

-- 
2.37.1 (Apple Git-137.1)


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

[PATCH v4 1/6] bnxt_en: Add auxiliary driver support

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 11520 bytes --]

Add auxiliary driver support.
An auxiliary device will be created if the hardware indicates
support for RDMA.
The bnxt_ulp_probe() function has been removed and a new
bnxt_rdma_aux_device_add() function has been added.
The bnxt_free_msix_vecs() and bnxt_req_msix_vecs() will now hold
the RTNL lock when they call the bnxt_close_nic()and bnxt_open_nic()
since the device close and open need to be protected under RTNL lock.
The operations between the bnxt_en and bnxt_re will be protected
using the en_ops_lock.
This will be used by the bnxt_re driver in a follow-on patch
to create ROCE interfaces.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |   8 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   8 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 177 +++++++++++++++---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |   7 +-
 4 files changed, 169 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d9ad325f7840..16265d177639 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -13145,6 +13145,9 @@ static void bnxt_remove_one(struct pci_dev *pdev)
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct bnxt *bp = netdev_priv(dev);
 
+	bnxt_rdma_aux_device_uninit(bp->aux_dev);
+	bnxt_aux_dev_free(bp);
+
 	if (BNXT_PF(bp))
 		bnxt_sriov_disable(bp);
 
@@ -13738,11 +13741,13 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	bnxt_dl_fw_reporters_create(bp);
 
+	bnxt_rdma_aux_device_init(bp);
+
 	bnxt_print_device_info(bp);
 
 	pci_save_state(pdev);
-	return 0;
 
+	return 0;
 init_err_cleanup:
 	bnxt_dl_unregister(bp);
 init_err_dl:
@@ -13786,7 +13791,6 @@ static void bnxt_shutdown(struct pci_dev *pdev)
 	if (netif_running(dev))
 		dev_close(dev);
 
-	bnxt_ulp_shutdown(bp);
 	bnxt_clear_int_mode(bp);
 	pci_disable_device(pdev);
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 7611bed4a196..222e1024ffde 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -24,6 +24,7 @@
 #include <linux/interrupt.h>
 #include <linux/rhashtable.h>
 #include <linux/crash_dump.h>
+#include <linux/auxiliary_bus.h>
 #include <net/devlink.h>
 #include <net/dst_metadata.h>
 #include <net/xdp.h>
@@ -1622,6 +1623,12 @@ struct bnxt_fw_health {
 #define BNXT_FW_RETRY			5
 #define BNXT_FW_IF_RETRY		10
 
+struct bnxt_aux_dev {
+	struct auxiliary_device aux_dev;
+	struct bnxt_en_dev *edev;
+	int id;
+};
+
 enum board_idx {
 	BCM57301,
 	BCM57302,
@@ -1843,6 +1850,7 @@ struct bnxt {
 #define BNXT_CHIP_P4_PLUS(bp)			\
 	(BNXT_CHIP_P4(bp) || BNXT_CHIP_P5(bp))
 
+	struct bnxt_aux_dev	*aux_dev;
 	struct bnxt_en_dev	*edev;
 
 	struct bnxt_napi	**bnapi;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 2e54bf4fc7a7..7ffd61ec2683 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -25,32 +25,37 @@
 #include "bnxt_hwrm.h"
 #include "bnxt_ulp.h"
 
+static DEFINE_IDA(bnxt_aux_dev_ids);
+
 static int bnxt_register_dev(struct bnxt_en_dev *edev, unsigned int ulp_id,
 			     struct bnxt_ulp_ops *ulp_ops, void *handle)
 {
 	struct net_device *dev = edev->net;
 	struct bnxt *bp = netdev_priv(dev);
 	struct bnxt_ulp *ulp;
+	int rc = 0;
 
-	ASSERT_RTNL();
 	if (ulp_id >= BNXT_MAX_ULP)
 		return -EINVAL;
 
 	ulp = &edev->ulp_tbl[ulp_id];
 	if (rcu_access_pointer(ulp->ulp_ops)) {
 		netdev_err(bp->dev, "ulp id %d already registered\n", ulp_id);
-		return -EBUSY;
+		rc = -EBUSY;
+		goto exit;
 	}
 	if (ulp_id == BNXT_ROCE_ULP) {
 		unsigned int max_stat_ctxs;
 
 		max_stat_ctxs = bnxt_get_max_func_stat_ctxs(bp);
 		if (max_stat_ctxs <= BNXT_MIN_ROCE_STAT_CTXS ||
-		    bp->cp_nr_rings == max_stat_ctxs)
-			return -ENOMEM;
+		    bp->cp_nr_rings == max_stat_ctxs) {
+			rc = -ENOMEM;
+			goto exit;
+		}
 	}
 
-	atomic_set(&ulp->ref_count, 0);
+	atomic_set(&ulp->ref_count, 1);
 	ulp->handle = handle;
 	rcu_assign_pointer(ulp->ulp_ops, ulp_ops);
 
@@ -59,7 +64,8 @@ static int bnxt_register_dev(struct bnxt_en_dev *edev, unsigned int ulp_id,
 			bnxt_hwrm_vnic_cfg(bp, 0);
 	}
 
-	return 0;
+exit:
+	return rc;
 }
 
 static int bnxt_unregister_dev(struct bnxt_en_dev *edev, unsigned int ulp_id)
@@ -69,10 +75,11 @@ static int bnxt_unregister_dev(struct bnxt_en_dev *edev, unsigned int ulp_id)
 	struct bnxt_ulp *ulp;
 	int i = 0;
 
-	ASSERT_RTNL();
 	if (ulp_id >= BNXT_MAX_ULP)
 		return -EINVAL;
 
+	edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
+
 	ulp = &edev->ulp_tbl[ulp_id];
 	if (!rcu_access_pointer(ulp->ulp_ops)) {
 		netdev_err(bp->dev, "ulp id %d not registered\n", ulp_id);
@@ -126,7 +133,6 @@ static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id,
 	int total_vecs;
 	int rc = 0;
 
-	ASSERT_RTNL();
 	if (ulp_id != BNXT_ROCE_ULP)
 		return -EINVAL;
 
@@ -149,6 +155,7 @@ static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id,
 		max_idx = min_t(int, bp->total_irqs, max_cp_rings);
 		idx = max_idx - avail_msix;
 	}
+
 	edev->ulp_tbl[ulp_id].msix_base = idx;
 	edev->ulp_tbl[ulp_id].msix_requested = avail_msix;
 	hw_resc = &bp->hw_resc;
@@ -156,8 +163,10 @@ static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id,
 	if (bp->total_irqs < total_vecs ||
 	    (BNXT_NEW_RM(bp) && hw_resc->resv_irqs < total_vecs)) {
 		if (netif_running(dev)) {
+			rtnl_lock();
 			bnxt_close_nic(bp, true, false);
 			rc = bnxt_open_nic(bp, true, false);
+			rtnl_unlock();
 		} else {
 			rc = bnxt_reserve_rings(bp, true);
 		}
@@ -184,7 +193,6 @@ static int bnxt_free_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id)
 	struct net_device *dev = edev->net;
 	struct bnxt *bp = netdev_priv(dev);
 
-	ASSERT_RTNL();
 	if (ulp_id != BNXT_ROCE_ULP)
 		return -EINVAL;
 
@@ -194,9 +202,12 @@ static int bnxt_free_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id)
 	edev->ulp_tbl[ulp_id].msix_requested = 0;
 	edev->flags &= ~BNXT_EN_FLAG_MSIX_REQUESTED;
 	if (netif_running(dev) && !(edev->flags & BNXT_EN_FLAG_ULP_STOPPED)) {
+		rtnl_lock();
 		bnxt_close_nic(bp, true, false);
 		bnxt_open_nic(bp, true, false);
+		rtnl_unlock();
 	}
+
 	return 0;
 }
 
@@ -347,25 +358,6 @@ void bnxt_ulp_sriov_cfg(struct bnxt *bp, int num_vfs)
 	}
 }
 
-void bnxt_ulp_shutdown(struct bnxt *bp)
-{
-	struct bnxt_en_dev *edev = bp->edev;
-	struct bnxt_ulp_ops *ops;
-	int i;
-
-	if (!edev)
-		return;
-
-	for (i = 0; i < BNXT_MAX_ULP; i++) {
-		struct bnxt_ulp *ulp = &edev->ulp_tbl[i];
-
-		ops = rtnl_dereference(ulp->ulp_ops);
-		if (!ops || !ops->ulp_shutdown)
-			continue;
-		ops->ulp_shutdown(ulp->handle);
-	}
-}
-
 void bnxt_ulp_irq_stop(struct bnxt *bp)
 {
 	struct bnxt_en_dev *edev = bp->edev;
@@ -475,6 +467,135 @@ static const struct bnxt_en_ops bnxt_en_ops_tbl = {
 	.bnxt_register_fw_async_events	= bnxt_register_async_events,
 };
 
+void bnxt_aux_dev_free(struct bnxt *bp)
+{
+	kfree(bp->aux_dev);
+	bp->aux_dev = NULL;
+}
+
+static struct bnxt_aux_dev *bnxt_aux_dev_alloc(struct bnxt *bp)
+{
+	struct bnxt_aux_dev *bnxt_adev;
+
+	bnxt_adev =  kzalloc(sizeof(*bnxt_adev), GFP_KERNEL);
+	if (!bnxt_adev)
+		return ERR_PTR(-ENOMEM);
+
+	return bnxt_adev;
+}
+
+void bnxt_rdma_aux_device_uninit(struct bnxt_aux_dev *bnxt_adev)
+{
+	struct auxiliary_device *adev;
+
+	if (IS_ERR_OR_NULL(bnxt_adev))
+		return;
+
+	adev = &bnxt_adev->aux_dev;
+	auxiliary_device_delete(adev);
+	auxiliary_device_uninit(adev);
+	if (bnxt_adev->id >= 0)
+		ida_free(&bnxt_aux_dev_ids, bnxt_adev->id);
+}
+
+void bnxt_rdma_aux_device_init(struct bnxt *bp)
+{
+	struct net_device *dev = bp->dev;
+	int rc;
+
+	if (bp->flags & BNXT_FLAG_ROCE_CAP) {
+		bp->aux_dev = bnxt_aux_dev_alloc(bp);
+		if (IS_ERR_OR_NULL(bp->aux_dev)) {
+			netdev_warn(dev, "Failed to init auxiliary device for ROCE\n");
+			goto skip_aux_init;
+		}
+
+		bp->aux_dev->id = ida_alloc(&bnxt_aux_dev_ids, GFP_KERNEL);
+		if (bp->aux_dev->id < 0) {
+			netdev_warn(dev, "ida alloc failed for ROCE auxiliary device\n");
+			bnxt_aux_dev_free(bp);
+			goto skip_aux_init;
+		}
+
+		/* If aux bus init fails, continue with netdev init. */
+		rc = bnxt_rdma_aux_device_add(bp);
+		if (rc) {
+			netdev_warn(dev, "Failed to add auxiliary device for ROCE\n");
+			ida_free(&bnxt_aux_dev_ids, bp->aux_dev->id);
+			bnxt_aux_dev_free(bp);
+		}
+	}
+skip_aux_init:
+	return;
+}
+
+void bnxt_aux_dev_release(struct device *dev)
+{
+	struct bnxt_aux_dev *bnxt_adev =
+		container_of(dev, struct bnxt_aux_dev, aux_dev.dev);
+	struct bnxt *bp = netdev_priv(bnxt_adev->edev->net);
+
+	bnxt_adev->edev->en_ops = NULL;
+	kfree(bnxt_adev->edev);
+	bnxt_adev->edev = NULL;
+	bp->edev = NULL;
+}
+
+static inline void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
+{
+	edev->en_ops = &bnxt_en_ops_tbl;
+	edev->net = bp->dev;
+	edev->pdev = bp->pdev;
+	edev->l2_db_size = bp->db_size;
+	edev->l2_db_size_nc = bp->db_size;
+
+	if (bp->flags & BNXT_FLAG_ROCEV1_CAP)
+		edev->flags |= BNXT_EN_FLAG_ROCEV1_CAP;
+	if (bp->flags & BNXT_FLAG_ROCEV2_CAP)
+		edev->flags |= BNXT_EN_FLAG_ROCEV2_CAP;
+}
+
+int bnxt_rdma_aux_device_add(struct bnxt *bp)
+{
+	struct bnxt_aux_dev *bnxt_adev = bp->aux_dev;
+	struct bnxt_en_dev *edev = bnxt_adev->edev;
+	struct auxiliary_device *aux_dev;
+	int ret;
+
+	aux_dev = &bnxt_adev->aux_dev;
+	aux_dev->id = bnxt_adev->id;
+	aux_dev->name = "rdma";
+	aux_dev->dev.parent = &bp->pdev->dev;
+	aux_dev->dev.release = bnxt_aux_dev_release;
+
+	if (!edev) {
+		edev = kzalloc(sizeof(*edev), GFP_KERNEL);
+		if (!edev)
+			return -ENOMEM;
+	}
+
+	bnxt_set_edev_info(edev, bp);
+	bnxt_adev->edev = edev;
+	bp->edev = edev;
+
+	ret = auxiliary_device_init(aux_dev);
+	if (ret) {
+		kfree(edev);
+		kfree(bnxt_adev);
+		return ret;
+	}
+
+	ret = auxiliary_device_add(aux_dev);
+	if (ret) {
+		kfree(edev);
+		kfree(bnxt_adev);
+		auxiliary_device_uninit(aux_dev);
+		return ret;
+	}
+
+	return 0;
+}
+
 struct bnxt_en_dev *bnxt_ulp_probe(struct net_device *dev)
 {
 	struct bnxt *bp = netdev_priv(dev);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index 42b50abc3e91..8b85c663d881 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -17,6 +17,7 @@
 #define BNXT_MIN_ROCE_STAT_CTXS	1
 
 struct hwrm_async_event_cmpl;
+struct bnxt_aux_dev;
 struct bnxt;
 
 struct bnxt_msix_entry {
@@ -102,10 +103,14 @@ int bnxt_get_ulp_stat_ctxs(struct bnxt *bp);
 void bnxt_ulp_stop(struct bnxt *bp);
 void bnxt_ulp_start(struct bnxt *bp, int err);
 void bnxt_ulp_sriov_cfg(struct bnxt *bp, int num_vfs);
-void bnxt_ulp_shutdown(struct bnxt *bp);
 void bnxt_ulp_irq_stop(struct bnxt *bp);
 void bnxt_ulp_irq_restart(struct bnxt *bp, int err);
 void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl);
+void bnxt_aux_dev_release(struct device *dev);
+int bnxt_rdma_aux_device_add(struct bnxt *bp);
+void bnxt_rdma_aux_device_uninit(struct bnxt_aux_dev *bnxt_adev);
+void bnxt_rdma_aux_device_init(struct bnxt *bp);
+void bnxt_aux_dev_free(struct bnxt *bp);
 struct bnxt_en_dev *bnxt_ulp_probe(struct net_device *dev);
 
 #endif
-- 
2.37.1 (Apple Git-137.1)


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

[PATCH v4 2/6] RDMA/bnxt_re: Use auxiliary driver interface

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 19490 bytes --]

Use auxiliary driver interface for driver load, unload ROCE driver.
The driver does not need to register the interface using the netdev
notifier anymore. Removed the bnxt_re_dev_list which is not needed.
Currently probe, remove and shutdown ops have been implemented for
the auxiliary device.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/bnxt_re.h       |   9 +-
 drivers/infiniband/hw/bnxt_re/main.c          | 389 ++++++------------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c |  29 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |   3 -
 4 files changed, 135 insertions(+), 295 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
index 785c37cae3c0..b0465c8d229a 100644
--- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
+++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
@@ -89,13 +89,6 @@ struct bnxt_re_ring_attr {
 	u8		mode;
 };
 
-struct bnxt_re_work {
-	struct work_struct	work;
-	unsigned long		event;
-	struct bnxt_re_dev      *rdev;
-	struct net_device	*vlan_dev;
-};
-
 struct bnxt_re_sqp_entries {
 	struct bnxt_qplib_sge sge;
 	u64 wrid;
@@ -132,6 +125,7 @@ struct bnxt_re_dev {
 #define BNXT_RE_FLAG_ERR_DEVICE_DETACHED       17
 #define BNXT_RE_FLAG_ISSUE_ROCE_STATS          29
 	struct net_device		*netdev;
+	struct notifier_block		nb;
 	unsigned int			version, major, minor;
 	struct bnxt_qplib_chip_ctx	*chip_ctx;
 	struct bnxt_en_dev		*en_dev;
@@ -194,5 +188,4 @@ static inline struct device *rdev_to_dev(struct bnxt_re_dev *rdev)
 		return  &rdev->ibdev.dev;
 	return NULL;
 }
-
 #endif
diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 8c0c80a8d338..a9fb1067c9a1 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -48,6 +48,7 @@
 #include <net/ipv6.h>
 #include <net/addrconf.h>
 #include <linux/if_ether.h>
+#include <linux/auxiliary_bus.h>
 
 #include <rdma/ib_verbs.h>
 #include <rdma/ib_user_verbs.h>
@@ -74,14 +75,14 @@ MODULE_DESCRIPTION(BNXT_RE_DESC " Driver");
 MODULE_LICENSE("Dual BSD/GPL");
 
 /* globals */
-static struct list_head bnxt_re_dev_list = LIST_HEAD_INIT(bnxt_re_dev_list);
-/* Mutex to protect the list of bnxt_re devices added */
-static DEFINE_MUTEX(bnxt_re_dev_lock);
-static struct workqueue_struct *bnxt_re_wq;
-static void bnxt_re_remove_device(struct bnxt_re_dev *rdev);
-static void bnxt_re_dealloc_driver(struct ib_device *ib_dev);
+static DEFINE_MUTEX(bnxt_re_mutex);
+
 static void bnxt_re_stop_irq(void *handle);
 static void bnxt_re_dev_stop(struct bnxt_re_dev *rdev);
+static int bnxt_re_netdev_event(struct notifier_block *notifier,
+				unsigned long event, void *ptr);
+static struct bnxt_re_dev *bnxt_re_from_netdev(struct net_device *netdev);
+static void bnxt_re_dev_uninit(struct bnxt_re_dev *rdev);
 
 static void bnxt_re_set_drv_mode(struct bnxt_re_dev *rdev, u8 mode)
 {
@@ -233,7 +234,6 @@ static void bnxt_re_stop(void *p)
 
 	if (!rdev)
 		return;
-	ASSERT_RTNL();
 
 	/* L2 driver invokes this callback during device error/crash or device
 	 * reset. Current RoCE driver doesn't recover the device in case of
@@ -282,16 +282,14 @@ static void bnxt_re_sriov_config(void *p, int num_vfs)
 	}
 }
 
-static void bnxt_re_shutdown(void *p)
+static void bnxt_re_shutdown(struct auxiliary_device *adev)
 {
-	struct bnxt_re_dev *rdev = p;
+	struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
 
 	if (!rdev)
 		return;
-	ASSERT_RTNL();
-	/* Release the MSIx vectors before queuing unregister */
-	bnxt_re_stop_irq(rdev);
-	ib_unregister_device_queued(&rdev->ibdev);
+	ib_unregister_device(&rdev->ibdev);
+	bnxt_re_dev_uninit(rdev);
 }
 
 static void bnxt_re_stop_irq(void *handle)
@@ -346,11 +344,9 @@ static void bnxt_re_start_irq(void *handle, struct bnxt_msix_entry *ent)
 }
 
 static struct bnxt_ulp_ops bnxt_re_ulp_ops = {
-	.ulp_async_notifier = NULL,
 	.ulp_stop = bnxt_re_stop,
 	.ulp_start = bnxt_re_start,
 	.ulp_sriov_config = bnxt_re_sriov_config,
-	.ulp_shutdown = bnxt_re_shutdown,
 	.ulp_irq_stop = bnxt_re_stop_irq,
 	.ulp_irq_restart = bnxt_re_start_irq
 };
@@ -401,7 +397,6 @@ static int bnxt_re_free_msix(struct bnxt_re_dev *rdev)
 
 	en_dev = rdev->en_dev;
 
-
 	rc = en_dev->en_ops->bnxt_free_msix(rdev->en_dev, BNXT_ROCE_ULP);
 
 	return rc;
@@ -458,12 +453,17 @@ static void bnxt_re_fill_fw_msg(struct bnxt_fw_msg *fw_msg, void *msg,
 static int bnxt_re_net_ring_free(struct bnxt_re_dev *rdev,
 				 u16 fw_ring_id, int type)
 {
-	struct bnxt_en_dev *en_dev = rdev->en_dev;
+	struct bnxt_en_dev *en_dev;
 	struct hwrm_ring_free_input req = {0};
 	struct hwrm_ring_free_output resp;
 	struct bnxt_fw_msg fw_msg;
 	int rc = -EINVAL;
 
+	if (!rdev)
+		return rc;
+
+	en_dev = rdev->en_dev;
+
 	if (!en_dev)
 		return rc;
 
@@ -584,21 +584,6 @@ static int bnxt_re_net_stats_ctx_alloc(struct bnxt_re_dev *rdev,
 
 /* Device */
 
-static bool is_bnxt_re_dev(struct net_device *netdev)
-{
-	struct ethtool_drvinfo drvinfo;
-
-	if (netdev->ethtool_ops && netdev->ethtool_ops->get_drvinfo) {
-		memset(&drvinfo, 0, sizeof(drvinfo));
-		netdev->ethtool_ops->get_drvinfo(netdev, &drvinfo);
-
-		if (strcmp(drvinfo.driver, "bnxt_en"))
-			return false;
-		return true;
-	}
-	return false;
-}
-
 static struct bnxt_re_dev *bnxt_re_from_netdev(struct net_device *netdev)
 {
 	struct ib_device *ibdev =
@@ -609,31 +594,6 @@ static struct bnxt_re_dev *bnxt_re_from_netdev(struct net_device *netdev)
 	return container_of(ibdev, struct bnxt_re_dev, ibdev);
 }
 
-static struct bnxt_en_dev *bnxt_re_dev_probe(struct net_device *netdev)
-{
-	struct bnxt_en_dev *en_dev;
-	struct pci_dev *pdev;
-
-	en_dev = bnxt_ulp_probe(netdev);
-	if (IS_ERR(en_dev))
-		return en_dev;
-
-	pdev = en_dev->pdev;
-	if (!pdev)
-		return ERR_PTR(-EINVAL);
-
-	if (!(en_dev->flags & BNXT_EN_FLAG_ROCE_CAP)) {
-		dev_info(&pdev->dev,
-			"%s: probe error: RoCE is not supported on this device",
-			ROCE_DRV_MODULE_NAME);
-		return ERR_PTR(-ENODEV);
-	}
-
-	dev_hold(netdev);
-
-	return en_dev;
-}
-
 static ssize_t hw_rev_show(struct device *device, struct device_attribute *attr,
 			   char *buf)
 {
@@ -679,7 +639,6 @@ static const struct ib_device_ops bnxt_re_dev_ops = {
 	.create_qp = bnxt_re_create_qp,
 	.create_srq = bnxt_re_create_srq,
 	.create_user_ah = bnxt_re_create_ah,
-	.dealloc_driver = bnxt_re_dealloc_driver,
 	.dealloc_pd = bnxt_re_dealloc_pd,
 	.dealloc_ucontext = bnxt_re_dealloc_ucontext,
 	.del_gid = bnxt_re_del_gid,
@@ -744,18 +703,7 @@ static int bnxt_re_register_ib(struct bnxt_re_dev *rdev)
 	return ib_register_device(ibdev, "bnxt_re%d", &rdev->en_dev->pdev->dev);
 }
 
-static void bnxt_re_dev_remove(struct bnxt_re_dev *rdev)
-{
-	dev_put(rdev->netdev);
-	rdev->netdev = NULL;
-	mutex_lock(&bnxt_re_dev_lock);
-	list_del_rcu(&rdev->list);
-	mutex_unlock(&bnxt_re_dev_lock);
-
-	synchronize_rcu();
-}
-
-static struct bnxt_re_dev *bnxt_re_dev_add(struct net_device *netdev,
+static struct bnxt_re_dev *bnxt_re_dev_add(struct bnxt_aux_dev *aux_dev,
 					   struct bnxt_en_dev *en_dev)
 {
 	struct bnxt_re_dev *rdev;
@@ -768,8 +716,8 @@ static struct bnxt_re_dev *bnxt_re_dev_add(struct net_device *netdev,
 		return NULL;
 	}
 	/* Default values */
-	rdev->netdev = netdev;
-	dev_hold(rdev->netdev);
+	rdev->nb.notifier_call = NULL;
+	rdev->netdev = en_dev->net;
 	rdev->en_dev = en_dev;
 	rdev->id = rdev->en_dev->pdev->devfn;
 	INIT_LIST_HEAD(&rdev->qp_list);
@@ -784,9 +732,6 @@ static struct bnxt_re_dev *bnxt_re_dev_add(struct net_device *netdev,
 	rdev->cosq[0] = 0xFFFF;
 	rdev->cosq[1] = 0xFFFF;
 
-	mutex_lock(&bnxt_re_dev_lock);
-	list_add_tail_rcu(&rdev->list, &bnxt_re_dev_list);
-	mutex_unlock(&bnxt_re_dev_lock);
 	return rdev;
 }
 
@@ -1147,6 +1092,9 @@ static void bnxt_re_dev_stop(struct bnxt_re_dev *rdev)
 	struct ib_qp_attr qp_attr;
 	struct bnxt_re_qp *qp;
 
+	if (!rdev)
+		return;
+
 	qp_attr.qp_state = IB_QPS_ERR;
 	mutex_lock(&rdev->qp_lock);
 	list_for_each_entry(qp, &rdev->qp_list, list) {
@@ -1323,7 +1271,7 @@ static int bnxt_re_ib_init(struct bnxt_re_dev *rdev)
 		pr_err("Failed to register with IB: %#x\n", rc);
 		return rc;
 	}
-	dev_info(rdev_to_dev(rdev), "Device registered successfully");
+	dev_info(rdev_to_dev(rdev), "Device registered with IB successfully");
 	ib_get_eth_speed(&rdev->ibdev, 1, &rdev->active_speed,
 			 &rdev->active_width);
 	set_bit(BNXT_RE_FLAG_ISSUE_ROCE_STATS, &rdev->flags);
@@ -1541,135 +1489,43 @@ static int bnxt_re_dev_init(struct bnxt_re_dev *rdev, u8 wqe_mode)
 	return rc;
 }
 
-static void bnxt_re_dev_unreg(struct bnxt_re_dev *rdev)
-{
-	struct net_device *netdev = rdev->netdev;
-
-	bnxt_re_dev_remove(rdev);
-
-	if (netdev)
-		dev_put(netdev);
-}
-
-static int bnxt_re_dev_reg(struct bnxt_re_dev **rdev, struct net_device *netdev)
+static int bnxt_re_add_device(struct auxiliary_device *adev, u8 wqe_mode)
 {
+	struct bnxt_aux_dev *aux_dev =
+		container_of(adev, struct bnxt_aux_dev, aux_dev);
 	struct bnxt_en_dev *en_dev;
+	struct bnxt_re_dev *rdev;
 	int rc = 0;
 
-	if (!is_bnxt_re_dev(netdev))
-		return -ENODEV;
+	/* en_dev should never be NULL as long as adev and aux_dev are valid. */
+	en_dev = aux_dev->edev;
 
-	en_dev = bnxt_re_dev_probe(netdev);
-	if (IS_ERR(en_dev)) {
-		if (en_dev != ERR_PTR(-ENODEV))
-			ibdev_err(&(*rdev)->ibdev, "%s: Failed to probe\n",
-				  ROCE_DRV_MODULE_NAME);
-		rc = PTR_ERR(en_dev);
-		goto exit;
-	}
-	*rdev = bnxt_re_dev_add(netdev, en_dev);
-	if (!*rdev) {
+	rdev = bnxt_re_dev_add(aux_dev, en_dev);
+	if (!rdev || !rdev_to_dev(rdev)) {
 		rc = -ENOMEM;
-		dev_put(netdev);
 		goto exit;
 	}
-exit:
-	return rc;
-}
-
-static void bnxt_re_remove_device(struct bnxt_re_dev *rdev)
-{
-	bnxt_re_dev_uninit(rdev);
-	pci_dev_put(rdev->en_dev->pdev);
-	bnxt_re_dev_unreg(rdev);
-}
-
-static int bnxt_re_add_device(struct bnxt_re_dev **rdev,
-			      struct net_device *netdev, u8 wqe_mode)
-{
-	int rc;
 
-	rc = bnxt_re_dev_reg(rdev, netdev);
-	if (rc == -ENODEV)
-		return rc;
-	if (rc) {
-		pr_err("Failed to register with the device %s: %#x\n",
-		       netdev->name, rc);
-		return rc;
-	}
+	rc = bnxt_re_dev_init(rdev, wqe_mode);
+	if (rc)
+		goto re_dev_dealloc;
 
-	pci_dev_get((*rdev)->en_dev->pdev);
-	rc = bnxt_re_dev_init(*rdev, wqe_mode);
+	rc = bnxt_re_ib_init(rdev);
 	if (rc) {
-		pci_dev_put((*rdev)->en_dev->pdev);
-		bnxt_re_dev_unreg(*rdev);
+		pr_err("Failed to register with IB: %s",
+			aux_dev->aux_dev.name);
+		goto re_dev_uninit;
 	}
+	auxiliary_set_drvdata(adev, rdev);
 
-	return rc;
-}
-
-static void bnxt_re_dealloc_driver(struct ib_device *ib_dev)
-{
-	struct bnxt_re_dev *rdev =
-		container_of(ib_dev, struct bnxt_re_dev, ibdev);
-
-	dev_info(rdev_to_dev(rdev), "Unregistering Device");
-
-	rtnl_lock();
-	bnxt_re_remove_device(rdev);
-	rtnl_unlock();
-}
-
-/* Handle all deferred netevents tasks */
-static void bnxt_re_task(struct work_struct *work)
-{
-	struct bnxt_re_work *re_work;
-	struct bnxt_re_dev *rdev;
-	int rc = 0;
-
-	re_work = container_of(work, struct bnxt_re_work, work);
-	rdev = re_work->rdev;
-
-	if (re_work->event == NETDEV_REGISTER) {
-		rc = bnxt_re_ib_init(rdev);
-		if (rc) {
-			ibdev_err(&rdev->ibdev,
-				  "Failed to register with IB: %#x", rc);
-			rtnl_lock();
-			bnxt_re_remove_device(rdev);
-			rtnl_unlock();
-			goto exit;
-		}
-		goto exit;
-	}
-
-	if (!ib_device_try_get(&rdev->ibdev))
-		goto exit;
+	return 0;
 
-	switch (re_work->event) {
-	case NETDEV_UP:
-		bnxt_re_dispatch_event(&rdev->ibdev, NULL, 1,
-				       IB_EVENT_PORT_ACTIVE);
-		break;
-	case NETDEV_DOWN:
-		bnxt_re_dev_stop(rdev);
-		break;
-	case NETDEV_CHANGE:
-		if (!netif_carrier_ok(rdev->netdev))
-			bnxt_re_dev_stop(rdev);
-		else if (netif_carrier_ok(rdev->netdev))
-			bnxt_re_dispatch_event(&rdev->ibdev, NULL, 1,
-					       IB_EVENT_PORT_ACTIVE);
-		ib_get_eth_speed(&rdev->ibdev, 1, &rdev->active_speed,
-				 &rdev->active_width);
-		break;
-	default:
-		break;
-	}
-	ib_device_put(&rdev->ibdev);
+re_dev_uninit:
+	bnxt_re_dev_uninit(rdev);
+re_dev_dealloc:
+	ib_dealloc_device(&rdev->ibdev);
 exit:
-	put_device(&rdev->ibdev.dev);
-	kfree(re_work);
+	return rc;
 }
 
 /*
@@ -1690,109 +1546,130 @@ static int bnxt_re_netdev_event(struct notifier_block *notifier,
 				unsigned long event, void *ptr)
 {
 	struct net_device *real_dev, *netdev = netdev_notifier_info_to_dev(ptr);
-	struct bnxt_re_work *re_work;
 	struct bnxt_re_dev *rdev;
-	int rc = 0;
-	bool sch_work = false;
-	bool release = true;
 
 	real_dev = rdma_vlan_dev_real_dev(netdev);
 	if (!real_dev)
 		real_dev = netdev;
 
-	rdev = bnxt_re_from_netdev(real_dev);
-	if (!rdev && event != NETDEV_REGISTER)
-		return NOTIFY_OK;
-
 	if (real_dev != netdev)
 		goto exit;
 
-	switch (event) {
-	case NETDEV_REGISTER:
-		if (rdev)
-			break;
-		rc = bnxt_re_add_device(&rdev, real_dev,
-					BNXT_QPLIB_WQE_MODE_STATIC);
-		if (!rc)
-			sch_work = true;
-		release = false;
-		break;
+	rdev = bnxt_re_from_netdev(real_dev);
+	if (!rdev)
+		return NOTIFY_DONE;
 
-	case NETDEV_UNREGISTER:
-		ib_unregister_device_queued(&rdev->ibdev);
-		break;
 
+	switch (event) {
+	case NETDEV_UP:
+	case NETDEV_DOWN:
+	case NETDEV_CHANGE:
+		bnxt_re_dispatch_event(&rdev->ibdev, NULL, 1,
+					netif_carrier_ok(real_dev) ?
+					IB_EVENT_PORT_ACTIVE :
+					IB_EVENT_PORT_ERR);
+		break;
 	default:
-		sch_work = true;
 		break;
 	}
-	if (sch_work) {
-		/* Allocate for the deferred task */
-		re_work = kzalloc(sizeof(*re_work), GFP_KERNEL);
-		if (re_work) {
-			get_device(&rdev->ibdev.dev);
-			re_work->rdev = rdev;
-			re_work->event = event;
-			re_work->vlan_dev = (real_dev == netdev ?
-					     NULL : netdev);
-			INIT_WORK(&re_work->work, bnxt_re_task);
-			queue_work(bnxt_re_wq, &re_work->work);
-		}
-	}
-
+	ib_device_put(&rdev->ibdev);
 exit:
-	if (rdev && release)
-		ib_device_put(&rdev->ibdev);
 	return NOTIFY_DONE;
 }
 
-static struct notifier_block bnxt_re_netdev_notifier = {
-	.notifier_call = bnxt_re_netdev_event
-};
+#define BNXT_ADEV_NAME "bnxt_en"
 
-static int __init bnxt_re_mod_init(void)
+static void bnxt_re_remove(struct auxiliary_device *adev)
 {
-	int rc = 0;
+	struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
 
-	pr_info("%s: %s", ROCE_DRV_MODULE_NAME, version);
+	if (!rdev)
+		return;
 
-	bnxt_re_wq = create_singlethread_workqueue("bnxt_re");
-	if (!bnxt_re_wq)
-		return -ENOMEM;
+	mutex_lock(&bnxt_re_mutex);
+	if (rdev->nb.notifier_call) {
+		unregister_netdevice_notifier(&rdev->nb);
+		rdev->nb.notifier_call = NULL;
+	} else {
+		/* If notifier is null, we should have already done a
+		 * clean up before coming here.
+		 */
+		goto skip_remove;
+	}
 
-	INIT_LIST_HEAD(&bnxt_re_dev_list);
+	ib_unregister_device(&rdev->ibdev);
+	ib_dealloc_device(&rdev->ibdev);
+	bnxt_re_dev_uninit(rdev);
+skip_remove:
+	mutex_unlock(&bnxt_re_mutex);
+}
 
-	rc = register_netdevice_notifier(&bnxt_re_netdev_notifier);
+static int bnxt_re_probe(struct auxiliary_device *adev,
+			 const struct auxiliary_device_id *id)
+{
+	struct bnxt_re_dev *rdev;
+	int rc;
+
+	mutex_lock(&bnxt_re_mutex);
+	rc = bnxt_re_add_device(adev, BNXT_QPLIB_WQE_MODE_STATIC);
 	if (rc) {
+		mutex_unlock(&bnxt_re_mutex);
+		return rc;
+	}
+
+	rdev = auxiliary_get_drvdata(adev);
+
+	rdev->nb.notifier_call = bnxt_re_netdev_event;
+	rc = register_netdevice_notifier(&rdev->nb);
+	if (rc) {
+		rdev->nb.notifier_call = NULL;
 		pr_err("%s: Cannot register to netdevice_notifier",
 		       ROCE_DRV_MODULE_NAME);
-		goto err_netdev;
+		goto err;
 	}
+
+	mutex_unlock(&bnxt_re_mutex);
 	return 0;
 
-err_netdev:
-	destroy_workqueue(bnxt_re_wq);
+err:
+	mutex_unlock(&bnxt_re_mutex);
+	bnxt_re_remove(adev);
 
 	return rc;
 }
 
-static void __exit bnxt_re_mod_exit(void)
+static const struct auxiliary_device_id bnxt_re_id_table[] = {
+	{ .name = BNXT_ADEV_NAME ".rdma", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(auxiliary, bnxt_re_id_table);
+
+static struct auxiliary_driver bnxt_re_driver = {
+	.name = "rdma",
+	.probe = bnxt_re_probe,
+	.remove = bnxt_re_remove,
+	.shutdown = bnxt_re_shutdown,
+	.id_table = bnxt_re_id_table,
+};
+
+static int __init bnxt_re_mod_init(void)
 {
-	struct bnxt_re_dev *rdev;
+	int rc = 0;
 
-	unregister_netdevice_notifier(&bnxt_re_netdev_notifier);
-	if (bnxt_re_wq)
-		destroy_workqueue(bnxt_re_wq);
-	list_for_each_entry(rdev, &bnxt_re_dev_list, list) {
-		/* VF device removal should be called before the removal
-		 * of PF device. Queue VFs unregister first, so that VFs
-		 * shall be removed before the PF during the call of
-		 * ib_unregister_driver.
-		 */
-		if (rdev->is_virtfn)
-			ib_unregister_device(&rdev->ibdev);
+	pr_info("%s: %s", ROCE_DRV_MODULE_NAME, version);
+	rc = auxiliary_driver_register(&bnxt_re_driver);
+	if (rc) {
+		pr_err("%s: Failed to register auxiliary driver\n",
+			ROCE_DRV_MODULE_NAME);
+		return rc;
 	}
-	ib_unregister_driver(RDMA_DRIVER_BNXT_RE);
+	return 0;
+}
+
+static void __exit bnxt_re_mod_exit(void)
+{
+	auxiliary_driver_unregister(&bnxt_re_driver);
 }
 
 module_init(bnxt_re_mod_init);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 7ffd61ec2683..1d2d30f97ed9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -19,6 +19,7 @@
 #include <linux/irq.h>
 #include <asm/byteorder.h>
 #include <linux/bitmap.h>
+#include <linux/auxiliary_bus.h>
 
 #include "bnxt_hsi.h"
 #include "bnxt.h"
@@ -78,8 +79,6 @@ static int bnxt_unregister_dev(struct bnxt_en_dev *edev, unsigned int ulp_id)
 	if (ulp_id >= BNXT_MAX_ULP)
 		return -EINVAL;
 
-	edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
-
 	ulp = &edev->ulp_tbl[ulp_id];
 	if (!rcu_access_pointer(ulp->ulp_ops)) {
 		netdev_err(bp->dev, "ulp id %d not registered\n", ulp_id);
@@ -595,29 +594,3 @@ int bnxt_rdma_aux_device_add(struct bnxt *bp)
 
 	return 0;
 }
-
-struct bnxt_en_dev *bnxt_ulp_probe(struct net_device *dev)
-{
-	struct bnxt *bp = netdev_priv(dev);
-	struct bnxt_en_dev *edev;
-
-	edev = bp->edev;
-	if (!edev) {
-		edev = kzalloc(sizeof(*edev), GFP_KERNEL);
-		if (!edev)
-			return ERR_PTR(-ENOMEM);
-		edev->en_ops = &bnxt_en_ops_tbl;
-		edev->net = dev;
-		edev->pdev = bp->pdev;
-		edev->l2_db_size = bp->db_size;
-		edev->l2_db_size_nc = bp->db_size;
-		bp->edev = edev;
-	}
-	edev->flags &= ~BNXT_EN_FLAG_ROCE_CAP;
-	if (bp->flags & BNXT_FLAG_ROCEV1_CAP)
-		edev->flags |= BNXT_EN_FLAG_ROCEV1_CAP;
-	if (bp->flags & BNXT_FLAG_ROCEV2_CAP)
-		edev->flags |= BNXT_EN_FLAG_ROCEV2_CAP;
-	return bp->edev;
-}
-EXPORT_SYMBOL(bnxt_ulp_probe);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index 8b85c663d881..aaf55d847505 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -32,7 +32,6 @@ struct bnxt_ulp_ops {
 	void (*ulp_stop)(void *);
 	void (*ulp_start)(void *);
 	void (*ulp_sriov_config)(void *, int);
-	void (*ulp_shutdown)(void *);
 	void (*ulp_irq_stop)(void *);
 	void (*ulp_irq_restart)(void *, struct bnxt_msix_entry *);
 };
@@ -111,6 +110,4 @@ int bnxt_rdma_aux_device_add(struct bnxt *bp);
 void bnxt_rdma_aux_device_uninit(struct bnxt_aux_dev *bnxt_adev);
 void bnxt_rdma_aux_device_init(struct bnxt *bp);
 void bnxt_aux_dev_free(struct bnxt *bp);
-struct bnxt_en_dev *bnxt_ulp_probe(struct net_device *dev);
-
 #endif
-- 
2.37.1 (Apple Git-137.1)


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

[PATCH v4 3/6] bnxt_en: Remove usage of ulp_id

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 19417 bytes --]

Since the driver continues to use the single ULP model,
the extra complexity and indirection is unnecessary.
Remove the usage of ulp_id from the code.

Suggested-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c          |  24 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 211 ++++++++----------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  26 +--
 4 files changed, 112 insertions(+), 151 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index a9fb1067c9a1..e60bc3ce9c3d 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -366,8 +366,7 @@ static int bnxt_re_unregister_netdev(struct bnxt_re_dev *rdev)
 
 	en_dev = rdev->en_dev;
 
-	rc = en_dev->en_ops->bnxt_unregister_device(rdev->en_dev,
-						    BNXT_ROCE_ULP);
+	rc = en_dev->en_ops->bnxt_unregister_device(rdev->en_dev);
 	return rc;
 }
 
@@ -381,7 +380,7 @@ static int bnxt_re_register_netdev(struct bnxt_re_dev *rdev)
 
 	en_dev = rdev->en_dev;
 
-	rc = en_dev->en_ops->bnxt_register_device(en_dev, BNXT_ROCE_ULP,
+	rc = en_dev->en_ops->bnxt_register_device(en_dev,
 						  &bnxt_re_ulp_ops, rdev);
 	rdev->qplib_res.pdev = rdev->en_dev->pdev;
 	return rc;
@@ -390,16 +389,15 @@ static int bnxt_re_register_netdev(struct bnxt_re_dev *rdev)
 static int bnxt_re_free_msix(struct bnxt_re_dev *rdev)
 {
 	struct bnxt_en_dev *en_dev;
-	int rc;
 
 	if (!rdev)
 		return -EINVAL;
 
 	en_dev = rdev->en_dev;
 
-	rc = en_dev->en_ops->bnxt_free_msix(rdev->en_dev, BNXT_ROCE_ULP);
+	en_dev->en_ops->bnxt_free_msix(rdev->en_dev);
 
-	return rc;
+	return 0;
 }
 
 static int bnxt_re_request_msix(struct bnxt_re_dev *rdev)
@@ -414,7 +412,7 @@ static int bnxt_re_request_msix(struct bnxt_re_dev *rdev)
 
 	num_msix_want = min_t(u32, BNXT_RE_MAX_MSIX, num_online_cpus());
 
-	num_msix_got = en_dev->en_ops->bnxt_request_msix(en_dev, BNXT_ROCE_ULP,
+	num_msix_got = en_dev->en_ops->bnxt_request_msix(en_dev,
 							 rdev->msix_entries,
 							 num_msix_want);
 	if (num_msix_got < BNXT_RE_MIN_MSIX) {
@@ -477,7 +475,7 @@ static int bnxt_re_net_ring_free(struct bnxt_re_dev *rdev,
 	req.ring_id = cpu_to_le16(fw_ring_id);
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, BNXT_ROCE_ULP, &fw_msg);
+	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
 	if (rc)
 		ibdev_err(&rdev->ibdev, "Failed to free HW ring:%d :%#x",
 			  req.ring_id, rc);
@@ -514,7 +512,7 @@ static int bnxt_re_net_ring_alloc(struct bnxt_re_dev *rdev,
 	req.int_mode = ring_attr->mode;
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, BNXT_ROCE_ULP, &fw_msg);
+	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
 	if (!rc)
 		*fw_ring_id = le16_to_cpu(resp.ring_id);
 
@@ -542,7 +540,7 @@ static int bnxt_re_net_stats_ctx_free(struct bnxt_re_dev *rdev,
 	req.stat_ctx_id = cpu_to_le32(fw_stats_ctx_id);
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, BNXT_ROCE_ULP, &fw_msg);
+	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
 	if (rc)
 		ibdev_err(&rdev->ibdev, "Failed to free HW stats context %#x",
 			  rc);
@@ -575,7 +573,7 @@ static int bnxt_re_net_stats_ctx_alloc(struct bnxt_re_dev *rdev,
 	req.stat_ctx_flags = STAT_CTX_ALLOC_REQ_STAT_CTX_FLAGS_ROCE;
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, BNXT_ROCE_ULP, &fw_msg);
+	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
 	if (!rc)
 		*fw_stats_ctx_id = le32_to_cpu(resp.stat_ctx_id);
 
@@ -1061,7 +1059,7 @@ static int bnxt_re_query_hwrm_pri2cos(struct bnxt_re_dev *rdev, u8 dir,
 
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, BNXT_ROCE_ULP, &fw_msg);
+	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
 	if (rc)
 		return rc;
 
@@ -1247,7 +1245,7 @@ static void bnxt_re_query_hwrm_intf_version(struct bnxt_re_dev *rdev)
 	req.hwrm_intf_upd = HWRM_VERSION_UPDATE;
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, BNXT_ROCE_ULP, &fw_msg);
+	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
 	if (rc) {
 		ibdev_err(&rdev->ibdev, "Failed to query HW version, rc = 0x%x",
 			  rc);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 16265d177639..1b573e015b5e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -5533,7 +5533,7 @@ int bnxt_hwrm_vnic_cfg(struct bnxt *bp, u16 vnic_id)
 #endif
 	if ((bp->flags & BNXT_FLAG_STRIP_VLAN) || def_vlan)
 		req->flags |= cpu_to_le32(VNIC_CFG_REQ_FLAGS_VLAN_STRIP_MODE);
-	if (!vnic_id && bnxt_ulp_registered(bp->edev, BNXT_ROCE_ULP))
+	if (!vnic_id && bnxt_ulp_registered(bp->edev))
 		req->flags |= cpu_to_le32(bnxt_get_roce_vnic_mode(bp));
 
 	return hwrm_req_send(bp, req);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 1d2d30f97ed9..3ea2e1de2e29 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -28,64 +28,44 @@
 
 static DEFINE_IDA(bnxt_aux_dev_ids);
 
-static int bnxt_register_dev(struct bnxt_en_dev *edev, unsigned int ulp_id,
-			     struct bnxt_ulp_ops *ulp_ops, void *handle)
+static int bnxt_register_dev(struct bnxt_en_dev *edev,
+			     struct bnxt_ulp_ops *ulp_ops,
+			     void *handle)
 {
 	struct net_device *dev = edev->net;
 	struct bnxt *bp = netdev_priv(dev);
+	unsigned int max_stat_ctxs;
 	struct bnxt_ulp *ulp;
-	int rc = 0;
 
-	if (ulp_id >= BNXT_MAX_ULP)
-		return -EINVAL;
+	max_stat_ctxs = bnxt_get_max_func_stat_ctxs(bp);
+	if (max_stat_ctxs <= BNXT_MIN_ROCE_STAT_CTXS ||
+	    bp->cp_nr_rings == max_stat_ctxs)
+		return -ENOMEM;
 
-	ulp = &edev->ulp_tbl[ulp_id];
-	if (rcu_access_pointer(ulp->ulp_ops)) {
-		netdev_err(bp->dev, "ulp id %d already registered\n", ulp_id);
-		rc = -EBUSY;
-		goto exit;
-	}
-	if (ulp_id == BNXT_ROCE_ULP) {
-		unsigned int max_stat_ctxs;
-
-		max_stat_ctxs = bnxt_get_max_func_stat_ctxs(bp);
-		if (max_stat_ctxs <= BNXT_MIN_ROCE_STAT_CTXS ||
-		    bp->cp_nr_rings == max_stat_ctxs) {
-			rc = -ENOMEM;
-			goto exit;
-		}
-	}
+	ulp = kzalloc(sizeof(*ulp), GFP_KERNEL);
+	if (!ulp)
+		return -ENOMEM;
 
-	atomic_set(&ulp->ref_count, 1);
+	edev->ulp_tbl = ulp;
 	ulp->handle = handle;
 	rcu_assign_pointer(ulp->ulp_ops, ulp_ops);
 
-	if (ulp_id == BNXT_ROCE_ULP) {
-		if (test_bit(BNXT_STATE_OPEN, &bp->state))
-			bnxt_hwrm_vnic_cfg(bp, 0);
-	}
+	if (test_bit(BNXT_STATE_OPEN, &bp->state))
+		bnxt_hwrm_vnic_cfg(bp, 0);
 
-exit:
-	return rc;
+	return 0;
 }
 
-static int bnxt_unregister_dev(struct bnxt_en_dev *edev, unsigned int ulp_id)
+static int bnxt_unregister_dev(struct bnxt_en_dev *edev)
 {
 	struct net_device *dev = edev->net;
 	struct bnxt *bp = netdev_priv(dev);
 	struct bnxt_ulp *ulp;
 	int i = 0;
 
-	if (ulp_id >= BNXT_MAX_ULP)
-		return -EINVAL;
-
-	ulp = &edev->ulp_tbl[ulp_id];
-	if (!rcu_access_pointer(ulp->ulp_ops)) {
-		netdev_err(bp->dev, "ulp id %d not registered\n", ulp_id);
-		return -EINVAL;
-	}
-	if (ulp_id == BNXT_ROCE_ULP && ulp->msix_requested)
-		edev->en_ops->bnxt_free_msix(edev, ulp_id);
+	ulp = edev->ulp_tbl;
+	if (ulp->msix_requested)
+		edev->en_ops->bnxt_free_msix(edev);
 
 	if (ulp->max_async_event_id)
 		bnxt_hwrm_func_drv_rgtr(bp, NULL, 0, true);
@@ -98,6 +78,8 @@ static int bnxt_unregister_dev(struct bnxt_en_dev *edev, unsigned int ulp_id)
 		msleep(100);
 		i++;
 	}
+	kfree(ulp);
+	edev->ulp_tbl = NULL;
 	return 0;
 }
 
@@ -106,8 +88,8 @@ static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
 	struct bnxt_en_dev *edev = bp->edev;
 	int num_msix, idx, i;
 
-	num_msix = edev->ulp_tbl[BNXT_ROCE_ULP].msix_requested;
-	idx = edev->ulp_tbl[BNXT_ROCE_ULP].msix_base;
+	num_msix = edev->ulp_tbl->msix_requested;
+	idx = edev->ulp_tbl->msix_base;
 	for (i = 0; i < num_msix; i++) {
 		ent[i].vector = bp->irq_tbl[idx + i].vector;
 		ent[i].ring_idx = idx + i;
@@ -121,8 +103,9 @@ static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
 	}
 }
 
-static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id,
-			      struct bnxt_msix_entry *ent, int num_msix)
+static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev,
+			      struct bnxt_msix_entry *ent,
+			      int num_msix)
 {
 	struct net_device *dev = edev->net;
 	struct bnxt *bp = netdev_priv(dev);
@@ -132,13 +115,10 @@ static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id,
 	int total_vecs;
 	int rc = 0;
 
-	if (ulp_id != BNXT_ROCE_ULP)
-		return -EINVAL;
-
 	if (!(bp->flags & BNXT_FLAG_USING_MSIX))
 		return -ENODEV;
 
-	if (edev->ulp_tbl[ulp_id].msix_requested)
+	if (edev->ulp_tbl->msix_requested)
 		return -EAGAIN;
 
 	max_cp_rings = bnxt_get_max_func_cp_rings(bp);
@@ -155,8 +135,8 @@ static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id,
 		idx = max_idx - avail_msix;
 	}
 
-	edev->ulp_tbl[ulp_id].msix_base = idx;
-	edev->ulp_tbl[ulp_id].msix_requested = avail_msix;
+	edev->ulp_tbl->msix_base = idx;
+	edev->ulp_tbl->msix_requested = avail_msix;
 	hw_resc = &bp->hw_resc;
 	total_vecs = idx + avail_msix;
 	if (bp->total_irqs < total_vecs ||
@@ -171,7 +151,7 @@ static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id,
 		}
 	}
 	if (rc) {
-		edev->ulp_tbl[ulp_id].msix_requested = 0;
+		edev->ulp_tbl->msix_requested = 0;
 		return -EAGAIN;
 	}
 
@@ -180,25 +160,22 @@ static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id,
 
 		resv_msix = hw_resc->resv_irqs - bp->cp_nr_rings;
 		avail_msix = min_t(int, resv_msix, avail_msix);
-		edev->ulp_tbl[ulp_id].msix_requested = avail_msix;
+		edev->ulp_tbl->msix_requested = avail_msix;
 	}
 	bnxt_fill_msix_vecs(bp, ent);
 	edev->flags |= BNXT_EN_FLAG_MSIX_REQUESTED;
 	return avail_msix;
 }
 
-static int bnxt_free_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id)
+static void bnxt_free_msix_vecs(struct bnxt_en_dev *edev)
 {
 	struct net_device *dev = edev->net;
 	struct bnxt *bp = netdev_priv(dev);
 
-	if (ulp_id != BNXT_ROCE_ULP)
-		return -EINVAL;
-
 	if (!(edev->flags & BNXT_EN_FLAG_MSIX_REQUESTED))
-		return 0;
+		return;
 
-	edev->ulp_tbl[ulp_id].msix_requested = 0;
+	edev->ulp_tbl->msix_requested = 0;
 	edev->flags &= ~BNXT_EN_FLAG_MSIX_REQUESTED;
 	if (netif_running(dev) && !(edev->flags & BNXT_EN_FLAG_ULP_STOPPED)) {
 		rtnl_lock();
@@ -207,43 +184,43 @@ static int bnxt_free_msix_vecs(struct bnxt_en_dev *edev, unsigned int ulp_id)
 		rtnl_unlock();
 	}
 
-	return 0;
+	return;
 }
 
 int bnxt_get_ulp_msix_num(struct bnxt *bp)
 {
-	if (bnxt_ulp_registered(bp->edev, BNXT_ROCE_ULP)) {
+	if (bnxt_ulp_registered(bp->edev)) {
 		struct bnxt_en_dev *edev = bp->edev;
 
-		return edev->ulp_tbl[BNXT_ROCE_ULP].msix_requested;
+		return edev->ulp_tbl->msix_requested;
 	}
 	return 0;
 }
 
 int bnxt_get_ulp_msix_base(struct bnxt *bp)
 {
-	if (bnxt_ulp_registered(bp->edev, BNXT_ROCE_ULP)) {
+	if (bnxt_ulp_registered(bp->edev)) {
 		struct bnxt_en_dev *edev = bp->edev;
 
-		if (edev->ulp_tbl[BNXT_ROCE_ULP].msix_requested)
-			return edev->ulp_tbl[BNXT_ROCE_ULP].msix_base;
+		if (edev->ulp_tbl->msix_requested)
+			return edev->ulp_tbl->msix_base;
 	}
 	return 0;
 }
 
 int bnxt_get_ulp_stat_ctxs(struct bnxt *bp)
 {
-	if (bnxt_ulp_registered(bp->edev, BNXT_ROCE_ULP)) {
+	if (bnxt_ulp_registered(bp->edev)) {
 		struct bnxt_en_dev *edev = bp->edev;
 
-		if (edev->ulp_tbl[BNXT_ROCE_ULP].msix_requested)
+		if (edev->ulp_tbl->msix_requested)
 			return BNXT_MIN_ROCE_STAT_CTXS;
 	}
 
 	return 0;
 }
 
-static int bnxt_send_msg(struct bnxt_en_dev *edev, unsigned int ulp_id,
+static int bnxt_send_msg(struct bnxt_en_dev *edev,
 			 struct bnxt_fw_msg *fw_msg)
 {
 	struct net_device *dev = edev->net;
@@ -253,7 +230,7 @@ static int bnxt_send_msg(struct bnxt_en_dev *edev, unsigned int ulp_id,
 	u32 resp_len;
 	int rc;
 
-	if (ulp_id != BNXT_ROCE_ULP && bp->fw_reset_state)
+	if (bp->fw_reset_state)
 		return -EBUSY;
 
 	rc = hwrm_req_init(bp, req, 0 /* don't care */);
@@ -292,27 +269,24 @@ void bnxt_ulp_stop(struct bnxt *bp)
 {
 	struct bnxt_en_dev *edev = bp->edev;
 	struct bnxt_ulp_ops *ops;
-	int i;
+	struct bnxt_ulp *ulp;
 
 	if (!edev)
 		return;
 
 	edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
-	for (i = 0; i < BNXT_MAX_ULP; i++) {
-		struct bnxt_ulp *ulp = &edev->ulp_tbl[i];
-
-		ops = rtnl_dereference(ulp->ulp_ops);
-		if (!ops || !ops->ulp_stop)
-			continue;
-		ops->ulp_stop(ulp->handle);
-	}
+	ulp = edev->ulp_tbl;
+	ops = rtnl_dereference(ulp->ulp_ops);
+	if (!ops || !ops->ulp_stop)
+		return;
+	ops->ulp_stop(ulp->handle);
 }
 
 void bnxt_ulp_start(struct bnxt *bp, int err)
 {
 	struct bnxt_en_dev *edev = bp->edev;
 	struct bnxt_ulp_ops *ops;
-	int i;
+	struct bnxt_ulp *ulp;
 
 	if (!edev)
 		return;
@@ -322,39 +296,33 @@ void bnxt_ulp_start(struct bnxt *bp, int err)
 	if (err)
 		return;
 
-	for (i = 0; i < BNXT_MAX_ULP; i++) {
-		struct bnxt_ulp *ulp = &edev->ulp_tbl[i];
-
-		ops = rtnl_dereference(ulp->ulp_ops);
-		if (!ops || !ops->ulp_start)
-			continue;
-		ops->ulp_start(ulp->handle);
-	}
+	ulp = edev->ulp_tbl;
+	ops = rtnl_dereference(ulp->ulp_ops);
+	if (!ops || !ops->ulp_start)
+		return;
+	ops->ulp_start(ulp->handle);
 }
 
 void bnxt_ulp_sriov_cfg(struct bnxt *bp, int num_vfs)
 {
 	struct bnxt_en_dev *edev = bp->edev;
 	struct bnxt_ulp_ops *ops;
-	int i;
+	struct bnxt_ulp *ulp;
 
 	if (!edev)
 		return;
+	ulp = edev->ulp_tbl;
 
-	for (i = 0; i < BNXT_MAX_ULP; i++) {
-		struct bnxt_ulp *ulp = &edev->ulp_tbl[i];
-
-		rcu_read_lock();
-		ops = rcu_dereference(ulp->ulp_ops);
-		if (!ops || !ops->ulp_sriov_config) {
-			rcu_read_unlock();
-			continue;
-		}
-		bnxt_ulp_get(ulp);
+	rcu_read_lock();
+	ops = rcu_dereference(ulp->ulp_ops);
+	if (!ops || !ops->ulp_sriov_config) {
 		rcu_read_unlock();
-		ops->ulp_sriov_config(ulp->handle, num_vfs);
-		bnxt_ulp_put(ulp);
+		return;
 	}
+	bnxt_ulp_get(ulp);
+	rcu_read_unlock();
+	ops->ulp_sriov_config(ulp->handle, num_vfs);
+	bnxt_ulp_put(ulp);
 }
 
 void bnxt_ulp_irq_stop(struct bnxt *bp)
@@ -365,8 +333,8 @@ void bnxt_ulp_irq_stop(struct bnxt *bp)
 	if (!edev || !(edev->flags & BNXT_EN_FLAG_MSIX_REQUESTED))
 		return;
 
-	if (bnxt_ulp_registered(bp->edev, BNXT_ROCE_ULP)) {
-		struct bnxt_ulp *ulp = &edev->ulp_tbl[BNXT_ROCE_ULP];
+	if (bnxt_ulp_registered(bp->edev)) {
+		struct bnxt_ulp *ulp = edev->ulp_tbl;
 
 		if (!ulp->msix_requested)
 			return;
@@ -386,8 +354,8 @@ void bnxt_ulp_irq_restart(struct bnxt *bp, int err)
 	if (!edev || !(edev->flags & BNXT_EN_FLAG_MSIX_REQUESTED))
 		return;
 
-	if (bnxt_ulp_registered(bp->edev, BNXT_ROCE_ULP)) {
-		struct bnxt_ulp *ulp = &edev->ulp_tbl[BNXT_ROCE_ULP];
+	if (bnxt_ulp_registered(bp->edev)) {
+		struct bnxt_ulp *ulp = edev->ulp_tbl;
 		struct bnxt_msix_entry *ent = NULL;
 
 		if (!ulp->msix_requested)
@@ -414,41 +382,36 @@ void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl)
 	u16 event_id = le16_to_cpu(cmpl->event_id);
 	struct bnxt_en_dev *edev = bp->edev;
 	struct bnxt_ulp_ops *ops;
-	int i;
+	struct bnxt_ulp *ulp;
 
 	if (!edev)
 		return;
+	ulp = edev->ulp_tbl;
 
 	rcu_read_lock();
-	for (i = 0; i < BNXT_MAX_ULP; i++) {
-		struct bnxt_ulp *ulp = &edev->ulp_tbl[i];
-
-		ops = rcu_dereference(ulp->ulp_ops);
-		if (!ops || !ops->ulp_async_notifier)
-			continue;
-		if (!ulp->async_events_bmap ||
-		    event_id > ulp->max_async_event_id)
-			continue;
-
-		/* Read max_async_event_id first before testing the bitmap. */
-		smp_rmb();
-		if (test_bit(event_id, ulp->async_events_bmap))
-			ops->ulp_async_notifier(ulp->handle, cmpl);
-	}
+
+	ops = rcu_dereference(ulp->ulp_ops);
+	if (!ops || !ops->ulp_async_notifier)
+		return;
+	if (!ulp->async_events_bmap || event_id > ulp->max_async_event_id)
+		return;
+
+	/* Read max_async_event_id first before testing the bitmap. */
+	smp_rmb();
+	if (test_bit(event_id, ulp->async_events_bmap))
+		ops->ulp_async_notifier(ulp->handle, cmpl);
 	rcu_read_unlock();
 }
 
-static int bnxt_register_async_events(struct bnxt_en_dev *edev, unsigned int ulp_id,
-				      unsigned long *events_bmap, u16 max_id)
+static int 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);
 	struct bnxt_ulp *ulp;
 
-	if (ulp_id >= BNXT_MAX_ULP)
-		return -EINVAL;
-
-	ulp = &edev->ulp_tbl[ulp_id];
+	ulp = edev->ulp_tbl;
 	ulp->async_events_bmap = events_bmap;
 	/* Make sure bnxt_ulp_async_events() sees this order */
 	smp_wmb();
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index aaf55d847505..7b2b829300b4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -65,7 +65,7 @@ struct bnxt_en_dev {
 	#define BNXT_EN_FLAG_MSIX_REQUESTED	0x4
 	#define BNXT_EN_FLAG_ULP_STOPPED	0x8
 	const struct bnxt_en_ops	*en_ops;
-	struct bnxt_ulp			ulp_tbl[BNXT_MAX_ULP];
+	struct bnxt_ulp			*ulp_tbl;
 	int				l2_db_size;	/* Doorbell BAR size in
 							 * bytes mapped by L2
 							 * driver.
@@ -77,21 +77,21 @@ struct bnxt_en_dev {
 };
 
 struct bnxt_en_ops {
-	int (*bnxt_register_device)(struct bnxt_en_dev *, unsigned int,
-				    struct bnxt_ulp_ops *, void *);
-	int (*bnxt_unregister_device)(struct bnxt_en_dev *, unsigned int);
-	int (*bnxt_request_msix)(struct bnxt_en_dev *, unsigned int,
-				 struct bnxt_msix_entry *, int);
-	int (*bnxt_free_msix)(struct bnxt_en_dev *, unsigned int);
-	int (*bnxt_send_fw_msg)(struct bnxt_en_dev *, unsigned int,
-				struct bnxt_fw_msg *);
-	int (*bnxt_register_fw_async_events)(struct bnxt_en_dev *, unsigned int,
-					     unsigned long *, u16);
+	int (*bnxt_register_device)(struct bnxt_en_dev *edev,
+				    struct bnxt_ulp_ops *ulp_ops, void *handle);
+	int (*bnxt_unregister_device)(struct bnxt_en_dev *edev);
+	int (*bnxt_request_msix)(struct bnxt_en_dev *edev,
+				 struct bnxt_msix_entry *ent, int num_msix);
+	void (*bnxt_free_msix)(struct bnxt_en_dev *edev);
+	int (*bnxt_send_fw_msg)(struct bnxt_en_dev *edev,
+				struct bnxt_fw_msg *fw_msg);
+	int (*bnxt_register_fw_async_events)(struct bnxt_en_dev *edev,
+					     unsigned long *events_bmap, u16 max_id);
 };
 
-static inline bool bnxt_ulp_registered(struct bnxt_en_dev *edev, int ulp_id)
+static inline bool bnxt_ulp_registered(struct bnxt_en_dev *edev)
 {
-	if (edev && rcu_access_pointer(edev->ulp_tbl[ulp_id].ulp_ops))
+	if (edev && edev->ulp_tbl)
 		return true;
 	return false;
 }
-- 
2.37.1 (Apple Git-137.1)


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

[PATCH v4 4/6] bnxt_en: Use direct API instead of indirection

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 13580 bytes --]

For a single ULP user there is no need for complicating function
indirection calls. Remove all this complexity in favour of direct
function calls exported by the bnxt_en driver. This allows to
simplify the code greatly.

Suggested-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c          | 71 ++++------------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 84 ++++---------------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h | 23 ++---
 3 files changed, 39 insertions(+), 139 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index e60bc3ce9c3d..021812956f73 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -353,23 +353,6 @@ static struct bnxt_ulp_ops bnxt_re_ulp_ops = {
 
 /* RoCE -> Net driver */
 
-/* Driver registration routines used to let the networking driver (bnxt_en)
- * to know that the RoCE driver is now installed
- */
-static int bnxt_re_unregister_netdev(struct bnxt_re_dev *rdev)
-{
-	struct bnxt_en_dev *en_dev;
-	int rc;
-
-	if (!rdev)
-		return -EINVAL;
-
-	en_dev = rdev->en_dev;
-
-	rc = en_dev->en_ops->bnxt_unregister_device(rdev->en_dev);
-	return rc;
-}
-
 static int bnxt_re_register_netdev(struct bnxt_re_dev *rdev)
 {
 	struct bnxt_en_dev *en_dev;
@@ -380,26 +363,12 @@ static int bnxt_re_register_netdev(struct bnxt_re_dev *rdev)
 
 	en_dev = rdev->en_dev;
 
-	rc = en_dev->en_ops->bnxt_register_device(en_dev,
-						  &bnxt_re_ulp_ops, rdev);
-	rdev->qplib_res.pdev = rdev->en_dev->pdev;
+	rc = bnxt_register_dev(en_dev, &bnxt_re_ulp_ops, rdev);
+	if (!rc)
+		rdev->qplib_res.pdev = rdev->en_dev->pdev;
 	return rc;
 }
 
-static int bnxt_re_free_msix(struct bnxt_re_dev *rdev)
-{
-	struct bnxt_en_dev *en_dev;
-
-	if (!rdev)
-		return -EINVAL;
-
-	en_dev = rdev->en_dev;
-
-	en_dev->en_ops->bnxt_free_msix(rdev->en_dev);
-
-	return 0;
-}
-
 static int bnxt_re_request_msix(struct bnxt_re_dev *rdev)
 {
 	int rc = 0, num_msix_want = BNXT_RE_MAX_MSIX, num_msix_got;
@@ -412,9 +381,9 @@ static int bnxt_re_request_msix(struct bnxt_re_dev *rdev)
 
 	num_msix_want = min_t(u32, BNXT_RE_MAX_MSIX, num_online_cpus());
 
-	num_msix_got = en_dev->en_ops->bnxt_request_msix(en_dev,
-							 rdev->msix_entries,
-							 num_msix_want);
+	num_msix_got = bnxt_req_msix_vecs(en_dev,
+					  rdev->msix_entries,
+					  num_msix_want);
 	if (num_msix_got < BNXT_RE_MIN_MSIX) {
 		rc = -EINVAL;
 		goto done;
@@ -475,7 +444,7 @@ static int bnxt_re_net_ring_free(struct bnxt_re_dev *rdev,
 	req.ring_id = cpu_to_le16(fw_ring_id);
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
+	rc = bnxt_send_msg(en_dev, &fw_msg);
 	if (rc)
 		ibdev_err(&rdev->ibdev, "Failed to free HW ring:%d :%#x",
 			  req.ring_id, rc);
@@ -512,7 +481,7 @@ static int bnxt_re_net_ring_alloc(struct bnxt_re_dev *rdev,
 	req.int_mode = ring_attr->mode;
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
+	rc = bnxt_send_msg(en_dev, &fw_msg);
 	if (!rc)
 		*fw_ring_id = le16_to_cpu(resp.ring_id);
 
@@ -540,7 +509,7 @@ static int bnxt_re_net_stats_ctx_free(struct bnxt_re_dev *rdev,
 	req.stat_ctx_id = cpu_to_le32(fw_stats_ctx_id);
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
+	rc = bnxt_send_msg(en_dev, &fw_msg);
 	if (rc)
 		ibdev_err(&rdev->ibdev, "Failed to free HW stats context %#x",
 			  rc);
@@ -573,7 +542,7 @@ static int bnxt_re_net_stats_ctx_alloc(struct bnxt_re_dev *rdev,
 	req.stat_ctx_flags = STAT_CTX_ALLOC_REQ_STAT_CTX_FLAGS_ROCE;
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
+	rc = bnxt_send_msg(en_dev, &fw_msg);
 	if (!rc)
 		*fw_stats_ctx_id = le32_to_cpu(resp.stat_ctx_id);
 
@@ -1059,7 +1028,7 @@ static int bnxt_re_query_hwrm_pri2cos(struct bnxt_re_dev *rdev, u8 dir,
 
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
+	rc = bnxt_send_msg(en_dev, &fw_msg);
 	if (rc)
 		return rc;
 
@@ -1245,7 +1214,7 @@ static void bnxt_re_query_hwrm_intf_version(struct bnxt_re_dev *rdev)
 	req.hwrm_intf_upd = HWRM_VERSION_UPDATE;
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
-	rc = en_dev->en_ops->bnxt_send_fw_msg(en_dev, &fw_msg);
+	rc = bnxt_send_msg(en_dev, &fw_msg);
 	if (rc) {
 		ibdev_err(&rdev->ibdev, "Failed to query HW version, rc = 0x%x",
 			  rc);
@@ -1308,20 +1277,12 @@ static void bnxt_re_dev_uninit(struct bnxt_re_dev *rdev)
 		bnxt_re_net_ring_free(rdev, rdev->rcfw.creq.ring_id, type);
 		bnxt_qplib_free_rcfw_channel(&rdev->rcfw);
 	}
-	if (test_and_clear_bit(BNXT_RE_FLAG_GOT_MSIX, &rdev->flags)) {
-		rc = bnxt_re_free_msix(rdev);
-		if (rc)
-			ibdev_warn(&rdev->ibdev,
-				   "Failed to free MSI-X vectors: %#x", rc);
-	}
+	if (test_and_clear_bit(BNXT_RE_FLAG_GOT_MSIX, &rdev->flags))
+		bnxt_free_msix_vecs(rdev->en_dev);
 
 	bnxt_re_destroy_chip_ctx(rdev);
-	if (test_and_clear_bit(BNXT_RE_FLAG_NETDEV_REGISTERED, &rdev->flags)) {
-		rc = bnxt_re_unregister_netdev(rdev);
-		if (rc)
-			ibdev_warn(&rdev->ibdev,
-				   "Failed to unregister with netdev: %#x", rc);
-	}
+	if (test_and_clear_bit(BNXT_RE_FLAG_NETDEV_REGISTERED, &rdev->flags))
+		bnxt_unregister_dev(rdev->en_dev);
 }
 
 /* worker thread for polling periodic events. Now used for QoS programming*/
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 3ea2e1de2e29..bc0982c12702 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -28,9 +28,9 @@
 
 static DEFINE_IDA(bnxt_aux_dev_ids);
 
-static int bnxt_register_dev(struct bnxt_en_dev *edev,
-			     struct bnxt_ulp_ops *ulp_ops,
-			     void *handle)
+int bnxt_register_dev(struct bnxt_en_dev *edev,
+		      struct bnxt_ulp_ops *ulp_ops,
+		      void *handle)
 {
 	struct net_device *dev = edev->net;
 	struct bnxt *bp = netdev_priv(dev);
@@ -55,8 +55,9 @@ static int bnxt_register_dev(struct bnxt_en_dev *edev,
 
 	return 0;
 }
+EXPORT_SYMBOL(bnxt_register_dev);
 
-static int bnxt_unregister_dev(struct bnxt_en_dev *edev)
+void bnxt_unregister_dev(struct bnxt_en_dev *edev)
 {
 	struct net_device *dev = edev->net;
 	struct bnxt *bp = netdev_priv(dev);
@@ -65,7 +66,7 @@ static int bnxt_unregister_dev(struct bnxt_en_dev *edev)
 
 	ulp = edev->ulp_tbl;
 	if (ulp->msix_requested)
-		edev->en_ops->bnxt_free_msix(edev);
+		bnxt_free_msix_vecs(edev);
 
 	if (ulp->max_async_event_id)
 		bnxt_hwrm_func_drv_rgtr(bp, NULL, 0, true);
@@ -80,8 +81,9 @@ static int bnxt_unregister_dev(struct bnxt_en_dev *edev)
 	}
 	kfree(ulp);
 	edev->ulp_tbl = NULL;
-	return 0;
+	return;
 }
+EXPORT_SYMBOL(bnxt_unregister_dev);
 
 static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
 {
@@ -103,7 +105,7 @@ static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
 	}
 }
 
-static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev,
+int bnxt_req_msix_vecs(struct bnxt_en_dev *edev,
 			      struct bnxt_msix_entry *ent,
 			      int num_msix)
 {
@@ -166,8 +168,9 @@ static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev,
 	edev->flags |= BNXT_EN_FLAG_MSIX_REQUESTED;
 	return avail_msix;
 }
+EXPORT_SYMBOL(bnxt_req_msix_vecs);
 
-static void bnxt_free_msix_vecs(struct bnxt_en_dev *edev)
+void bnxt_free_msix_vecs(struct bnxt_en_dev *edev)
 {
 	struct net_device *dev = edev->net;
 	struct bnxt *bp = netdev_priv(dev);
@@ -186,6 +189,7 @@ static void bnxt_free_msix_vecs(struct bnxt_en_dev *edev)
 
 	return;
 }
+EXPORT_SYMBOL(bnxt_free_msix_vecs);
 
 int bnxt_get_ulp_msix_num(struct bnxt *bp)
 {
@@ -220,7 +224,7 @@ int bnxt_get_ulp_stat_ctxs(struct bnxt *bp)
 	return 0;
 }
 
-static int bnxt_send_msg(struct bnxt_en_dev *edev,
+int bnxt_send_msg(struct bnxt_en_dev *edev,
 			 struct bnxt_fw_msg *fw_msg)
 {
 	struct net_device *dev = edev->net;
@@ -254,6 +258,7 @@ static int bnxt_send_msg(struct bnxt_en_dev *edev,
 	hwrm_req_drop(bp, req);
 	return rc;
 }
+EXPORT_SYMBOL(bnxt_send_msg);
 
 static void bnxt_ulp_get(struct bnxt_ulp *ulp)
 {
@@ -313,14 +318,11 @@ void bnxt_ulp_sriov_cfg(struct bnxt *bp, int num_vfs)
 		return;
 	ulp = edev->ulp_tbl;
 
-	rcu_read_lock();
 	ops = rcu_dereference(ulp->ulp_ops);
-	if (!ops || !ops->ulp_sriov_config) {
-		rcu_read_unlock();
+	if (!ops || !ops->ulp_sriov_config)
 		return;
-	}
+
 	bnxt_ulp_get(ulp);
-	rcu_read_unlock();
 	ops->ulp_sriov_config(ulp->handle, num_vfs);
 	bnxt_ulp_put(ulp);
 }
@@ -377,58 +379,6 @@ void bnxt_ulp_irq_restart(struct bnxt *bp, int err)
 	}
 }
 
-void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl)
-{
-	u16 event_id = le16_to_cpu(cmpl->event_id);
-	struct bnxt_en_dev *edev = bp->edev;
-	struct bnxt_ulp_ops *ops;
-	struct bnxt_ulp *ulp;
-
-	if (!edev)
-		return;
-	ulp = edev->ulp_tbl;
-
-	rcu_read_lock();
-
-	ops = rcu_dereference(ulp->ulp_ops);
-	if (!ops || !ops->ulp_async_notifier)
-		return;
-	if (!ulp->async_events_bmap || event_id > ulp->max_async_event_id)
-		return;
-
-	/* Read max_async_event_id first before testing the bitmap. */
-	smp_rmb();
-	if (test_bit(event_id, ulp->async_events_bmap))
-		ops->ulp_async_notifier(ulp->handle, cmpl);
-	rcu_read_unlock();
-}
-
-static int 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);
-	struct bnxt_ulp *ulp;
-
-	ulp = edev->ulp_tbl;
-	ulp->async_events_bmap = events_bmap;
-	/* Make sure bnxt_ulp_async_events() sees this order */
-	smp_wmb();
-	ulp->max_async_event_id = max_id;
-	bnxt_hwrm_func_drv_rgtr(bp, events_bmap, max_id + 1, true);
-	return 0;
-}
-
-static const struct bnxt_en_ops bnxt_en_ops_tbl = {
-	.bnxt_register_device	= bnxt_register_dev,
-	.bnxt_unregister_device	= bnxt_unregister_dev,
-	.bnxt_request_msix	= bnxt_req_msix_vecs,
-	.bnxt_free_msix		= bnxt_free_msix_vecs,
-	.bnxt_send_fw_msg	= bnxt_send_msg,
-	.bnxt_register_fw_async_events	= bnxt_register_async_events,
-};
-
 void bnxt_aux_dev_free(struct bnxt *bp)
 {
 	kfree(bp->aux_dev);
@@ -497,7 +447,6 @@ void bnxt_aux_dev_release(struct device *dev)
 		container_of(dev, struct bnxt_aux_dev, aux_dev.dev);
 	struct bnxt *bp = netdev_priv(bnxt_adev->edev->net);
 
-	bnxt_adev->edev->en_ops = NULL;
 	kfree(bnxt_adev->edev);
 	bnxt_adev->edev = NULL;
 	bp->edev = NULL;
@@ -505,7 +454,6 @@ void bnxt_aux_dev_release(struct device *dev)
 
 static inline void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
 {
-	edev->en_ops = &bnxt_en_ops_tbl;
 	edev->net = bp->dev;
 	edev->pdev = bp->pdev;
 	edev->l2_db_size = bp->db_size;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index 7b2b829300b4..ed03a2da6233 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -27,8 +27,6 @@ struct bnxt_msix_entry {
 };
 
 struct bnxt_ulp_ops {
-	/* async_notifier() cannot sleep (in BH context) */
-	void (*ulp_async_notifier)(void *, struct hwrm_async_event_cmpl *);
 	void (*ulp_stop)(void *);
 	void (*ulp_start)(void *);
 	void (*ulp_sriov_config)(void *, int);
@@ -64,7 +62,6 @@ struct bnxt_en_dev {
 						 BNXT_EN_FLAG_ROCEV2_CAP)
 	#define BNXT_EN_FLAG_MSIX_REQUESTED	0x4
 	#define BNXT_EN_FLAG_ULP_STOPPED	0x8
-	const struct bnxt_en_ops	*en_ops;
 	struct bnxt_ulp			*ulp_tbl;
 	int				l2_db_size;	/* Doorbell BAR size in
 							 * bytes mapped by L2
@@ -76,19 +73,6 @@ struct bnxt_en_dev {
 							 */
 };
 
-struct bnxt_en_ops {
-	int (*bnxt_register_device)(struct bnxt_en_dev *edev,
-				    struct bnxt_ulp_ops *ulp_ops, void *handle);
-	int (*bnxt_unregister_device)(struct bnxt_en_dev *edev);
-	int (*bnxt_request_msix)(struct bnxt_en_dev *edev,
-				 struct bnxt_msix_entry *ent, int num_msix);
-	void (*bnxt_free_msix)(struct bnxt_en_dev *edev);
-	int (*bnxt_send_fw_msg)(struct bnxt_en_dev *edev,
-				struct bnxt_fw_msg *fw_msg);
-	int (*bnxt_register_fw_async_events)(struct bnxt_en_dev *edev,
-					     unsigned long *events_bmap, u16 max_id);
-};
-
 static inline bool bnxt_ulp_registered(struct bnxt_en_dev *edev)
 {
 	if (edev && edev->ulp_tbl)
@@ -110,4 +94,11 @@ int bnxt_rdma_aux_device_add(struct bnxt *bp);
 void bnxt_rdma_aux_device_uninit(struct bnxt_aux_dev *bnxt_adev);
 void bnxt_rdma_aux_device_init(struct bnxt *bp);
 void bnxt_aux_dev_free(struct bnxt *bp);
+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_req_msix_vecs(struct bnxt_en_dev *edev, struct bnxt_msix_entry *ent,
+		       int num_msix);
+void bnxt_free_msix_vecs(struct bnxt_en_dev *edev);
+int bnxt_send_msg(struct bnxt_en_dev *edev, struct bnxt_fw_msg *fw_msg);
 #endif
-- 
2.37.1 (Apple Git-137.1)


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

[PATCH v4 5/6] bnxt_en: Use auxiliary bus calls over proprietary calls

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 7302 bytes --]

Wherever possible use the function ops provided by auxiliary bus
instead of using proprietary ops.

Defined bnxt_re_suspend and bnxt_re_resume calls which can be
invoked by the bnxt_en driver instead of the ULP stop/start calls.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c          | 102 +++++++++++-------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c |  40 ++++---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |   2 -
 3 files changed, 87 insertions(+), 57 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 021812956f73..b2d9667c02af 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -226,45 +226,6 @@ static void bnxt_re_set_resource_limits(struct bnxt_re_dev *rdev)
 		bnxt_re_limit_vf_res(&rdev->qplib_ctx, num_vfs);
 }
 
-/* for handling bnxt_en callbacks later */
-static void bnxt_re_stop(void *p)
-{
-	struct bnxt_re_dev *rdev = p;
-	struct bnxt *bp;
-
-	if (!rdev)
-		return;
-
-	/* L2 driver invokes this callback during device error/crash or device
-	 * reset. Current RoCE driver doesn't recover the device in case of
-	 * error. Handle the error by dispatching fatal events to all qps
-	 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
-	 * L2 driver want to modify the MSIx table.
-	 */
-	bp = netdev_priv(rdev->netdev);
-
-	ibdev_info(&rdev->ibdev, "Handle device stop call from L2 driver");
-	/* Check the current device state from L2 structure and move the
-	 * device to detached state if FW_FATAL_COND is set.
-	 * This prevents more commands to HW during clean-up,
-	 * in case the device is already in error.
-	 */
-	if (test_bit(BNXT_STATE_FW_FATAL_COND, &bp->state))
-		set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
-
-	bnxt_re_dev_stop(rdev);
-	bnxt_re_stop_irq(rdev);
-	/* Move the device states to detached and  avoid sending any more
-	 * commands to HW
-	 */
-	set_bit(BNXT_RE_FLAG_ERR_DEVICE_DETACHED, &rdev->flags);
-	set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
-}
-
-static void bnxt_re_start(void *p)
-{
-}
-
 static void bnxt_re_sriov_config(void *p, int num_vfs)
 {
 	struct bnxt_re_dev *rdev = p;
@@ -344,8 +305,6 @@ static void bnxt_re_start_irq(void *handle, struct bnxt_msix_entry *ent)
 }
 
 static struct bnxt_ulp_ops bnxt_re_ulp_ops = {
-	.ulp_stop = bnxt_re_stop,
-	.ulp_start = bnxt_re_start,
 	.ulp_sriov_config = bnxt_re_sriov_config,
 	.ulp_irq_stop = bnxt_re_stop_irq,
 	.ulp_irq_restart = bnxt_re_start_irq
@@ -1597,6 +1556,65 @@ static int bnxt_re_probe(struct auxiliary_device *adev,
 	return rc;
 }
 
+static int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
+{
+	struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
+	struct bnxt *bp;
+
+	if (!rdev)
+		return 0;
+
+	mutex_lock(&bnxt_re_mutex);
+	/* L2 driver may invoke this callback during device error/crash or device
+	 * reset. Current RoCE driver doesn't recover the device in case of
+	 * error. Handle the error by dispatching fatal events to all qps
+	 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
+	 * L2 driver want to modify the MSIx table.
+	 */
+	bp = netdev_priv(rdev->netdev);
+
+	ibdev_info(&rdev->ibdev, "Handle device suspend call");
+	/* Check the current device state from L2 structure and move the
+	 * device to detached state if FW_FATAL_COND is set.
+	 * This prevents more commands to HW during clean-up,
+	 * in case the device is already in error.
+	 */
+	if (test_bit(BNXT_STATE_FW_FATAL_COND, &bp->state))
+		set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
+
+	bnxt_re_dev_stop(rdev);
+	bnxt_re_stop_irq(rdev);
+	/* Move the device states to detached and  avoid sending any more
+	 * commands to HW
+	 */
+	set_bit(BNXT_RE_FLAG_ERR_DEVICE_DETACHED, &rdev->flags);
+	set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
+	mutex_unlock(&bnxt_re_mutex);
+
+	return 0;
+}
+
+static int bnxt_re_resume(struct auxiliary_device *adev)
+{
+	struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
+
+	if (!rdev)
+		return 0;
+
+	mutex_lock(&bnxt_re_mutex);
+	/* L2 driver may invoke this callback during device recovery, resume.
+	 * reset. Current RoCE driver doesn't recover the device in case of
+	 * error. Handle the error by dispatching fatal events to all qps
+	 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
+	 * L2 driver want to modify the MSIx table.
+	 */
+
+	ibdev_info(&rdev->ibdev, "Handle device resume call");
+	mutex_unlock(&bnxt_re_mutex);
+
+	return 0;
+}
+
 static const struct auxiliary_device_id bnxt_re_id_table[] = {
 	{ .name = BNXT_ADEV_NAME ".rdma", },
 	{},
@@ -1609,6 +1627,8 @@ static struct auxiliary_driver bnxt_re_driver = {
 	.probe = bnxt_re_probe,
 	.remove = bnxt_re_remove,
 	.shutdown = bnxt_re_shutdown,
+	.suspend = bnxt_re_suspend,
+	.resume = bnxt_re_resume,
 	.id_table = bnxt_re_id_table,
 };
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index bc0982c12702..0df44c14b3fc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -272,26 +272,31 @@ static void bnxt_ulp_put(struct bnxt_ulp *ulp)
 
 void bnxt_ulp_stop(struct bnxt *bp)
 {
+	struct bnxt_aux_dev *bnxt_aux = bp->aux_dev;
 	struct bnxt_en_dev *edev = bp->edev;
-	struct bnxt_ulp_ops *ops;
-	struct bnxt_ulp *ulp;
 
 	if (!edev)
 		return;
 
 	edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
-	ulp = edev->ulp_tbl;
-	ops = rtnl_dereference(ulp->ulp_ops);
-	if (!ops || !ops->ulp_stop)
-		return;
-	ops->ulp_stop(ulp->handle);
+	if (bnxt_aux) {
+		struct auxiliary_device *adev;
+
+		adev = &bnxt_aux->aux_dev;
+		if (adev->dev.driver) {
+			struct auxiliary_driver *adrv;
+			pm_message_t pm = {};
+
+			adrv = to_auxiliary_drv(adev->dev.driver);
+			adrv->suspend(adev, pm);
+		}
+	}
 }
 
 void bnxt_ulp_start(struct bnxt *bp, int err)
 {
+	struct bnxt_aux_dev *bnxt_aux = bp->aux_dev;
 	struct bnxt_en_dev *edev = bp->edev;
-	struct bnxt_ulp_ops *ops;
-	struct bnxt_ulp *ulp;
 
 	if (!edev)
 		return;
@@ -301,11 +306,18 @@ void bnxt_ulp_start(struct bnxt *bp, int err)
 	if (err)
 		return;
 
-	ulp = edev->ulp_tbl;
-	ops = rtnl_dereference(ulp->ulp_ops);
-	if (!ops || !ops->ulp_start)
-		return;
-	ops->ulp_start(ulp->handle);
+	if (bnxt_aux) {
+		struct auxiliary_device *adev;
+
+		adev = &bnxt_aux->aux_dev;
+		if (adev->dev.driver) {
+			struct auxiliary_driver *adrv;
+
+			adrv = to_auxiliary_drv(adev->dev.driver);
+			adrv->resume(adev);
+		}
+	}
+
 }
 
 void bnxt_ulp_sriov_cfg(struct bnxt *bp, int num_vfs)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index ed03a2da6233..f76f06413a88 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -27,8 +27,6 @@ struct bnxt_msix_entry {
 };
 
 struct bnxt_ulp_ops {
-	void (*ulp_stop)(void *);
-	void (*ulp_start)(void *);
 	void (*ulp_sriov_config)(void *, int);
 	void (*ulp_irq_stop)(void *);
 	void (*ulp_irq_restart)(void *, struct bnxt_msix_entry *);
-- 
2.37.1 (Apple Git-137.1)


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

[PATCH v4 6/6] bnxt_en: Remove struct bnxt access from RoCE driver

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 6268 bytes --]

From: Hongguang Gao <hongguang.gao@broadcom.com>

Decouple RoCE driver from directly accessing L2's private bnxt
structure. Move the fields needed by RoCE driver into bnxt_en_dev.
They'll be passed to RoCE driver by bnxt_rdma_aux_device_add()
function.

Signed-off-by: Hongguang Gao <hongguang.gao@broadcom.com>
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Reviewed-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c          | 22 ++++++-------------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c |  9 ++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h | 11 ++++++++++
 3 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index b2d9667c02af..2997b1949de9 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -112,16 +112,14 @@ static int bnxt_re_setup_chip_ctx(struct bnxt_re_dev *rdev, u8 wqe_mode)
 {
 	struct bnxt_qplib_chip_ctx *chip_ctx;
 	struct bnxt_en_dev *en_dev;
-	struct bnxt *bp;
 
 	en_dev = rdev->en_dev;
-	bp = netdev_priv(en_dev->net);
 
 	chip_ctx = kzalloc(sizeof(*chip_ctx), GFP_KERNEL);
 	if (!chip_ctx)
 		return -ENOMEM;
-	chip_ctx->chip_num = bp->chip_num;
-	chip_ctx->hw_stats_size = bp->hw_ring_stats_size;
+	chip_ctx->chip_num = en_dev->chip_num;
+	chip_ctx->hw_stats_size = en_dev->hw_ring_stats_size;
 
 	rdev->chip_ctx = chip_ctx;
 	/* rest members to follow eventually */
@@ -129,7 +127,7 @@ static int bnxt_re_setup_chip_ctx(struct bnxt_re_dev *rdev, u8 wqe_mode)
 	rdev->qplib_res.cctx = rdev->chip_ctx;
 	rdev->rcfw.res = &rdev->qplib_res;
 	rdev->qplib_res.dattr = &rdev->dev_attr;
-	rdev->qplib_res.is_vf = BNXT_VF(bp);
+	rdev->qplib_res.is_vf = BNXT_EN_VF(en_dev);
 
 	bnxt_re_set_drv_mode(rdev, wqe_mode);
 	if (bnxt_qplib_determine_atomics(en_dev->pdev))
@@ -142,10 +140,7 @@ static int bnxt_re_setup_chip_ctx(struct bnxt_re_dev *rdev, u8 wqe_mode)
 
 static void bnxt_re_get_sriov_func_type(struct bnxt_re_dev *rdev)
 {
-	struct bnxt *bp;
-
-	bp = netdev_priv(rdev->en_dev->net);
-	if (BNXT_VF(bp))
+	if (BNXT_EN_VF(rdev->en_dev))
 		rdev->is_virtfn = 1;
 }
 
@@ -966,7 +961,6 @@ static int bnxt_re_query_hwrm_pri2cos(struct bnxt_re_dev *rdev, u8 dir,
 				      u64 *cid_map)
 {
 	struct hwrm_queue_pri2cos_qcfg_input req = {0};
-	struct bnxt *bp = netdev_priv(rdev->netdev);
 	struct hwrm_queue_pri2cos_qcfg_output resp;
 	struct bnxt_en_dev *en_dev = rdev->en_dev;
 	struct bnxt_fw_msg fw_msg;
@@ -983,7 +977,7 @@ static int bnxt_re_query_hwrm_pri2cos(struct bnxt_re_dev *rdev, u8 dir,
 	flags |= (dir & 0x01);
 	flags |= HWRM_QUEUE_PRI2COS_QCFG_INPUT_FLAGS_IVLAN;
 	req.flags = cpu_to_le32(flags);
-	req.port_id = bp->pf.port_id;
+	req.port_id = en_dev->pf_port_id;
 
 	bnxt_re_fill_fw_msg(&fw_msg, (void *)&req, sizeof(req), (void *)&resp,
 			    sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
@@ -1559,7 +1553,6 @@ static int bnxt_re_probe(struct auxiliary_device *adev,
 static int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
 {
 	struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
-	struct bnxt *bp;
 
 	if (!rdev)
 		return 0;
@@ -1571,15 +1564,14 @@ static int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
 	 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
 	 * L2 driver want to modify the MSIx table.
 	 */
-	bp = netdev_priv(rdev->netdev);
 
 	ibdev_info(&rdev->ibdev, "Handle device suspend call");
-	/* Check the current device state from L2 structure and move the
+	/* Check the current device state from bnxt_en_dev and move the
 	 * device to detached state if FW_FATAL_COND is set.
 	 * This prevents more commands to HW during clean-up,
 	 * in case the device is already in error.
 	 */
-	if (test_bit(BNXT_STATE_FW_FATAL_COND, &bp->state))
+	if (test_bit(BNXT_STATE_FW_FATAL_COND, &rdev->en_dev->en_state))
 		set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
 
 	bnxt_re_dev_stop(rdev);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 0df44c14b3fc..a8357fc503bb 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -288,6 +288,7 @@ void bnxt_ulp_stop(struct bnxt *bp)
 			pm_message_t pm = {};
 
 			adrv = to_auxiliary_drv(adev->dev.driver);
+			edev->en_state = bp->state;
 			adrv->suspend(adev, pm);
 		}
 	}
@@ -314,6 +315,7 @@ void bnxt_ulp_start(struct bnxt *bp, int err)
 			struct auxiliary_driver *adrv;
 
 			adrv = to_auxiliary_drv(adev->dev.driver);
+			edev->en_state = bp->state;
 			adrv->resume(adev);
 		}
 	}
@@ -475,6 +477,13 @@ static inline void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
 		edev->flags |= BNXT_EN_FLAG_ROCEV1_CAP;
 	if (bp->flags & BNXT_FLAG_ROCEV2_CAP)
 		edev->flags |= BNXT_EN_FLAG_ROCEV2_CAP;
+	if (bp->flags & BNXT_FLAG_VF)
+		edev->flags |= BNXT_EN_FLAG_VF;
+
+	edev->chip_num = bp->chip_num;
+	edev->hw_ring_stats_size = bp->hw_ring_stats_size;
+	edev->pf_port_id = bp->pf.port_id;
+	edev->en_state = bp->state;
 }
 
 int bnxt_rdma_aux_device_add(struct bnxt *bp)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index f76f06413a88..a065ecda6838 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -60,6 +60,9 @@ struct bnxt_en_dev {
 						 BNXT_EN_FLAG_ROCEV2_CAP)
 	#define BNXT_EN_FLAG_MSIX_REQUESTED	0x4
 	#define BNXT_EN_FLAG_ULP_STOPPED	0x8
+	#define BNXT_EN_FLAG_VF			0x10
+#define BNXT_EN_VF(edev)	((edev)->flags & BNXT_EN_FLAG_VF)
+
 	struct bnxt_ulp			*ulp_tbl;
 	int				l2_db_size;	/* Doorbell BAR size in
 							 * bytes mapped by L2
@@ -69,6 +72,14 @@ struct bnxt_en_dev {
 							 * bytes mapped as non-
 							 * cacheable.
 							 */
+	u16				chip_num;
+	u16				hw_ring_stats_size;
+	u16				pf_port_id;
+	unsigned long			en_state;	/* Could be checked in
+							 * RoCE driver suspend
+							 * mode only. Will be
+							 * updated in resume.
+							 */
 };
 
 static inline bool bnxt_ulp_registered(struct bnxt_en_dev *edev)
-- 
2.37.1 (Apple Git-137.1)


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH v4 0/6] Add Auxiliary driver support

[Leon Romanovsky]

On Wed, Nov 09, 2022 at 10:42:38AM -0800, Ajit Khaparde wrote:
> Add auxiliary device driver for Broadcom devices.
> The bnxt_en driver will register and initialize an aux device
> if RDMA is enabled in the underlying device.
> The bnxt_re driver will then probe and initialize the
> RoCE interfaces with the infiniband stack.
> 
> We got rid of the bnxt_en_ops which the bnxt_re driver used to
> communicate with bnxt_en.
> Similarly  We have tried to clean up most of the bnxt_ulp_ops.
> In most of the cases we used the functions and entry points provided
> by the auxiliary bus driver framework.
> And now these are the minimal functions needed to support the functionality.
> 
> We will try to work on getting rid of the remaining if we find any
> other viable option in future.

I still see extra checks for something that was already checked in upper
functions, for example in bnxt_re_register_netdev() you check rdev, which
you already checked before.

However, the most important part is still existence of bnxt_ulp_ops,
which shows completely no-go thing - SR-IOV config in RDMA code.

   302 static struct bnxt_ulp_ops bnxt_re_ulp_ops = {
   303         .ulp_sriov_config = bnxt_re_sriov_config,
   304         .ulp_irq_stop = bnxt_re_stop_irq,
   305         .ulp_irq_restart = bnxt_re_start_irq
   306 };

All PCI management logic and interfaces are needed to be inside eth part
of your driver and only that part should implement SR-IOV config. Once
user enabled SR-IOV, the PCI driver should create auxiliary devices for
each VF. These device will have RDMA capabilities and it will trigger RDMA
driver to bind to them.

Thanks

Re: [PATCH v4 0/6] Add Auxiliary driver support

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 4347 bytes --]

Leon,
We appreciate your valuable feedback.
Please see inline.

On Thu, Nov 10, 2022 at 2:53 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Wed, Nov 09, 2022 at 10:42:38AM -0800, Ajit Khaparde wrote:
> > Add auxiliary device driver for Broadcom devices.
> > The bnxt_en driver will register and initialize an aux device
> > if RDMA is enabled in the underlying device.
> > The bnxt_re driver will then probe and initialize the
> > RoCE interfaces with the infiniband stack.
> >
> > We got rid of the bnxt_en_ops which the bnxt_re driver used to
> > communicate with bnxt_en.
> > Similarly  We have tried to clean up most of the bnxt_ulp_ops.
> > In most of the cases we used the functions and entry points provided
> > by the auxiliary bus driver framework.
> > And now these are the minimal functions needed to support the functionality.
> >
> > We will try to work on getting rid of the remaining if we find any
> > other viable option in future.
>
> I still see extra checks for something that was already checked in upper
> functions, for example in bnxt_re_register_netdev() you check rdev, which
> you already checked before.
Sure. I will do another sweep and clean up.

>
> However, the most important part is still existence of bnxt_ulp_ops,
> which shows completely no-go thing - SR-IOV config in RDMA code.
>
>    302 static struct bnxt_ulp_ops bnxt_re_ulp_ops = {
>    303         .ulp_sriov_config = bnxt_re_sriov_config,
>    304         .ulp_irq_stop = bnxt_re_stop_irq,
>    305         .ulp_irq_restart = bnxt_re_start_irq
>    306 };
>
> All PCI management logic and interfaces are needed to be inside eth part
> of your driver and only that part should implement SR-IOV config. Once
> user enabled SR-IOV, the PCI driver should create auxiliary devices for
> each VF. These device will have RDMA capabilities and it will trigger RDMA
> driver to bind to them.
I agree and once the PF creates the auxiliary devices for the VF, the RoCE
Vf indeed get probed and created. But the twist in bnxt_en/bnxt_re
design is that
the RoCE driver is responsible for making adjustments to the RoCE resources.

So once the VF's are created and the bnxt_en driver enables SRIOV adjusts the
NIC resources for the VF,  and such, it tries to call into the bnxt_re
driver for the
same purpose.

1. We do something like this to the auxiliary_device structure:

diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
index de21d9d24a95..4e581fbf458f 100644
--- a/include/linux/auxiliary_bus.h
+++ b/include/linux/auxiliary_bus.h
@@ -148,6 +148,7 @@ struct auxiliary_device {
  * @shutdown: Called at shut-down time to quiesce the device.
  * @suspend: Called to put the device to sleep mode. Usually to a power state.
  * @resume: Called to bring a device from sleep mode.
+ * @sriov_configure: Called to allow configuration of VFs .
  * @name: Driver name.
  * @driver: Core driver structure.
  * @id_table: Table of devices this driver should match on the bus.
@@ -183,6 +184,7 @@ struct auxiliary_driver {
        void (*shutdown)(struct auxiliary_device *auxdev);
        int (*suspend)(struct auxiliary_device *auxdev, pm_message_t state);
        int (*resume)(struct auxiliary_device *auxdev);
+       int (*sriov_configure)(struct auxiliary_device *auxdev, int
num_vfs); /* On PF */
        const char *name;
        struct device_driver driver;
        const struct auxiliary_device_id *id_table;

Then the bnxt_en driver could call into bnxt_re via that interface.

Please let me know what you feel.

2. While it may take care of the first function in the ulp_ops, it
leaves us with two.
And that is where I will need some input if we need to absolutely get
rid of the ops.

2a. We may be able to use the auxiliary_device suspend & resume with a
private flag
in the driver's aux_dev pointer.
2b. Or just like (1) above, add another hook to auxiliary_driver
void (*restart)(struct auxiliary_device *auxdev);
And then use the auxiliary_driver shutdown & restart with a private flag.

Note that we may get creative right now and get rid of the ulp_ops.
But the bnxt_en driver having a need to update the bnxt_re driver is a
part of the
design. So it will help if we can consider beyond the ulp_irq_stop,
ulp_irq_restart.
2c. Maybe keep the bnxt_ulp_ops for that reason?

Thank you for your time.

Thanks
Ajit

>
> Thanks

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH v4 0/6] Add Auxiliary driver support

[Leon Romanovsky]

On Mon, Nov 14, 2022 at 04:47:31PM -0800, Ajit Khaparde wrote:
> Leon,
> We appreciate your valuable feedback.
> Please see inline.
> 
> On Thu, Nov 10, 2022 at 2:53 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Wed, Nov 09, 2022 at 10:42:38AM -0800, Ajit Khaparde wrote:
> > > Add auxiliary device driver for Broadcom devices.
> > > The bnxt_en driver will register and initialize an aux device
> > > if RDMA is enabled in the underlying device.
> > > The bnxt_re driver will then probe and initialize the
> > > RoCE interfaces with the infiniband stack.
> > >
> > > We got rid of the bnxt_en_ops which the bnxt_re driver used to
> > > communicate with bnxt_en.
> > > Similarly  We have tried to clean up most of the bnxt_ulp_ops.
> > > In most of the cases we used the functions and entry points provided
> > > by the auxiliary bus driver framework.
> > > And now these are the minimal functions needed to support the functionality.
> > >
> > > We will try to work on getting rid of the remaining if we find any
> > > other viable option in future.
> >
> > I still see extra checks for something that was already checked in upper
> > functions, for example in bnxt_re_register_netdev() you check rdev, which
> > you already checked before.
> Sure. I will do another sweep and clean up.
> 
> >
> > However, the most important part is still existence of bnxt_ulp_ops,
> > which shows completely no-go thing - SR-IOV config in RDMA code.
> >
> >    302 static struct bnxt_ulp_ops bnxt_re_ulp_ops = {
> >    303         .ulp_sriov_config = bnxt_re_sriov_config,
> >    304         .ulp_irq_stop = bnxt_re_stop_irq,
> >    305         .ulp_irq_restart = bnxt_re_start_irq
> >    306 };
> >
> > All PCI management logic and interfaces are needed to be inside eth part
> > of your driver and only that part should implement SR-IOV config. Once
> > user enabled SR-IOV, the PCI driver should create auxiliary devices for
> > each VF. These device will have RDMA capabilities and it will trigger RDMA
> > driver to bind to them.
> I agree and once the PF creates the auxiliary devices for the VF, the RoCE
> Vf indeed get probed and created. But the twist in bnxt_en/bnxt_re
> design is that
> the RoCE driver is responsible for making adjustments to the RoCE resources.

You can still do these adjustments by checking type of function that
called to RDMA .probe. PCI core exposes some functions to help distinguish between
PF and VFs.

> 
> So once the VF's are created and the bnxt_en driver enables SRIOV adjusts the
> NIC resources for the VF,  and such, it tries to call into the bnxt_re
> driver for the
> same purpose.

If I read code correctly, all these resources are for one PCI function.

Something like this:

bnxt_re_probe()
{
  ...
	if (is_virtfn(p))
		 bnxt_re_sriov_config(p);
  ...
}



> 
> 1. We do something like this to the auxiliary_device structure:
> 
> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
> index de21d9d24a95..4e581fbf458f 100644
> --- a/include/linux/auxiliary_bus.h
> +++ b/include/linux/auxiliary_bus.h
> @@ -148,6 +148,7 @@ struct auxiliary_device {
>   * @shutdown: Called at shut-down time to quiesce the device.
>   * @suspend: Called to put the device to sleep mode. Usually to a power state.
>   * @resume: Called to bring a device from sleep mode.
> + * @sriov_configure: Called to allow configuration of VFs .
>   * @name: Driver name.
>   * @driver: Core driver structure.
>   * @id_table: Table of devices this driver should match on the bus.
> @@ -183,6 +184,7 @@ struct auxiliary_driver {
>         void (*shutdown)(struct auxiliary_device *auxdev);
>         int (*suspend)(struct auxiliary_device *auxdev, pm_message_t state);
>         int (*resume)(struct auxiliary_device *auxdev);
> +       int (*sriov_configure)(struct auxiliary_device *auxdev, int
> num_vfs); /* On PF */
>         const char *name;
>         struct device_driver driver;
>         const struct auxiliary_device_id *id_table;
> 
> Then the bnxt_en driver could call into bnxt_re via that interface.
> 

@sriov_configure callback is PCI specific and doesn't belong to aux
devices.

> Please let me know what you feel.
> 
> 2. While it may take care of the first function in the ulp_ops, it
> leaves us with two.
> And that is where I will need some input if we need to absolutely get
> rid of the ops.
> 
> 2a. We may be able to use the auxiliary_device suspend & resume with a
> private flag
> in the driver's aux_dev pointer.
> 2b. Or just like (1) above, add another hook to auxiliary_driver
> void (*restart)(struct auxiliary_device *auxdev);
> And then use the auxiliary_driver shutdown & restart with a private flag.
> 
> Note that we may get creative right now and get rid of the ulp_ops.
> But the bnxt_en driver having a need to update the bnxt_re driver is a
> part of the
> design. So it will help if we can consider beyond the ulp_irq_stop,
> ulp_irq_restart.
> 2c. Maybe keep the bnxt_ulp_ops for that reason?

It is nicer to get rid from bnxt_ulp_ops completely, but it is not must.
To get rid from sriov_configure is the most important comment here.

Thanks

> 
> Thank you for your time.
> 
> Thanks
> Ajit
> 
> >
> > Thanks

Re: [PATCH v4 0/6] Add Auxiliary driver support

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]

On Wed, Nov 16, 2022 at 5:22 AM Leon Romanovsky <leon@kernel.org> wrote:
>
::snip::
> > > All PCI management logic and interfaces are needed to be inside eth part
> > > of your driver and only that part should implement SR-IOV config. Once
> > > user enabled SR-IOV, the PCI driver should create auxiliary devices for
> > > each VF. These device will have RDMA capabilities and it will trigger RDMA
> > > driver to bind to them.
> > I agree and once the PF creates the auxiliary devices for the VF, the RoCE
> > Vf indeed get probed and created. But the twist in bnxt_en/bnxt_re
> > design is that
> > the RoCE driver is responsible for making adjustments to the RoCE resources.
>
> You can still do these adjustments by checking type of function that
> called to RDMA .probe. PCI core exposes some functions to help distinguish between
> PF and VFs.
>
> >
> > So once the VF's are created and the bnxt_en driver enables SRIOV adjusts the
> > NIC resources for the VF,  and such, it tries to call into the bnxt_re
> > driver for the
> > same purpose.
>
> If I read code correctly, all these resources are for one PCI function.
>
> Something like this:
>
> bnxt_re_probe()
> {
>   ...
>         if (is_virtfn(p))
>                  bnxt_re_sriov_config(p);
>   ...
> }
I understand what you are suggesting.
But what I want is a way to do this in the context of the PF
preferably before the VFs are probed. So we are trying to call the
bnxt_re_sriov_config in the context of handling the PF's
sriov_configure implementation.  Having the ulp_ops is allowing us to
avoid resource wastage and assumptions in the bnxt_re driver.

::snip::

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH v4 0/6] Add Auxiliary driver support

[Leon Romanovsky]

On Tue, Nov 22, 2022 at 07:02:45AM -0800, Ajit Khaparde wrote:
> On Wed, Nov 16, 2022 at 5:22 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> ::snip::
> > > > All PCI management logic and interfaces are needed to be inside eth part
> > > > of your driver and only that part should implement SR-IOV config. Once
> > > > user enabled SR-IOV, the PCI driver should create auxiliary devices for
> > > > each VF. These device will have RDMA capabilities and it will trigger RDMA
> > > > driver to bind to them.
> > > I agree and once the PF creates the auxiliary devices for the VF, the RoCE
> > > Vf indeed get probed and created. But the twist in bnxt_en/bnxt_re
> > > design is that
> > > the RoCE driver is responsible for making adjustments to the RoCE resources.
> >
> > You can still do these adjustments by checking type of function that
> > called to RDMA .probe. PCI core exposes some functions to help distinguish between
> > PF and VFs.
> >
> > >
> > > So once the VF's are created and the bnxt_en driver enables SRIOV adjusts the
> > > NIC resources for the VF,  and such, it tries to call into the bnxt_re
> > > driver for the
> > > same purpose.
> >
> > If I read code correctly, all these resources are for one PCI function.
> >
> > Something like this:
> >
> > bnxt_re_probe()
> > {
> >   ...
> >         if (is_virtfn(p))
> >                  bnxt_re_sriov_config(p);
> >   ...
> > }
> I understand what you are suggesting.
> But what I want is a way to do this in the context of the PF
> preferably before the VFs are probed. 

I don't understand the last sentence. You call to this sriov_config in
bnxt_re driver without any protection from VFs being probed,

> So we are trying to call the
> bnxt_re_sriov_config in the context of handling the PF's
> sriov_configure implementation.  Having the ulp_ops is allowing us to
> avoid resource wastage and assumptions in the bnxt_re driver.

To which resource wastage are you referring?

There are no differences if same limits will be in bnxt_en driver when
RDMA bnxt device is created or in bnxt_re which will be called once RDMA
device is created.

Thanks

> 
> ::snip::

Re: [PATCH v4 0/6] Add Auxiliary driver support

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 3330 bytes --]

On Tue, Nov 22, 2022 at 10:59 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Tue, Nov 22, 2022 at 07:02:45AM -0800, Ajit Khaparde wrote:
> > On Wed, Nov 16, 2022 at 5:22 AM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > ::snip::
> > > > > All PCI management logic and interfaces are needed to be inside eth part
> > > > > of your driver and only that part should implement SR-IOV config. Once
> > > > > user enabled SR-IOV, the PCI driver should create auxiliary devices for
> > > > > each VF. These device will have RDMA capabilities and it will trigger RDMA
> > > > > driver to bind to them.
> > > > I agree and once the PF creates the auxiliary devices for the VF, the RoCE
> > > > Vf indeed get probed and created. But the twist in bnxt_en/bnxt_re
> > > > design is that
> > > > the RoCE driver is responsible for making adjustments to the RoCE resources.
> > >
> > > You can still do these adjustments by checking type of function that
> > > called to RDMA .probe. PCI core exposes some functions to help distinguish between
> > > PF and VFs.
> > >
> > > >
> > > > So once the VF's are created and the bnxt_en driver enables SRIOV adjusts the
> > > > NIC resources for the VF,  and such, it tries to call into the bnxt_re
> > > > driver for the
> > > > same purpose.
> > >
> > > If I read code correctly, all these resources are for one PCI function.
> > >
> > > Something like this:
> > >
> > > bnxt_re_probe()
> > > {
> > >   ...
> > >         if (is_virtfn(p))
> > >                  bnxt_re_sriov_config(p);
> > >   ...
> > > }
> > I understand what you are suggesting.
> > But what I want is a way to do this in the context of the PF
> > preferably before the VFs are probed.
>
> I don't understand the last sentence. You call to this sriov_config in
> bnxt_re driver without any protection from VFs being probed,

Let me elaborate -
When a user sets num_vfs to a non-zero number, the PCI driver hook
sriov_configure calls bnxt_sriov_configure(). Once pci_enable_sriov()
succeeds, bnxt_ulp_sriov_cfg() is issued under bnxt_sriov_configure().
All this happens under bnxt_en.
bnxt_ulp_sriov_cfg() ultimately calls into the bnxt_re driver.
Since bnxt_sriov_configure() is called only for PFs, bnxt_ulp_sriov_cfg()
is called for PFs only.

Once bnxt_ulp_sriov_cfg() calls into the bnxt_re via the ulp_ops,
it adjusts the QPs, SRQs, CQs, MRs, GIDs and such.

>
> > So we are trying to call the
> > bnxt_re_sriov_config in the context of handling the PF's
> > sriov_configure implementation.  Having the ulp_ops is allowing us to
> > avoid resource wastage and assumptions in the bnxt_re driver.
>
> To which resource wastage are you referring?
Essentially the PF driver reserves a set of above resources for the PF,
and divides the remaining resources among the VFs.
If the calculation is based on sriov_totalvfs instead of sriov_numvfs,
there can be a difference in the resources provisioned for a VF.
And that is because a user may create a subset of VFs instead of the
total VFs allowed in the PCI SR-IOV capability register.
I was referring to the resource wastage in that deployment scenario.

Thanks
Ajit

>
> There are no differences if same limits will be in bnxt_en driver when
> RDMA bnxt device is created or in bnxt_re which will be called once RDMA
> device is created.
>
> Thanks
>
> >
> > ::snip::
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH v4 0/6] Add Auxiliary driver support

[Leon Romanovsky]

On Mon, Nov 28, 2022 at 06:01:13PM -0800, Ajit Khaparde wrote:
> On Tue, Nov 22, 2022 at 10:59 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Tue, Nov 22, 2022 at 07:02:45AM -0800, Ajit Khaparde wrote:
> > > On Wed, Nov 16, 2022 at 5:22 AM Leon Romanovsky <leon@kernel.org> wrote:
> > > >
> > > ::snip::
> > > > > > All PCI management logic and interfaces are needed to be inside eth part
> > > > > > of your driver and only that part should implement SR-IOV config. Once
> > > > > > user enabled SR-IOV, the PCI driver should create auxiliary devices for
> > > > > > each VF. These device will have RDMA capabilities and it will trigger RDMA
> > > > > > driver to bind to them.
> > > > > I agree and once the PF creates the auxiliary devices for the VF, the RoCE
> > > > > Vf indeed get probed and created. But the twist in bnxt_en/bnxt_re
> > > > > design is that
> > > > > the RoCE driver is responsible for making adjustments to the RoCE resources.
> > > >
> > > > You can still do these adjustments by checking type of function that
> > > > called to RDMA .probe. PCI core exposes some functions to help distinguish between
> > > > PF and VFs.
> > > >
> > > > >
> > > > > So once the VF's are created and the bnxt_en driver enables SRIOV adjusts the
> > > > > NIC resources for the VF,  and such, it tries to call into the bnxt_re
> > > > > driver for the
> > > > > same purpose.
> > > >
> > > > If I read code correctly, all these resources are for one PCI function.
> > > >
> > > > Something like this:
> > > >
> > > > bnxt_re_probe()
> > > > {
> > > >   ...
> > > >         if (is_virtfn(p))
> > > >                  bnxt_re_sriov_config(p);
> > > >   ...
> > > > }
> > > I understand what you are suggesting.
> > > But what I want is a way to do this in the context of the PF
> > > preferably before the VFs are probed.
> >
> > I don't understand the last sentence. You call to this sriov_config in
> > bnxt_re driver without any protection from VFs being probed,
> 
> Let me elaborate -
> When a user sets num_vfs to a non-zero number, the PCI driver hook
> sriov_configure calls bnxt_sriov_configure(). Once pci_enable_sriov()
> succeeds, bnxt_ulp_sriov_cfg() is issued under bnxt_sriov_configure().
> All this happens under bnxt_en.
> bnxt_ulp_sriov_cfg() ultimately calls into the bnxt_re driver.
> Since bnxt_sriov_configure() is called only for PFs, bnxt_ulp_sriov_cfg()
> is called for PFs only.
> 
> Once bnxt_ulp_sriov_cfg() calls into the bnxt_re via the ulp_ops,
> it adjusts the QPs, SRQs, CQs, MRs, GIDs and such.

Once you called to pci_enable_sriov(), PCI core created sysfs entries
and it triggers udev rules and VFs probe. Because you are calling it
in bnxt_sriov_configure(), you will have inherit protection for PF
with PCI lock, but not for VFs.

> 
> >
> > > So we are trying to call the
> > > bnxt_re_sriov_config in the context of handling the PF's
> > > sriov_configure implementation.  Having the ulp_ops is allowing us to
> > > avoid resource wastage and assumptions in the bnxt_re driver.
> >
> > To which resource wastage are you referring?
> Essentially the PF driver reserves a set of above resources for the PF,
> and divides the remaining resources among the VFs.
> If the calculation is based on sriov_totalvfs instead of sriov_numvfs,
> there can be a difference in the resources provisioned for a VF.
> And that is because a user may create a subset of VFs instead of the
> total VFs allowed in the PCI SR-IOV capability register.
> I was referring to the resource wastage in that deployment scenario.

It is ok, set all needed limits in bnxt_en. You don't need to call to
bnxt_re for that.

> 
> Thanks
> Ajit
> 
> >
> > There are no differences if same limits will be in bnxt_en driver when
> > RDMA bnxt device is created or in bnxt_re which will be called once RDMA
> > device is created.
> >
> > Thanks
> >
> > >
> > > ::snip::
> >
> >

Re: [PATCH v4 0/6] Add Auxiliary driver support

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]

On Tue, Nov 29, 2022 at 1:13 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, Nov 28, 2022 at 06:01:13PM -0800, Ajit Khaparde wrote:
> > On Tue, Nov 22, 2022 at 10:59 PM Leon Romanovsky <leon@kernel.org> wrote:
> > >
::: snip :::
> >
> > >
> > > > So we are trying to call the
> > > > bnxt_re_sriov_config in the context of handling the PF's
> > > > sriov_configure implementation.  Having the ulp_ops is allowing us to
> > > > avoid resource wastage and assumptions in the bnxt_re driver.
> > >
> > > To which resource wastage are you referring?
> > Essentially the PF driver reserves a set of above resources for the PF,
> > and divides the remaining resources among the VFs.
> > If the calculation is based on sriov_totalvfs instead of sriov_numvfs,
> > there can be a difference in the resources provisioned for a VF.
> > And that is because a user may create a subset of VFs instead of the
> > total VFs allowed in the PCI SR-IOV capability register.
> > I was referring to the resource wastage in that deployment scenario.
>
> It is ok, set all needed limits in bnxt_en. You don't need to call to
> bnxt_re for that.
Thanks. We have removed the sriov_config callback.
I will send the fresh patchset in a few minutes.

>
> >
> > Thanks
> > Ajit
> >
> > >
> > > There are no differences if same limits will be in bnxt_en driver when
> > > RDMA bnxt device is created or in bnxt_re which will be called once RDMA
> > > device is created.
> > >
> > > Thanks
> > >
> > > >
> > > > ::snip::
> > >
> > >
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]