[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aYPZ5O3mFzxCzyc9@sx113>
Date: Wed, 4 Feb 2026 15:44:36 -0800
From: Saeed Mahameed <saeedm@...dia.com>
To: Pavan Chebbi <pavan.chebbi@...adcom.com>
Cc: jgg@...pe.ca, michael.chan@...adcom.com, linux-kernel@...r.kernel.org,
dave.jiang@...el.com, Jonathan.Cameron@...wei.com,
gospo@...adcom.com, selvin.xavier@...adcom.com, leon@...nel.org,
kalesh-anakkur.purayil@...adcom.com
Subject: Re: [PATCH v3 fwctl 2/5] fwctl/bnxt_en: Refactor aux bus functions
to be more generic
On 29 Jan 07:54, Pavan Chebbi wrote:
>Up until now there was only one auxiliary device that bnxt
>created and that was for RoCE driver. bnxt fwctl is also
>going to use an aux bus device that bnxt should create.
>This requires some nomenclature changes and refactoring of
>the existing bnxt aux dev functions.
>
>Convert 'aux_priv' and 'edev' members of struct bnxt into
>arrays where each element contains supported auxbus device's
>data. Move struct bnxt_aux_priv from bnxt.h to ulp.h because
>that is where it belongs. Make aux bus init/uninit/add/del
>functions more generic which will loop through all the aux
>device types. Make bnxt_ulp_start/stop functions (the only
>other common functions applicable to any aux device) loop
>through the aux devices to update their config and states.
>Make callers of bnxt_ulp_start() call it only when there
>are no errors.
>
>Also, as an improvement in code, bnxt_register_dev() can skip
>unnecessary dereferencing of edev from bp, instead use the
>edev pointer from the function parameter.
>
>Future patches will reuse these functions to add an aux bus
>device for fwctl.
>
>Reviewed-by: Andy Gospodarek <gospo@...adcom.com>
>Signed-off-by: Pavan Chebbi <pavan.chebbi@...adcom.com>
>---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 47 ++-
> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 19 +-
> .../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 8 +-
> .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 2 +-
> drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 339 +++++++++++-------
> include/linux/bnxt/ulp.h | 25 +-
> 6 files changed, 273 insertions(+), 167 deletions(-)
>
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>index 4481d80cdfc2..a9bfbfabf121 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>@@ -6859,7 +6859,8 @@ int bnxt_hwrm_vnic_cfg(struct bnxt *bp, struct bnxt_vnic_info *vnic)
> #endif
> if ((bp->flags & BNXT_FLAG_STRIP_VLAN) || def_vlan)
> req->flags |= cpu_to_le32(VNIC_CFG_REQ_FLAGS_VLAN_STRIP_MODE);
>- if (vnic->vnic_id == BNXT_VNIC_DEFAULT && bnxt_ulp_registered(bp->edev))
>+ if (vnic->vnic_id == BNXT_VNIC_DEFAULT &&
>+ bnxt_ulp_registered(bp->edev[BNXT_AUXDEV_RDMA]))
> req->flags |= cpu_to_le32(bnxt_get_roce_vnic_mode(bp));
>
> return hwrm_req_send(bp, req);
>@@ -7974,6 +7975,7 @@ static int bnxt_get_avail_msix(struct bnxt *bp, int num);
>
> static int __bnxt_reserve_rings(struct bnxt *bp)
> {
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
> struct bnxt_hw_rings hwr = {0};
> int rx_rings, old_rx_rings, rc;
> int cp = bp->cp_nr_rings;
>@@ -7984,7 +7986,7 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
> if (!bnxt_need_reserve_rings(bp))
> return 0;
>
>- if (BNXT_NEW_RM(bp) && !bnxt_ulp_registered(bp->edev)) {
>+ if (BNXT_NEW_RM(bp) && !bnxt_ulp_registered(edev)) {
> ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
> if (!ulp_msix)
> bnxt_set_ulp_stat_ctxs(bp, 0);
>@@ -8035,8 +8037,7 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
> }
> rx_rings = min_t(int, rx_rings, hwr.grp);
> hwr.cp = min_t(int, hwr.cp, bp->cp_nr_rings);
>- if (bnxt_ulp_registered(bp->edev) &&
>- hwr.stat > bnxt_get_ulp_stat_ctxs(bp))
>+ if (bnxt_ulp_registered(edev) && hwr.stat > bnxt_get_ulp_stat_ctxs(bp))
> hwr.stat -= bnxt_get_ulp_stat_ctxs(bp);
> hwr.cp = min_t(int, hwr.cp, hwr.stat);
> rc = bnxt_trim_rings(bp, &rx_rings, &hwr.tx, hwr.cp, sh);
>@@ -8075,7 +8076,7 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
> !netif_is_rxfh_configured(bp->dev))
> bnxt_set_dflt_rss_indir_tbl(bp, NULL);
>
>- if (!bnxt_ulp_registered(bp->edev) && BNXT_NEW_RM(bp)) {
>+ if (!bnxt_ulp_registered(edev) && BNXT_NEW_RM(bp)) {
> int resv_msix, resv_ctx, ulp_ctxs;
> struct bnxt_hw_resc *hw_resc;
>
>@@ -11430,6 +11431,7 @@ static void bnxt_clear_int_mode(struct bnxt *bp)
>
> int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init)
> {
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
> bool irq_cleared = false;
> bool irq_change = false;
> int tcs = bp->num_tc;
>@@ -11439,7 +11441,7 @@ int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init)
> if (!bnxt_need_reserve_rings(bp))
> return 0;
>
>- if (BNXT_NEW_RM(bp) && !bnxt_ulp_registered(bp->edev)) {
>+ if (BNXT_NEW_RM(bp) && !bnxt_ulp_registered(edev)) {
> int ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
>
> if (ulp_msix > bp->ulp_num_msix_want)
>@@ -14522,7 +14524,7 @@ static void bnxt_fw_echo_reply(struct bnxt *bp)
> static void bnxt_ulp_restart(struct bnxt *bp)
> {
> bnxt_ulp_stop(bp);
>- bnxt_ulp_start(bp, 0);
>+ bnxt_ulp_start(bp);
> }
>
> static void bnxt_sp_task(struct work_struct *work)
>@@ -14679,7 +14681,7 @@ int bnxt_check_rings(struct bnxt *bp, int tx, int rx, bool sh, int tcs,
> hwr.cp_p5 = hwr.tx + rx;
> rc = bnxt_hwrm_check_rings(bp, &hwr);
> if (!rc && pci_msix_can_alloc_dyn(bp->pdev)) {
>- if (!bnxt_ulp_registered(bp->edev)) {
>+ if (!bnxt_ulp_registered(bp->edev[BNXT_AUXDEV_RDMA])) {
> hwr.cp += bnxt_get_ulp_msix_num(bp);
> hwr.cp = min_t(int, hwr.cp, bnxt_get_max_func_irqs(bp));
> }
>@@ -15199,7 +15201,7 @@ static void bnxt_fw_reset_task(struct work_struct *work)
> bnxt_dl_health_fw_status_update(bp, true);
> }
> netdev_unlock(bp->dev);
>- bnxt_ulp_start(bp, 0);
>+ bnxt_ulp_start(bp);
> bnxt_reenable_sriov(bp);
> netdev_lock(bp->dev);
> bnxt_vf_reps_alloc(bp);
>@@ -15221,7 +15223,8 @@ static void bnxt_fw_reset_task(struct work_struct *work)
> bnxt_fw_reset_abort(bp, rc);
> netdev_unlock(bp->dev);
> ulp_start:
>- bnxt_ulp_start(bp, rc);
>+ if (!rc)
>+ bnxt_ulp_start(bp);
> }
>
> static int bnxt_init_board(struct pci_dev *pdev, struct net_device *dev)
>@@ -16211,12 +16214,13 @@ static void bnxt_remove_one(struct pci_dev *pdev)
> if (BNXT_PF(bp))
> __bnxt_sriov_disable(bp);
>
>- bnxt_rdma_aux_device_del(bp);
>+ bnxt_aux_devices_del(bp);
>
> unregister_netdev(dev);
> bnxt_ptp_clear(bp);
>
>- bnxt_rdma_aux_device_uninit(bp);
>+ bnxt_aux_devices_uninit(bp);
>+ bnxt_auxdev_id_free(bp, bp->auxdev_id);
>
> bnxt_free_l2_filters(bp, true);
> bnxt_free_ntp_fltrs(bp, true);
>@@ -16802,7 +16806,9 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> bnxt_set_tpa_flags(bp);
> bnxt_init_ring_params(bp);
> bnxt_set_ring_params(bp);
>- bnxt_rdma_aux_device_init(bp);
>+ mutex_init(&bp->auxdev_lock);
>+ if (!bnxt_auxdev_id_alloc(bp))
>+ bnxt_aux_devices_init(bp);
> rc = bnxt_set_dflt_rings(bp, true);
> if (rc) {
> if (BNXT_VF(bp) && rc == -ENODEV) {
>@@ -16866,7 +16872,7 @@ 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_add(bp);
>+ bnxt_aux_devices_add(bp);
>
> bnxt_print_device_info(bp);
>
>@@ -16874,7 +16880,8 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> return 0;
> init_err_cleanup:
>- bnxt_rdma_aux_device_uninit(bp);
>+ bnxt_aux_devices_uninit(bp);
>+ bnxt_auxdev_id_free(bp, bp->auxdev_id);
> bnxt_dl_unregister(bp);
> init_err_dl:
> bnxt_shutdown_tc(bp);
>@@ -17008,9 +17015,10 @@ static int bnxt_resume(struct device *device)
>
> resume_exit:
> netdev_unlock(bp->dev);
>- bnxt_ulp_start(bp, rc);
>- if (!rc)
>+ if (!rc) {
>+ bnxt_ulp_start(bp);
> bnxt_reenable_sriov(bp);
>+ }
> return rc;
> }
>
>@@ -17190,9 +17198,10 @@ static void bnxt_io_resume(struct pci_dev *pdev)
> netif_device_attach(netdev);
>
> netdev_unlock(netdev);
>- bnxt_ulp_start(bp, err);
>- if (!err)
>+ if (!err) {
>+ bnxt_ulp_start(bp);
> bnxt_reenable_sriov(bp);
>+ }
> }
>
> static const struct pci_error_handlers bnxt_err_handler = {
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
>index f5f07a7e6b29..a739e7d52bdc 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
>@@ -24,12 +24,12 @@
> #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>
> #include <linux/dim.h>
> #include <linux/io-64-nonatomic-lo-hi.h>
>+#include <linux/bnxt/ulp.h>
> #ifdef CONFIG_TEE_BNXT_FW
> #include <linux/firmware/broadcom/tee_bnxt_fw.h>
> #endif
>@@ -2076,12 +2076,6 @@ struct bnxt_fw_health {
> #define BNXT_FW_IF_RETRY 10
> #define BNXT_FW_SLOT_RESET_RETRY 4
>
>-struct bnxt_aux_priv {
>- struct auxiliary_device aux_dev;
>- struct bnxt_en_dev *edev;
>- int id;
>-};
>-
> enum board_idx {
> BCM57301,
> BCM57302,
>@@ -2341,8 +2335,8 @@ struct bnxt {
> #define BNXT_CHIP_P5_AND_MINUS(bp) \
> (BNXT_CHIP_P3(bp) || BNXT_CHIP_P4(bp) || BNXT_CHIP_P5(bp))
>
>- struct bnxt_aux_priv *aux_priv;
>- struct bnxt_en_dev *edev;
>+ struct bnxt_aux_priv *aux_priv[__BNXT_AUXDEV_MAX];
>+ struct bnxt_en_dev *edev[__BNXT_AUXDEV_MAX];
>
> struct bnxt_napi **bnapi;
>
>@@ -2751,6 +2745,13 @@ struct bnxt {
> struct bnxt_ctx_pg_info *fw_crash_mem;
> u32 fw_crash_len;
> struct bnxt_bs_trace_info bs_trace[BNXT_TRACE_MAX];
>+ int auxdev_id;
>+ /* synchronize validity checks of available aux devices */
>+ struct mutex auxdev_lock;
>+ u8 auxdev_state[__BNXT_AUXDEV_MAX];
>+#define BNXT_ADEV_STATE_NONE 0
>+#define BNXT_ADEV_STATE_INIT 1
>+#define BNXT_ADEV_STATE_ADD 2
> };
>
> #define BNXT_NUM_RX_RING_STATS 8
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>index 230cd95d30a2..835f2b413931 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>@@ -440,13 +440,13 @@ static int bnxt_dl_reload_down(struct devlink *dl, bool netns_change,
> "reload is unsupported while VFs are allocated or being configured");
> netdev_unlock(bp->dev);
> rtnl_unlock();
>- bnxt_ulp_start(bp, 0);
>+ bnxt_ulp_start(bp);
> return -EOPNOTSUPP;
> }
> if (bp->dev->reg_state == NETREG_UNREGISTERED) {
> netdev_unlock(bp->dev);
> rtnl_unlock();
>- bnxt_ulp_start(bp, 0);
>+ bnxt_ulp_start(bp);
> return -ENODEV;
> }
> if (netif_running(bp->dev))
>@@ -578,8 +578,8 @@ static int bnxt_dl_reload_up(struct devlink *dl, enum devlink_reload_action acti
> }
> netdev_unlock(bp->dev);
> rtnl_unlock();
>- if (action == DEVLINK_RELOAD_ACTION_DRIVER_REINIT)
>- bnxt_ulp_start(bp, rc);
>+ if (!rc && action == DEVLINK_RELOAD_ACTION_DRIVER_REINIT)
>+ bnxt_ulp_start(bp);
> return rc;
> }
>
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
>index 8cad7b982664..064d7bc4ce8d 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
>@@ -5100,7 +5100,7 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
>
> memset(buf, 0, sizeof(u64) * bp->num_tests);
> if (etest->flags & ETH_TEST_FL_OFFLINE &&
>- bnxt_ulp_registered(bp->edev)) {
>+ bnxt_ulp_registered(bp->edev[BNXT_AUXDEV_RDMA])) {
> etest->flags |= ETH_TEST_FL_FAILED;
> netdev_warn(dev, "Offline tests cannot be run with RoCE driver loaded\n");
> return;
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
>index fa513892db50..3097fc5755e6 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
>@@ -29,9 +29,32 @@
>
> static DEFINE_IDA(bnxt_aux_dev_ids);
>
>+struct bnxt_aux_device {
>+ const char *name;
>+};
>+
>+static void bnxt_auxdev_set_state(struct bnxt *bp, int idx, int state)
>+{
>+ bp->auxdev_state[idx] = state;
>+}
>+
>+static bool bnxt_auxdev_is_init(struct bnxt *bp, int idx)
>+{
>+ return (bp->auxdev_state[idx] == BNXT_ADEV_STATE_INIT);
>+}
>+
>+static bool bnxt_auxdev_is_active(struct bnxt *bp, int idx)
>+{
>+ return (bp->auxdev_state[idx] == BNXT_ADEV_STATE_ADD);
>+}
>+
>+static struct bnxt_aux_device bnxt_aux_devices[__BNXT_AUXDEV_MAX] = {{
>+ .name = "rdma",
>+}};
>+
> static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
> {
>- struct bnxt_en_dev *edev = bp->edev;
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
> int num_msix, i;
>
> if (!edev->ulp_tbl->msix_requested) {
>@@ -51,61 +74,75 @@ static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
>
> int bnxt_get_ulp_msix_num(struct bnxt *bp)
> {
>- if (bp->edev)
>- return bp->edev->ulp_num_msix_vec;
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
>+
>+ if (edev)
>+ return edev->ulp_num_msix_vec;
> return 0;
> }
>
> void bnxt_set_ulp_msix_num(struct bnxt *bp, int num)
> {
>- if (bp->edev)
>- bp->edev->ulp_num_msix_vec = num;
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
>+
>+ if (edev)
>+ edev->ulp_num_msix_vec = num;
> }
>
> int bnxt_get_ulp_msix_num_in_use(struct bnxt *bp)
> {
>- if (bnxt_ulp_registered(bp->edev))
>- return bp->edev->ulp_num_msix_vec;
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
>+
>+ if (bnxt_ulp_registered(edev))
>+ return edev->ulp_num_msix_vec;
> return 0;
> }
>
> int bnxt_get_ulp_stat_ctxs(struct bnxt *bp)
> {
>- if (bp->edev)
>- return bp->edev->ulp_num_ctxs;
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
>+
>+ if (edev)
>+ return edev->ulp_num_ctxs;
> return 0;
> }
>
> void bnxt_set_ulp_stat_ctxs(struct bnxt *bp, int num_ulp_ctx)
> {
>- if (bp->edev)
>- bp->edev->ulp_num_ctxs = num_ulp_ctx;
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
>+
>+ if (edev)
>+ edev->ulp_num_ctxs = num_ulp_ctx;
> }
>
> int bnxt_get_ulp_stat_ctxs_in_use(struct bnxt *bp)
> {
>- if (bnxt_ulp_registered(bp->edev))
>- return bp->edev->ulp_num_ctxs;
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
>+
>+ if (bnxt_ulp_registered(edev))
>+ return edev->ulp_num_ctxs;
> return 0;
> }
>
> void bnxt_set_dflt_ulp_stat_ctxs(struct bnxt *bp)
> {
>- if (bp->edev) {
>- bp->edev->ulp_num_ctxs = BNXT_MIN_ROCE_STAT_CTXS;
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
>+
>+ if (edev) {
>+ edev->ulp_num_ctxs = BNXT_MIN_ROCE_STAT_CTXS;
> /* Reserve one additional stat_ctx for PF0 (except
> * on 1-port NICs) as it also creates one stat_ctx
> * for PF1 in case of RoCE bonding.
> */
> if (BNXT_PF(bp) && !bp->pf.port_id &&
> bp->port_count > 1)
>- bp->edev->ulp_num_ctxs++;
>+ edev->ulp_num_ctxs++;
>
> /* Reserve one additional stat_ctx when the device is capable
> * of supporting port mirroring on RDMA device.
> */
> if (BNXT_MIRROR_ON_ROCE_CAP(bp))
>- bp->edev->ulp_num_ctxs++;
>+ edev->ulp_num_ctxs++;
> }
> }
>
>@@ -141,7 +178,7 @@ int bnxt_register_dev(struct bnxt_en_dev *edev,
>
> edev->ulp_tbl->msix_requested = bnxt_get_ulp_msix_num(bp);
>
>- bnxt_fill_msix_vecs(bp, bp->edev->msix_entries);
>+ bnxt_fill_msix_vecs(bp, edev->msix_entries);
> exit:
> mutex_unlock(&edev->en_dev_lock);
> netdev_unlock(dev);
>@@ -227,20 +264,27 @@ EXPORT_SYMBOL(bnxt_send_msg);
>
> void bnxt_ulp_stop(struct bnxt *bp)
> {
>- struct bnxt_aux_priv *aux_priv = bp->aux_priv;
>- struct bnxt_en_dev *edev = bp->edev;
>+ int i;
>
>- if (!edev)
>- return;
>-
>- mutex_lock(&edev->en_dev_lock);
>- if (!bnxt_ulp_registered(edev) ||
>- (edev->flags & BNXT_EN_FLAG_ULP_STOPPED))
>- goto ulp_stop_exit;
>-
>- edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
>- if (aux_priv) {
>+ mutex_lock(&bp->auxdev_lock);
>+ for (i = 0; i < __BNXT_AUXDEV_MAX; i++) {
>+ struct bnxt_aux_priv *aux_priv;
> struct auxiliary_device *adev;
>+ struct bnxt_en_dev *edev;
>+
>+ if (!bnxt_auxdev_is_active(bp, i))
>+ continue;
>+
>+ aux_priv = bp->aux_priv[i];
>+ edev = bp->edev[i];
>+ mutex_lock(&edev->en_dev_lock);
>+ if (!bnxt_ulp_registered(edev) ||
>+ (edev->flags & BNXT_EN_FLAG_ULP_STOPPED)) {
>+ mutex_unlock(&edev->en_dev_lock);
>+ continue;
>+ }
>+
>+ edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
>
> adev = &aux_priv->aux_dev;
> if (adev->dev.driver) {
>@@ -251,29 +295,35 @@ void bnxt_ulp_stop(struct bnxt *bp)
> edev->en_state = bp->state;
> adrv->suspend(adev, pm);
> }
>+ mutex_unlock(&edev->en_dev_lock);
> }
>-ulp_stop_exit:
>- mutex_unlock(&edev->en_dev_lock);
>+ mutex_unlock(&bp->auxdev_lock);
> }
>
>-void bnxt_ulp_start(struct bnxt *bp, int err)
>+void bnxt_ulp_start(struct bnxt *bp)
> {
>- struct bnxt_aux_priv *aux_priv = bp->aux_priv;
>- struct bnxt_en_dev *edev = bp->edev;
>+ int i;
>
>- if (!edev || err)
>- return;
>+ mutex_lock(&bp->auxdev_lock);
>+ for (i = 0; i < __BNXT_AUXDEV_MAX; i++) {
>+ struct bnxt_aux_priv *aux_priv;
>+ struct auxiliary_device *adev;
>+ struct bnxt_en_dev *edev;
>
>- mutex_lock(&edev->en_dev_lock);
>- if (!bnxt_ulp_registered(edev) ||
>- !(edev->flags & BNXT_EN_FLAG_ULP_STOPPED))
>- goto ulp_start_exit;
>+ if (!bnxt_auxdev_is_active(bp, i))
>+ continue;
>
>- if (edev->ulp_tbl->msix_requested)
>- bnxt_fill_msix_vecs(bp, edev->msix_entries);
>+ aux_priv = bp->aux_priv[i];
>+ edev = bp->edev[i];
>+ mutex_lock(&edev->en_dev_lock);
>+ if (!bnxt_ulp_registered(edev) ||
>+ !(edev->flags & BNXT_EN_FLAG_ULP_STOPPED)) {
>+ goto clear_flag_continue;
>+ }
>+
>+ if (edev->ulp_tbl->msix_requested)
>+ bnxt_fill_msix_vecs(bp, edev->msix_entries);
>
>- if (aux_priv) {
>- struct auxiliary_device *adev;
>
> adev = &aux_priv->aux_dev;
> if (adev->dev.driver) {
>@@ -283,22 +333,23 @@ void bnxt_ulp_start(struct bnxt *bp, int err)
> edev->en_state = bp->state;
> adrv->resume(adev);
> }
>+clear_flag_continue:
>+ edev->flags &= ~BNXT_EN_FLAG_ULP_STOPPED;
>+ mutex_unlock(&edev->en_dev_lock);
> }
>-ulp_start_exit:
>- edev->flags &= ~BNXT_EN_FLAG_ULP_STOPPED;
>- mutex_unlock(&edev->en_dev_lock);
>+ mutex_unlock(&bp->auxdev_lock);
> }
>
> void bnxt_ulp_irq_stop(struct bnxt *bp)
> {
>- struct bnxt_en_dev *edev = bp->edev;
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
> struct bnxt_ulp_ops *ops;
> bool reset = false;
>
> if (!edev)
> return;
>
>- if (bnxt_ulp_registered(bp->edev)) {
>+ if (bnxt_ulp_registered(edev)) {
> struct bnxt_ulp *ulp = edev->ulp_tbl;
>
> if (!ulp->msix_requested)
>@@ -315,13 +366,13 @@ void bnxt_ulp_irq_stop(struct bnxt *bp)
>
> void bnxt_ulp_irq_restart(struct bnxt *bp, int err)
> {
>- struct bnxt_en_dev *edev = bp->edev;
>+ struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
> struct bnxt_ulp_ops *ops;
>
> if (!edev)
> return;
>
>- if (bnxt_ulp_registered(bp->edev)) {
>+ if (bnxt_ulp_registered(edev)) {
> struct bnxt_ulp *ulp = edev->ulp_tbl;
> struct bnxt_msix_entry *ent = NULL;
>
>@@ -347,7 +398,7 @@ 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_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
> struct bnxt_ulp_ops *ops;
> struct bnxt_ulp *ulp;
>
>@@ -388,18 +439,21 @@ void bnxt_register_async_events(struct bnxt_en_dev *edev,
> }
> EXPORT_SYMBOL(bnxt_register_async_events);
>
>-void bnxt_rdma_aux_device_uninit(struct bnxt *bp)
>+void bnxt_aux_devices_uninit(struct bnxt *bp)
> {
> struct bnxt_aux_priv *aux_priv;
> struct auxiliary_device *adev;
>-
>- /* Skip if no auxiliary device init was done. */
>- if (!bp->aux_priv)
>- return;
>-
>- aux_priv = bp->aux_priv;
>- adev = &aux_priv->aux_dev;
>- auxiliary_device_uninit(adev);
>+ int idx;
>+
>+ mutex_lock(&bp->auxdev_lock);
>+ for (idx = 0; idx < __BNXT_AUXDEV_MAX; idx++) {
>+ if (bnxt_auxdev_is_init(bp, idx)) {
>+ aux_priv = bp->aux_priv[idx];
>+ adev = &aux_priv->aux_dev;
>+ auxiliary_device_uninit(adev);
>+ }
>+ }
>+ mutex_unlock(&bp->auxdev_lock);
> }
>
> static void bnxt_aux_dev_release(struct device *dev)
>@@ -408,20 +462,25 @@ static void bnxt_aux_dev_release(struct device *dev)
> container_of(dev, struct bnxt_aux_priv, aux_dev.dev);
> struct bnxt *bp = netdev_priv(aux_priv->edev->net);
>
>- ida_free(&bnxt_aux_dev_ids, aux_priv->id);
> kfree(aux_priv->edev->ulp_tbl);
>- bp->edev = NULL;
>+ bp->edev[aux_priv->id] = NULL;
> kfree(aux_priv->edev);
> kfree(aux_priv);
>- bp->aux_priv = NULL;
>+ bp->aux_priv[aux_priv->id] = NULL;
> }
>
>-void bnxt_rdma_aux_device_del(struct bnxt *bp)
>+void bnxt_aux_devices_del(struct bnxt *bp)
> {
>- if (!bp->edev)
>- return;
>+ int idx;
>
>- auxiliary_device_delete(&bp->aux_priv->aux_dev);
>+ mutex_lock(&bp->auxdev_lock);
>+ for (idx = 0; idx < __BNXT_AUXDEV_MAX; idx++) {
>+ if (bnxt_auxdev_is_active(bp, idx)) {
>+ auxiliary_device_delete(&bp->aux_priv[idx]->aux_dev);
>+ bnxt_auxdev_set_state(bp, idx, BNXT_ADEV_STATE_INIT);
I can't really follow why you need to maintain a dangling device state !
Why not completely remove and free an aux dev and then on probe/add init
and add from scratch, simplified patch #3 which prompted me to come and
look back here.
>+ }
>+ }
>+ mutex_unlock(&bp->auxdev_lock);
> }
>
> static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
>@@ -451,83 +510,105 @@ static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
> edev->bar0 = bp->bar0;
> }
>
>-void bnxt_rdma_aux_device_add(struct bnxt *bp)
>+void bnxt_aux_devices_add(struct bnxt *bp)
> {
> struct auxiliary_device *aux_dev;
>- int rc;
>-
>- if (!bp->edev)
>- return;
>-
>- aux_dev = &bp->aux_priv->aux_dev;
>- rc = auxiliary_device_add(aux_dev);
>- if (rc) {
>- netdev_warn(bp->dev, "Failed to add auxiliary device for ROCE\n");
>- auxiliary_device_uninit(aux_dev);
>- bp->flags &= ~BNXT_FLAG_ROCE_CAP;
>+ int rc, idx;
>+
>+ mutex_lock(&bp->auxdev_lock);
>+ for (idx = 0; idx < __BNXT_AUXDEV_MAX; idx++) {
>+ if (bnxt_auxdev_is_init(bp, idx)) {
>+ aux_dev = &bp->aux_priv[idx]->aux_dev;
>+ rc = auxiliary_device_add(aux_dev);
>+ if (rc) {
>+ netdev_warn(bp->dev, "Failed to add auxiliary device for ROCE\n");
>+ auxiliary_device_uninit(aux_dev);
>+ if (idx == BNXT_AUXDEV_RDMA)
>+ bp->flags &= ~BNXT_FLAG_ROCE_CAP;
>+ continue;
>+ }
>+ bnxt_auxdev_set_state(bp, idx, BNXT_ADEV_STATE_ADD);
>+ }
> }
>+ mutex_unlock(&bp->auxdev_lock);
> }
>
>-void bnxt_rdma_aux_device_init(struct bnxt *bp)
>+void bnxt_aux_devices_init(struct bnxt *bp)
> {
> struct auxiliary_device *aux_dev;
> struct bnxt_aux_priv *aux_priv;
> struct bnxt_en_dev *edev;
> struct bnxt_ulp *ulp;
>- int rc;
>+ int rc, idx;
>+
>+ mutex_lock(&bp->auxdev_lock);
>+ for (idx = 0; idx < __BNXT_AUXDEV_MAX; idx++) {
>+ bnxt_auxdev_set_state(bp, idx, BNXT_ADEV_STATE_NONE);
>+
>+ if (idx == BNXT_AUXDEV_RDMA &&
>+ !(bp->flags & BNXT_FLAG_ROCE_CAP))
>+ continue;
>+
>+ aux_priv = kzalloc(sizeof(*aux_priv), GFP_KERNEL);
>+ if (!aux_priv)
>+ goto next_auxdev;
>+
>+ aux_dev = &aux_priv->aux_dev;
>+ aux_dev->id = bp->auxdev_id;
>+ aux_dev->name = bnxt_aux_devices[idx].name;
>+ aux_dev->dev.parent = &bp->pdev->dev;
>+ aux_dev->dev.release = bnxt_aux_dev_release;
>+
>+ rc = auxiliary_device_init(aux_dev);
>+ if (rc) {
>+ kfree(aux_priv);
>+ goto next_auxdev;
>+ }
>+ bp->aux_priv[idx] = aux_priv;
>
>- if (!(bp->flags & BNXT_FLAG_ROCE_CAP))
>- return;
>+ /* From this point, all cleanup will happen via the .release
>+ * callback & any error unwinding will need to include a call
>+ * to auxiliary_device_uninit.
>+ */
>+ edev = kzalloc(sizeof(*edev), GFP_KERNEL);
>+ if (!edev)
>+ goto aux_dev_uninit;
>
>- aux_priv = kzalloc(sizeof(*bp->aux_priv), GFP_KERNEL);
>- if (!aux_priv)
>- goto exit;
>+ aux_priv->edev = edev;
>+ bnxt_set_edev_info(edev, bp);
>
>- aux_priv->id = ida_alloc(&bnxt_aux_dev_ids, GFP_KERNEL);
>- if (aux_priv->id < 0) {
>- netdev_warn(bp->dev,
>- "ida alloc failed for ROCE auxiliary device\n");
>- kfree(aux_priv);
>- goto exit;
>- }
>+ ulp = kzalloc(sizeof(*ulp), GFP_KERNEL);
>+ if (!ulp)
>+ goto aux_dev_uninit;
>
>- aux_dev = &aux_priv->aux_dev;
>- aux_dev->id = aux_priv->id;
>- aux_dev->name = "rdma";
>- aux_dev->dev.parent = &bp->pdev->dev;
>- aux_dev->dev.release = bnxt_aux_dev_release;
>+ edev->ulp_tbl = ulp;
>+ bp->edev[idx] = edev;
>+ if (idx == BNXT_AUXDEV_RDMA)
>+ bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
>+ aux_priv->id = idx;
>+ bnxt_auxdev_set_state(bp, idx, BNXT_ADEV_STATE_INIT);
>
>- rc = auxiliary_device_init(aux_dev);
>- if (rc) {
>- ida_free(&bnxt_aux_dev_ids, aux_priv->id);
>- kfree(aux_priv);
>- goto exit;
>+ continue;
>+aux_dev_uninit:
>+ auxiliary_device_uninit(aux_dev);
>+next_auxdev:
>+ if (idx == BNXT_AUXDEV_RDMA)
>+ bp->flags &= ~BNXT_FLAG_ROCE_CAP;
> }
>- bp->aux_priv = aux_priv;
>-
>- /* From this point, all cleanup will happen via the .release callback &
>- * any error unwinding will need to include a call to
>- * auxiliary_device_uninit.
>- */
>- edev = kzalloc(sizeof(*edev), GFP_KERNEL);
>- if (!edev)
>- goto aux_dev_uninit;
>-
>- aux_priv->edev = edev;
>-
>- ulp = kzalloc(sizeof(*ulp), GFP_KERNEL);
>- if (!ulp)
>- goto aux_dev_uninit;
>+ mutex_unlock(&bp->auxdev_lock);
>+}
>
>- edev->ulp_tbl = ulp;
>- bp->edev = edev;
>- bnxt_set_edev_info(edev, bp);
>- bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
>+int bnxt_auxdev_id_alloc(struct bnxt *bp)
>+{
>+ bp->auxdev_id = ida_alloc(&bnxt_aux_dev_ids, GFP_KERNEL);
>+ if (bp->auxdev_id < 0)
>+ return bp->auxdev_id;
>
>- return;
>+ return 0;
>+}
>
>-aux_dev_uninit:
>- auxiliary_device_uninit(aux_dev);
>-exit:
>- bp->flags &= ~BNXT_FLAG_ROCE_CAP;
>+void bnxt_auxdev_id_free(struct bnxt *bp, int id)
>+{
>+ if (bp->auxdev_id >= 0)
>+ ida_free(&bnxt_aux_dev_ids, id);
> }
>diff --git a/include/linux/bnxt/ulp.h b/include/linux/bnxt/ulp.h
>index 3c5b8a53f715..1a4643c46f86 100644
>--- a/include/linux/bnxt/ulp.h
>+++ b/include/linux/bnxt/ulp.h
>@@ -10,6 +10,8 @@
> #ifndef BNXT_ULP_H
> #define BNXT_ULP_H
>
>+#include <linux/auxiliary_bus.h>
>+
> #define BNXT_MIN_ROCE_CP_RINGS 2
> #define BNXT_MIN_ROCE_STAT_CTXS 1
>
>@@ -20,6 +22,17 @@
> struct hwrm_async_event_cmpl;
> struct bnxt;
>
>+enum bnxt_auxdev_type {
>+ BNXT_AUXDEV_RDMA = 0,
>+ __BNXT_AUXDEV_MAX
>+};
>+
>+struct bnxt_aux_priv {
>+ struct auxiliary_device aux_dev;
>+ struct bnxt_en_dev *edev;
>+ int id;
>+};
>+
> struct bnxt_msix_entry {
> u32 vector;
> u32 ring_idx;
>@@ -110,19 +123,21 @@ void bnxt_set_ulp_stat_ctxs(struct bnxt *bp, int num_ctxs);
> int bnxt_get_ulp_stat_ctxs_in_use(struct bnxt *bp);
> void bnxt_set_dflt_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_start(struct bnxt *bp);
> void bnxt_ulp_sriov_cfg(struct bnxt *bp, int num_vfs);
> 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_rdma_aux_device_uninit(struct bnxt *bp);
>-void bnxt_rdma_aux_device_del(struct bnxt *bp);
>-void bnxt_rdma_aux_device_add(struct bnxt *bp);
>-void bnxt_rdma_aux_device_init(struct bnxt *bp);
>+void bnxt_aux_devices_uninit(struct bnxt *bp);
>+void bnxt_aux_devices_del(struct bnxt *bp);
>+void bnxt_aux_devices_add(struct bnxt *bp);
>+void bnxt_aux_devices_init(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_send_msg(struct bnxt_en_dev *edev, struct bnxt_fw_msg *fw_msg);
> void bnxt_register_async_events(struct bnxt_en_dev *edev,
> unsigned long *events_bmap, u16 max_id);
>+int bnxt_auxdev_id_alloc(struct bnxt *bp);
>+void bnxt_auxdev_id_free(struct bnxt *bp, int id);
> #endif
>--
>2.39.1
>
Powered by blists - more mailing lists