[PATCH net-next v2 0/6] bnxt_fwctl: fwctl for Broadcom Netxtreme devices

[PATCH net-next v2 0/6] bnxt_fwctl: fwctl for Broadcom Netxtreme devices

[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/

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/

Pavan Chebbi (6):
  bnxt_en: Move common definitions to include/linux/bnxt/
  bnxt_en: Refactor aux bus functions to be generic
  bnxt_en: Make a lookup table for supported aux bus devices
  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        |  27 ++
 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                     | 297 ++++++++++++++++++
 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     |  13 +-
 .../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 | 266 ++++++++++++----
 include/linux/bnxt/common.h                   |  20 ++
 .../bnxt_ulp.h => include/linux/bnxt/ulp.h    |  14 +-
 include/uapi/fwctl/bnxt.h                     |  63 ++++
 include/uapi/fwctl/fwctl.h                    |   1 +
 22 files changed, 683 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
 create mode 100644 include/linux/bnxt/common.h
 rename drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h => include/linux/bnxt/ulp.h (90%)
 create mode 100644 include/uapi/fwctl/bnxt.h

-- 
2.39.1

[PATCH net-next v2 1/6] bnxt_en: Move common definitions to include/linux/bnxt/

[Pavan Chebbi]

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.
Have a new common.h, also at the same place that will
have some non-ulp but shared bnxt declarations.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
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.h     |  7 +------
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |  2 +-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  2 +-
 .../net/ethernet/broadcom/bnxt/bnxt_sriov.c   |  2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c |  2 +-
 include/linux/bnxt/common.h                   | 20 +++++++++++++++++++
 .../bnxt_ulp.h => include/linux/bnxt/ulp.h    |  0
 12 files changed, 30 insertions(+), 15 deletions(-)
 create mode 100644 include/linux/bnxt/common.h
 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 d59612d1e176..917a39f8865c 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.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 06a4c2afdf8a..2578bac16f6c 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -33,6 +33,7 @@
 #ifdef CONFIG_TEE_BNXT_FW
 #include <linux/firmware/broadcom/tee_bnxt_fw.h>
 #endif
+#include <linux/bnxt/common.h>
 
 #define BNXT_DEFAULT_RX_COPYBREAK 256
 #define BNXT_MAX_RX_COPYBREAK 1024
@@ -2075,12 +2076,6 @@ struct bnxt_fw_health {
 #define BNXT_FW_IF_RETRY		10
 #define BNXT_FW_SLOT_RESET_RETRY	4
 
-struct bnxt_aux_priv {
-	struct auxiliary_device aux_dev;
-	struct bnxt_en_dev *edev;
-	int id;
-};
-
 enum board_idx {
 	BCM57301,
 	BCM57302,
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 be32ef8f5c96..3231d3c022dc 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/include/linux/bnxt/common.h b/include/linux/bnxt/common.h
new file mode 100644
index 000000000000..2ee75a0a1feb
--- /dev/null
+++ b/include/linux/bnxt/common.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (c) 2025, Broadcom Corporation
+ *
+ */
+
+#ifndef BNXT_COMN_H
+#define BNXT_COMN_H
+
+#include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
+#include <linux/auxiliary_bus.h>
+
+struct bnxt_aux_priv {
+	struct auxiliary_device aux_dev;
+	struct bnxt_en_dev *edev;
+	int id;
+};
+
+#endif /* BNXT_COMN_H */
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

Re: [PATCH net-next v2 1/6] bnxt_en: Move common definitions to include/linux/bnxt/

[Jonathan Cameron]

On Tue, 23 Sep 2025 02:58:20 -0700
Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:

> 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.
> Have a new common.h, also at the same place that will
> have some non-ulp but shared bnxt declarations.
> 
> Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
One really minor comment inline. Given this does exactly what you say
FWIW.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> diff --git a/include/linux/bnxt/common.h b/include/linux/bnxt/common.h
> new file mode 100644
> index 000000000000..2ee75a0a1feb
> --- /dev/null
> +++ b/include/linux/bnxt/common.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (c) 2025, Broadcom Corporation
> + *

Totally trivial but this blank line adds nothing useful.

> + */
> +
> +#ifndef BNXT_COMN_H
> +#define BNXT_COMN_H
> +
> +#include <linux/bnxt/hsi.h>
> +#include <linux/bnxt/ulp.h>
> +#include <linux/auxiliary_bus.h>
> +
> +struct bnxt_aux_priv {
> +	struct auxiliary_device aux_dev;
> +	struct bnxt_en_dev *edev;
> +	int id;
> +};
> +
> +#endif /* BNXT_COMN_H */
> 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

Re: [PATCH net-next v2 1/6] bnxt_en: Move common definitions to include/linux/bnxt/

[Saeed Mahameed]

On 23 Sep 02:58, Pavan Chebbi wrote:
>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.
>Have a new common.h, also at the same place that will
>have some non-ulp but shared bnxt declarations.
>
>Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
>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.h     |  7 +------
> .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |  2 +-
> .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  2 +-
> .../net/ethernet/broadcom/bnxt/bnxt_sriov.c   |  2 +-
> drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c |  2 +-
> include/linux/bnxt/common.h                   | 20 +++++++++++++++++++
> .../bnxt_ulp.h => include/linux/bnxt/ulp.h    |  0
> 12 files changed, 30 insertions(+), 15 deletions(-)
> create mode 100644 include/linux/bnxt/common.h
> 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 d59612d1e176..917a39f8865c 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.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
>index 06a4c2afdf8a..2578bac16f6c 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
>@@ -33,6 +33,7 @@
> #ifdef CONFIG_TEE_BNXT_FW
> #include <linux/firmware/broadcom/tee_bnxt_fw.h>
> #endif
>+#include <linux/bnxt/common.h>
>
> #define BNXT_DEFAULT_RX_COPYBREAK 256
> #define BNXT_MAX_RX_COPYBREAK 1024
>@@ -2075,12 +2076,6 @@ struct bnxt_fw_health {
> #define BNXT_FW_IF_RETRY		10
> #define BNXT_FW_SLOT_RESET_RETRY	4
>
>-struct bnxt_aux_priv {
>-	struct auxiliary_device aux_dev;
>-	struct bnxt_en_dev *edev;
>-	int id;
>-};
>-
> enum board_idx {
> 	BCM57301,
> 	BCM57302,
>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 be32ef8f5c96..3231d3c022dc 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/include/linux/bnxt/common.h b/include/linux/bnxt/common.h
>new file mode 100644
>index 000000000000..2ee75a0a1feb
>--- /dev/null
>+++ b/include/linux/bnxt/common.h
>@@ -0,0 +1,20 @@
>+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>+/*
>+ * Copyright (c) 2025, Broadcom Corporation
>+ *
>+ */
>+
>+#ifndef BNXT_COMN_H
>+#define BNXT_COMN_H
>+
>+#include <linux/bnxt/hsi.h>
>+#include <linux/bnxt/ulp.h>
>+#include <linux/auxiliary_bus.h>
>+
>+struct bnxt_aux_priv {
>+	struct auxiliary_device aux_dev;
>+	struct bnxt_en_dev *edev;
>+	int id;
>+};
>+

This file is redundant since ulp.h already holds every thing "aux", so this
struct belongs there. Also the only place you include this is file:
   drivers/net/ethernet/broadcom/bnxt/bnxt.h

So I am not sure if you have your include paths properly setup to avoid
cross subsystem includes, in-case this was the point of this patch :).

>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
>
>

Re: [PATCH net-next v2 1/6] bnxt_en: Move common definitions to include/linux/bnxt/

[Pavan Chebbi]

.

On Tue, Sep 23, 2025 at 10:03 PM Saeed Mahameed <saeed@kernel.org> wrote:

> >diff --git a/include/linux/bnxt/common.h b/include/linux/bnxt/common.h
> >new file mode 100644
> >index 000000000000..2ee75a0a1feb
> >--- /dev/null
> >+++ b/include/linux/bnxt/common.h
> >@@ -0,0 +1,20 @@
> >+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> >+/*
> >+ * Copyright (c) 2025, Broadcom Corporation
> >+ *
> >+ */
> >+
> >+#ifndef BNXT_COMN_H
> >+#define BNXT_COMN_H
> >+
> >+#include <linux/bnxt/hsi.h>
> >+#include <linux/bnxt/ulp.h>
> >+#include <linux/auxiliary_bus.h>
> >+
> >+struct bnxt_aux_priv {
> >+      struct auxiliary_device aux_dev;
> >+      struct bnxt_en_dev *edev;
> >+      int id;
> >+};
> >+
>
> This file is redundant since ulp.h already holds every thing "aux", so this
> struct belongs there. Also the only place you include this is file:
>    drivers/net/ethernet/broadcom/bnxt/bnxt.h

Hi Saeed, later bnxt fwctl will include it as well. You could say it
can still be
inside ulp.h but fwctl is only going to need bnxt_aux_priv. So I
carved it out of
earlier bnxt.h.

>
> So I am not sure if you have your include paths properly setup to avoid
> cross subsystem includes, in-case this was the point of this patch :).
>
> >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
> >
> >

Re: [PATCH net-next v2 1/6] bnxt_en: Move common definitions to include/linux/bnxt/

[Pavan Chebbi]

On Tue, Sep 23, 2025 at 10:46 PM Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:
>
> >
> > This file is redundant since ulp.h already holds every thing "aux", so this
> > struct belongs there. Also the only place you include this is file:
> >    drivers/net/ethernet/broadcom/bnxt/bnxt.h
>
> Hi Saeed, later bnxt fwctl will include it as well. You could say it
> can still be
> inside ulp.h but fwctl is only going to need bnxt_aux_priv. So I
> carved it out of
> earlier bnxt.h.
>
On second thought, the struct might not have belonged to bnxt.h in the
first place.
So moving it to ulp.h seems like a better thing to do. Thanks.

[PATCH net-next v2 2/6] bnxt_en: Refactor aux bus functions to be generic

[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.

Make aux bus init/uninit/add/del functions generic which will
accept aux device type as a parameter. Change 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 | 102 +++++++++---------
 include/linux/bnxt/ulp.h                      |  13 ++-
 5 files changed, 77 insertions(+), 67 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 917a39f8865c..feb11b9ea4dd 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)
@@ -14657,7 +14658,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));
 		}
@@ -16187,12 +16188,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);
@@ -16778,7 +16779,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) {
@@ -16842,7 +16843,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);
 
@@ -16850,7 +16851,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 2578bac16f6c..b2f139eddfec 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2335,8 +2335,8 @@ struct bnxt {
 #define BNXT_CHIP_P5_AND_MINUS(bp)		\
 	(BNXT_CHIP_P3(bp) || BNXT_CHIP_P4(bp) || BNXT_CHIP_P5(bp))
 
-	struct bnxt_aux_priv	*aux_priv;
-	struct bnxt_en_dev	*edev;
+	struct bnxt_aux_priv	*aux_priv_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 3231d3c022dc..6a175fd082c1 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -5085,7 +5085,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..665850753f90 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -27,11 +27,11 @@
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
 
-static DEFINE_IDA(bnxt_aux_dev_ids);
+static DEFINE_IDA(bnxt_rdma_aux_dev_ids);
 
 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 +51,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 +135,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 +224,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 +255,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 +288,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 +312,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 +344,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 +385,41 @@ 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;
 
 	/* Skip if no auxiliary device init was done. */
-	if (!bp->aux_priv)
+	if (!bp->aux_priv_rdma)
 		return;
 
-	aux_priv = bp->aux_priv;
+	aux_priv = bp->aux_priv_rdma;
 	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 (!bp->edev_rdma)
 		return;
 
-	auxiliary_device_delete(&bp->aux_priv->aux_dev);
+	auxiliary_device_delete(&bp->aux_priv_rdma->aux_dev);
 }
 
 static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
@@ -448,15 +449,15 @@ 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 (!bp->edev_rdma)
 		return;
 
-	aux_dev = &bp->aux_priv->aux_dev;
+	aux_dev = &bp->aux_priv_rdma->aux_dev;
 	rc = auxiliary_device_add(aux_dev);
 	if (rc) {
 		netdev_warn(bp->dev, "Failed to add auxiliary device for ROCE\n");
@@ -465,7 +466,8 @@ void bnxt_rdma_aux_device_add(struct bnxt *bp)
 	}
 }
 
-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 +475,15 @@ 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_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_rdma_aux_dev_ids, GFP_KERNEL);
 	if (aux_priv->id < 0) {
 		netdev_warn(bp->dev,
 			    "ida alloc failed for ROCE auxiliary device\n");
@@ -492,15 +495,15 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp)
 	aux_dev->id = aux_priv->id;
 	aux_dev->name = "rdma";
 	aux_dev->dev.parent = &bp->pdev->dev;
-	aux_dev->dev.release = bnxt_aux_dev_release;
+	aux_dev->dev.release = bnxt_rdma_aux_dev_release;
 
 	rc = auxiliary_device_init(aux_dev);
 	if (rc) {
-		ida_free(&bnxt_aux_dev_ids, aux_priv->id);
+		ida_free(&bnxt_rdma_aux_dev_ids, aux_priv->id);
 		kfree(aux_priv);
 		goto exit;
 	}
-	bp->aux_priv = aux_priv;
+	bp->aux_priv_rdma = aux_priv;
 
 	/* From this point, all cleanup will happen via the .release callback &
 	 * any error unwinding will need to include a call to
@@ -517,9 +520,10 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp)
 		goto aux_dev_uninit;
 
 	edev->ulp_tbl = ulp;
-	bp->edev = edev;
+	bp->edev_rdma = edev;
 	bnxt_set_edev_info(edev, bp);
-	bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
+	if (auxdev_type == BNXT_AUXDEV_RDMA)
+		bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
 
 	return;
 
diff --git a/include/linux/bnxt/ulp.h b/include/linux/bnxt/ulp.h
index 7b9dd8ebe4bc..baac0dd44078 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

Re: [PATCH net-next v2 2/6] bnxt_en: Refactor aux bus functions to be generic

[Jonathan Cameron]

On Tue, 23 Sep 2025 02:58:21 -0700
Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:

> Up until now there was only one auxiliary device that bnxt
> created and that was for RoCE driver. bnxt fwctl is also
> going to use an aux bus device that bnxt should create.
> This requires some nomenclature changes and refactoring of
> the existing bnxt aux dev functions.
> 
> Make aux bus init/uninit/add/del functions generic which will
> accept aux device type as a parameter. Change 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.
> 
Hi Pavan,

It might just be a question of patch break up, but the code here
doesn't really match with what you suggest when talking about making these
functions generic.  They still have a lot of what looks to be unconditional
RDMA specific code in them after this patch.

I think if this 'generic' approach makes sense this patch needs to
be much clearer on what is rdma specific than it currently is. I'm not
yet convinced that this approach is preferable to a few helper functions
(for the generic bits) that rdma and fwctl specific registration functions call.

Thanks,

Jonathan



> Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> index 992eec874345..665850753f90 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c

I stopped reading that this point as the same issue on how generic things are
continued and would have lead to many similar comments.

> @@ -465,7 +466,8 @@ void bnxt_rdma_aux_device_add(struct bnxt *bp)
>  	}
>  }
>  
> -void bnxt_rdma_aux_device_init(struct bnxt *bp)
> +void bnxt_aux_device_init(struct bnxt *bp,

This confuses me a bit.  The patch description says it will make them
generic and this has a bunch of code that really doesn't look generic.

> +			  enum bnxt_ulp_auxdev_type auxdev_type)
>  {
>  	struct auxiliary_device *aux_dev;
>  	struct bnxt_aux_priv *aux_priv;
> @@ -473,14 +475,15 @@ 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_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_rdma_aux_dev_ids, GFP_KERNEL);
>  	if (aux_priv->id < 0) {
>  		netdev_warn(bp->dev,
>  			    "ida alloc failed for ROCE auxiliary device\n");
> @@ -492,15 +495,15 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp)
>  	aux_dev->id = aux_priv->id;
>  	aux_dev->name = "rdma";
>  	aux_dev->dev.parent = &bp->pdev->dev;
> -	aux_dev->dev.release = bnxt_aux_dev_release;
> +	aux_dev->dev.release = bnxt_rdma_aux_dev_release;

Another call that looks very rmda specific.
I would put these all under conditional checks even if that means
that if any other value is passed in for type the function doesn't
yet do anything useful.

>  
>  	rc = auxiliary_device_init(aux_dev);
>  	if (rc) {
> -		ida_free(&bnxt_aux_dev_ids, aux_priv->id);
> +		ida_free(&bnxt_rdma_aux_dev_ids, aux_priv->id);
>  		kfree(aux_priv);
>  		goto exit;
>  	}
> -	bp->aux_priv = aux_priv;
> +	bp->aux_priv_rdma = aux_priv;

As below. This feels like an odd thing to not make conditional on the type.

>  
>  	/* From this point, all cleanup will happen via the .release callback &
>  	 * any error unwinding will need to include a call to
> @@ -517,9 +520,10 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp)
>  		goto aux_dev_uninit;
>  
>  	edev->ulp_tbl = ulp;
> -	bp->edev = edev;
> +	bp->edev_rdma = edev;

This seems to have a slightly odd mix of conditional assignment like
the ulp_num_msix_want below and unconditional assignment of clearly
RDMA specific things like evdev_rdma.

>  	bnxt_set_edev_info(edev, bp);
> -	bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
> +	if (auxdev_type == BNXT_AUXDEV_RDMA)
> +		bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
>  
>  	return;
>  
> diff --git a/include/linux/bnxt/ulp.h b/include/linux/bnxt/ulp.h
> index 7b9dd8ebe4bc..baac0dd44078 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,

Trivial but no point in a trailing comma after a entry that will always
terminate this list (like this one).  Having the comma just makes an
accidental addition of stuff after this harder to spot!


> +};

[PATCH net-next v2 3/6] bnxt_en: Make a lookup table for supported aux bus devices

[Pavan Chebbi]

We could maintain a look up table of aux bus devices supported
by bnxt. This way, the aux bus init/add/uninit/del could have
generic code to work on any of bnxt's aux devices.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 105 ++++++++++++++++--
 1 file changed, 93 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 665850753f90..ecad1947ccb5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -29,6 +29,70 @@
 
 static DEFINE_IDA(bnxt_rdma_aux_dev_ids);
 
+struct bnxt_aux_device {
+	const char *name;
+	const u32 id;
+	u32 (*alloc_ida)(void);
+	void (*free_ida)(struct bnxt_aux_priv *priv);
+	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 u32 bnxt_rdma_aux_dev_alloc_ida(void)
+{
+	return ida_alloc(&bnxt_rdma_aux_dev_ids, GFP_KERNEL);
+}
+
+static void bnxt_rdma_aux_dev_free_ida(struct bnxt_aux_priv *aux_priv)
+{
+	ida_free(&bnxt_rdma_aux_dev_ids, aux_priv->id);
+}
+
+static struct bnxt_aux_device bnxt_aux_devices[__BNXT_AUXDEV_MAX] = {{
+	.name		= "rdma",
+	.alloc_ida	= bnxt_rdma_aux_dev_alloc_ida,
+	.free_ida	= bnxt_rdma_aux_dev_free_ida,
+	.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_rdma;
@@ -391,11 +455,15 @@ void bnxt_aux_device_uninit(struct bnxt *bp,
 	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_rdma)
+	if (!bnxt_aux_devices[auxdev_type].get_priv(bp))
 		return;
 
-	aux_priv = bp->aux_priv_rdma;
+	aux_priv = bnxt_aux_devices[auxdev_type].get_priv(bp);
 	adev = &aux_priv->aux_dev;
 	auxiliary_device_uninit(adev);
 }
@@ -416,10 +484,14 @@ static void bnxt_rdma_aux_dev_release(struct device *dev)
 
 void bnxt_aux_device_del(struct bnxt *bp, enum bnxt_ulp_auxdev_type auxdev_type)
 {
-	if (!bp->edev_rdma)
+	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_rdma->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)
@@ -454,10 +526,14 @@ 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_rdma)
+	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_rdma->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");
@@ -475,6 +551,11 @@ void bnxt_aux_device_init(struct bnxt *bp,
 	struct bnxt_ulp *ulp;
 	int rc;
 
+	if (auxdev_type >= __BNXT_AUXDEV_MAX) {
+		netdev_warn(bp->dev, "Failed to init: unrecognized auxiliary device\n");
+		return;
+	}
+
 	if (auxdev_type == BNXT_AUXDEV_RDMA &&
 	    !(bp->flags & BNXT_FLAG_ROCE_CAP))
 		return;
@@ -483,7 +564,7 @@ void bnxt_aux_device_init(struct bnxt *bp,
 	if (!aux_priv)
 		goto exit;
 
-	aux_priv->id = ida_alloc(&bnxt_rdma_aux_dev_ids, GFP_KERNEL);
+	aux_priv->id = bnxt_aux_devices[auxdev_type].alloc_ida();
 	if (aux_priv->id < 0) {
 		netdev_warn(bp->dev,
 			    "ida alloc failed for ROCE auxiliary device\n");
@@ -493,17 +574,17 @@ void bnxt_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_rdma_aux_dev_release;
+	aux_dev->dev.release = bnxt_aux_devices[auxdev_type].release;
 
 	rc = auxiliary_device_init(aux_dev);
 	if (rc) {
-		ida_free(&bnxt_rdma_aux_dev_ids, aux_priv->id);
+		bnxt_aux_devices[auxdev_type].free_ida(aux_priv);
 		kfree(aux_priv);
 		goto exit;
 	}
-	bp->aux_priv_rdma = 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
@@ -520,7 +601,7 @@ void bnxt_aux_device_init(struct bnxt *bp,
 		goto aux_dev_uninit;
 
 	edev->ulp_tbl = ulp;
-	bp->edev_rdma = edev;
+	bnxt_aux_devices[auxdev_type].set_edev(bp, edev);
 	bnxt_set_edev_info(edev, bp);
 	if (auxdev_type == BNXT_AUXDEV_RDMA)
 		bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
-- 
2.39.1

Re: [PATCH net-next v2 3/6] bnxt_en: Make a lookup table for supported aux bus devices

[Jonathan Cameron]

On Tue, 23 Sep 2025 02:58:22 -0700
Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:

> We could maintain a look up table of aux bus devices supported
> by bnxt. This way, the aux bus init/add/uninit/del could have
> generic code to work on any of bnxt's aux devices.
> 
> Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

Ah. Ok. This does make it more generic.  Smash this and patch 2 together
so we don't have the intermediate state where stuff is partly generic.

Key is perhaps to remember that reviewers almost always end up looking at patches
in isolation before they look at the overall result.

Jonathan

> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 105 ++++++++++++++++--
>  1 file changed, 93 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> index 665850753f90..ecad1947ccb5 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> @@ -29,6 +29,70 @@
>  
>  static DEFINE_IDA(bnxt_rdma_aux_dev_ids);
>  
> +struct bnxt_aux_device {
> +	const char *name;
> +	const u32 id;
> +	u32 (*alloc_ida)(void);
> +	void (*free_ida)(struct bnxt_aux_priv *priv);
> +	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);
> +};

Re: [PATCH net-next v2 3/6] bnxt_en: Make a lookup table for supported aux bus devices

[Pavan Chebbi]

On Tue, Sep 23, 2025 at 4:19 PM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> On Tue, 23 Sep 2025 02:58:22 -0700
> Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:
>
> > We could maintain a look up table of aux bus devices supported
> > by bnxt. This way, the aux bus init/add/uninit/del could have
> > generic code to work on any of bnxt's aux devices.
> >
> > Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> > Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
>
> Ah. Ok. This does make it more generic.  Smash this and patch 2 together
> so we don't have the intermediate state where stuff is partly generic.
>
Hi Jonathan, thanks for the review. Right, I assumed context while
preparing the patches.
I will address this patch break-up and other comments in v3. Thanks again.

[PATCH net-next v2 4/6] bnxt_en: Create an aux device for fwctl

[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 | 79 +++++++++++++++++--
 include/linux/bnxt/ulp.h                      |  1 +
 4 files changed, 82 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index feb11b9ea4dd..58a7c0af89a7 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -16189,11 +16189,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);
@@ -16780,6 +16782,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) {
@@ -16844,6 +16847,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);
 
@@ -16852,6 +16856,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 b2f139eddfec..1eeea0884e09 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2340,6 +2340,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 ecad1947ccb5..c06a9503b551 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;
@@ -43,6 +44,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)
@@ -81,6 +83,43 @@ static void bnxt_rdma_aux_dev_free_ida(struct bnxt_aux_priv *aux_priv)
 	ida_free(&bnxt_rdma_aux_dev_ids, aux_priv->id);
 }
 
+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 u32 bnxt_fwctl_aux_dev_alloc_ida(void)
+{
+	return ida_alloc(&bnxt_fwctl_aux_dev_ids, GFP_KERNEL);
+}
+
+static void bnxt_fwctl_aux_dev_free_ida(struct bnxt_aux_priv *aux_priv)
+{
+	ida_free(&bnxt_fwctl_aux_dev_ids, aux_priv->id);
+}
+
 static struct bnxt_aux_device bnxt_aux_devices[__BNXT_AUXDEV_MAX] = {{
 	.name		= "rdma",
 	.alloc_ida	= bnxt_rdma_aux_dev_alloc_ida,
@@ -91,6 +130,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",
+	.alloc_ida	= bnxt_fwctl_aux_dev_alloc_ida,
+	.free_ida	= bnxt_fwctl_aux_dev_free_ida,
+	.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)
@@ -314,6 +363,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);
 }
 
@@ -347,6 +398,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);
 }
 
@@ -536,12 +589,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);
-		bp->flags &= ~BNXT_FLAG_ROCE_CAP;
+		if (auxdev_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)
 {
@@ -566,8 +634,8 @@ void bnxt_aux_device_init(struct bnxt *bp,
 
 	aux_priv->id = bnxt_aux_devices[auxdev_type].alloc_ida();
 	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;
 	}
@@ -611,5 +679,6 @@ void bnxt_aux_device_init(struct bnxt *bp,
 aux_dev_uninit:
 	auxiliary_device_uninit(aux_dev);
 exit:
-	bp->flags &= ~BNXT_FLAG_ROCE_CAP;
+	if (auxdev_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 baac0dd44078..5a580bc37a06 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

Re: [PATCH net-next v2 4/6] bnxt_en: Create an aux device for fwctl

[Jonathan Cameron]

On Tue, 23 Sep 2025 02:58:23 -0700
Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:

> 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>
Just a few minor comments in here.

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index b2f139eddfec..1eeea0884e09 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2340,6 +2340,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 ecad1947ccb5..c06a9503b551 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c

> +static u32 bnxt_fwctl_aux_dev_alloc_ida(void)
> +{
> +	return ida_alloc(&bnxt_fwctl_aux_dev_ids, GFP_KERNEL);

Could maybe add a pointer to the ida to the type specific structure instead of a callback?
Small increase in shared code.


> +}
> +
> +static void bnxt_fwctl_aux_dev_free_ida(struct bnxt_aux_priv *aux_priv)
> +{
> +	ida_free(&bnxt_fwctl_aux_dev_ids, aux_priv->id);
> +}

>  
> @@ -536,12 +589,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);
> -		bp->flags &= ~BNXT_FLAG_ROCE_CAP;
> +		if (auxdev_type == BNXT_AUXDEV_RDMA)
> +			bp->flags &= ~BNXT_FLAG_ROCE_CAP;

Same comment as below.

>  	}
>  }

> @@ -611,5 +679,6 @@ void bnxt_aux_device_init(struct bnxt *bp,
>  aux_dev_uninit:
>  	auxiliary_device_uninit(aux_dev);
>  exit:
> -	bp->flags &= ~BNXT_FLAG_ROCE_CAP;
> +	if (auxdev_type == BNXT_AUXDEV_RDMA)
Mixing and matching between a 'type' specific structure and specific
ID based checks normally doesn't scale well if you ever end up adding more
types.  I'd suggest moving this to data or a callback in the bnxt_auxdev struture.

> +		bp->flags &= ~BNXT_FLAG_ROCE_CAP;
>  }

>  

[PATCH net-next v2 5/6] bnxt_fwctl: Add bnxt fwctl device

[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.

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   | 297 ++++++++++++++++++++++++++++++++++++
 include/uapi/fwctl/bnxt.h   |  63 ++++++++
 include/uapi/fwctl/fwctl.h  |   1 +
 7 files changed, 383 insertions(+)
 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 a8a770714101..8954da3e9203 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10115,6 +10115,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..1bec4567e35c
--- /dev/null
+++ b/drivers/fwctl/bnxt/main.c
@@ -0,0 +1,297 @@
+// 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/common.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)
+{
+	struct input *req = (struct input *)hwrm_in->msg;
+
+	mutex_lock(&edev->en_dev_lock);
+	if (edev->flags & BNXT_EN_FLAG_ULP_STOPPED) {
+		mutex_unlock(&edev->en_dev_lock);
+		return false;
+	}
+	mutex_unlock(&edev->en_dev_lock);
+
+	if (le16_to_cpu(req->req_type) <= HWRM_LAST)
+		return true;
+
+	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->read_from_device)) {
+			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 rc;
+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;
+	int dma_buf_size;
+
+	rpc_in.msg = kzalloc(msg->req_len, GFP_KERNEL);
+	if (!rpc_in.msg) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+	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 err_out;
+	}
+
+	if (!bnxtctl_validate_rpc(bnxt_aux_priv->edev, &rpc_in))
+		return ERR_PTR(-EPERM);
+
+	rpc_in.msg_len = msg->req_len;
+	rpc_in.resp = kzalloc(*out_len, GFP_KERNEL);
+	if (!rpc_in.resp) {
+		err = -ENOMEM;
+		goto err_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 err_out;
+		}
+		dma_buf_size = msg->num_dma * sizeof(*dma_buf);
+		dma_buf = kzalloc(dma_buf_size, GFP_KERNEL);
+		if (!dma_buf) {
+			dev_err(dev, "Failed to allocate dma buffers\n");
+			err = -ENOMEM;
+			goto err_out;
+		}
+
+		if (copy_from_user(dma_buf, u64_to_user_ptr(msg->payload),
+				   dma_buf_size)) {
+			dev_dbg(dev, "Failed to copy payload from user\n");
+			err = -EFAULT;
+			goto err_out;
+		}
+
+		rc = bnxt_fw_setup_input_dma(bnxtctl, dev, msg->num_dma,
+					     dma_buf, &rpc_in);
+		if (rc) {
+			err = -EIO;
+			goto err_out;
+		}
+	}
+
+	rc = bnxt_send_msg(bnxt_aux_priv->edev, &rpc_in);
+	if (rc) {
+		err = -EIO;
+		goto err_out;
+	}
+
+	for (i = 0; i < msg->num_dma; i++) {
+		if (dma_buf[i].read_from_device) {
+			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;
+			}
+		}
+	}
+	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]);
+
+err_out:
+	kfree(dma_buf);
+	kfree(rpc_in.msg);
+
+	if (err)
+		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/include/uapi/fwctl/bnxt.h b/include/uapi/fwctl/bnxt.h
new file mode 100644
index 000000000000..cf8f2b80f3de
--- /dev/null
+++ b/include/uapi/fwctl/bnxt.h
@@ -0,0 +1,63 @@
+/* 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
+
+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
+ * @read_from_device: DMA direction, 0 or 1
+ * @unused: pad
+ */
+struct fwctl_dma_info_bnxt {
+	__aligned_u64 data;
+	__u32 len;
+	__u16 offset;
+	__u8 read_from_device;
+	__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

Re: [PATCH net-next v2 5/6] bnxt_fwctl: Add bnxt fwctl device

[Jonathan Cameron]

On Tue, 23 Sep 2025 02:58:24 -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.
> 
> Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
> Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

I'm failing to find where this driver applies the fwctl_rpc_scope
to commands issued.  I suppose maybe they are all entirely safe
non invasive requests for data?

That scope stuff is probably the most important thing that fwctl
provides so all drivers need to deal with it.

Thanks,

Jonathan

> --- /dev/null
> +++ b/drivers/fwctl/bnxt/main.c
> @@ -0,0 +1,297 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025, Broadcom Corporation
> + *
This blank line doesn't add anything.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/slab.h>
> +#include <linux/pci.h>

Is there anything pci specific in here?  I'd check all the includes
to ensure they follow (approx) include what you used iwyu principles.

> +#include <linux/fwctl.h>
> +#include <uapi/fwctl/fwctl.h>
> +#include <uapi/fwctl/bnxt.h>
> +#include <linux/bnxt/common.h>
> +#include <linux/bnxt/ulp.h>

> +static bool bnxtctl_validate_rpc(struct bnxt_en_dev *edev,
> +				 struct bnxt_fw_msg *hwrm_in)
> +{
> +	struct input *req = (struct input *)hwrm_in->msg;

> +
> +	mutex_lock(&edev->en_dev_lock);

Neater if you use guard() 

> +	if (edev->flags & BNXT_EN_FLAG_ULP_STOPPED) {
> +		mutex_unlock(&edev->en_dev_lock);
> +		return false;
> +	}
> +	mutex_unlock(&edev->en_dev_lock);
> +
> +	if (le16_to_cpu(req->req_type) <= HWRM_LAST)
> +		return true;
> +
> +	return false;

I was kind of expecting something called validate_rpc to do
the scope checks that we see in other drivers.
e.g. mlx5ctl_validate_rpc()

> +}

> +
> +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;
> +	int dma_buf_size;
> +
> +	rpc_in.msg = kzalloc(msg->req_len, GFP_KERNEL);
> +	if (!rpc_in.msg) {
> +		err = -ENOMEM;
> +		goto err_out;

Nothing to clean up at this point, so returning here would be simpler.

> +	}
> +	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 err_out;
> +	}
> +
> +	if (!bnxtctl_validate_rpc(bnxt_aux_priv->edev, &rpc_in))
> +		return ERR_PTR(-EPERM);
> +
> +	rpc_in.msg_len = msg->req_len;
> +	rpc_in.resp = kzalloc(*out_len, GFP_KERNEL);
> +	if (!rpc_in.resp) {
> +		err = -ENOMEM;
> +		goto err_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 err_out;
> +		}
> +		dma_buf_size = msg->num_dma * sizeof(*dma_buf);

kcalloc probably more appropriate given it looks like an array.

> +		dma_buf = kzalloc(dma_buf_size, GFP_KERNEL);
> +		if (!dma_buf) {
> +			dev_err(dev, "Failed to allocate dma buffers\n");
> +			err = -ENOMEM;

General (growing) convention is don't bother printing messages on memory
failure as the allocator is very noisy if this happen away.

> +			goto err_out;
> +		}
> +
> +		if (copy_from_user(dma_buf, u64_to_user_ptr(msg->payload),
> +				   dma_buf_size)) {
> +			dev_dbg(dev, "Failed to copy payload from user\n");
> +			err = -EFAULT;
> +			goto err_out;
> +		}
> +
> +		rc = bnxt_fw_setup_input_dma(bnxtctl, dev, msg->num_dma,
> +					     dma_buf, &rpc_in);
> +		if (rc) {
> +			err = -EIO;
> +			goto err_out;
> +		}
> +	}
> +
> +	rc = bnxt_send_msg(bnxt_aux_priv->edev, &rpc_in);
> +	if (rc) {
> +		err = -EIO;
> +		goto err_out;
> +	}
> +
> +	for (i = 0; i < msg->num_dma; i++) {
> +		if (dma_buf[i].read_from_device) {
> +			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;
> +			}
> +		}
> +	}
> +	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]);
> +
> +err_out:
> +	kfree(dma_buf);
> +	kfree(rpc_in.msg);
> +
> +	if (err)
> +		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 const struct auxiliary_device_id bnxtctl_id_table[] = {
> +	{ .name = "bnxt_en.fwctl", },
> +	{},

No need for trailing comma.

> +};
> +MODULE_DEVICE_TABLE(auxiliary, bnxtctl_id_table);

> diff --git a/include/uapi/fwctl/bnxt.h b/include/uapi/fwctl/bnxt.h
> new file mode 100644
> index 000000000000..cf8f2b80f3de
> --- /dev/null
> +++ b/include/uapi/fwctl/bnxt.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (c) 2025, Broadcom Corporation
> + *

Trivial, blank line here adds nothing useful.

> + */
> +
> +#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

Re: [PATCH net-next v2 5/6] bnxt_fwctl: Add bnxt fwctl device

[Pavan Chebbi]

On Tue, Sep 23, 2025 at 4:47 PM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> I was kind of expecting something called validate_rpc to do
> the scope checks that we see in other drivers.
> e.g. mlx5ctl_validate_rpc()
>
Right, skipped applying scope with the intention to support all the commands,
assuming good faith in the sender. Thanks for your comment, I realize besides
missing to implement an important fwctl construct, it also does make me appear
lazy, to segregate commands from a really large list. But I will do it in v3.
Thanks for pointing out.

Re: [External] : [PATCH net-next v2 5/6] bnxt_fwctl: Add bnxt fwctl device

[ALOK TIWARI]

On 9/23/2025 3:28 PM, Pavan Chebbi wrote:
> +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++) {

The driver caps num_dma at 10 and rejects values greater than that.
It would be clearer and more consistent to use u8 num_dma here instead 
of int.

> +		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

Thanks,
Alok

Re: [PATCH net-next v2 5/6] bnxt_fwctl: Add bnxt fwctl device

[Dave Jiang]

On 9/23/25 2:58 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.
> 
> 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   | 297 ++++++++++++++++++++++++++++++++++++
>  include/uapi/fwctl/bnxt.h   |  63 ++++++++
>  include/uapi/fwctl/fwctl.h  |   1 +
>  7 files changed, 383 insertions(+)
>  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 a8a770714101..8954da3e9203 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10115,6 +10115,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..1bec4567e35c
> --- /dev/null
> +++ b/drivers/fwctl/bnxt/main.c
> @@ -0,0 +1,297 @@
> +// 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/common.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)
> +{
> +	struct input *req = (struct input *)hwrm_in->msg;
> +
> +	mutex_lock(&edev->en_dev_lock);
> +	if (edev->flags & BNXT_EN_FLAG_ULP_STOPPED) {
> +		mutex_unlock(&edev->en_dev_lock);
> +		return false;
> +	}
> +	mutex_unlock(&edev->en_dev_lock);
> +
> +	if (le16_to_cpu(req->req_type) <= HWRM_LAST)
> +		return true;
> +
> +	return false;

guard(mutex)(&edev->en_dev_lock);
if (edev->flags & BNXT_EN_FLAG_ULP_STOPPED)
	return false;

return le16_to_cpu(req->req_type) <= HWRM_LAST;


> +}
> +
> +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->read_from_device)) {

unnecessary () around msg->read_from_device?

> +			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 rc;

return 0 should be ok?

> +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;
> +	int dma_buf_size;
> +
> +	rpc_in.msg = kzalloc(msg->req_len, GFP_KERNEL);

I think if you use __free(kfree) for all the allocations in the function, you can be rid of the gotos.

> +	if (!rpc_in.msg) {
> +		err = -ENOMEM;
> +		goto err_out;

nothing to free here, no need to go to err_out

> +	}
> +	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 err_out;
> +	}
> +
> +	if (!bnxtctl_validate_rpc(bnxt_aux_priv->edev, &rpc_in))
> +		return ERR_PTR(-EPERM);
> +
> +	rpc_in.msg_len = msg->req_len;
> +	rpc_in.resp = kzalloc(*out_len, GFP_KERNEL);

I think you missed freeing this buffer on error later on. __free() should help you avoid that.

> +	if (!rpc_in.resp) {
> +		err = -ENOMEM;
> +		goto err_out;

calling kfree(dma_buf) when unnecessary
> +	}
> +
> +	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 err_out;
calling kfree(dma_buf) when unnecessary
> +		}
> +		dma_buf_size = msg->num_dma * sizeof(*dma_buf);
> +		dma_buf = kzalloc(dma_buf_size, GFP_KERNEL);
> +		if (!dma_buf) {
> +			dev_err(dev, "Failed to allocate dma buffers\n");
> +			err = -ENOMEM;
> +			goto err_out;
calling kfree(dma_buf) when unnecessary
> +		}
> +
> +		if (copy_from_user(dma_buf, u64_to_user_ptr(msg->payload),
> +				   dma_buf_size)) {
> +			dev_dbg(dev, "Failed to copy payload from user\n");
> +			err = -EFAULT;
> +			goto err_out;
> +		}
> +
> +		rc = bnxt_fw_setup_input_dma(bnxtctl, dev, msg->num_dma,
> +					     dma_buf, &rpc_in);
> +		if (rc) {
> +			err = -EIO;
> +			goto err_out;
> +		}
> +	}
> +
> +	rc = bnxt_send_msg(bnxt_aux_priv->edev, &rpc_in);
> +	if (rc) {
> +		err = -EIO;
> +		goto err_out;

Do all the DMA buffers allocated from bnxt_fw_setup_input_dma() need to be freed here? Maybe DEFINE_FREE() a macro for bnxt_fw_setup_input_dma() might help since you are freeing everything at the end of the function anyways. 

> +	}
> +
> +	for (i = 0; i < msg->num_dma; i++) {
> +		if (dma_buf[i].read_from_device) {
> +			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;
should we break out of this loop on error?

> +			}
> +		}
> +	}
> +	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]);
> +
> +err_out:
> +	kfree(dma_buf);
> +	kfree(rpc_in.msg);
> +
> +	if (err)
> +		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/include/uapi/fwctl/bnxt.h b/include/uapi/fwctl/bnxt.h
> new file mode 100644
> index 000000000000..cf8f2b80f3de
> --- /dev/null
> +++ b/include/uapi/fwctl/bnxt.h
> @@ -0,0 +1,63 @@
> +/* 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
> +
> +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
> + * @read_from_device: DMA direction, 0 or 1
> + * @unused: pad
> + */
> +struct fwctl_dma_info_bnxt {
> +	__aligned_u64 data;
> +	__u32 len;
> +	__u16 offset;
> +	__u8 read_from_device;

variable name doesn't convey writing to device on value of 1. Maybe 'dma_direction'? Also create enum or define for DEVICE_READ and DEVICE_WRITE so it's obvoius when setting or checking the variable. Although you can always use 'enum dma_data_direction' if you don't mind changing the values.

> +	__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,
>  };
>  

Re: [PATCH net-next v2 5/6] bnxt_fwctl: Add bnxt fwctl device

[Pavan Chebbi]

On Thu, Sep 25, 2025 at 4:02 AM Dave Jiang <dave.jiang@intel.com> wrote:
>

> > +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;
> > +     int dma_buf_size;
> > +
> > +     rpc_in.msg = kzalloc(msg->req_len, GFP_KERNEL);
>
> I think if you use __free(kfree) for all the allocations in the function, you can be rid of the gotos.
>
Thanks Dave for the review. Would you be fine if I defer using scope
based cleanup for later?
I need some time to understand the mechanism better and correctly
define the macros as some
pointers holding the memory are members within a stack variable. I
will fix the goto/free issues
you highlighted in the next revision. I hope that is going to be OK?

Re: [PATCH net-next v2 5/6] bnxt_fwctl: Add bnxt fwctl device

[Dave Jiang]

On 9/24/25 9:31 PM, Pavan Chebbi wrote:
> On Thu, Sep 25, 2025 at 4:02 AM Dave Jiang <dave.jiang@intel.com> wrote:
>>
> 
>>> +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;
>>> +     int dma_buf_size;
>>> +
>>> +     rpc_in.msg = kzalloc(msg->req_len, GFP_KERNEL);
>>
>> I think if you use __free(kfree) for all the allocations in the function, you can be rid of the gotos.
>>
> Thanks Dave for the review. Would you be fine if I defer using scope
> based cleanup for later?
> I need some time to understand the mechanism better and correctly
> define the macros as some
> pointers holding the memory are members within a stack variable. I
> will fix the goto/free issues
> you highlighted in the next revision. I hope that is going to be OK?

Sure that is fine. The way things are done in this function makes things a bit tricky to do cleanup properly via the scope based cleanup. I might play with it a bit and see if I can come up with something. It looks like it needs a bit of refactoring to split some things out. Probably not a bad thing in the long run. 

Re: [PATCH net-next v2 5/6] bnxt_fwctl: Add bnxt fwctl device

[Dave Jiang]

On 9/25/25 8:46 AM, Dave Jiang wrote:
> 
> 
> On 9/24/25 9:31 PM, Pavan Chebbi wrote:
>> On Thu, Sep 25, 2025 at 4:02 AM Dave Jiang <dave.jiang@intel.com> wrote:
>>>
>>
>>>> +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;
>>>> +     int dma_buf_size;
>>>> +
>>>> +     rpc_in.msg = kzalloc(msg->req_len, GFP_KERNEL);
>>>
>>> I think if you use __free(kfree) for all the allocations in the function, you can be rid of the gotos.
>>>
>> Thanks Dave for the review. Would you be fine if I defer using scope
>> based cleanup for later?
>> I need some time to understand the mechanism better and correctly
>> define the macros as some
>> pointers holding the memory are members within a stack variable. I
>> will fix the goto/free issues
>> you highlighted in the next revision. I hope that is going to be OK?
> 
> Sure that is fine. The way things are done in this function makes things a bit tricky to do cleanup properly via the scope based cleanup. I might play with it a bit and see if I can come up with something. It looks like it needs a bit of refactoring to split some things out. Probably not a bad thing in the long run. 
> 

Here's a potential template for doing it with scoped based cleanup. It applies on top of this current patch. I only compile tested. There probably will be errors in the conversion. Feel free to use it.

---

diff --git a/drivers/fwctl/bnxt/main.c b/drivers/fwctl/bnxt/main.c
index 1bec4567e35c..714106fd1033 100644
--- a/drivers/fwctl/bnxt/main.c
+++ b/drivers/fwctl/bnxt/main.c
@@ -22,8 +22,6 @@ struct bnxtctl_uctx {
 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))
@@ -76,61 +74,133 @@ static bool bnxtctl_validate_rpc(struct bnxt_en_dev *edev,
 	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)
+struct bnxtctl_dma {
+	struct device *dev;
+	int num_dma;
+	void *dma_virt_addr[MAX_NUM_DMA_INDICATIONS];
+	dma_addr_t dma_addr[MAX_NUM_DMA_INDICATIONS];
+};
+
+struct dma_context {
+	struct bnxtctl_dma *dma;
+	struct fwctl_dma_info_bnxt *dma_info;
+};
+
+static void free_dma(struct bnxtctl_dma *dma)
+{
+	int i;
+
+	for (i = 0; i < dma->num_dma; i++) {
+		if (dma->dma_virt_addr[i])
+			dma_free_coherent(dma->dev, 0, dma->dma_virt_addr[i],
+					  dma->dma_addr[i]);
+	}
+	kfree(dma);
+}
+DEFINE_FREE(free_dma, struct bnxtctl_dma *, if (_T) free_dma(_T))
+
+static struct bnxtctl_dma *
+allocate_and_setup_dma_bufs(struct device *dev,
+			    struct fwctl_dma_info_bnxt *dma_info,
+			    int num_dma,
+			    struct bnxt_fw_msg *fw_msg)
 {
-	u8 i, num_allocated = 0;
 	void *dma_ptr;
-	int rc = 0;
+	int i;
 
+	struct bnxtctl_dma *dma __free(free_dma) =
+		kzalloc(sizeof(*dma), GFP_KERNEL);
+	if (!dma)
+		return ERR_PTR(-ENOMEM);
+
+	dma->dev = dev->parent;
 	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->read_from_device)) {
-			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;
+		__le64 *dmap;
 
-		if ((PTR_ALIGN(dma_ptr, 8) == dma_ptr) &&
-		    msg->offset < fw_msg->msg_len) {
-			__le64 *dmap = dma_ptr;
+		if (dma_info->len == 0 || dma_info->len > MAX_DMA_MEM_SIZE)
+			return ERR_PTR(-EINVAL);
 
-			*dmap = cpu_to_le64(bnxt_dev->dma_addr[i]);
-		} else {
-			rc = -EINVAL;
-			goto err;
+		dma->dma_virt_addr[i] =
+			dma_alloc_coherent(dma->dev, dma_info->len,
+					   &dma->dma_addr[i], GFP_KERNEL);
+		if (!dma->dma_virt_addr[i])
+			return ERR_PTR(-ENOMEM);
+
+		dma->num_dma++;
+		if (!(dma_info->read_from_device)) {
+			if (copy_from_user(dma->dma_virt_addr[i],
+					   u64_to_user_ptr(dma_info->data),
+					   dma_info->len))
+				return ERR_PTR(-EFAULT);
 		}
-		msg += 1;
+		dma_ptr = fw_msg->msg + dma_info->offset;
+
+		if (PTR_ALIGN(dma_ptr, 8) != dma_ptr ||
+		    dma_info->offset >= fw_msg->msg_len)
+			return ERR_PTR(-EINVAL);
+
+		dmap = dma_ptr;
+		*dmap = cpu_to_le64(dma->dma_addr[i]);
+		dma_info += 1;
+	}
+
+	return no_free_ptr(dma);
+}
+
+static void free_dma_context(struct dma_context *dma_ctx)
+{
+	if (dma_ctx->dma)
+		free_dma(dma_ctx->dma);
+	if (dma_ctx->dma_info)
+		kfree(dma_ctx->dma_info);
+	kfree(dma_ctx);
+}
+DEFINE_FREE(free_dma_ctx, struct dma_context *, if (_T) free_dma_context(_T))
+
+static struct dma_context *
+allocate_and_setup_dma_context(struct device *dev,
+			       struct fwctl_rpc_bnxt *rpc_msg,
+			       struct bnxt_fw_msg *fw_msg)
+{
+	int num_dma = rpc_msg->num_dma;
+	int dma_buf_size;
+
+	if (!num_dma)
+		return NULL;
+
+	struct dma_context *dma_ctx __free(free_dma_ctx) =
+		kzalloc(sizeof(*dma_ctx), GFP_KERNEL);
+	if (!dma_ctx)
+		return ERR_PTR(-ENOMEM);
+
+	if (num_dma > MAX_NUM_DMA_INDICATIONS) {
+		dev_err(dev, "DMA buffers exceed the number supported\n");
+		return ERR_PTR(-EINVAL);
 	}
 
-	return rc;
-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]);
+	dma_buf_size = num_dma * sizeof(struct fwctl_dma_info_bnxt);
+	struct fwctl_dma_info_bnxt *dma_info __free(kfree)
+		= kzalloc(dma_buf_size, GFP_KERNEL);
+	if (!dma_info) {
+		dev_err(dev, "Failed to allocate dma buffers\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	if (copy_from_user(dma_info, u64_to_user_ptr(rpc_msg->payload),
+			   dma_buf_size)) {
+		dev_dbg(dev, "Failed to copy payload from user\n");
+		return ERR_PTR(-EFAULT);
+	}
+
+	struct bnxtctl_dma *dma __free(free_dma) =
+		allocate_and_setup_dma_bufs(dev, dma_info, num_dma, fw_msg);
+	if (IS_ERR(dma))
+		return ERR_CAST(dma);
+
+	dma_ctx->dma_info = no_free_ptr(dma_info);
+	dma_ctx->dma = no_free_ptr(dma);
 
-	return rc;
+	return no_free_ptr(dma_ctx);
 }
 
 static void *bnxtctl_fw_rpc(struct fwctl_uctx *uctx,
@@ -140,34 +210,28 @@ static void *bnxtctl_fw_rpc(struct fwctl_uctx *uctx,
 	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;
-	int dma_buf_size;
+	int i, rc;
+
+	void *rpc_in_msg __free(kfree) = kzalloc(msg->req_len, GFP_KERNEL);
+	if (!rpc_in_msg)
+		return ERR_PTR(-ENOMEM);
 
-	rpc_in.msg = kzalloc(msg->req_len, GFP_KERNEL);
-	if (!rpc_in.msg) {
-		err = -ENOMEM;
-		goto err_out;
-	}
 	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 err_out;
+		return ERR_PTR(-EFAULT);
 	}
 
 	if (!bnxtctl_validate_rpc(bnxt_aux_priv->edev, &rpc_in))
 		return ERR_PTR(-EPERM);
 
 	rpc_in.msg_len = msg->req_len;
-	rpc_in.resp = kzalloc(*out_len, GFP_KERNEL);
-	if (!rpc_in.resp) {
-		err = -ENOMEM;
-		goto err_out;
-	}
+	void *rpc_in_resp __free(kfree) = kzalloc(*out_len, GFP_KERNEL);
+	if (!rpc_in_resp)
+		return ERR_PTR(-ENOMEM);
 
 	rpc_in.resp_max_len = *out_len;
 	if (!msg->timeout)
@@ -175,64 +239,37 @@ static void *bnxtctl_fw_rpc(struct fwctl_uctx *uctx,
 	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 err_out;
-		}
-		dma_buf_size = msg->num_dma * sizeof(*dma_buf);
-		dma_buf = kzalloc(dma_buf_size, GFP_KERNEL);
-		if (!dma_buf) {
-			dev_err(dev, "Failed to allocate dma buffers\n");
-			err = -ENOMEM;
-			goto err_out;
-		}
-
-		if (copy_from_user(dma_buf, u64_to_user_ptr(msg->payload),
-				   dma_buf_size)) {
-			dev_dbg(dev, "Failed to copy payload from user\n");
-			err = -EFAULT;
-			goto err_out;
-		}
-
-		rc = bnxt_fw_setup_input_dma(bnxtctl, dev, msg->num_dma,
-					     dma_buf, &rpc_in);
-		if (rc) {
-			err = -EIO;
-			goto err_out;
-		}
-	}
+	struct dma_context *dma_ctx __free(free_dma_ctx) =
+		allocate_and_setup_dma_context(dev, msg, &rpc_in);
+	if (IS_ERR(dma_ctx))
+		return ERR_CAST(dma_ctx);
 
+	rpc_in.msg = rpc_in_msg;
+	rpc_in.resp = rpc_in_resp;
 	rc = bnxt_send_msg(bnxt_aux_priv->edev, &rpc_in);
-	if (rc) {
-		err = -EIO;
-		goto err_out;
-	}
+	if (rc)
+		return ERR_PTR(rc);
+
+	if (!dma_ctx)
+		return no_free_ptr(rpc_in_resp);
 
 	for (i = 0; i < msg->num_dma; i++) {
-		if (dma_buf[i].read_from_device) {
-			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;
-			}
+		struct fwctl_dma_info_bnxt *dma_info =
+			&dma_ctx->dma_info[i];
+		struct bnxtctl_dma *dma = dma_ctx->dma;
+
+		if (!dma_info->read_from_device)
+			continue;
+
+		if (copy_to_user(u64_to_user_ptr(dma_info->data),
+				 dma->dma_virt_addr[i],
+				 dma_info->len)) {
+			dev_dbg(dev, "Failed to copy resp to user\n");
+			return ERR_PTR(-EFAULT);
 		}
 	}
-	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]);
 
-err_out:
-	kfree(dma_buf);
-	kfree(rpc_in.msg);
-
-	if (err)
-		return ERR_PTR(err);
-
-	return rpc_in.resp;
+	return no_free_ptr(rpc_in_resp);
 }
 
 static const struct fwctl_ops bnxtctl_ops = {

Re: [PATCH net-next v2 5/6] bnxt_fwctl: Add bnxt fwctl device

[Pavan Chebbi]

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

On Thu, Sep 25, 2025 at 11:47 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
>
> Here's a potential template for doing it with scoped based cleanup. It applies on top of this current patch. I only compile tested. There probably will be errors in the conversion. Feel free to use it.

Thanks Dave. I will probably build on top of this.
BTW, I was wondering how could you use scoped free for response and
dma until I saw return no_free_ptr() for them :)
Anyway I will send the v3 as said previously and follow it up with
scoped cleanups later.

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

[PATCH net-next v2 6/6] bnxt_fwctl: Add documentation entries

[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        | 27 +++++++++++++++++++
 Documentation/userspace-api/fwctl/fwctl.rst   |  1 +
 Documentation/userspace-api/fwctl/index.rst   |  1 +
 3 files changed, 29 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..78f24004af02
--- /dev/null
+++ b/Documentation/userspace-api/fwctl/bnxt_fwctl.rst
@@ -0,0 +1,27 @@
+.. 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 ULP conduit provided by bnxt to send requests (HWRM commands)
+to firmware.
+
+bnxt_fwctl User API
+==================
+
+Each RPC request contains a message request structure (HWRM input) its,
+legth, 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.
diff --git a/Documentation/userspace-api/fwctl/fwctl.rst b/Documentation/userspace-api/fwctl/fwctl.rst
index a74eab8d14c6..e9f345797ca0 100644
--- a/Documentation/userspace-api/fwctl/fwctl.rst
+++ b/Documentation/userspace-api/fwctl/fwctl.rst
@@ -151,6 +151,7 @@ fwctl User API
 .. kernel-doc:: include/uapi/fwctl/fwctl.h
 .. kernel-doc:: include/uapi/fwctl/mlx5.h
 .. kernel-doc:: include/uapi/fwctl/pds.h
+.. kernel-doc:: include/uapi/fwctl/bnxt.h
 
 sysfs Class
 -----------
diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
index 316ac456ad3b..c0630d27afeb 100644
--- a/Documentation/userspace-api/fwctl/index.rst
+++ b/Documentation/userspace-api/fwctl/index.rst
@@ -12,3 +12,4 @@ to securely construct and execute RPCs inside device firmware.
    fwctl
    fwctl-cxl
    pds_fwctl
+   bnxt_fwctl
-- 
2.39.1

Re: [PATCH net-next v2 6/6] bnxt_fwctl: Add documentation entries

[Jonathan Cameron]

On Tue, 23 Sep 2025 02:58:25 -0700
Pavan Chebbi <pavan.chebbi@broadcom.com> 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>
> ---
>  .../userspace-api/fwctl/bnxt_fwctl.rst        | 27 +++++++++++++++++++
>  Documentation/userspace-api/fwctl/fwctl.rst   |  1 +
>  Documentation/userspace-api/fwctl/index.rst   |  1 +
>  3 files changed, 29 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..78f24004af02
> --- /dev/null
> +++ b/Documentation/userspace-api/fwctl/bnxt_fwctl.rst
> @@ -0,0 +1,27 @@
> +.. 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 ULP conduit provided by bnxt to send requests (HWRM commands)
> +to firmware.

It would be nice to have a little detail on what 'sort' of commands
are available even if only in a hand wavy way.

> +
> +bnxt_fwctl User API
> +==================
> +
> +Each RPC request contains a message request structure (HWRM input) its,
> +legth, optional request timeout, and dma buffers' information if the

length

> +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.
> diff --git a/Documentation/userspace-api/fwctl/fwctl.rst b/Documentation/userspace-api/fwctl/fwctl.rst
> index a74eab8d14c6..e9f345797ca0 100644
> --- a/Documentation/userspace-api/fwctl/fwctl.rst
> +++ b/Documentation/userspace-api/fwctl/fwctl.rst
> @@ -151,6 +151,7 @@ fwctl User API
>  .. kernel-doc:: include/uapi/fwctl/fwctl.h
>  .. kernel-doc:: include/uapi/fwctl/mlx5.h
>  .. kernel-doc:: include/uapi/fwctl/pds.h
> +.. kernel-doc:: include/uapi/fwctl/bnxt.h

As below.

>  
>  sysfs Class
>  -----------
> diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
> index 316ac456ad3b..c0630d27afeb 100644
> --- a/Documentation/userspace-api/fwctl/index.rst
> +++ b/Documentation/userspace-api/fwctl/index.rst
> @@ -12,3 +12,4 @@ to securely construct and execute RPCs inside device firmware.
>     fwctl
>     fwctl-cxl
>     pds_fwctl
> +   bnxt_fwctl
Perhaps we should keep the specific driver bits of this in alphabetical order?  Once we have
a bit list that will make it easier for people to find what they want.

Re: [PATCH net-next v2 6/6] bnxt_fwctl: Add documentation entries

[Dave Jiang]

On 9/23/25 2:58 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>
> ---
>  .../userspace-api/fwctl/bnxt_fwctl.rst        | 27 +++++++++++++++++++
>  Documentation/userspace-api/fwctl/fwctl.rst   |  1 +
>  Documentation/userspace-api/fwctl/index.rst   |  1 +
>  3 files changed, 29 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..78f24004af02
> --- /dev/null
> +++ b/Documentation/userspace-api/fwctl/bnxt_fwctl.rst
> @@ -0,0 +1,27 @@
> +.. 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 ULP conduit provided by bnxt to send requests (HWRM commands)

Probably should expand the TLA on first time usage in documentation. What's ULP? What's HWRM?

> +to firmware.
> +
> +bnxt_fwctl User API
> +==================
> +
> +Each RPC request contains a message request structure (HWRM input) its,

s/) its,/), its /

> +legth, optional request timeout, and dma buffers' information if the

s/legth/length/

> +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.

Would be helpful to explain the ioctls provided and perhaps some sample code for user side.

> diff --git a/Documentation/userspace-api/fwctl/fwctl.rst b/Documentation/userspace-api/fwctl/fwctl.rst
> index a74eab8d14c6..e9f345797ca0 100644
> --- a/Documentation/userspace-api/fwctl/fwctl.rst
> +++ b/Documentation/userspace-api/fwctl/fwctl.rst
> @@ -151,6 +151,7 @@ fwctl User API
>  .. kernel-doc:: include/uapi/fwctl/fwctl.h
>  .. kernel-doc:: include/uapi/fwctl/mlx5.h
>  .. kernel-doc:: include/uapi/fwctl/pds.h
> +.. kernel-doc:: include/uapi/fwctl/bnxt.h
>  
>  sysfs Class
>  -----------
> diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
> index 316ac456ad3b..c0630d27afeb 100644
> --- a/Documentation/userspace-api/fwctl/index.rst
> +++ b/Documentation/userspace-api/fwctl/index.rst
> @@ -12,3 +12,4 @@ to securely construct and execute RPCs inside device firmware.
>     fwctl
>     fwctl-cxl
>     pds_fwctl
> +   bnxt_fwctl

Re: [PATCH net-next v2 0/6] bnxt_fwctl: fwctl for Broadcom Netxtreme devices

[Saeed Mahameed]

On 23 Sep 02:58, 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/
>
>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/
>
>Pavan Chebbi (6):
>  bnxt_en: Move common definitions to include/linux/bnxt/
>  bnxt_en: Refactor aux bus functions to be generic
>  bnxt_en: Make a lookup table for supported aux bus devices
>  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        |  27 ++
> 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                     | 297 ++++++++++++++++++
> 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     |  13 +-
> .../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 | 266 ++++++++++++----
> include/linux/bnxt/common.h                   |  20 ++
> .../bnxt_ulp.h => include/linux/bnxt/ulp.h    |  14 +-
> include/uapi/fwctl/bnxt.h                     |  63 ++++
> include/uapi/fwctl/fwctl.h                    |   1 +
> 22 files changed, 683 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
> create mode 100644 include/linux/bnxt/common.h
> rename drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h => include/linux/bnxt/ulp.h (90%)
> create mode 100644 include/uapi/fwctl/bnxt.h
>

We need to better plan the target tree, this series is touching 3
sub-systems, this is very dangerous this late in the release cycle.

To apply this I would recommend a side branch to be pulled into all
subsystems at once before merge window,

 From the patch planing it seems that:
1. First patch infra common code movement. (rdma + netdev)
2. 2nd..4th patches aux refactoring (netdev only)
3. final patch bnxt fwctl (fwctl only)



>-- 
>2.39.1
>
>