Re: [PATCH net-next v2 4/6] bnxt_en: Create an aux device for fwctl
From: Jonathan Cameron <jonathan.cameron@huawei.com>
Date: 2025-09-23 10:56:11
On Tue, 23 Sep 2025 02:58:23 -0700 Pavan Chebbi [off-list ref] 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 <redacted> Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Just a few minor comments in here.
quoted hunk ↗ jump to hunk
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);
+}quoted hunk ↗ jump to hunk
@@ -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.
} }
quoted hunk ↗ jump to hunk
@@ -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; }