netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/5]  bnxt_fwctl: fwctl for Broadcom Netxtreme devices
@ 2025-09-27  9:39 Pavan Chebbi
  2025-09-27  9:39 ` [PATCH net-next v4 1/5] bnxt_en: Move common definitions to include/linux/bnxt/ Pavan Chebbi
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Pavan Chebbi @ 2025-09-27  9:39 UTC (permalink / raw)
  To: jgg, michael.chan
  Cc: dave.jiang, saeedm, Jonathan.Cameron, davem, corbet, edumazet,
	gospo, kuba, netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil, Pavan Chebbi

Introducing bnxt_fwctl which follows along Jason's work [1].
It is an aux bus driver that enables fwctl for Broadcom
NetXtreme 574xx, 575xx and 576xx series chipsets by using
bnxt driver's capability to talk to devices' firmware.

The first patch moves the ULP definitions to a common place
inside include/linux/bnxt/. The second and third patches
refactor and extend the existing bnxt aux bus functions to
be able to add more than one auxiliary device. The last three
patches create an additional bnxt aux device, add bnxt_fwctl,
and the documentation.

[1] https://lore.kernel.org/netdev/0-v5-642aa0c94070+4447f-fwctl_jgg@nvidia.com/

v4: In patch #4, added the missing kfree on error for response
buffer. Improved documentation in patch #5 based on comments
from Dave.

v3: Addressed the review comments as below
Patch #1: Removed redundant common.h [thanks Saeed]
Patch #2 and #3 merged into a single patch [thanks Jonathan]
Patch #3: Addressed comments from Jonathan
Patch #4 and #5: Addressed comments from Jonathan and Dave

v2: In patch #5, fixed a sparse warning where a __le16 was
degraded to an integer. Also addressed kdoc warnings for
include/uapi/fwctl/bnxt.h in the same patch.

v1: https://lore.kernel.org/netdev/20250922090851.719913-1-pavan.chebbi@broadcom.com/

The following are changes since commit fec734e8d564d55fb6bd4909ae2e68814d21d0a1:
  Merge tag 'riscv-for-linus-v6.17-rc8' of git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux
and are available in the git repository at:
  https://github.com/pavanchebbi/linux/tree/bnxt_fwctl_v4

Pavan Chebbi (5):
  bnxt_en: Move common definitions to include/linux/bnxt/
  bnxt_en: Refactor aux bus functions to be more generic
  bnxt_en: Create an aux device for fwctl
  bnxt_fwctl: Add bnxt fwctl device
  bnxt_fwctl: Add documentation entries

 .../userspace-api/fwctl/bnxt_fwctl.rst        |  78 +++
 Documentation/userspace-api/fwctl/fwctl.rst   |   1 +
 Documentation/userspace-api/fwctl/index.rst   |   1 +
 MAINTAINERS                                   |   6 +
 drivers/fwctl/Kconfig                         |  11 +
 drivers/fwctl/Makefile                        |   1 +
 drivers/fwctl/bnxt/Makefile                   |   4 +
 drivers/fwctl/bnxt/main.c                     | 454 ++++++++++++++++++
 drivers/infiniband/hw/bnxt_re/debugfs.c       |   2 +-
 drivers/infiniband/hw/bnxt_re/main.c          |   2 +-
 drivers/infiniband/hw/bnxt_re/qplib_fp.c      |   2 +-
 drivers/infiniband/hw/bnxt_re/qplib_res.h     |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  30 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  12 +-
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |   2 +-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |   4 +-
 .../net/ethernet/broadcom/bnxt/bnxt_sriov.c   |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 245 +++++++---
 .../bnxt_ulp.h => include/linux/bnxt/ulp.h    |  22 +-
 include/uapi/fwctl/bnxt.h                     |  64 +++
 include/uapi/fwctl/fwctl.h                    |   1 +
 21 files changed, 858 insertions(+), 88 deletions(-)
 create mode 100644 Documentation/userspace-api/fwctl/bnxt_fwctl.rst
 create mode 100644 drivers/fwctl/bnxt/Makefile
 create mode 100644 drivers/fwctl/bnxt/main.c
 rename drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h => include/linux/bnxt/ulp.h (87%)
 create mode 100644 include/uapi/fwctl/bnxt.h

-- 
2.39.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH net-next v4 1/5] bnxt_en: Move common definitions to include/linux/bnxt/
  2025-09-27  9:39 [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
@ 2025-09-27  9:39 ` Pavan Chebbi
  2025-09-27  9:39 ` [PATCH net-next v4 2/5] bnxt_en: Refactor aux bus functions to be more generic Pavan Chebbi
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Pavan Chebbi @ 2025-09-27  9:39 UTC (permalink / raw)
  To: jgg, michael.chan
  Cc: dave.jiang, saeedm, Jonathan.Cameron, davem, corbet, edumazet,
	gospo, kuba, netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil, Pavan Chebbi, linux-rdma

We have common definitions that are now going to be used
by more than one component outside of bnxt (bnxt_re and
fwctl)

Move bnxt_ulp.h to include/linux/bnxt/ as ulp.h.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/debugfs.c                         | 2 +-
 drivers/infiniband/hw/bnxt_re/main.c                            | 2 +-
 drivers/infiniband/hw/bnxt_re/qplib_fp.c                        | 2 +-
 drivers/infiniband/hw/bnxt_re/qplib_res.h                       | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c                       | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c               | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c               | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c                 | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c                   | 2 +-
 .../broadcom/bnxt/bnxt_ulp.h => include/linux/bnxt/ulp.h        | 0
 10 files changed, 9 insertions(+), 9 deletions(-)
 rename drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h => include/linux/bnxt/ulp.h (100%)

diff --git a/drivers/infiniband/hw/bnxt_re/debugfs.c b/drivers/infiniband/hw/bnxt_re/debugfs.c
index e632f1661b92..a9dd3597cfbc 100644
--- a/drivers/infiniband/hw/bnxt_re/debugfs.c
+++ b/drivers/infiniband/hw/bnxt_re/debugfs.c
@@ -9,8 +9,8 @@
 #include <linux/debugfs.h>
 #include <linux/pci.h>
 #include <rdma/ib_addr.h>
+#include <linux/bnxt/ulp.h>
 
-#include "bnxt_ulp.h"
 #include "roce_hsi.h"
 #include "qplib_res.h"
 #include "qplib_sp.h"
diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index df7cf8d68e27..b773556fc5e9 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -55,8 +55,8 @@
 #include <rdma/ib_umem.h>
 #include <rdma/ib_addr.h>
 #include <linux/hashtable.h>
+#include <linux/bnxt/ulp.h>
 
-#include "bnxt_ulp.h"
 #include "roce_hsi.h"
 #include "qplib_res.h"
 #include "qplib_sp.h"
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_fp.c b/drivers/infiniband/hw/bnxt_re/qplib_fp.c
index ee36b3d82cc0..bb252cd8509b 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_fp.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_fp.c
@@ -46,6 +46,7 @@
 #include <linux/delay.h>
 #include <linux/prefetch.h>
 #include <linux/if_ether.h>
+#include <linux/bnxt/ulp.h>
 #include <rdma/ib_mad.h>
 
 #include "roce_hsi.h"
@@ -55,7 +56,6 @@
 #include "qplib_sp.h"
 #include "qplib_fp.h"
 #include <rdma/ib_addr.h>
-#include "bnxt_ulp.h"
 #include "bnxt_re.h"
 #include "ib_verbs.h"
 
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_res.h b/drivers/infiniband/hw/bnxt_re/qplib_res.h
index 6a13927674b4..7cdddf921b48 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_res.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_res.h
@@ -39,7 +39,7 @@
 #ifndef __BNXT_QPLIB_RES_H__
 #define __BNXT_QPLIB_RES_H__
 
-#include "bnxt_ulp.h"
+#include <linux/bnxt/ulp.h>
 
 extern const struct bnxt_qplib_gid bnxt_qplib_gid_zero;
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 1d0e0e7362bd..785a6146b968 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -59,10 +59,10 @@
 #include <net/netdev_rx_queue.h>
 #include <linux/pci-tph.h>
 #include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
 
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
-#include "bnxt_ulp.h"
 #include "bnxt_sriov.h"
 #include "bnxt_ethtool.h"
 #include "bnxt_dcb.h"
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 02961d93ed35..cfcd3335a2d3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -13,12 +13,12 @@
 #include <net/devlink.h>
 #include <net/netdev_lock.h>
 #include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
 #include "bnxt_vfr.h"
 #include "bnxt_devlink.h"
 #include "bnxt_ethtool.h"
-#include "bnxt_ulp.h"
 #include "bnxt_ptp.h"
 #include "bnxt_coredump.h"
 #include "bnxt_nvm_defs.h"
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 41686a6f84b5..818bd0fa0a7d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -27,9 +27,9 @@
 #include <net/netdev_queues.h>
 #include <net/netlink.h>
 #include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
-#include "bnxt_ulp.h"
 #include "bnxt_xdp.h"
 #include "bnxt_ptp.h"
 #include "bnxt_ethtool.h"
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
index 80fed2c07b9e..84c43f83193a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
@@ -17,9 +17,9 @@
 #include <linux/etherdevice.h>
 #include <net/dcbnl.h>
 #include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
-#include "bnxt_ulp.h"
 #include "bnxt_sriov.h"
 #include "bnxt_vfr.h"
 #include "bnxt_ethtool.h"
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 61cf201bb0dc..992eec874345 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -22,10 +22,10 @@
 #include <linux/auxiliary_bus.h>
 #include <net/netdev_lock.h>
 #include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
 
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
-#include "bnxt_ulp.h"
 
 static DEFINE_IDA(bnxt_aux_dev_ids);
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/include/linux/bnxt/ulp.h
similarity index 100%
rename from drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
rename to include/linux/bnxt/ulp.h
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v4 2/5] bnxt_en: Refactor aux bus functions to be more generic
  2025-09-27  9:39 [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
  2025-09-27  9:39 ` [PATCH net-next v4 1/5] bnxt_en: Move common definitions to include/linux/bnxt/ Pavan Chebbi
@ 2025-09-27  9:39 ` Pavan Chebbi
  2025-09-27  9:39 ` [PATCH net-next v4 3/5] bnxt_en: Create an aux device for fwctl Pavan Chebbi
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Pavan Chebbi @ 2025-09-27  9:39 UTC (permalink / raw)
  To: jgg, michael.chan
  Cc: dave.jiang, saeedm, Jonathan.Cameron, davem, corbet, edumazet,
	gospo, kuba, netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil, Pavan Chebbi

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.

We could maintain a look up table of supported aux bus devices
so that we can fetch the aux device info by calling the table's
functions. This way, the aux bus init/add/uninit/del could have
generic code to work on any of bnxt's aux devices.

Make aux bus init/uninit/add/del functions more generic which
will accept aux device type as a parameter. Change the existing
'aux_dev_ids' to 'aux_dev_rdma_ids' to mean it is for RoCE driver.

Also rename the 'aux_priv' and 'edev' members of struct bp to
'aux_priv_rdma' and 'edev_rdma' respectively, to mean they belong
to rdma.
Rename bnxt_aux_device_release() as bnxt_rdma_aux_device_release()

Future patches will reuse these functions to add an aux bus device
for fwctl.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  23 +--
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   4 +-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 180 +++++++++++++-----
 include/linux/bnxt/ulp.h                      |  13 +-
 5 files changed, 152 insertions(+), 70 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 785a6146b968..bd567f776fe8 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6848,7 +6848,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_rdma))
 		req->flags |= cpu_to_le32(bnxt_get_roce_vnic_mode(bp));
 
 	return hwrm_req_send(bp, req);
@@ -7973,7 +7974,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(bp->edev_rdma)) {
 		ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
 		if (!ulp_msix)
 			bnxt_set_ulp_stat_ctxs(bp, 0);
@@ -8024,7 +8025,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) &&
+	if (bnxt_ulp_registered(bp->edev_rdma) &&
 	    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);
@@ -8064,7 +8065,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(bp->edev_rdma) && BNXT_NEW_RM(bp)) {
 		int resv_msix, resv_ctx, ulp_ctxs;
 		struct bnxt_hw_resc *hw_resc;
 
@@ -11419,7 +11420,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(bp->edev_rdma)) {
 		int ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
 
 		if (ulp_msix > bp->ulp_num_msix_want)
@@ -14651,7 +14652,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_rdma)) {
 			hwr.cp += bnxt_get_ulp_msix_num(bp);
 			hwr.cp = min_t(int, hwr.cp, bnxt_get_max_func_irqs(bp));
 		}
@@ -16183,12 +16184,12 @@ 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_device_del(bp, BNXT_AUXDEV_RDMA);
 
 	unregister_netdev(dev);
 	bnxt_ptp_clear(bp);
 
-	bnxt_rdma_aux_device_uninit(bp);
+	bnxt_aux_device_uninit(bp, BNXT_AUXDEV_RDMA);
 
 	bnxt_free_l2_filters(bp, true);
 	bnxt_free_ntp_fltrs(bp, true);
@@ -16774,7 +16775,7 @@ 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);
+	bnxt_aux_device_init(bp, BNXT_AUXDEV_RDMA);
 	rc = bnxt_set_dflt_rings(bp, true);
 	if (rc) {
 		if (BNXT_VF(bp) && rc == -ENODEV) {
@@ -16838,7 +16839,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_device_add(bp, BNXT_AUXDEV_RDMA);
 
 	bnxt_print_device_info(bp);
 
@@ -16846,7 +16847,7 @@ 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_device_uninit(bp, BNXT_AUXDEV_RDMA);
 	bnxt_dl_unregister(bp);
 init_err_dl:
 	bnxt_shutdown_tc(bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 06a4c2afdf8a..b3cba97bb9ea 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2340,8 +2340,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_rdma;
+	struct bnxt_en_dev	*edev_rdma;
 
 	struct bnxt_napi	**bnapi;
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 818bd0fa0a7d..0c1f7450ea55 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -5086,7 +5086,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_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 992eec874345..8009964da698 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -27,11 +27,64 @@
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
 
-static DEFINE_IDA(bnxt_aux_dev_ids);
+static DEFINE_IDA(bnxt_rdma_aux_dev_ids);
+
+struct bnxt_aux_device {
+	const char *name;
+	const u32 type;
+	struct ida *ida;
+	void (*release)(struct device *dev);
+	void (*set_priv)(struct bnxt *bp, struct bnxt_aux_priv *priv);
+	struct bnxt_aux_priv *(*get_priv)(struct bnxt *bp);
+	void (*set_edev)(struct bnxt *bp, struct bnxt_en_dev *edev);
+	struct bnxt_en_dev *(*get_edev)(struct bnxt *bp);
+	struct auxiliary_device *(*get_auxdev)(struct bnxt *bp);
+};
+
+static void bnxt_rdma_aux_dev_release(struct device *dev);
+
+static void bnxt_rdma_aux_dev_set_priv(struct bnxt *bp,
+				       struct bnxt_aux_priv *priv)
+{
+	bp->aux_priv_rdma = priv;
+}
+
+static struct bnxt_aux_priv *bnxt_rdma_aux_dev_get_priv(struct bnxt *bp)
+{
+	return bp->aux_priv_rdma;
+}
+
+static struct auxiliary_device *bnxt_rdma_aux_dev_get_auxdev(struct bnxt *bp)
+{
+	return &bp->aux_priv_rdma->aux_dev;
+}
+
+static void bnxt_rdma_aux_dev_set_edev(struct bnxt *bp,
+				       struct bnxt_en_dev *edev)
+{
+	bp->edev_rdma = edev;
+}
+
+static struct bnxt_en_dev *bnxt_rdma_aux_dev_get_edev(struct bnxt *bp)
+{
+	return bp->edev_rdma;
+}
+
+static struct bnxt_aux_device bnxt_aux_devices[__BNXT_AUXDEV_MAX] = {{
+	.name		= "rdma",
+	.type		= BNXT_AUXDEV_RDMA,
+	.ida		= &bnxt_rdma_aux_dev_ids,
+	.release	= bnxt_rdma_aux_dev_release,
+	.set_priv       = bnxt_rdma_aux_dev_set_priv,
+	.get_priv	= bnxt_rdma_aux_dev_get_priv,
+	.set_edev       = bnxt_rdma_aux_dev_set_edev,
+	.get_edev	= bnxt_rdma_aux_dev_get_edev,
+	.get_auxdev	= bnxt_rdma_aux_dev_get_auxdev,
+}};
 
 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_rdma;
 	int num_msix, i;
 
 	if (!edev->ulp_tbl->msix_requested) {
@@ -51,55 +104,55 @@ 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;
+	if (bp->edev_rdma)
+		return bp->edev_rdma->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;
+	if (bp->edev_rdma)
+		bp->edev_rdma->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;
+	if (bnxt_ulp_registered(bp->edev_rdma))
+		return bp->edev_rdma->ulp_num_msix_vec;
 	return 0;
 }
 
 int bnxt_get_ulp_stat_ctxs(struct bnxt *bp)
 {
-	if (bp->edev)
-		return bp->edev->ulp_num_ctxs;
+	if (bp->edev_rdma)
+		return bp->edev_rdma->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;
+	if (bp->edev_rdma)
+		bp->edev_rdma->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;
+	if (bnxt_ulp_registered(bp->edev_rdma))
+		return bp->edev_rdma->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;
+	if (bp->edev_rdma) {
+		bp->edev_rdma->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++;
+			bp->edev_rdma->ulp_num_ctxs++;
 	}
 }
 
@@ -135,7 +188,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, bp->edev_rdma->msix_entries);
 	edev->flags |= BNXT_EN_FLAG_MSIX_REQUESTED;
 exit:
 	mutex_unlock(&edev->en_dev_lock);
@@ -224,8 +277,8 @@ 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;
+	struct bnxt_aux_priv *aux_priv = bp->aux_priv_rdma;
+	struct bnxt_en_dev *edev = bp->edev_rdma;
 
 	if (!edev)
 		return;
@@ -255,8 +308,8 @@ void bnxt_ulp_stop(struct bnxt *bp)
 
 void bnxt_ulp_start(struct bnxt *bp, int err)
 {
-	struct bnxt_aux_priv *aux_priv = bp->aux_priv;
-	struct bnxt_en_dev *edev = bp->edev;
+	struct bnxt_aux_priv *aux_priv = bp->aux_priv_rdma;
+	struct bnxt_en_dev *edev = bp->edev_rdma;
 
 	if (!edev || err)
 		return;
@@ -288,14 +341,14 @@ void bnxt_ulp_start(struct bnxt *bp, int err)
 
 void bnxt_ulp_irq_stop(struct bnxt *bp)
 {
-	struct bnxt_en_dev *edev = bp->edev;
+	struct bnxt_en_dev *edev = bp->edev_rdma;
 	struct bnxt_ulp_ops *ops;
 	bool reset = false;
 
 	if (!edev || !(edev->flags & BNXT_EN_FLAG_MSIX_REQUESTED))
 		return;
 
-	if (bnxt_ulp_registered(bp->edev)) {
+	if (bnxt_ulp_registered(bp->edev_rdma)) {
 		struct bnxt_ulp *ulp = edev->ulp_tbl;
 
 		if (!ulp->msix_requested)
@@ -312,13 +365,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_rdma;
 	struct bnxt_ulp_ops *ops;
 
 	if (!edev || !(edev->flags & BNXT_EN_FLAG_MSIX_REQUESTED))
 		return;
 
-	if (bnxt_ulp_registered(bp->edev)) {
+	if (bnxt_ulp_registered(bp->edev_rdma)) {
 		struct bnxt_ulp *ulp = edev->ulp_tbl;
 		struct bnxt_msix_entry *ent = NULL;
 
@@ -344,7 +397,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_rdma;
 	struct bnxt_ulp_ops *ops;
 	struct bnxt_ulp *ulp;
 
@@ -385,40 +438,49 @@ 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_device_uninit(struct bnxt *bp,
+			    enum bnxt_ulp_auxdev_type auxdev_type)
 {
 	struct bnxt_aux_priv *aux_priv;
 	struct auxiliary_device *adev;
 
+	if (auxdev_type >= __BNXT_AUXDEV_MAX) {
+		netdev_warn(bp->dev, "Failed to uninit: unrecognized auxiliary device\n");
+		return;
+	}
 	/* Skip if no auxiliary device init was done. */
-	if (!bp->aux_priv)
+	if (!bnxt_aux_devices[auxdev_type].get_priv(bp))
 		return;
 
-	aux_priv = bp->aux_priv;
+	aux_priv = bnxt_aux_devices[auxdev_type].get_priv(bp);
 	adev = &aux_priv->aux_dev;
 	auxiliary_device_uninit(adev);
 }
 
-static void bnxt_aux_dev_release(struct device *dev)
+static void bnxt_rdma_aux_dev_release(struct device *dev)
 {
 	struct bnxt_aux_priv *aux_priv =
 		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);
+	ida_free(&bnxt_rdma_aux_dev_ids, aux_priv->id);
 	kfree(aux_priv->edev->ulp_tbl);
-	bp->edev = NULL;
+	bp->edev_rdma = NULL;
 	kfree(aux_priv->edev);
 	kfree(aux_priv);
-	bp->aux_priv = NULL;
+	bp->aux_priv_rdma = NULL;
 }
 
-void bnxt_rdma_aux_device_del(struct bnxt *bp)
+void bnxt_aux_device_del(struct bnxt *bp, enum bnxt_ulp_auxdev_type auxdev_type)
 {
-	if (!bp->edev)
+	if (auxdev_type >= __BNXT_AUXDEV_MAX) {
+		netdev_warn(bp->dev, "Failed to del: unrecognized auxiliary device\n");
+		return;
+	}
+	if (!bnxt_aux_devices[auxdev_type].get_edev(bp))
 		return;
 
-	auxiliary_device_delete(&bp->aux_priv->aux_dev);
+	auxiliary_device_delete(bnxt_aux_devices[auxdev_type].get_auxdev(bp));
 }
 
 static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
@@ -448,24 +510,30 @@ 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_device_add(struct bnxt *bp, enum bnxt_ulp_auxdev_type auxdev_type)
 {
 	struct auxiliary_device *aux_dev;
 	int rc;
 
-	if (!bp->edev)
+	if (auxdev_type >= __BNXT_AUXDEV_MAX) {
+		netdev_warn(bp->dev, "Failed to add: unrecognized auxiliary device\n");
+		return;
+	}
+	if (!bnxt_aux_devices[auxdev_type].get_edev(bp))
 		return;
 
-	aux_dev = &bp->aux_priv->aux_dev;
+	aux_dev = bnxt_aux_devices[auxdev_type].get_auxdev(bp);
 	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;
+		if (bnxt_aux_devices[auxdev_type].type == BNXT_AUXDEV_RDMA)
+			bp->flags &= ~BNXT_FLAG_ROCE_CAP;
 	}
 }
 
-void bnxt_rdma_aux_device_init(struct bnxt *bp)
+void bnxt_aux_device_init(struct bnxt *bp,
+			  enum bnxt_ulp_auxdev_type auxdev_type)
 {
 	struct auxiliary_device *aux_dev;
 	struct bnxt_aux_priv *aux_priv;
@@ -473,14 +541,20 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp)
 	struct bnxt_ulp *ulp;
 	int rc;
 
-	if (!(bp->flags & BNXT_FLAG_ROCE_CAP))
+	if (auxdev_type >= __BNXT_AUXDEV_MAX) {
+		netdev_warn(bp->dev, "Failed to init: unrecognized auxiliary device\n");
+		return;
+	}
+
+	if (bnxt_aux_devices[auxdev_type].type == BNXT_AUXDEV_RDMA &&
+	    !(bp->flags & BNXT_FLAG_ROCE_CAP))
 		return;
 
-	aux_priv = kzalloc(sizeof(*bp->aux_priv), GFP_KERNEL);
+	aux_priv = kzalloc(sizeof(*bp->aux_priv_rdma), GFP_KERNEL);
 	if (!aux_priv)
 		goto exit;
 
-	aux_priv->id = ida_alloc(&bnxt_aux_dev_ids, GFP_KERNEL);
+	aux_priv->id = ida_alloc(bnxt_aux_devices[auxdev_type].ida, GFP_KERNEL);
 	if (aux_priv->id < 0) {
 		netdev_warn(bp->dev,
 			    "ida alloc failed for ROCE auxiliary device\n");
@@ -490,17 +564,17 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp)
 
 	aux_dev = &aux_priv->aux_dev;
 	aux_dev->id = aux_priv->id;
-	aux_dev->name = "rdma";
+	aux_dev->name = bnxt_aux_devices[auxdev_type].name;
 	aux_dev->dev.parent = &bp->pdev->dev;
-	aux_dev->dev.release = bnxt_aux_dev_release;
+	aux_dev->dev.release = bnxt_aux_devices[auxdev_type].release;
 
 	rc = auxiliary_device_init(aux_dev);
 	if (rc) {
-		ida_free(&bnxt_aux_dev_ids, aux_priv->id);
+		ida_free(bnxt_aux_devices[auxdev_type].ida, aux_priv->id);
 		kfree(aux_priv);
 		goto exit;
 	}
-	bp->aux_priv = aux_priv;
+	bnxt_aux_devices[auxdev_type].set_priv(bp, aux_priv);
 
 	/* From this point, all cleanup will happen via the .release callback &
 	 * any error unwinding will need to include a call to
@@ -517,14 +591,16 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp)
 		goto aux_dev_uninit;
 
 	edev->ulp_tbl = ulp;
-	bp->edev = edev;
+	bnxt_aux_devices[auxdev_type].set_edev(bp, edev);
 	bnxt_set_edev_info(edev, bp);
-	bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
+	if (bnxt_aux_devices[auxdev_type].type == BNXT_AUXDEV_RDMA)
+		bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
 
 	return;
 
 aux_dev_uninit:
 	auxiliary_device_uninit(aux_dev);
 exit:
-	bp->flags &= ~BNXT_FLAG_ROCE_CAP;
+	if (bnxt_aux_devices[auxdev_type].type == BNXT_AUXDEV_RDMA)
+		bp->flags &= ~BNXT_FLAG_ROCE_CAP;
 }
diff --git a/include/linux/bnxt/ulp.h b/include/linux/bnxt/ulp.h
index 7b9dd8ebe4bc..01b7100dcf4d 100644
--- a/include/linux/bnxt/ulp.h
+++ b/include/linux/bnxt/ulp.h
@@ -20,6 +20,11 @@
 struct hwrm_async_event_cmpl;
 struct bnxt;
 
+enum bnxt_ulp_auxdev_type {
+	BNXT_AUXDEV_RDMA = 0,
+	__BNXT_AUXDEV_MAX
+};
+
 struct bnxt_msix_entry {
 	u32	vector;
 	u32	ring_idx;
@@ -116,10 +121,10 @@ 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_device_uninit(struct bnxt *bp, enum bnxt_ulp_auxdev_type type);
+void bnxt_aux_device_init(struct bnxt *bp, enum bnxt_ulp_auxdev_type type);
+void bnxt_aux_device_add(struct bnxt *bp, enum bnxt_ulp_auxdev_type type);
+void bnxt_aux_device_del(struct bnxt *bp, enum bnxt_ulp_auxdev_type type);
 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);
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v4 3/5] bnxt_en: Create an aux device for fwctl
  2025-09-27  9:39 [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
  2025-09-27  9:39 ` [PATCH net-next v4 1/5] bnxt_en: Move common definitions to include/linux/bnxt/ Pavan Chebbi
  2025-09-27  9:39 ` [PATCH net-next v4 2/5] bnxt_en: Refactor aux bus functions to be more generic Pavan Chebbi
@ 2025-09-27  9:39 ` Pavan Chebbi
  2025-09-27  9:39 ` [PATCH net-next v4 4/5] bnxt_fwctl: Add bnxt fwctl device Pavan Chebbi
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Pavan Chebbi @ 2025-09-27  9:39 UTC (permalink / raw)
  To: jgg, michael.chan
  Cc: dave.jiang, saeedm, Jonathan.Cameron, davem, corbet, edumazet,
	gospo, kuba, netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil, Pavan Chebbi

Create an additional auxiliary device to support fwctl.
The next patch will create bnxt_fwctl and bind to this
device.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  5 ++
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  2 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 63 ++++++++++++++++++-
 include/linux/bnxt/ulp.h                      |  1 +
 4 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index bd567f776fe8..82301fc4f53b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -16185,11 +16185,13 @@ static void bnxt_remove_one(struct pci_dev *pdev)
 		__bnxt_sriov_disable(bp);
 
 	bnxt_aux_device_del(bp, BNXT_AUXDEV_RDMA);
+	bnxt_aux_device_del(bp, BNXT_AUXDEV_FWCTL);
 
 	unregister_netdev(dev);
 	bnxt_ptp_clear(bp);
 
 	bnxt_aux_device_uninit(bp, BNXT_AUXDEV_RDMA);
+	bnxt_aux_device_uninit(bp, BNXT_AUXDEV_FWCTL);
 
 	bnxt_free_l2_filters(bp, true);
 	bnxt_free_ntp_fltrs(bp, true);
@@ -16776,6 +16778,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	bnxt_init_ring_params(bp);
 	bnxt_set_ring_params(bp);
 	bnxt_aux_device_init(bp, BNXT_AUXDEV_RDMA);
+	bnxt_aux_device_init(bp, BNXT_AUXDEV_FWCTL);
 	rc = bnxt_set_dflt_rings(bp, true);
 	if (rc) {
 		if (BNXT_VF(bp) && rc == -ENODEV) {
@@ -16840,6 +16843,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	bnxt_dl_fw_reporters_create(bp);
 
 	bnxt_aux_device_add(bp, BNXT_AUXDEV_RDMA);
+	bnxt_aux_device_add(bp, BNXT_AUXDEV_FWCTL);
 
 	bnxt_print_device_info(bp);
 
@@ -16848,6 +16852,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return 0;
 init_err_cleanup:
 	bnxt_aux_device_uninit(bp, BNXT_AUXDEV_RDMA);
+	bnxt_aux_device_uninit(bp, BNXT_AUXDEV_FWCTL);
 	bnxt_dl_unregister(bp);
 init_err_dl:
 	bnxt_shutdown_tc(bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index b3cba97bb9ea..ea1d10c50da6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2345,6 +2345,8 @@ struct bnxt {
 
 	struct bnxt_napi	**bnapi;
 
+	struct bnxt_en_dev	*edev_fwctl;
+	struct bnxt_aux_priv	*aux_priv_fwctl;
 	struct bnxt_rx_ring_info	*rx_ring;
 	struct bnxt_tx_ring_info	*tx_ring;
 	u16			*tx_ring_map;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 8009964da698..7fe8848ac9fc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -28,6 +28,7 @@
 #include "bnxt_hwrm.h"
 
 static DEFINE_IDA(bnxt_rdma_aux_dev_ids);
+static DEFINE_IDA(bnxt_fwctl_aux_dev_ids);
 
 struct bnxt_aux_device {
 	const char *name;
@@ -42,6 +43,7 @@ struct bnxt_aux_device {
 };
 
 static void bnxt_rdma_aux_dev_release(struct device *dev);
+static void bnxt_fwctl_aux_dev_release(struct device *dev);
 
 static void bnxt_rdma_aux_dev_set_priv(struct bnxt *bp,
 				       struct bnxt_aux_priv *priv)
@@ -70,6 +72,33 @@ static struct bnxt_en_dev *bnxt_rdma_aux_dev_get_edev(struct bnxt *bp)
 	return bp->edev_rdma;
 }
 
+static void bnxt_fwctl_aux_dev_set_priv(struct bnxt *bp,
+					struct bnxt_aux_priv *priv)
+{
+	bp->aux_priv_fwctl = priv;
+}
+
+static struct bnxt_aux_priv *bnxt_fwctl_aux_dev_get_priv(struct bnxt *bp)
+{
+	return bp->aux_priv_fwctl;
+}
+
+static struct auxiliary_device *bnxt_fwctl_aux_dev_get_auxdev(struct bnxt *bp)
+{
+	return &bp->aux_priv_fwctl->aux_dev;
+}
+
+static void bnxt_fwctl_aux_dev_set_edev(struct bnxt *bp,
+					struct bnxt_en_dev *edev)
+{
+	bp->edev_fwctl = edev;
+}
+
+static struct bnxt_en_dev *bnxt_fwctl_aux_dev_get_edev(struct bnxt *bp)
+{
+	return bp->edev_fwctl;
+}
+
 static struct bnxt_aux_device bnxt_aux_devices[__BNXT_AUXDEV_MAX] = {{
 	.name		= "rdma",
 	.type		= BNXT_AUXDEV_RDMA,
@@ -80,6 +109,16 @@ static struct bnxt_aux_device bnxt_aux_devices[__BNXT_AUXDEV_MAX] = {{
 	.set_edev       = bnxt_rdma_aux_dev_set_edev,
 	.get_edev	= bnxt_rdma_aux_dev_get_edev,
 	.get_auxdev	= bnxt_rdma_aux_dev_get_auxdev,
+}, {
+	.name		= "fwctl",
+	.type		= BNXT_AUXDEV_FWCTL,
+	.ida		= &bnxt_fwctl_aux_dev_ids,
+	.release	= bnxt_fwctl_aux_dev_release,
+	.set_priv       = bnxt_fwctl_aux_dev_set_priv,
+	.get_priv	= bnxt_fwctl_aux_dev_get_priv,
+	.set_edev       = bnxt_fwctl_aux_dev_set_edev,
+	.get_edev	= bnxt_fwctl_aux_dev_get_edev,
+	.get_auxdev	= bnxt_fwctl_aux_dev_get_auxdev,
 }};
 
 static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
@@ -303,6 +342,8 @@ void bnxt_ulp_stop(struct bnxt *bp)
 		}
 	}
 ulp_stop_exit:
+	if (bp->edev_fwctl)
+		bp->edev_fwctl->flags |= BNXT_EN_FLAG_ULP_STOPPED;
 	mutex_unlock(&edev->en_dev_lock);
 }
 
@@ -336,6 +377,8 @@ void bnxt_ulp_start(struct bnxt *bp, int err)
 	}
 ulp_start_exit:
 	edev->flags &= ~BNXT_EN_FLAG_ULP_STOPPED;
+	if (bp->edev_fwctl)
+		bp->edev_fwctl->flags &= ~BNXT_EN_FLAG_ULP_STOPPED;
 	mutex_unlock(&edev->en_dev_lock);
 }
 
@@ -525,13 +568,27 @@ void bnxt_aux_device_add(struct bnxt *bp, enum bnxt_ulp_auxdev_type auxdev_type)
 	aux_dev = bnxt_aux_devices[auxdev_type].get_auxdev(bp);
 	rc = auxiliary_device_add(aux_dev);
 	if (rc) {
-		netdev_warn(bp->dev, "Failed to add auxiliary device for ROCE\n");
+		netdev_warn(bp->dev, "Failed to add auxiliary device for auxdev type %d\n",
+			    auxdev_type);
 		auxiliary_device_uninit(aux_dev);
 		if (bnxt_aux_devices[auxdev_type].type == BNXT_AUXDEV_RDMA)
 			bp->flags &= ~BNXT_FLAG_ROCE_CAP;
 	}
 }
 
+static void bnxt_fwctl_aux_dev_release(struct device *dev)
+{
+	struct bnxt_aux_priv *aux_priv =
+		container_of(dev, struct bnxt_aux_priv, aux_dev.dev);
+	struct bnxt *bp = netdev_priv(aux_priv->edev->net);
+
+	ida_free(&bnxt_fwctl_aux_dev_ids, aux_priv->id);
+	kfree(aux_priv->edev);
+	bp->edev_fwctl = NULL;
+	kfree(bp->aux_priv_fwctl);
+	bp->aux_priv_fwctl = NULL;
+}
+
 void bnxt_aux_device_init(struct bnxt *bp,
 			  enum bnxt_ulp_auxdev_type auxdev_type)
 {
@@ -556,8 +613,8 @@ void bnxt_aux_device_init(struct bnxt *bp,
 
 	aux_priv->id = ida_alloc(bnxt_aux_devices[auxdev_type].ida, GFP_KERNEL);
 	if (aux_priv->id < 0) {
-		netdev_warn(bp->dev,
-			    "ida alloc failed for ROCE auxiliary device\n");
+		netdev_warn(bp->dev, "ida alloc failed for %d auxiliary device\n",
+			    auxdev_type);
 		kfree(aux_priv);
 		goto exit;
 	}
diff --git a/include/linux/bnxt/ulp.h b/include/linux/bnxt/ulp.h
index 01b7100dcf4d..b1ec40cf00fa 100644
--- a/include/linux/bnxt/ulp.h
+++ b/include/linux/bnxt/ulp.h
@@ -22,6 +22,7 @@ struct bnxt;
 
 enum bnxt_ulp_auxdev_type {
 	BNXT_AUXDEV_RDMA = 0,
+	BNXT_AUXDEV_FWCTL,
 	__BNXT_AUXDEV_MAX
 };
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v4 4/5] bnxt_fwctl: Add bnxt fwctl device
  2025-09-27  9:39 [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
                   ` (2 preceding siblings ...)
  2025-09-27  9:39 ` [PATCH net-next v4 3/5] bnxt_en: Create an aux device for fwctl Pavan Chebbi
@ 2025-09-27  9:39 ` Pavan Chebbi
  2025-09-29 18:34   ` Dave Jiang
  2025-10-01 15:03   ` Jonathan Cameron
  2025-09-27  9:39 ` [PATCH net-next v4 5/5] bnxt_fwctl: Add documentation entries Pavan Chebbi
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 17+ messages in thread
From: Pavan Chebbi @ 2025-09-27  9:39 UTC (permalink / raw)
  To: jgg, michael.chan
  Cc: dave.jiang, saeedm, Jonathan.Cameron, davem, corbet, edumazet,
	gospo, kuba, netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil, Pavan Chebbi

Create bnxt_fwctl device. This will bind to bnxt's aux device.
On the upper edge, it will register with the fwctl subsystem.
It will make use of bnxt's ULP functions to send FW commands.

Also move 'bnxt_aux_priv' definition required by bnxt_fwctl
from bnxt.h to ulp.h.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 MAINTAINERS                               |   6 +
 drivers/fwctl/Kconfig                     |  11 +
 drivers/fwctl/Makefile                    |   1 +
 drivers/fwctl/bnxt/Makefile               |   4 +
 drivers/fwctl/bnxt/main.c                 | 454 ++++++++++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |   6 -
 include/linux/bnxt/ulp.h                  |   8 +
 include/uapi/fwctl/bnxt.h                 |  64 +++
 include/uapi/fwctl/fwctl.h                |   1 +
 9 files changed, 549 insertions(+), 6 deletions(-)
 create mode 100644 drivers/fwctl/bnxt/Makefile
 create mode 100644 drivers/fwctl/bnxt/main.c
 create mode 100644 include/uapi/fwctl/bnxt.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 2ba1e447f720..e30e600b23d8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10120,6 +10120,12 @@ L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	drivers/fwctl/pds/
 
+FWCTL BNXT DRIVER
+M:	Pavan Chebbi <pavan.chebbi@broadcom.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	drivers/fwctl/bnxt/
+
 GALAXYCORE GC0308 CAMERA SENSOR DRIVER
 M:	Sebastian Reichel <sre@kernel.org>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
index b5583b12a011..203b6ebb06fc 100644
--- a/drivers/fwctl/Kconfig
+++ b/drivers/fwctl/Kconfig
@@ -29,5 +29,16 @@ config FWCTL_PDS
 	  to access the debug and configuration information of the AMD/Pensando
 	  DSC hardware family.
 
+	  If you don't know what to do here, say N.
+
+config FWCTL_BNXT
+	tristate "bnxt control fwctl driver"
+	depends on BNXT
+	help
+	  BNXT provides interface for the user process to access the debug and
+	  configuration registers of the Broadcom NIC hardware family
+	  This will allow configuration and debug tools to work out of the box on
+	  mainstream kernel.
+
 	  If you don't know what to do here, say N.
 endif
diff --git a/drivers/fwctl/Makefile b/drivers/fwctl/Makefile
index c093b5f661d6..fdd46f3a0e4e 100644
--- a/drivers/fwctl/Makefile
+++ b/drivers/fwctl/Makefile
@@ -2,5 +2,6 @@
 obj-$(CONFIG_FWCTL) += fwctl.o
 obj-$(CONFIG_FWCTL_MLX5) += mlx5/
 obj-$(CONFIG_FWCTL_PDS) += pds/
+obj-$(CONFIG_FWCTL_BNXT) += bnxt/
 
 fwctl-y += main.o
diff --git a/drivers/fwctl/bnxt/Makefile b/drivers/fwctl/bnxt/Makefile
new file mode 100644
index 000000000000..b47172761f1e
--- /dev/null
+++ b/drivers/fwctl/bnxt/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_FWCTL_BNXT) += bnxt_fwctl.o
+
+bnxt_fwctl-y += main.o
diff --git a/drivers/fwctl/bnxt/main.c b/drivers/fwctl/bnxt/main.c
new file mode 100644
index 000000000000..397b85671bab
--- /dev/null
+++ b/drivers/fwctl/bnxt/main.c
@@ -0,0 +1,454 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Broadcom Corporation
+ */
+
+#include <linux/kernel.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/slab.h>
+#include <linux/pci.h>
+#include <linux/fwctl.h>
+#include <uapi/fwctl/fwctl.h>
+#include <uapi/fwctl/bnxt.h>
+#include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
+
+struct bnxtctl_uctx {
+	struct fwctl_uctx uctx;
+	u32 uctx_caps;
+};
+
+struct bnxtctl_dev {
+	struct fwctl_device fwctl;
+	struct bnxt_aux_priv *aux_priv;
+	void *dma_virt_addr[MAX_NUM_DMA_INDICATIONS];
+	dma_addr_t dma_addr[MAX_NUM_DMA_INDICATIONS];
+};
+
+DEFINE_FREE(bnxtctl, struct bnxtctl_dev *, if (_T) fwctl_put(&_T->fwctl))
+
+static int bnxtctl_open_uctx(struct fwctl_uctx *uctx)
+{
+	struct bnxtctl_uctx *bnxtctl_uctx =
+		container_of(uctx, struct bnxtctl_uctx, uctx);
+
+	bnxtctl_uctx->uctx_caps = BIT(FWCTL_BNXT_QUERY_COMMANDS) |
+				  BIT(FWCTL_BNXT_SEND_COMMAND);
+	return 0;
+}
+
+static void bnxtctl_close_uctx(struct fwctl_uctx *uctx)
+{
+}
+
+static void *bnxtctl_info(struct fwctl_uctx *uctx, size_t *length)
+{
+	struct bnxtctl_uctx *bnxtctl_uctx =
+		container_of(uctx, struct bnxtctl_uctx, uctx);
+	struct fwctl_info_bnxt *info;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return ERR_PTR(-ENOMEM);
+
+	info->uctx_caps = bnxtctl_uctx->uctx_caps;
+
+	*length = sizeof(*info);
+	return info;
+}
+
+static bool bnxtctl_validate_rpc(struct bnxt_en_dev *edev,
+				 struct bnxt_fw_msg *hwrm_in,
+				 enum fwctl_rpc_scope scope)
+{
+	struct input *req = (struct input *)hwrm_in->msg;
+
+	guard(mutex)(&edev->en_dev_lock);
+	if (edev->flags & BNXT_EN_FLAG_ULP_STOPPED)
+		return false;
+
+	switch (le16_to_cpu(req->req_type)) {
+	case HWRM_FUNC_VF_CFG:
+	case HWRM_FUNC_RESET:
+	case HWRM_FUNC_CFG:
+	case HWRM_PORT_PHY_CFG:
+	case HWRM_PORT_MAC_CFG:
+	case HWRM_PORT_CLR_STATS:
+	case HWRM_QUEUE_PRI2COS_CFG:
+	case HWRM_QUEUE_COS2BW_CFG:
+	case HWRM_QUEUE_DSCP2PRI_CFG:
+	case HWRM_QUEUE_ADPTV_QOS_RX_FEATURE_CFG:
+	case HWRM_QUEUE_ADPTV_QOS_TX_FEATURE_CFG:
+	case HWRM_QUEUE_ADPTV_QOS_RX_TUNING_CFG:
+	case HWRM_VNIC_RSS_CFG:
+	case HWRM_TUNNEL_DST_PORT_ALLOC:
+	case HWRM_TUNNEL_DST_PORT_FREE:
+	case HWRM_QUEUE_ADPTV_QOS_TX_TUNING_CFG:
+	case HWRM_PORT_TX_FIR_CFG:
+	case HWRM_FW_SET_STRUCTURED_DATA:
+	case HWRM_PORT_PRBS_TEST:
+	case HWRM_PORT_EP_TX_CFG:
+	case HWRM_CFA_REDIRECT_TUNNEL_TYPE_INFO:
+	case HWRM_CFA_FLOW_FLUSH:
+	case HWRM_CFA_L2_FILTER_ALLOC:
+	case HWRM_CFA_NTUPLE_FILTER_FREE:
+	case HWRM_CFA_REDIRECT_TUNNEL_TYPE_ALLOC:
+	case HWRM_CFA_REDIRECT_TUNNEL_TYPE_FREE:
+	case HWRM_FW_LIVEPATCH:
+	case HWRM_FW_RESET:
+	case HWRM_FW_SYNC:
+	case HWRM_FW_SET_TIME:
+	case HWRM_PORT_CFG:
+	case HWRM_FUNC_PTP_PIN_CFG:
+	case HWRM_FUNC_PTP_CFG:
+	case HWRM_FUNC_PTP_EXT_CFG:
+	case HWRM_FUNC_SYNCE_CFG:
+	case HWRM_MFG_OTP_CFG:
+	case HWRM_MFG_TESTS:
+	case HWRM_UDCC_CFG:
+	case HWRM_DBG_SERDES_TEST:
+	case HWRM_DBG_LOG_BUFFER_FLUSH:
+	case HWRM_DBG_DUMP:
+	case HWRM_DBG_ERASE_NVM:
+	case HWRM_DBG_CFG:
+	case HWRM_DBG_COREDUMP_LIST:
+	case HWRM_DBG_COREDUMP_INITIATE:
+	case HWRM_DBG_COREDUMP_RETRIEVE:
+	case HWRM_DBG_CRASHDUMP_HEADER:
+	case HWRM_DBG_CRASHDUMP_ERASE:
+	case HWRM_DBG_PTRACE:
+	case HWRM_DBG_TOKEN_CFG:
+	case HWRM_NVM_DEFRAG:
+	case HWRM_NVM_FACTORY_DEFAULTS:
+	case HWRM_NVM_FLUSH:
+	case HWRM_NVM_INSTALL_UPDATE:
+	case HWRM_NVM_MODIFY:
+	case HWRM_NVM_VERIFY_UPDATE:
+	case HWRM_NVM_ERASE_DIR_ENTRY:
+	case HWRM_NVM_MOD_DIR_ENTRY:
+	case HWRM_NVM_FIND_DIR_ENTRY:
+	case HWRM_NVM_RAW_DUMP:
+		return scope >= FWCTL_RPC_CONFIGURATION;
+
+	case HWRM_VER_GET:
+	case HWRM_FW_GET_STRUCTURED_DATA:
+	case HWRM_ERROR_RECOVERY_QCFG:
+	case HWRM_FUNC_QCAPS:
+	case HWRM_FUNC_QCFG:
+	case HWRM_FUNC_QSTATS:
+	case HWRM_PORT_QSTATS:
+	case HWRM_PORT_PHY_QCFG:
+	case HWRM_PORT_MAC_QCFG:
+	case HWRM_PORT_PHY_QCAPS:
+	case HWRM_PORT_PHY_I2C_READ:
+	case HWRM_PORT_PHY_MDIO_READ:
+	case HWRM_QUEUE_PRI2COS_QCFG:
+	case HWRM_QUEUE_COS2BW_QCFG:
+	case HWRM_QUEUE_DSCP2PRI_QCFG:
+	case HWRM_VNIC_RSS_QCFG:
+	case HWRM_QUEUE_GLOBAL_QCFG:
+	case HWRM_QUEUE_ADPTV_QOS_RX_FEATURE_QCFG:
+	case HWRM_QUEUE_ADPTV_QOS_TX_FEATURE_QCFG:
+	case HWRM_QUEUE_QCAPS:
+	case HWRM_QUEUE_ADPTV_QOS_RX_TUNING_QCFG:
+	case HWRM_QUEUE_ADPTV_QOS_TX_TUNING_QCFG:
+	case HWRM_TUNNEL_DST_PORT_QUERY:
+	case HWRM_PORT_QSTATS_EXT:
+	case HWRM_PORT_TX_FIR_QCFG:
+	case HWRM_FW_LIVEPATCH_QUERY:
+	case HWRM_FW_QSTATUS:
+	case HWRM_FW_HEALTH_CHECK:
+	case HWRM_FW_GET_TIME:
+	case HWRM_PORT_DSC_DUMP:
+	case HWRM_PORT_EP_TX_QCFG:
+	case HWRM_PORT_QCFG:
+	case HWRM_PORT_MAC_QCAPS:
+	case HWRM_TEMP_MONITOR_QUERY:
+	case HWRM_REG_POWER_QUERY:
+	case HWRM_CORE_FREQUENCY_QUERY:
+	case HWRM_STAT_QUERY_ROCE_STATS:
+	case HWRM_STAT_QUERY_ROCE_STATS_EXT:
+	case HWRM_CFA_REDIRECT_QUERY_TUNNEL_TYPE:
+	case HWRM_CFA_FLOW_INFO:
+	case HWRM_CFA_ADV_FLOW_MGNT_QCAPS:
+	case HWRM_FUNC_RESOURCE_QCAPS:
+	case HWRM_FUNC_BACKING_STORE_QCAPS:
+	case HWRM_FUNC_BACKING_STORE_QCFG:
+	case HWRM_FUNC_QSTATS_EXT:
+	case HWRM_FUNC_PTP_PIN_QCFG:
+	case HWRM_FUNC_PTP_EXT_QCFG:
+	case HWRM_FUNC_BACKING_STORE_QCFG_V2:
+	case HWRM_FUNC_BACKING_STORE_QCAPS_V2:
+	case HWRM_FUNC_SYNCE_QCFG:
+	case HWRM_FUNC_TTX_PACING_RATE_PROF_QUERY:
+	case HWRM_PCIE_QSTATS:
+	case HWRM_MFG_OTP_QCFG:
+	case HWRM_MFG_FRU_EEPROM_READ:
+	case HWRM_MFG_GET_NVM_MEASUREMENT:
+	case HWRM_STAT_GENERIC_QSTATS:
+	case HWRM_PORT_PHY_FDRSTAT:
+	case HWRM_UDCC_QCAPS:
+	case HWRM_UDCC_QCFG:
+	case HWRM_UDCC_SESSION_QCFG:
+	case HWRM_UDCC_SESSION_QUERY:
+	case HWRM_UDCC_COMP_QCFG:
+	case HWRM_UDCC_COMP_QUERY:
+	case HWRM_QUEUE_ADPTV_QOS_RX_QCFG:
+	case HWRM_QUEUE_ADPTV_QOS_TX_QCFG:
+	case HWRM_TF_RESC_USAGE_QUERY:
+	case HWRM_TFC_RESC_USAGE_QUERY:
+	case HWRM_DBG_READ_DIRECT:
+	case HWRM_DBG_READ_INDIRECT:
+	case HWRM_DBG_RING_INFO_GET:
+	case HWRM_DBG_QCAPS:
+	case HWRM_DBG_QCFG:
+	case HWRM_DBG_USEQ_FLUSH:
+	case HWRM_DBG_USEQ_QCAPS:
+	case HWRM_DBG_SIM_CABLE_STATE:
+	case HWRM_DBG_TOKEN_QUERY_AUTH_IDS:
+	case HWRM_NVM_GET_VARIABLE:
+	case HWRM_NVM_GET_DEV_INFO:
+	case HWRM_NVM_GET_DIR_ENTRIES:
+	case HWRM_NVM_GET_DIR_INFO:
+	case HWRM_NVM_READ:
+	case HWRM_SELFTEST_QLIST:
+	case HWRM_SELFTEST_RETRIEVE_SERDES_DATA:
+		return scope >= FWCTL_RPC_DEBUG_READ_ONLY;
+
+	case HWRM_PORT_PHY_I2C_WRITE:
+	case HWRM_MFG_FRU_WRITE_CONTROL:
+	case HWRM_MFG_FRU_EEPROM_WRITE:
+	case HWRM_DBG_WRITE_DIRECT:
+	case HWRM_NVM_SET_VARIABLE:
+	case HWRM_NVM_WRITE:
+	case HWRM_NVM_RAW_WRITE_BLK:
+	case HWRM_PORT_PHY_MDIO_WRITE:
+		return scope >= FWCTL_RPC_DEBUG_WRITE;
+
+	default:
+		return false;
+	}
+}
+
+static int bnxt_fw_setup_input_dma(struct bnxtctl_dev *bnxt_dev,
+				   struct device *dev,
+				   int num_dma,
+				   struct fwctl_dma_info_bnxt *msg,
+				   struct bnxt_fw_msg *fw_msg)
+{
+	u8 i, num_allocated = 0;
+	void *dma_ptr;
+	int rc = 0;
+
+	for (i = 0; i < num_dma; i++) {
+		if (msg->len == 0 || msg->len > MAX_DMA_MEM_SIZE) {
+			rc = -EINVAL;
+			goto err;
+		}
+		bnxt_dev->dma_virt_addr[i] = dma_alloc_coherent(dev->parent,
+								msg->len,
+								&bnxt_dev->dma_addr[i],
+								GFP_KERNEL);
+		if (!bnxt_dev->dma_virt_addr[i]) {
+			rc = -ENOMEM;
+			goto err;
+		}
+		num_allocated++;
+		if (msg->dma_direction == DEVICE_WRITE) {
+			if (copy_from_user(bnxt_dev->dma_virt_addr[i],
+					   u64_to_user_ptr(msg->data),
+					   msg->len)) {
+				rc = -EFAULT;
+				goto err;
+			}
+		}
+		dma_ptr = fw_msg->msg + msg->offset;
+
+		if ((PTR_ALIGN(dma_ptr, 8) == dma_ptr) &&
+		    msg->offset < fw_msg->msg_len) {
+			__le64 *dmap = dma_ptr;
+
+			*dmap = cpu_to_le64(bnxt_dev->dma_addr[i]);
+		} else {
+			rc = -EINVAL;
+			goto err;
+		}
+		msg += 1;
+	}
+
+	return 0;
+err:
+	for (i = 0; i < num_allocated; i++)
+		dma_free_coherent(dev->parent,
+				  msg->len,
+				  bnxt_dev->dma_virt_addr[i],
+				  bnxt_dev->dma_addr[i]);
+
+	return rc;
+}
+
+static void *bnxtctl_fw_rpc(struct fwctl_uctx *uctx,
+			    enum fwctl_rpc_scope scope,
+			    void *in, size_t in_len, size_t *out_len)
+{
+	struct bnxtctl_dev *bnxtctl =
+		container_of(uctx->fwctl, struct bnxtctl_dev, fwctl);
+	struct bnxt_aux_priv *bnxt_aux_priv = bnxtctl->aux_priv;
+	struct fwctl_dma_info_bnxt *dma_buf = NULL;
+	struct device *dev = &uctx->fwctl->dev;
+	struct fwctl_rpc_bnxt *msg = in;
+	struct bnxt_fw_msg rpc_in;
+	int i, rc, err = 0;
+
+	rpc_in.msg = kzalloc(msg->req_len, GFP_KERNEL);
+	if (!rpc_in.msg)
+		return ERR_PTR(-ENOMEM);
+
+	if (copy_from_user(rpc_in.msg, u64_to_user_ptr(msg->req),
+			   msg->req_len)) {
+		dev_dbg(dev, "Failed to copy in_payload from user\n");
+		err = -EFAULT;
+		goto free_msg_out;
+	}
+
+	if (!bnxtctl_validate_rpc(bnxt_aux_priv->edev, &rpc_in, scope)) {
+		err = -EPERM;
+		goto free_msg_out;
+	}
+
+	rpc_in.msg_len = msg->req_len;
+	rpc_in.resp = kzalloc(*out_len, GFP_KERNEL);
+	if (!rpc_in.resp) {
+		err = -ENOMEM;
+		goto free_msg_out;
+	}
+
+	rpc_in.resp_max_len = *out_len;
+	if (!msg->timeout)
+		rpc_in.timeout = DFLT_HWRM_CMD_TIMEOUT;
+	else
+		rpc_in.timeout = msg->timeout;
+
+	if (msg->num_dma) {
+		if (msg->num_dma > MAX_NUM_DMA_INDICATIONS) {
+			dev_err(dev, "DMA buffers exceed the number supported\n");
+			err = -EINVAL;
+			goto free_msg_out;
+		}
+
+		dma_buf = kcalloc(msg->num_dma, sizeof(*dma_buf), GFP_KERNEL);
+		if (!dma_buf) {
+			err = -ENOMEM;
+			goto free_msg_out;
+		}
+
+		if (copy_from_user(dma_buf, u64_to_user_ptr(msg->payload),
+				   msg->num_dma * sizeof(*dma_buf))) {
+			dev_dbg(dev, "Failed to copy payload from user\n");
+			err = -EFAULT;
+			goto free_dmabuf_out;
+		}
+
+		rc = bnxt_fw_setup_input_dma(bnxtctl, dev, msg->num_dma,
+					     dma_buf, &rpc_in);
+		if (rc) {
+			err = -EIO;
+			goto free_dmabuf_out;
+		}
+	}
+
+	rc = bnxt_send_msg(bnxt_aux_priv->edev, &rpc_in);
+	if (rc) {
+		err = -EIO;
+		goto free_dma_out;
+	}
+
+	for (i = 0; i < msg->num_dma; i++) {
+		if (dma_buf[i].dma_direction == DEVICE_READ) {
+			if (copy_to_user(u64_to_user_ptr(dma_buf[i].data),
+					 bnxtctl->dma_virt_addr[i],
+					 dma_buf[i].len)) {
+				dev_dbg(dev, "Failed to copy resp to user\n");
+				err = -EFAULT;
+				break;
+			}
+		}
+	}
+free_dma_out:
+	for (i = 0; i < msg->num_dma; i++)
+		dma_free_coherent(dev->parent, dma_buf[i].len,
+				  bnxtctl->dma_virt_addr[i],
+				  bnxtctl->dma_addr[i]);
+free_dmabuf_out:
+	kfree(dma_buf);
+free_msg_out:
+	kfree(rpc_in.msg);
+
+	if (err) {
+		kfree(rpc_in.resp);
+		return ERR_PTR(err);
+	}
+
+	return rpc_in.resp;
+}
+
+static const struct fwctl_ops bnxtctl_ops = {
+	.device_type = FWCTL_DEVICE_TYPE_BNXT,
+	.uctx_size = sizeof(struct bnxtctl_uctx),
+	.open_uctx = bnxtctl_open_uctx,
+	.close_uctx = bnxtctl_close_uctx,
+	.info = bnxtctl_info,
+	.fw_rpc = bnxtctl_fw_rpc,
+};
+
+static int bnxtctl_probe(struct auxiliary_device *adev,
+			 const struct auxiliary_device_id *id)
+{
+	struct bnxt_aux_priv *aux_priv =
+		container_of(adev, struct bnxt_aux_priv, aux_dev);
+	struct bnxtctl_dev *bnxtctl __free(bnxtctl) =
+		fwctl_alloc_device(&aux_priv->edev->pdev->dev, &bnxtctl_ops,
+				   struct bnxtctl_dev, fwctl);
+	int rc;
+
+	if (!bnxtctl)
+		return -ENOMEM;
+
+	bnxtctl->aux_priv = aux_priv;
+
+	rc = fwctl_register(&bnxtctl->fwctl);
+	if (rc)
+		return rc;
+
+	auxiliary_set_drvdata(adev, no_free_ptr(bnxtctl));
+	return 0;
+}
+
+static void bnxtctl_remove(struct auxiliary_device *adev)
+{
+	struct bnxtctl_dev *ctldev = auxiliary_get_drvdata(adev);
+
+	fwctl_unregister(&ctldev->fwctl);
+	fwctl_put(&ctldev->fwctl);
+}
+
+static const struct auxiliary_device_id bnxtctl_id_table[] = {
+	{ .name = "bnxt_en.fwctl", },
+	{}
+};
+MODULE_DEVICE_TABLE(auxiliary, bnxtctl_id_table);
+
+static struct auxiliary_driver bnxtctl_driver = {
+	.name = "bnxt_fwctl",
+	.probe = bnxtctl_probe,
+	.remove = bnxtctl_remove,
+	.id_table = bnxtctl_id_table,
+};
+
+module_auxiliary_driver(bnxtctl_driver);
+
+MODULE_IMPORT_NS("FWCTL");
+MODULE_DESCRIPTION("BNXT fwctl driver");
+MODULE_AUTHOR("Pavan Chebbi <pavan.chebbi@broadcom.com>");
+MODULE_AUTHOR("Andy Gospodarek <gospo@broadcom.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index ea1d10c50da6..a7bca802a3e7 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2075,12 +2075,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,
diff --git a/include/linux/bnxt/ulp.h b/include/linux/bnxt/ulp.h
index b1ec40cf00fa..df06f1bd210a 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
 
@@ -26,6 +28,12 @@ enum bnxt_ulp_auxdev_type {
 	__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;
diff --git a/include/uapi/fwctl/bnxt.h b/include/uapi/fwctl/bnxt.h
new file mode 100644
index 000000000000..a4686a45eb35
--- /dev/null
+++ b/include/uapi/fwctl/bnxt.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (c) 2025, Broadcom Corporation
+ */
+
+#ifndef _UAPI_FWCTL_BNXT_H_
+#define _UAPI_FWCTL_BNXT_H_
+
+#include <linux/types.h>
+
+#define MAX_DMA_MEM_SIZE		0x10000 /*64K*/
+#define DFLT_HWRM_CMD_TIMEOUT		500
+#define DEVICE_WRITE			0
+#define DEVICE_READ			1
+
+enum fwctl_bnxt_commands {
+	FWCTL_BNXT_QUERY_COMMANDS = 0,
+	FWCTL_BNXT_SEND_COMMAND
+};
+
+/**
+ * struct fwctl_info_bnxt - ioctl(FWCTL_INFO) out_device_data
+ * @uctx_caps: The command capabilities driver accepts.
+ *
+ * Return basic information about the FW interface available.
+ */
+struct fwctl_info_bnxt {
+	__u32 uctx_caps;
+};
+
+#define MAX_NUM_DMA_INDICATIONS 10
+
+/**
+ * struct fwctl_dma_info_bnxt - describe the buffer that should be DMAed
+ * @data: DMA-intended buffer
+ * @len: length of the @data
+ * @offset: offset at which FW (HWRM) input structure needs DMA address
+ * @dma_direction: DMA direction, DEVICE_READ or DEVICE_WRITE
+ * @unused: pad
+ */
+struct fwctl_dma_info_bnxt {
+	__aligned_u64 data;
+	__u32 len;
+	__u16 offset;
+	__u8 dma_direction;
+	__u8 unused;
+};
+
+/**
+ * struct fwctl_rpc_bnxt - describe the fwctl message for bnxt
+ * @req: FW (HWRM) command input structure
+ * @req_len: length of @req
+ * @timeout: if the user wants to override the driver's default, 0 otherwise
+ * @num_dma: number of DMA buffers to be added to @req
+ * @payload: DMA buffer details in struct fwctl_dma_info_bnxt format
+ */
+struct fwctl_rpc_bnxt {
+	__aligned_u64 req;
+	__u32 req_len;
+	__u32 timeout;
+	__u32 num_dma;
+	__aligned_u64 payload;
+};
+#endif
diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
index 716ac0eee42d..2d6d4049c205 100644
--- a/include/uapi/fwctl/fwctl.h
+++ b/include/uapi/fwctl/fwctl.h
@@ -44,6 +44,7 @@ enum fwctl_device_type {
 	FWCTL_DEVICE_TYPE_ERROR = 0,
 	FWCTL_DEVICE_TYPE_MLX5 = 1,
 	FWCTL_DEVICE_TYPE_CXL = 2,
+	FWCTL_DEVICE_TYPE_BNXT = 3,
 	FWCTL_DEVICE_TYPE_PDS = 4,
 };
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v4 5/5] bnxt_fwctl: Add documentation entries
  2025-09-27  9:39 [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
                   ` (3 preceding siblings ...)
  2025-09-27  9:39 ` [PATCH net-next v4 4/5] bnxt_fwctl: Add bnxt fwctl device Pavan Chebbi
@ 2025-09-27  9:39 ` Pavan Chebbi
  2025-09-29 18:36   ` Dave Jiang
  2025-09-28  6:35 ` [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
  2025-09-29 20:02 ` Leon Romanovsky
  6 siblings, 1 reply; 17+ messages in thread
From: Pavan Chebbi @ 2025-09-27  9:39 UTC (permalink / raw)
  To: jgg, michael.chan
  Cc: dave.jiang, saeedm, Jonathan.Cameron, davem, corbet, edumazet,
	gospo, kuba, netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil, Pavan Chebbi

Add bnxt_fwctl to the driver and fwctl documentation pages.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 .../userspace-api/fwctl/bnxt_fwctl.rst        | 78 +++++++++++++++++++
 Documentation/userspace-api/fwctl/fwctl.rst   |  1 +
 Documentation/userspace-api/fwctl/index.rst   |  1 +
 3 files changed, 80 insertions(+)
 create mode 100644 Documentation/userspace-api/fwctl/bnxt_fwctl.rst

diff --git a/Documentation/userspace-api/fwctl/bnxt_fwctl.rst b/Documentation/userspace-api/fwctl/bnxt_fwctl.rst
new file mode 100644
index 000000000000..7cf2b65ea34b
--- /dev/null
+++ b/Documentation/userspace-api/fwctl/bnxt_fwctl.rst
@@ -0,0 +1,78 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=================
+fwctl bnxt driver
+=================
+
+:Author: Pavan Chebbi
+
+Overview
+========
+
+BNXT driver makes a fwctl service available through an auxiliary_device.
+The bnxt_fwctl driver binds to this device and registers itself with the
+fwctl subsystem.
+
+The bnxt_fwctl driver is agnostic to the device firmware internals. It
+uses the Upper Layer Protocol (ULP) conduit provided by bnxt to send
+HardWare Resource Manager (HWRM) commands to firmware.
+
+These commands can query or change firmware driven device configurations
+and read/write registers that are useful for debugging.
+
+bnxt_fwctl User API
+===================
+
+Each RPC request contains a message request structure (HWRM input),
+its length, optional request timeout, and dma buffers' information
+if the command needs any DMA. The request is then put together with
+the request data and sent through bnxt's message queue to the firmware,
+and the results are returned to the caller.
+
+A typical user application can send a FWCTL_INFO command using ioctl()
+to discover bnxt_fwctl's RPC capabilities as shown below:
+
+        ioctl(fd, FWCTL_INFO, &fwctl_info_msg);
+
+where fwctl_info_msg (of type struct fwctl_info) describes bnxt_info_msg
+(of type struct fwctl_info_bnxt) as shown below:
+
+        size = sizeof(fwctl_info_msg);
+        flags = 0;
+        device_data_len = sizeof(bnxt_info_msg);
+        out_device_data = (__aligned_u64)&bnxt_info_msg;
+
+The uctx_caps of bnxt_info_msg represents the capabilities as described
+in fwctl_bnxt_commands of include/uapi/fwctl/bnxt.h
+
+The FW RPC itself, FWCTL_RPC can be sent using ioctl() as:
+
+        ioctl(fd, FWCTL_RPC, &fwctl_rpc_msg);
+
+where fwctl_rpc_msg (of type struct fwctl_rpc) encapsulates fwctl_rpc_bnxt
+(see bnxt_rpc_msg below). fwctl_rpc_bnxt members are set up as per the
+requirements of specific HWRM commands described in include/linux/bnxt/hsi.h.
+An example for HWRM_VER_GET is shown below:
+
+        struct fwctl_rpc_bnxt bnxt_rpc_msg;
+        struct hwrm_ver_get_output resp;
+        struct fwctl_rpc fwctl_rpc_msg;
+        struct hwrm_ver_get_input req;
+
+        req.req_type = HWRM_VER_GET;
+        req.hwrm_intf_maj = HWRM_VERSION_MAJOR;
+        req.hwrm_intf_min = HWRM_VERSION_MINOR;
+        req.hwrm_intf_upd = HWRM_VERSION_UPDATE;
+        req.cmpl_ring = -1;
+        req.target_id = -1;
+
+        bnxt_rpc_msg.req_len = sizeof(req);
+        bnxt_rpc_msg.num_dma = 0;
+        bnxt_rpc_msg.req = (__aligned_u64)&req;
+
+        fwctl_rpc_msg.size = sizeof(fwctl_rpc_msg);
+        fwctl_rpc_msg.scope = FWCTL_RPC_DEBUG_READ_ONLY;
+        fwctl_rpc_msg.in_len = sizeof(bnxt_rpc_msg) + sizeof(req);
+        fwctl_rpc_msg.out_len = sizeof(resp);
+        fwctl_rpc_msg.in = (__aligned_u64)&bnxt_rpc_msg;
+        fwctl_rpc_msg.out = (__aligned_u64)&resp;
diff --git a/Documentation/userspace-api/fwctl/fwctl.rst b/Documentation/userspace-api/fwctl/fwctl.rst
index a74eab8d14c6..826817bfd54d 100644
--- a/Documentation/userspace-api/fwctl/fwctl.rst
+++ b/Documentation/userspace-api/fwctl/fwctl.rst
@@ -148,6 +148,7 @@ area resulting in clashes will be resolved in favour of a kernel implementation.
 fwctl User API
 ==============
 
+.. kernel-doc:: include/uapi/fwctl/bnxt.h
 .. kernel-doc:: include/uapi/fwctl/fwctl.h
 .. kernel-doc:: include/uapi/fwctl/mlx5.h
 .. kernel-doc:: include/uapi/fwctl/pds.h
diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
index 316ac456ad3b..8062f7629654 100644
--- a/Documentation/userspace-api/fwctl/index.rst
+++ b/Documentation/userspace-api/fwctl/index.rst
@@ -10,5 +10,6 @@ to securely construct and execute RPCs inside device firmware.
    :maxdepth: 1
 
    fwctl
+   bnxt_fwctl
    fwctl-cxl
    pds_fwctl
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices
  2025-09-27  9:39 [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
                   ` (4 preceding siblings ...)
  2025-09-27  9:39 ` [PATCH net-next v4 5/5] bnxt_fwctl: Add documentation entries Pavan Chebbi
@ 2025-09-28  6:35 ` Pavan Chebbi
  2025-09-29 18:46   ` Jakub Kicinski
  2025-09-29 20:02 ` Leon Romanovsky
  6 siblings, 1 reply; 17+ messages in thread
From: Pavan Chebbi @ 2025-09-28  6:35 UTC (permalink / raw)
  To: jgg, michael.chan
  Cc: dave.jiang, saeedm, Jonathan.Cameron, davem, corbet, edumazet,
	gospo, kuba, netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil

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

On Sat, Sep 27, 2025 at 3:02 PM Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:
>
> Introducing bnxt_fwctl which follows along Jason's work [1].
> It is an aux bus driver that enables fwctl for Broadcom
> NetXtreme 574xx, 575xx and 576xx series chipsets by using
> bnxt driver's capability to talk to devices' firmware.
>
> The first patch moves the ULP definitions to a common place
> inside include/linux/bnxt/. The second and third patches
> refactor and extend the existing bnxt aux bus functions to
> be able to add more than one auxiliary device. The last three
> patches create an additional bnxt aux device, add bnxt_fwctl,
> and the documentation.
>
> [1] https://lore.kernel.org/netdev/0-v5-642aa0c94070+4447f-fwctl_jgg@nvidia.com/
>
> v4: In patch #4, added the missing kfree on error for response
> buffer. Improved documentation in patch #5 based on comments
> from Dave.
>

Dear maintainers, my not-yet-reviewed v4 series is moved to 'Changes Requested'.
I am not sure if I missed anything. Can you pls help me know!

> v3: Addressed the review comments as below
> Patch #1: Removed redundant common.h [thanks Saeed]
> Patch #2 and #3 merged into a single patch [thanks Jonathan]
> Patch #3: Addressed comments from Jonathan
> Patch #4 and #5: Addressed comments from Jonathan and Dave
>
> v2: In patch #5, fixed a sparse warning where a __le16 was
> degraded to an integer. Also addressed kdoc warnings for
> include/uapi/fwctl/bnxt.h in the same patch.
>
> v1: https://lore.kernel.org/netdev/20250922090851.719913-1-pavan.chebbi@broadcom.com/
>
> The following are changes since commit fec734e8d564d55fb6bd4909ae2e68814d21d0a1:
>   Merge tag 'riscv-for-linus-v6.17-rc8' of git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux
> and are available in the git repository at:
>   https://github.com/pavanchebbi/linux/tree/bnxt_fwctl_v4

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 4/5] bnxt_fwctl: Add bnxt fwctl device
  2025-09-27  9:39 ` [PATCH net-next v4 4/5] bnxt_fwctl: Add bnxt fwctl device Pavan Chebbi
@ 2025-09-29 18:34   ` Dave Jiang
  2025-10-02  8:27     ` Pavan Chebbi
  2025-10-01 15:03   ` Jonathan Cameron
  1 sibling, 1 reply; 17+ messages in thread
From: Dave Jiang @ 2025-09-29 18:34 UTC (permalink / raw)
  To: Pavan Chebbi, jgg, michael.chan
  Cc: saeedm, Jonathan.Cameron, davem, corbet, edumazet, gospo, kuba,
	netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil



On 9/27/25 2:39 AM, Pavan Chebbi wrote:
> Create bnxt_fwctl device. This will bind to bnxt's aux device.
> On the upper edge, it will register with the fwctl subsystem.
> It will make use of bnxt's ULP functions to send FW commands.
> 
> Also move 'bnxt_aux_priv' definition required by bnxt_fwctl
> from bnxt.h to ulp.h.
> 
> Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

just a minor comment below
> ---
>  MAINTAINERS                               |   6 +
>  drivers/fwctl/Kconfig                     |  11 +
>  drivers/fwctl/Makefile                    |   1 +
>  drivers/fwctl/bnxt/Makefile               |   4 +
>  drivers/fwctl/bnxt/main.c                 | 454 ++++++++++++++++++++++
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h |   6 -
>  include/linux/bnxt/ulp.h                  |   8 +
>  include/uapi/fwctl/bnxt.h                 |  64 +++
>  include/uapi/fwctl/fwctl.h                |   1 +
>  9 files changed, 549 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/fwctl/bnxt/Makefile
>  create mode 100644 drivers/fwctl/bnxt/main.c
>  create mode 100644 include/uapi/fwctl/bnxt.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2ba1e447f720..e30e600b23d8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10120,6 +10120,12 @@ L:	linux-kernel@vger.kernel.org
>  S:	Maintained
>  F:	drivers/fwctl/pds/
>  
> +FWCTL BNXT DRIVER
> +M:	Pavan Chebbi <pavan.chebbi@broadcom.com>
> +L:	linux-kernel@vger.kernel.org
> +S:	Maintained
> +F:	drivers/fwctl/bnxt/
> +
>  GALAXYCORE GC0308 CAMERA SENSOR DRIVER
>  M:	Sebastian Reichel <sre@kernel.org>
>  L:	linux-media@vger.kernel.org
> diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
> index b5583b12a011..203b6ebb06fc 100644
> --- a/drivers/fwctl/Kconfig
> +++ b/drivers/fwctl/Kconfig
> @@ -29,5 +29,16 @@ config FWCTL_PDS
>  	  to access the debug and configuration information of the AMD/Pensando
>  	  DSC hardware family.
>  
> +	  If you don't know what to do here, say N.
> +
> +config FWCTL_BNXT
> +	tristate "bnxt control fwctl driver"
> +	depends on BNXT
> +	help
> +	  BNXT provides interface for the user process to access the debug and
> +	  configuration registers of the Broadcom NIC hardware family
> +	  This will allow configuration and debug tools to work out of the box on
> +	  mainstream kernel.
> +
>  	  If you don't know what to do here, say N.
>  endif
> diff --git a/drivers/fwctl/Makefile b/drivers/fwctl/Makefile
> index c093b5f661d6..fdd46f3a0e4e 100644
> --- a/drivers/fwctl/Makefile
> +++ b/drivers/fwctl/Makefile
> @@ -2,5 +2,6 @@
>  obj-$(CONFIG_FWCTL) += fwctl.o
>  obj-$(CONFIG_FWCTL_MLX5) += mlx5/
>  obj-$(CONFIG_FWCTL_PDS) += pds/
> +obj-$(CONFIG_FWCTL_BNXT) += bnxt/
>  
>  fwctl-y += main.o
> diff --git a/drivers/fwctl/bnxt/Makefile b/drivers/fwctl/bnxt/Makefile
> new file mode 100644
> index 000000000000..b47172761f1e
> --- /dev/null
> +++ b/drivers/fwctl/bnxt/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_FWCTL_BNXT) += bnxt_fwctl.o
> +
> +bnxt_fwctl-y += main.o
> diff --git a/drivers/fwctl/bnxt/main.c b/drivers/fwctl/bnxt/main.c
> new file mode 100644
> index 000000000000..397b85671bab
> --- /dev/null
> +++ b/drivers/fwctl/bnxt/main.c
> @@ -0,0 +1,454 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025, Broadcom Corporation
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/slab.h>
> +#include <linux/pci.h>
> +#include <linux/fwctl.h>
> +#include <uapi/fwctl/fwctl.h>
> +#include <uapi/fwctl/bnxt.h>
> +#include <linux/bnxt/hsi.h>
> +#include <linux/bnxt/ulp.h>
> +
> +struct bnxtctl_uctx {
> +	struct fwctl_uctx uctx;
> +	u32 uctx_caps;
> +};
> +
> +struct bnxtctl_dev {
> +	struct fwctl_device fwctl;
> +	struct bnxt_aux_priv *aux_priv;

> +	void *dma_virt_addr[MAX_NUM_DMA_INDICATIONS];
> +	dma_addr_t dma_addr[MAX_NUM_DMA_INDICATIONS];
I think these 2 don't need to be in bnxtctl_dev and can be temporary variables. Since they all get freed at the end of the function that uses it.

DJ


> +};
> +
> +DEFINE_FREE(bnxtctl, struct bnxtctl_dev *, if (_T) fwctl_put(&_T->fwctl))
> +
> +static int bnxtctl_open_uctx(struct fwctl_uctx *uctx)
> +{
> +	struct bnxtctl_uctx *bnxtctl_uctx =
> +		container_of(uctx, struct bnxtctl_uctx, uctx);
> +
> +	bnxtctl_uctx->uctx_caps = BIT(FWCTL_BNXT_QUERY_COMMANDS) |
> +				  BIT(FWCTL_BNXT_SEND_COMMAND);
> +	return 0;
> +}
> +
> +static void bnxtctl_close_uctx(struct fwctl_uctx *uctx)
> +{
> +}
> +
> +static void *bnxtctl_info(struct fwctl_uctx *uctx, size_t *length)
> +{
> +	struct bnxtctl_uctx *bnxtctl_uctx =
> +		container_of(uctx, struct bnxtctl_uctx, uctx);
> +	struct fwctl_info_bnxt *info;
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return ERR_PTR(-ENOMEM);
> +
> +	info->uctx_caps = bnxtctl_uctx->uctx_caps;
> +
> +	*length = sizeof(*info);
> +	return info;
> +}
> +
> +static bool bnxtctl_validate_rpc(struct bnxt_en_dev *edev,
> +				 struct bnxt_fw_msg *hwrm_in,
> +				 enum fwctl_rpc_scope scope)
> +{
> +	struct input *req = (struct input *)hwrm_in->msg;
> +
> +	guard(mutex)(&edev->en_dev_lock);
> +	if (edev->flags & BNXT_EN_FLAG_ULP_STOPPED)
> +		return false;
> +
> +	switch (le16_to_cpu(req->req_type)) {
> +	case HWRM_FUNC_VF_CFG:
> +	case HWRM_FUNC_RESET:
> +	case HWRM_FUNC_CFG:
> +	case HWRM_PORT_PHY_CFG:
> +	case HWRM_PORT_MAC_CFG:
> +	case HWRM_PORT_CLR_STATS:
> +	case HWRM_QUEUE_PRI2COS_CFG:
> +	case HWRM_QUEUE_COS2BW_CFG:
> +	case HWRM_QUEUE_DSCP2PRI_CFG:
> +	case HWRM_QUEUE_ADPTV_QOS_RX_FEATURE_CFG:
> +	case HWRM_QUEUE_ADPTV_QOS_TX_FEATURE_CFG:
> +	case HWRM_QUEUE_ADPTV_QOS_RX_TUNING_CFG:
> +	case HWRM_VNIC_RSS_CFG:
> +	case HWRM_TUNNEL_DST_PORT_ALLOC:
> +	case HWRM_TUNNEL_DST_PORT_FREE:
> +	case HWRM_QUEUE_ADPTV_QOS_TX_TUNING_CFG:
> +	case HWRM_PORT_TX_FIR_CFG:
> +	case HWRM_FW_SET_STRUCTURED_DATA:
> +	case HWRM_PORT_PRBS_TEST:
> +	case HWRM_PORT_EP_TX_CFG:
> +	case HWRM_CFA_REDIRECT_TUNNEL_TYPE_INFO:
> +	case HWRM_CFA_FLOW_FLUSH:
> +	case HWRM_CFA_L2_FILTER_ALLOC:
> +	case HWRM_CFA_NTUPLE_FILTER_FREE:
> +	case HWRM_CFA_REDIRECT_TUNNEL_TYPE_ALLOC:
> +	case HWRM_CFA_REDIRECT_TUNNEL_TYPE_FREE:
> +	case HWRM_FW_LIVEPATCH:
> +	case HWRM_FW_RESET:
> +	case HWRM_FW_SYNC:
> +	case HWRM_FW_SET_TIME:
> +	case HWRM_PORT_CFG:
> +	case HWRM_FUNC_PTP_PIN_CFG:
> +	case HWRM_FUNC_PTP_CFG:
> +	case HWRM_FUNC_PTP_EXT_CFG:
> +	case HWRM_FUNC_SYNCE_CFG:
> +	case HWRM_MFG_OTP_CFG:
> +	case HWRM_MFG_TESTS:
> +	case HWRM_UDCC_CFG:
> +	case HWRM_DBG_SERDES_TEST:
> +	case HWRM_DBG_LOG_BUFFER_FLUSH:
> +	case HWRM_DBG_DUMP:
> +	case HWRM_DBG_ERASE_NVM:
> +	case HWRM_DBG_CFG:
> +	case HWRM_DBG_COREDUMP_LIST:
> +	case HWRM_DBG_COREDUMP_INITIATE:
> +	case HWRM_DBG_COREDUMP_RETRIEVE:
> +	case HWRM_DBG_CRASHDUMP_HEADER:
> +	case HWRM_DBG_CRASHDUMP_ERASE:
> +	case HWRM_DBG_PTRACE:
> +	case HWRM_DBG_TOKEN_CFG:
> +	case HWRM_NVM_DEFRAG:
> +	case HWRM_NVM_FACTORY_DEFAULTS:
> +	case HWRM_NVM_FLUSH:
> +	case HWRM_NVM_INSTALL_UPDATE:
> +	case HWRM_NVM_MODIFY:
> +	case HWRM_NVM_VERIFY_UPDATE:
> +	case HWRM_NVM_ERASE_DIR_ENTRY:
> +	case HWRM_NVM_MOD_DIR_ENTRY:
> +	case HWRM_NVM_FIND_DIR_ENTRY:
> +	case HWRM_NVM_RAW_DUMP:
> +		return scope >= FWCTL_RPC_CONFIGURATION;
> +
> +	case HWRM_VER_GET:
> +	case HWRM_FW_GET_STRUCTURED_DATA:
> +	case HWRM_ERROR_RECOVERY_QCFG:
> +	case HWRM_FUNC_QCAPS:
> +	case HWRM_FUNC_QCFG:
> +	case HWRM_FUNC_QSTATS:
> +	case HWRM_PORT_QSTATS:
> +	case HWRM_PORT_PHY_QCFG:
> +	case HWRM_PORT_MAC_QCFG:
> +	case HWRM_PORT_PHY_QCAPS:
> +	case HWRM_PORT_PHY_I2C_READ:
> +	case HWRM_PORT_PHY_MDIO_READ:
> +	case HWRM_QUEUE_PRI2COS_QCFG:
> +	case HWRM_QUEUE_COS2BW_QCFG:
> +	case HWRM_QUEUE_DSCP2PRI_QCFG:
> +	case HWRM_VNIC_RSS_QCFG:
> +	case HWRM_QUEUE_GLOBAL_QCFG:
> +	case HWRM_QUEUE_ADPTV_QOS_RX_FEATURE_QCFG:
> +	case HWRM_QUEUE_ADPTV_QOS_TX_FEATURE_QCFG:
> +	case HWRM_QUEUE_QCAPS:
> +	case HWRM_QUEUE_ADPTV_QOS_RX_TUNING_QCFG:
> +	case HWRM_QUEUE_ADPTV_QOS_TX_TUNING_QCFG:
> +	case HWRM_TUNNEL_DST_PORT_QUERY:
> +	case HWRM_PORT_QSTATS_EXT:
> +	case HWRM_PORT_TX_FIR_QCFG:
> +	case HWRM_FW_LIVEPATCH_QUERY:
> +	case HWRM_FW_QSTATUS:
> +	case HWRM_FW_HEALTH_CHECK:
> +	case HWRM_FW_GET_TIME:
> +	case HWRM_PORT_DSC_DUMP:
> +	case HWRM_PORT_EP_TX_QCFG:
> +	case HWRM_PORT_QCFG:
> +	case HWRM_PORT_MAC_QCAPS:
> +	case HWRM_TEMP_MONITOR_QUERY:
> +	case HWRM_REG_POWER_QUERY:
> +	case HWRM_CORE_FREQUENCY_QUERY:
> +	case HWRM_STAT_QUERY_ROCE_STATS:
> +	case HWRM_STAT_QUERY_ROCE_STATS_EXT:
> +	case HWRM_CFA_REDIRECT_QUERY_TUNNEL_TYPE:
> +	case HWRM_CFA_FLOW_INFO:
> +	case HWRM_CFA_ADV_FLOW_MGNT_QCAPS:
> +	case HWRM_FUNC_RESOURCE_QCAPS:
> +	case HWRM_FUNC_BACKING_STORE_QCAPS:
> +	case HWRM_FUNC_BACKING_STORE_QCFG:
> +	case HWRM_FUNC_QSTATS_EXT:
> +	case HWRM_FUNC_PTP_PIN_QCFG:
> +	case HWRM_FUNC_PTP_EXT_QCFG:
> +	case HWRM_FUNC_BACKING_STORE_QCFG_V2:
> +	case HWRM_FUNC_BACKING_STORE_QCAPS_V2:
> +	case HWRM_FUNC_SYNCE_QCFG:
> +	case HWRM_FUNC_TTX_PACING_RATE_PROF_QUERY:
> +	case HWRM_PCIE_QSTATS:
> +	case HWRM_MFG_OTP_QCFG:
> +	case HWRM_MFG_FRU_EEPROM_READ:
> +	case HWRM_MFG_GET_NVM_MEASUREMENT:
> +	case HWRM_STAT_GENERIC_QSTATS:
> +	case HWRM_PORT_PHY_FDRSTAT:
> +	case HWRM_UDCC_QCAPS:
> +	case HWRM_UDCC_QCFG:
> +	case HWRM_UDCC_SESSION_QCFG:
> +	case HWRM_UDCC_SESSION_QUERY:
> +	case HWRM_UDCC_COMP_QCFG:
> +	case HWRM_UDCC_COMP_QUERY:
> +	case HWRM_QUEUE_ADPTV_QOS_RX_QCFG:
> +	case HWRM_QUEUE_ADPTV_QOS_TX_QCFG:
> +	case HWRM_TF_RESC_USAGE_QUERY:
> +	case HWRM_TFC_RESC_USAGE_QUERY:
> +	case HWRM_DBG_READ_DIRECT:
> +	case HWRM_DBG_READ_INDIRECT:
> +	case HWRM_DBG_RING_INFO_GET:
> +	case HWRM_DBG_QCAPS:
> +	case HWRM_DBG_QCFG:
> +	case HWRM_DBG_USEQ_FLUSH:
> +	case HWRM_DBG_USEQ_QCAPS:
> +	case HWRM_DBG_SIM_CABLE_STATE:
> +	case HWRM_DBG_TOKEN_QUERY_AUTH_IDS:
> +	case HWRM_NVM_GET_VARIABLE:
> +	case HWRM_NVM_GET_DEV_INFO:
> +	case HWRM_NVM_GET_DIR_ENTRIES:
> +	case HWRM_NVM_GET_DIR_INFO:
> +	case HWRM_NVM_READ:
> +	case HWRM_SELFTEST_QLIST:
> +	case HWRM_SELFTEST_RETRIEVE_SERDES_DATA:
> +		return scope >= FWCTL_RPC_DEBUG_READ_ONLY;
> +
> +	case HWRM_PORT_PHY_I2C_WRITE:
> +	case HWRM_MFG_FRU_WRITE_CONTROL:
> +	case HWRM_MFG_FRU_EEPROM_WRITE:
> +	case HWRM_DBG_WRITE_DIRECT:
> +	case HWRM_NVM_SET_VARIABLE:
> +	case HWRM_NVM_WRITE:
> +	case HWRM_NVM_RAW_WRITE_BLK:
> +	case HWRM_PORT_PHY_MDIO_WRITE:
> +		return scope >= FWCTL_RPC_DEBUG_WRITE;
> +
> +	default:
> +		return false;
> +	}
> +}
> +
> +static int bnxt_fw_setup_input_dma(struct bnxtctl_dev *bnxt_dev,
> +				   struct device *dev,
> +				   int num_dma,
> +				   struct fwctl_dma_info_bnxt *msg,
> +				   struct bnxt_fw_msg *fw_msg)
> +{
> +	u8 i, num_allocated = 0;
> +	void *dma_ptr;
> +	int rc = 0;
> +
> +	for (i = 0; i < num_dma; i++) {
> +		if (msg->len == 0 || msg->len > MAX_DMA_MEM_SIZE) {
> +			rc = -EINVAL;
> +			goto err;
> +		}
> +		bnxt_dev->dma_virt_addr[i] = dma_alloc_coherent(dev->parent,
> +								msg->len,
> +								&bnxt_dev->dma_addr[i],
> +								GFP_KERNEL);
> +		if (!bnxt_dev->dma_virt_addr[i]) {
> +			rc = -ENOMEM;
> +			goto err;
> +		}
> +		num_allocated++;
> +		if (msg->dma_direction == DEVICE_WRITE) {
> +			if (copy_from_user(bnxt_dev->dma_virt_addr[i],
> +					   u64_to_user_ptr(msg->data),
> +					   msg->len)) {
> +				rc = -EFAULT;
> +				goto err;
> +			}
> +		}
> +		dma_ptr = fw_msg->msg + msg->offset;
> +
> +		if ((PTR_ALIGN(dma_ptr, 8) == dma_ptr) &&
> +		    msg->offset < fw_msg->msg_len) {
> +			__le64 *dmap = dma_ptr;
> +
> +			*dmap = cpu_to_le64(bnxt_dev->dma_addr[i]);
> +		} else {
> +			rc = -EINVAL;
> +			goto err;
> +		}
> +		msg += 1;
> +	}
> +
> +	return 0;
> +err:
> +	for (i = 0; i < num_allocated; i++)
> +		dma_free_coherent(dev->parent,
> +				  msg->len,
> +				  bnxt_dev->dma_virt_addr[i],
> +				  bnxt_dev->dma_addr[i]);
> +
> +	return rc;
> +}
> +
> +static void *bnxtctl_fw_rpc(struct fwctl_uctx *uctx,
> +			    enum fwctl_rpc_scope scope,
> +			    void *in, size_t in_len, size_t *out_len)
> +{
> +	struct bnxtctl_dev *bnxtctl =
> +		container_of(uctx->fwctl, struct bnxtctl_dev, fwctl);
> +	struct bnxt_aux_priv *bnxt_aux_priv = bnxtctl->aux_priv;
> +	struct fwctl_dma_info_bnxt *dma_buf = NULL;
> +	struct device *dev = &uctx->fwctl->dev;
> +	struct fwctl_rpc_bnxt *msg = in;
> +	struct bnxt_fw_msg rpc_in;
> +	int i, rc, err = 0;
> +
> +	rpc_in.msg = kzalloc(msg->req_len, GFP_KERNEL);
> +	if (!rpc_in.msg)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (copy_from_user(rpc_in.msg, u64_to_user_ptr(msg->req),
> +			   msg->req_len)) {
> +		dev_dbg(dev, "Failed to copy in_payload from user\n");
> +		err = -EFAULT;
> +		goto free_msg_out;
> +	}
> +
> +	if (!bnxtctl_validate_rpc(bnxt_aux_priv->edev, &rpc_in, scope)) {
> +		err = -EPERM;
> +		goto free_msg_out;
> +	}
> +
> +	rpc_in.msg_len = msg->req_len;
> +	rpc_in.resp = kzalloc(*out_len, GFP_KERNEL);
> +	if (!rpc_in.resp) {
> +		err = -ENOMEM;
> +		goto free_msg_out;
> +	}
> +
> +	rpc_in.resp_max_len = *out_len;
> +	if (!msg->timeout)
> +		rpc_in.timeout = DFLT_HWRM_CMD_TIMEOUT;
> +	else
> +		rpc_in.timeout = msg->timeout;
> +
> +	if (msg->num_dma) {
> +		if (msg->num_dma > MAX_NUM_DMA_INDICATIONS) {
> +			dev_err(dev, "DMA buffers exceed the number supported\n");
> +			err = -EINVAL;
> +			goto free_msg_out;
> +		}
> +
> +		dma_buf = kcalloc(msg->num_dma, sizeof(*dma_buf), GFP_KERNEL);
> +		if (!dma_buf) {
> +			err = -ENOMEM;
> +			goto free_msg_out;
> +		}
> +
> +		if (copy_from_user(dma_buf, u64_to_user_ptr(msg->payload),
> +				   msg->num_dma * sizeof(*dma_buf))) {
> +			dev_dbg(dev, "Failed to copy payload from user\n");
> +			err = -EFAULT;
> +			goto free_dmabuf_out;
> +		}
> +
> +		rc = bnxt_fw_setup_input_dma(bnxtctl, dev, msg->num_dma,
> +					     dma_buf, &rpc_in);
> +		if (rc) {
> +			err = -EIO;
> +			goto free_dmabuf_out;
> +		}
> +	}
> +
> +	rc = bnxt_send_msg(bnxt_aux_priv->edev, &rpc_in);
> +	if (rc) {
> +		err = -EIO;
> +		goto free_dma_out;
> +	}
> +
> +	for (i = 0; i < msg->num_dma; i++) {
> +		if (dma_buf[i].dma_direction == DEVICE_READ) {
> +			if (copy_to_user(u64_to_user_ptr(dma_buf[i].data),
> +					 bnxtctl->dma_virt_addr[i],
> +					 dma_buf[i].len)) {
> +				dev_dbg(dev, "Failed to copy resp to user\n");
> +				err = -EFAULT;
> +				break;
> +			}
> +		}
> +	}
> +free_dma_out:
> +	for (i = 0; i < msg->num_dma; i++)
> +		dma_free_coherent(dev->parent, dma_buf[i].len,
> +				  bnxtctl->dma_virt_addr[i],
> +				  bnxtctl->dma_addr[i]);
> +free_dmabuf_out:
> +	kfree(dma_buf);
> +free_msg_out:
> +	kfree(rpc_in.msg);
> +
> +	if (err) {
> +		kfree(rpc_in.resp);
> +		return ERR_PTR(err);
> +	}
> +
> +	return rpc_in.resp;
> +}
> +
> +static const struct fwctl_ops bnxtctl_ops = {
> +	.device_type = FWCTL_DEVICE_TYPE_BNXT,
> +	.uctx_size = sizeof(struct bnxtctl_uctx),
> +	.open_uctx = bnxtctl_open_uctx,
> +	.close_uctx = bnxtctl_close_uctx,
> +	.info = bnxtctl_info,
> +	.fw_rpc = bnxtctl_fw_rpc,
> +};
> +
> +static int bnxtctl_probe(struct auxiliary_device *adev,
> +			 const struct auxiliary_device_id *id)
> +{
> +	struct bnxt_aux_priv *aux_priv =
> +		container_of(adev, struct bnxt_aux_priv, aux_dev);
> +	struct bnxtctl_dev *bnxtctl __free(bnxtctl) =
> +		fwctl_alloc_device(&aux_priv->edev->pdev->dev, &bnxtctl_ops,
> +				   struct bnxtctl_dev, fwctl);
> +	int rc;
> +
> +	if (!bnxtctl)
> +		return -ENOMEM;
> +
> +	bnxtctl->aux_priv = aux_priv;
> +
> +	rc = fwctl_register(&bnxtctl->fwctl);
> +	if (rc)
> +		return rc;
> +
> +	auxiliary_set_drvdata(adev, no_free_ptr(bnxtctl));
> +	return 0;
> +}
> +
> +static void bnxtctl_remove(struct auxiliary_device *adev)
> +{
> +	struct bnxtctl_dev *ctldev = auxiliary_get_drvdata(adev);
> +
> +	fwctl_unregister(&ctldev->fwctl);
> +	fwctl_put(&ctldev->fwctl);
> +}
> +
> +static const struct auxiliary_device_id bnxtctl_id_table[] = {
> +	{ .name = "bnxt_en.fwctl", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(auxiliary, bnxtctl_id_table);
> +
> +static struct auxiliary_driver bnxtctl_driver = {
> +	.name = "bnxt_fwctl",
> +	.probe = bnxtctl_probe,
> +	.remove = bnxtctl_remove,
> +	.id_table = bnxtctl_id_table,
> +};
> +
> +module_auxiliary_driver(bnxtctl_driver);
> +
> +MODULE_IMPORT_NS("FWCTL");
> +MODULE_DESCRIPTION("BNXT fwctl driver");
> +MODULE_AUTHOR("Pavan Chebbi <pavan.chebbi@broadcom.com>");
> +MODULE_AUTHOR("Andy Gospodarek <gospo@broadcom.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index ea1d10c50da6..a7bca802a3e7 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2075,12 +2075,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,
> diff --git a/include/linux/bnxt/ulp.h b/include/linux/bnxt/ulp.h
> index b1ec40cf00fa..df06f1bd210a 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
>  
> @@ -26,6 +28,12 @@ enum bnxt_ulp_auxdev_type {
>  	__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;
> diff --git a/include/uapi/fwctl/bnxt.h b/include/uapi/fwctl/bnxt.h
> new file mode 100644
> index 000000000000..a4686a45eb35
> --- /dev/null
> +++ b/include/uapi/fwctl/bnxt.h
> @@ -0,0 +1,64 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (c) 2025, Broadcom Corporation
> + */
> +
> +#ifndef _UAPI_FWCTL_BNXT_H_
> +#define _UAPI_FWCTL_BNXT_H_
> +
> +#include <linux/types.h>
> +
> +#define MAX_DMA_MEM_SIZE		0x10000 /*64K*/
> +#define DFLT_HWRM_CMD_TIMEOUT		500
> +#define DEVICE_WRITE			0
> +#define DEVICE_READ			1
> +
> +enum fwctl_bnxt_commands {
> +	FWCTL_BNXT_QUERY_COMMANDS = 0,
> +	FWCTL_BNXT_SEND_COMMAND
> +};
> +
> +/**
> + * struct fwctl_info_bnxt - ioctl(FWCTL_INFO) out_device_data
> + * @uctx_caps: The command capabilities driver accepts.
> + *
> + * Return basic information about the FW interface available.
> + */
> +struct fwctl_info_bnxt {
> +	__u32 uctx_caps;
> +};
> +
> +#define MAX_NUM_DMA_INDICATIONS 10
> +
> +/**
> + * struct fwctl_dma_info_bnxt - describe the buffer that should be DMAed
> + * @data: DMA-intended buffer
> + * @len: length of the @data
> + * @offset: offset at which FW (HWRM) input structure needs DMA address
> + * @dma_direction: DMA direction, DEVICE_READ or DEVICE_WRITE
> + * @unused: pad
> + */
> +struct fwctl_dma_info_bnxt {
> +	__aligned_u64 data;
> +	__u32 len;
> +	__u16 offset;
> +	__u8 dma_direction;
> +	__u8 unused;
> +};
> +
> +/**
> + * struct fwctl_rpc_bnxt - describe the fwctl message for bnxt
> + * @req: FW (HWRM) command input structure
> + * @req_len: length of @req
> + * @timeout: if the user wants to override the driver's default, 0 otherwise
> + * @num_dma: number of DMA buffers to be added to @req
> + * @payload: DMA buffer details in struct fwctl_dma_info_bnxt format
> + */
> +struct fwctl_rpc_bnxt {
> +	__aligned_u64 req;
> +	__u32 req_len;
> +	__u32 timeout;
> +	__u32 num_dma;
> +	__aligned_u64 payload;
> +};
> +#endif
> diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
> index 716ac0eee42d..2d6d4049c205 100644
> --- a/include/uapi/fwctl/fwctl.h
> +++ b/include/uapi/fwctl/fwctl.h
> @@ -44,6 +44,7 @@ enum fwctl_device_type {
>  	FWCTL_DEVICE_TYPE_ERROR = 0,
>  	FWCTL_DEVICE_TYPE_MLX5 = 1,
>  	FWCTL_DEVICE_TYPE_CXL = 2,
> +	FWCTL_DEVICE_TYPE_BNXT = 3,
>  	FWCTL_DEVICE_TYPE_PDS = 4,
>  };
>  


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 5/5] bnxt_fwctl: Add documentation entries
  2025-09-27  9:39 ` [PATCH net-next v4 5/5] bnxt_fwctl: Add documentation entries Pavan Chebbi
@ 2025-09-29 18:36   ` Dave Jiang
  0 siblings, 0 replies; 17+ messages in thread
From: Dave Jiang @ 2025-09-29 18:36 UTC (permalink / raw)
  To: Pavan Chebbi, jgg, michael.chan
  Cc: saeedm, Jonathan.Cameron, davem, corbet, edumazet, gospo, kuba,
	netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil



On 9/27/25 2:39 AM, Pavan Chebbi wrote:
> Add bnxt_fwctl to the driver and fwctl documentation pages.
> 
> Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  .../userspace-api/fwctl/bnxt_fwctl.rst        | 78 +++++++++++++++++++
>  Documentation/userspace-api/fwctl/fwctl.rst   |  1 +
>  Documentation/userspace-api/fwctl/index.rst   |  1 +
>  3 files changed, 80 insertions(+)
>  create mode 100644 Documentation/userspace-api/fwctl/bnxt_fwctl.rst
> 
> diff --git a/Documentation/userspace-api/fwctl/bnxt_fwctl.rst b/Documentation/userspace-api/fwctl/bnxt_fwctl.rst
> new file mode 100644
> index 000000000000..7cf2b65ea34b
> --- /dev/null
> +++ b/Documentation/userspace-api/fwctl/bnxt_fwctl.rst
> @@ -0,0 +1,78 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=================
> +fwctl bnxt driver
> +=================
> +
> +:Author: Pavan Chebbi
> +
> +Overview
> +========
> +
> +BNXT driver makes a fwctl service available through an auxiliary_device.
> +The bnxt_fwctl driver binds to this device and registers itself with the
> +fwctl subsystem.
> +
> +The bnxt_fwctl driver is agnostic to the device firmware internals. It
> +uses the Upper Layer Protocol (ULP) conduit provided by bnxt to send
> +HardWare Resource Manager (HWRM) commands to firmware.
> +
> +These commands can query or change firmware driven device configurations
> +and read/write registers that are useful for debugging.
> +
> +bnxt_fwctl User API
> +===================
> +
> +Each RPC request contains a message request structure (HWRM input),
> +its length, optional request timeout, and dma buffers' information
> +if the command needs any DMA. The request is then put together with
> +the request data and sent through bnxt's message queue to the firmware,
> +and the results are returned to the caller.
> +
> +A typical user application can send a FWCTL_INFO command using ioctl()
> +to discover bnxt_fwctl's RPC capabilities as shown below:
> +
> +        ioctl(fd, FWCTL_INFO, &fwctl_info_msg);
> +
> +where fwctl_info_msg (of type struct fwctl_info) describes bnxt_info_msg
> +(of type struct fwctl_info_bnxt) as shown below:
> +
> +        size = sizeof(fwctl_info_msg);
> +        flags = 0;
> +        device_data_len = sizeof(bnxt_info_msg);
> +        out_device_data = (__aligned_u64)&bnxt_info_msg;
> +
> +The uctx_caps of bnxt_info_msg represents the capabilities as described
> +in fwctl_bnxt_commands of include/uapi/fwctl/bnxt.h
> +
> +The FW RPC itself, FWCTL_RPC can be sent using ioctl() as:
> +
> +        ioctl(fd, FWCTL_RPC, &fwctl_rpc_msg);
> +
> +where fwctl_rpc_msg (of type struct fwctl_rpc) encapsulates fwctl_rpc_bnxt
> +(see bnxt_rpc_msg below). fwctl_rpc_bnxt members are set up as per the
> +requirements of specific HWRM commands described in include/linux/bnxt/hsi.h.
> +An example for HWRM_VER_GET is shown below:
> +
> +        struct fwctl_rpc_bnxt bnxt_rpc_msg;
> +        struct hwrm_ver_get_output resp;
> +        struct fwctl_rpc fwctl_rpc_msg;
> +        struct hwrm_ver_get_input req;
> +
> +        req.req_type = HWRM_VER_GET;
> +        req.hwrm_intf_maj = HWRM_VERSION_MAJOR;
> +        req.hwrm_intf_min = HWRM_VERSION_MINOR;
> +        req.hwrm_intf_upd = HWRM_VERSION_UPDATE;
> +        req.cmpl_ring = -1;
> +        req.target_id = -1;
> +
> +        bnxt_rpc_msg.req_len = sizeof(req);
> +        bnxt_rpc_msg.num_dma = 0;
> +        bnxt_rpc_msg.req = (__aligned_u64)&req;
> +
> +        fwctl_rpc_msg.size = sizeof(fwctl_rpc_msg);
> +        fwctl_rpc_msg.scope = FWCTL_RPC_DEBUG_READ_ONLY;
> +        fwctl_rpc_msg.in_len = sizeof(bnxt_rpc_msg) + sizeof(req);
> +        fwctl_rpc_msg.out_len = sizeof(resp);
> +        fwctl_rpc_msg.in = (__aligned_u64)&bnxt_rpc_msg;
> +        fwctl_rpc_msg.out = (__aligned_u64)&resp;
> diff --git a/Documentation/userspace-api/fwctl/fwctl.rst b/Documentation/userspace-api/fwctl/fwctl.rst
> index a74eab8d14c6..826817bfd54d 100644
> --- a/Documentation/userspace-api/fwctl/fwctl.rst
> +++ b/Documentation/userspace-api/fwctl/fwctl.rst
> @@ -148,6 +148,7 @@ area resulting in clashes will be resolved in favour of a kernel implementation.
>  fwctl User API
>  ==============
>  
> +.. kernel-doc:: include/uapi/fwctl/bnxt.h
>  .. kernel-doc:: include/uapi/fwctl/fwctl.h
>  .. kernel-doc:: include/uapi/fwctl/mlx5.h
>  .. kernel-doc:: include/uapi/fwctl/pds.h
> diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
> index 316ac456ad3b..8062f7629654 100644
> --- a/Documentation/userspace-api/fwctl/index.rst
> +++ b/Documentation/userspace-api/fwctl/index.rst
> @@ -10,5 +10,6 @@ to securely construct and execute RPCs inside device firmware.
>     :maxdepth: 1
>  
>     fwctl
> +   bnxt_fwctl
>     fwctl-cxl
>     pds_fwctl


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices
  2025-09-28  6:35 ` [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
@ 2025-09-29 18:46   ` Jakub Kicinski
  2025-09-30  0:25     ` Pavan Chebbi
  0 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2025-09-29 18:46 UTC (permalink / raw)
  To: Pavan Chebbi
  Cc: jgg, michael.chan, dave.jiang, saeedm, Jonathan.Cameron, davem,
	corbet, edumazet, gospo, netdev, pabeni, andrew+netdev,
	selvin.xavier, leon, kalesh-anakkur.purayil

On Sun, 28 Sep 2025 12:05:36 +0530 Pavan Chebbi wrote:
> Dear maintainers, my not-yet-reviewed v4 series is moved to 'Changes Requested'.
> I am not sure if I missed anything. Can you pls help me know!

There is 

drivers/fwctl/bnxt/main.c:303:14-21: WARNING opportunity for memdup_user

somewhere in the series.

Please note that net-next is closed.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 0/5]  bnxt_fwctl: fwctl for Broadcom Netxtreme devices
  2025-09-27  9:39 [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
                   ` (5 preceding siblings ...)
  2025-09-28  6:35 ` [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
@ 2025-09-29 20:02 ` Leon Romanovsky
  6 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2025-09-29 20:02 UTC (permalink / raw)
  To: Pavan Chebbi
  Cc: jgg, michael.chan, dave.jiang, saeedm, Jonathan.Cameron, davem,
	corbet, edumazet, gospo, kuba, netdev, pabeni, andrew+netdev,
	selvin.xavier, kalesh-anakkur.purayil

On Sat, Sep 27, 2025 at 02:39:25AM -0700, Pavan Chebbi wrote:
> Introducing bnxt_fwctl which follows along Jason's work [1].
> It is an aux bus driver that enables fwctl for Broadcom
> NetXtreme 574xx, 575xx and 576xx series chipsets by using
> bnxt driver's capability to talk to devices' firmware.
> 
> The first patch moves the ULP definitions to a common place
> inside include/linux/bnxt/. The second and third patches
> refactor and extend the existing bnxt aux bus functions to
> be able to add more than one auxiliary device. The last three
> patches create an additional bnxt aux device, add bnxt_fwctl,
> and the documentation.
> 
> [1] https://lore.kernel.org/netdev/0-v5-642aa0c94070+4447f-fwctl_jgg@nvidia.com/
> 
> v4: In patch #4, added the missing kfree on error for response
> buffer. Improved documentation in patch #5 based on comments
> from Dave.
> 
> v3: Addressed the review comments as below
> Patch #1: Removed redundant common.h [thanks Saeed]
> Patch #2 and #3 merged into a single patch [thanks Jonathan]
> Patch #3: Addressed comments from Jonathan
> Patch #4 and #5: Addressed comments from Jonathan and Dave
> 
> v2: In patch #5, fixed a sparse warning where a __le16 was
> degraded to an integer. Also addressed kdoc warnings for
> include/uapi/fwctl/bnxt.h in the same patch.
> 
> v1: https://lore.kernel.org/netdev/20250922090851.719913-1-pavan.chebbi@broadcom.com/
> 
> The following are changes since commit fec734e8d564d55fb6bd4909ae2e68814d21d0a1:
>   Merge tag 'riscv-for-linus-v6.17-rc8' of git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux
> and are available in the git repository at:
>   https://github.com/pavanchebbi/linux/tree/bnxt_fwctl_v4
> 
> Pavan Chebbi (5):
>   bnxt_en: Move common definitions to include/linux/bnxt/
>   bnxt_en: Refactor aux bus functions to be more generic
>   bnxt_en: Create an aux device for fwctl

Like I said in v0, https://lore.kernel.org/all/20250929190601.GC324804@unreal
I think that aux logic is over engineered and shouldn't exist.

Thanks


>   bnxt_fwctl: Add bnxt fwctl device
>   bnxt_fwctl: Add documentation entries
> 
>  .../userspace-api/fwctl/bnxt_fwctl.rst        |  78 +++
>  Documentation/userspace-api/fwctl/fwctl.rst   |   1 +
>  Documentation/userspace-api/fwctl/index.rst   |   1 +
>  MAINTAINERS                                   |   6 +
>  drivers/fwctl/Kconfig                         |  11 +
>  drivers/fwctl/Makefile                        |   1 +
>  drivers/fwctl/bnxt/Makefile                   |   4 +
>  drivers/fwctl/bnxt/main.c                     | 454 ++++++++++++++++++
>  drivers/infiniband/hw/bnxt_re/debugfs.c       |   2 +-
>  drivers/infiniband/hw/bnxt_re/main.c          |   2 +-
>  drivers/infiniband/hw/bnxt_re/qplib_fp.c      |   2 +-
>  drivers/infiniband/hw/bnxt_re/qplib_res.h     |   2 +-
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  30 +-
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  12 +-
>  .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |   2 +-
>  .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |   4 +-
>  .../net/ethernet/broadcom/bnxt/bnxt_sriov.c   |   2 +-
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 245 +++++++---
>  .../bnxt_ulp.h => include/linux/bnxt/ulp.h    |  22 +-
>  include/uapi/fwctl/bnxt.h                     |  64 +++
>  include/uapi/fwctl/fwctl.h                    |   1 +
>  21 files changed, 858 insertions(+), 88 deletions(-)
>  create mode 100644 Documentation/userspace-api/fwctl/bnxt_fwctl.rst
>  create mode 100644 drivers/fwctl/bnxt/Makefile
>  create mode 100644 drivers/fwctl/bnxt/main.c
>  rename drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h => include/linux/bnxt/ulp.h (87%)
>  create mode 100644 include/uapi/fwctl/bnxt.h
> 
> -- 
> 2.39.1
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices
  2025-09-29 18:46   ` Jakub Kicinski
@ 2025-09-30  0:25     ` Pavan Chebbi
  2025-10-06 16:21       ` Andrew Lunn
  0 siblings, 1 reply; 17+ messages in thread
From: Pavan Chebbi @ 2025-09-30  0:25 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: jgg, Michael Chan, Dave Jiang, Saeed Mahameed, Jonathan Cameron,
	David S . Miller, Jonathan Corbet, Eric Dumazet,
	Andrew Gospodarek, Linux Netdev List, Paolo Abeni, Andrew Lunn,
	Selvin Xavier, Leon Romanovsky, Kalesh Anakkur Purayil


[-- Attachment #1.1: Type: text/plain, Size: 587 bytes --]

On Tue, 30 Sept, 2025, 12:16 am Jakub Kicinski, <kuba@kernel.org> wrote:

> On Sun, 28 Sep 2025 12:05:36 +0530 Pavan Chebbi wrote:
> > Dear maintainers, my not-yet-reviewed v4 series is moved to 'Changes
> Requested'.
> > I am not sure if I missed anything. Can you pls help me know!
>
> There is
>
> drivers/fwctl/bnxt/main.c:303:14-21: WARNING opportunity for memdup_user
>

Shouldn't it be treated more as a suggestion than a real warning? Are you
insisting that I should change to use it?


> somewhere in the series.
>
> Please note that net-next is closed.
>

[-- Attachment #1.2: Type: text/html, Size: 1177 bytes --]

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 4/5] bnxt_fwctl: Add bnxt fwctl device
  2025-09-27  9:39 ` [PATCH net-next v4 4/5] bnxt_fwctl: Add bnxt fwctl device Pavan Chebbi
  2025-09-29 18:34   ` Dave Jiang
@ 2025-10-01 15:03   ` Jonathan Cameron
  2025-10-02  8:29     ` Pavan Chebbi
  1 sibling, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2025-10-01 15:03 UTC (permalink / raw)
  To: Pavan Chebbi
  Cc: jgg, michael.chan, dave.jiang, saeedm, davem, corbet, edumazet,
	gospo, kuba, netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil

On Sat, 27 Sep 2025 02:39:29 -0700
Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:

> Create bnxt_fwctl device. This will bind to bnxt's aux device.
> On the upper edge, it will register with the fwctl subsystem.
> It will make use of bnxt's ULP functions to send FW commands.
> 
> Also move 'bnxt_aux_priv' definition required by bnxt_fwctl
> from bnxt.h to ulp.h.
> 
> Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

A few minor things inline.

J
> diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
> index b5583b12a011..203b6ebb06fc 100644
> --- a/drivers/fwctl/Kconfig
> +++ b/drivers/fwctl/Kconfig
> @@ -29,5 +29,16 @@ config FWCTL_PDS
>  	  to access the debug and configuration information of the AMD/Pensando
>  	  DSC hardware family.
>  
> +	  If you don't know what to do here, say N.
> +
> +config FWCTL_BNXT
> +	tristate "bnxt control fwctl driver"
> +	depends on BNXT
> +	help
> +	  BNXT provides interface for the user process to access the debug and
> +	  configuration registers of the Broadcom NIC hardware family

Full stop missing.

> +	  This will allow configuration and debug tools to work out of the box on
> +	  mainstream kernel.
> +
>  	  If you don't know what to do here, say N.
>  endif

> diff --git a/drivers/fwctl/bnxt/main.c b/drivers/fwctl/bnxt/main.c
> new file mode 100644
> index 000000000000..397b85671bab
> --- /dev/null
> +++ b/drivers/fwctl/bnxt/main.c

> +
> +static int bnxt_fw_setup_input_dma(struct bnxtctl_dev *bnxt_dev,
> +				   struct device *dev,
> +				   int num_dma,
> +				   struct fwctl_dma_info_bnxt *msg,
> +				   struct bnxt_fw_msg *fw_msg)
> +{
> +	u8 i, num_allocated = 0;
> +	void *dma_ptr;
> +	int rc = 0;

The compiler should be able to figure out that rc is always set in paths to where
it is used.   If not fair enough leaving this.

> +
> +	for (i = 0; i < num_dma; i++) {
> +		if (msg->len == 0 || msg->len > MAX_DMA_MEM_SIZE) {
> +			rc = -EINVAL;
> +			goto err;
> +		}
> +		bnxt_dev->dma_virt_addr[i] = dma_alloc_coherent(dev->parent,
> +								msg->len,
> +								&bnxt_dev->dma_addr[i],
> +								GFP_KERNEL);
> +		if (!bnxt_dev->dma_virt_addr[i]) {
> +			rc = -ENOMEM;
> +			goto err;
> +		}
> +		num_allocated++;
> +		if (msg->dma_direction == DEVICE_WRITE) {
> +			if (copy_from_user(bnxt_dev->dma_virt_addr[i],
> +					   u64_to_user_ptr(msg->data),
> +					   msg->len)) {
> +				rc = -EFAULT;
> +				goto err;
> +			}
> +		}
> +		dma_ptr = fw_msg->msg + msg->offset;
> +
> +		if ((PTR_ALIGN(dma_ptr, 8) == dma_ptr) &&
> +		    msg->offset < fw_msg->msg_len) {
> +			__le64 *dmap = dma_ptr;
> +
> +			*dmap = cpu_to_le64(bnxt_dev->dma_addr[i]);
> +		} else {
> +			rc = -EINVAL;
> +			goto err;
> +		}
> +		msg += 1;
> +	}
> +
> +	return 0;
> +err:
> +	for (i = 0; i < num_allocated; i++)
> +		dma_free_coherent(dev->parent,
> +				  msg->len,
> +				  bnxt_dev->dma_virt_addr[i],
> +				  bnxt_dev->dma_addr[i]);
> +
> +	return rc;
> +}
> +
> +static void *bnxtctl_fw_rpc(struct fwctl_uctx *uctx,
> +			    enum fwctl_rpc_scope scope,
> +			    void *in, size_t in_len, size_t *out_len)
> +{
> +	struct bnxtctl_dev *bnxtctl =
> +		container_of(uctx->fwctl, struct bnxtctl_dev, fwctl);
> +	struct bnxt_aux_priv *bnxt_aux_priv = bnxtctl->aux_priv;
> +	struct fwctl_dma_info_bnxt *dma_buf = NULL;
> +	struct device *dev = &uctx->fwctl->dev;
> +	struct fwctl_rpc_bnxt *msg = in;
> +	struct bnxt_fw_msg rpc_in;
> +	int i, rc, err = 0;
> +
> +	rpc_in.msg = kzalloc(msg->req_len, GFP_KERNEL);
> +	if (!rpc_in.msg)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (copy_from_user(rpc_in.msg, u64_to_user_ptr(msg->req),
> +			   msg->req_len)) {
> +		dev_dbg(dev, "Failed to copy in_payload from user\n");
> +		err = -EFAULT;
> +		goto free_msg_out;
> +	}
> +
> +	if (!bnxtctl_validate_rpc(bnxt_aux_priv->edev, &rpc_in, scope)) {
> +		err = -EPERM;
> +		goto free_msg_out;
> +	}
> +
> +	rpc_in.msg_len = msg->req_len;
> +	rpc_in.resp = kzalloc(*out_len, GFP_KERNEL);
> +	if (!rpc_in.resp) {
> +		err = -ENOMEM;
> +		goto free_msg_out;
> +	}
> +
> +	rpc_in.resp_max_len = *out_len;
> +	if (!msg->timeout)
> +		rpc_in.timeout = DFLT_HWRM_CMD_TIMEOUT;
> +	else
> +		rpc_in.timeout = msg->timeout;
> +
> +	if (msg->num_dma) {
> +		if (msg->num_dma > MAX_NUM_DMA_INDICATIONS) {
> +			dev_err(dev, "DMA buffers exceed the number supported\n");
> +			err = -EINVAL;
> +			goto free_msg_out;
> +		}
> +
> +		dma_buf = kcalloc(msg->num_dma, sizeof(*dma_buf), GFP_KERNEL);
> +		if (!dma_buf) {
> +			err = -ENOMEM;
> +			goto free_msg_out;
> +		}
> +
> +		if (copy_from_user(dma_buf, u64_to_user_ptr(msg->payload),
> +				   msg->num_dma * sizeof(*dma_buf))) {
> +			dev_dbg(dev, "Failed to copy payload from user\n");
> +			err = -EFAULT;
> +			goto free_dmabuf_out;
> +		}
> +
> +		rc = bnxt_fw_setup_input_dma(bnxtctl, dev, msg->num_dma,
> +					     dma_buf, &rpc_in);
> +		if (rc) {
If there is a strong reason to override the value of rc rather than returning
that I'd add a comment.

> +			err = -EIO;
> +			goto free_dmabuf_out;
> +		}
> +	}
> +
> +	rc = bnxt_send_msg(bnxt_aux_priv->edev, &rpc_in);
> +	if (rc) {
> +		err = -EIO;
Likewise here.

Overall I'd just use a single rc variable rather than having separate one for err.

> +		goto free_dma_out;
> +	}
> +
> +	for (i = 0; i < msg->num_dma; i++) {
> +		if (dma_buf[i].dma_direction == DEVICE_READ) {
> +			if (copy_to_user(u64_to_user_ptr(dma_buf[i].data),
> +					 bnxtctl->dma_virt_addr[i],
> +					 dma_buf[i].len)) {
> +				dev_dbg(dev, "Failed to copy resp to user\n");
> +				err = -EFAULT;
> +				break;
> +			}
> +		}
> +	}
> +free_dma_out:
> +	for (i = 0; i < msg->num_dma; i++)
> +		dma_free_coherent(dev->parent, dma_buf[i].len,
> +				  bnxtctl->dma_virt_addr[i],
> +				  bnxtctl->dma_addr[i]);
> +free_dmabuf_out:
> +	kfree(dma_buf);
> +free_msg_out:
> +	kfree(rpc_in.msg);
> +
> +	if (err) {
> +		kfree(rpc_in.resp);
> +		return ERR_PTR(err);
> +	}
> +
> +	return rpc_in.resp;
> +}

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index ea1d10c50da6..a7bca802a3e7 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2075,12 +2075,6 @@ struct bnxt_fw_health {
>  #define BNXT_FW_IF_RETRY		10
>  #define BNXT_FW_SLOT_RESET_RETRY	4
>  

Can you drop the linux/auxiliary_bus.h include given I think this
is the only use in here?

> -struct bnxt_aux_priv {
> -	struct auxiliary_device aux_dev;
> -	struct bnxt_en_dev *edev;
> -	int id;
> -};
> -
>  enum board_idx {
>  	BCM57301,
>  	BCM57302,

> diff --git a/include/uapi/fwctl/bnxt.h b/include/uapi/fwctl/bnxt.h
> new file mode 100644
> index 000000000000..a4686a45eb35
> --- /dev/null
> +++ b/include/uapi/fwctl/bnxt.h
> @@ -0,0 +1,64 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (c) 2025, Broadcom Corporation
> + */
> +
> +#ifndef _UAPI_FWCTL_BNXT_H_
> +#define _UAPI_FWCTL_BNXT_H_
> +
> +#include <linux/types.h>
> +
> +#define MAX_DMA_MEM_SIZE		0x10000 /*64K*/
> +#define DFLT_HWRM_CMD_TIMEOUT		500
> +#define DEVICE_WRITE			0
> +#define DEVICE_READ			1
> +
> +enum fwctl_bnxt_commands {
> +	FWCTL_BNXT_QUERY_COMMANDS = 0,
> +	FWCTL_BNXT_SEND_COMMAND

Maybe there will be more commands in future. So add a trailing comma.
General convention is always do this unless it is a terminating entry
that is just there to count the elements above.

> +};





^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 4/5] bnxt_fwctl: Add bnxt fwctl device
  2025-09-29 18:34   ` Dave Jiang
@ 2025-10-02  8:27     ` Pavan Chebbi
  0 siblings, 0 replies; 17+ messages in thread
From: Pavan Chebbi @ 2025-10-02  8:27 UTC (permalink / raw)
  To: Dave Jiang
  Cc: jgg, michael.chan, saeedm, Jonathan.Cameron, davem, corbet,
	edumazet, gospo, kuba, netdev, pabeni, andrew+netdev,
	selvin.xavier, leon, kalesh-anakkur.purayil

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

On Tue, Sep 30, 2025 at 12:04 AM Dave Jiang <dave.jiang@intel.com> wrote:
>
>
>
> On 9/27/25 2:39 AM, Pavan Chebbi wrote:
> > Create bnxt_fwctl device. This will bind to bnxt's aux device.
> > On the upper edge, it will register with the fwctl subsystem.
> > It will make use of bnxt's ULP functions to send FW commands.
> >
> > Also move 'bnxt_aux_priv' definition required by bnxt_fwctl
> > from bnxt.h to ulp.h.
> >
> > Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> > Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>
> just a minor comment below

Thanks for the review, Dave. Yes, the DMA address holders can be
temporary variables.
I can make that change since I must spin a new revision anyway.

<-->
> I think these 2 don't need to be in bnxtctl_dev and can be temporary variables. Since they all get freed at the end of the function that uses it.
>
> DJ
>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 4/5] bnxt_fwctl: Add bnxt fwctl device
  2025-10-01 15:03   ` Jonathan Cameron
@ 2025-10-02  8:29     ` Pavan Chebbi
  0 siblings, 0 replies; 17+ messages in thread
From: Pavan Chebbi @ 2025-10-02  8:29 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: jgg, michael.chan, dave.jiang, saeedm, davem, corbet, edumazet,
	gospo, kuba, netdev, pabeni, andrew+netdev, selvin.xavier, leon,
	kalesh-anakkur.purayil

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

On Wed, Oct 1, 2025 at 8:33 PM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> On Sat, 27 Sep 2025 02:39:29 -0700
> Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:
>
> > Create bnxt_fwctl device. This will bind to bnxt's aux device.
> > On the upper edge, it will register with the fwctl subsystem.
> > It will make use of bnxt's ULP functions to send FW commands.
> >
> > Also move 'bnxt_aux_priv' definition required by bnxt_fwctl
> > from bnxt.h to ulp.h.
> >
> > Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> > Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
>
> A few minor things inline.
>
> J

Thanks Jonathan for the additional comments. I will address them in
the next revision after
net-next opens.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices
  2025-09-30  0:25     ` Pavan Chebbi
@ 2025-10-06 16:21       ` Andrew Lunn
  2025-10-06 16:40         ` Pavan Chebbi
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Lunn @ 2025-10-06 16:21 UTC (permalink / raw)
  To: Pavan Chebbi
  Cc: Jakub Kicinski, jgg, Michael Chan, Dave Jiang, Saeed Mahameed,
	Jonathan Cameron, David S . Miller, Jonathan Corbet, Eric Dumazet,
	Andrew Gospodarek, Linux Netdev List, Paolo Abeni, Andrew Lunn,
	Selvin Xavier, Leon Romanovsky, Kalesh Anakkur Purayil

On Tue, Sep 30, 2025 at 05:55:38AM +0530, Pavan Chebbi wrote:
> 
> 
> On Tue, 30 Sept, 2025, 12:16 am Jakub Kicinski, <kuba@kernel.org> wrote:
> 
>     On Sun, 28 Sep 2025 12:05:36 +0530 Pavan Chebbi wrote:
>     > Dear maintainers, my not-yet-reviewed v4 series is moved to 'Changes
>     Requested'.
>     > I am not sure if I missed anything. Can you pls help me know!
> 
>     There is
> 
>     drivers/fwctl/bnxt/main.c:303:14-21: WARNING opportunity for memdup_user
> 
> 
> Shouldn't it be treated more as a suggestion than a real warning? Are you
> insisting that I should change to use it? 

There is some danger of "Cannot see the forest for the trees". If you
ignore this warning, are you going to miss other warnings which should
be addressed because you have got used to just ignoring warnings? It
is much better if your code is totally free of warnings.

	Andrew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices
  2025-10-06 16:21       ` Andrew Lunn
@ 2025-10-06 16:40         ` Pavan Chebbi
  0 siblings, 0 replies; 17+ messages in thread
From: Pavan Chebbi @ 2025-10-06 16:40 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jakub Kicinski, jgg, Michael Chan, Dave Jiang, Saeed Mahameed,
	Jonathan Cameron, David S . Miller, Jonathan Corbet, Eric Dumazet,
	Andrew Gospodarek, Linux Netdev List, Paolo Abeni, Andrew Lunn,
	Selvin Xavier, Leon Romanovsky, Kalesh Anakkur Purayil

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

On Mon, Oct 6, 2025 at 9:52 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Tue, Sep 30, 2025 at 05:55:38AM +0530, Pavan Chebbi wrote:
> >
> >
> > On Tue, 30 Sept, 2025, 12:16 am Jakub Kicinski, <kuba@kernel.org> wrote:
> >
> >     On Sun, 28 Sep 2025 12:05:36 +0530 Pavan Chebbi wrote:
> >     > Dear maintainers, my not-yet-reviewed v4 series is moved to 'Changes
> >     Requested'.
> >     > I am not sure if I missed anything. Can you pls help me know!
> >
> >     There is
> >
> >     drivers/fwctl/bnxt/main.c:303:14-21: WARNING opportunity for memdup_user
> >
> >
> > Shouldn't it be treated more as a suggestion than a real warning? Are you
> > insisting that I should change to use it?
>
> There is some danger of "Cannot see the forest for the trees". If you
> ignore this warning, are you going to miss other warnings which should
> be addressed because you have got used to just ignoring warnings? It
> is much better if your code is totally free of warnings.
>
>         Andrew

Sure, true for real warnings. My submission was that this particular
'WARNING' sounds more like a suggestion that's all.
And I chose not to address it, and did not ignore it. Anyway, I will
try to incorporate it in the next revision.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-10-06 16:40 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-27  9:39 [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
2025-09-27  9:39 ` [PATCH net-next v4 1/5] bnxt_en: Move common definitions to include/linux/bnxt/ Pavan Chebbi
2025-09-27  9:39 ` [PATCH net-next v4 2/5] bnxt_en: Refactor aux bus functions to be more generic Pavan Chebbi
2025-09-27  9:39 ` [PATCH net-next v4 3/5] bnxt_en: Create an aux device for fwctl Pavan Chebbi
2025-09-27  9:39 ` [PATCH net-next v4 4/5] bnxt_fwctl: Add bnxt fwctl device Pavan Chebbi
2025-09-29 18:34   ` Dave Jiang
2025-10-02  8:27     ` Pavan Chebbi
2025-10-01 15:03   ` Jonathan Cameron
2025-10-02  8:29     ` Pavan Chebbi
2025-09-27  9:39 ` [PATCH net-next v4 5/5] bnxt_fwctl: Add documentation entries Pavan Chebbi
2025-09-29 18:36   ` Dave Jiang
2025-09-28  6:35 ` [PATCH net-next v4 0/5] bnxt_fwctl: fwctl for Broadcom Netxtreme devices Pavan Chebbi
2025-09-29 18:46   ` Jakub Kicinski
2025-09-30  0:25     ` Pavan Chebbi
2025-10-06 16:21       ` Andrew Lunn
2025-10-06 16:40         ` Pavan Chebbi
2025-09-29 20:02 ` Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).