Thread (15 messages) 15 messages, 5 authors, 2026-01-15

Re: [PATCH net-next v5 2/5] bnxt_en: Refactor aux bus functions to be more generic

From: Leon Romanovsky <leon@kernel.org>
Date: 2026-01-15 11:00:44

On Thu, Jan 15, 2026 at 02:58:55PM +0530, Pavan Chebbi wrote:
On Sun, Oct 19, 2025 at 6:22 PM Leon Romanovsky [off-list ref] wrote:
quoted
On Tue, Oct 14, 2025 at 01:10:30AM -0700, Pavan Chebbi wrote:
quoted
Up until now there was only one auxiliary device that bnxt
created and that was for RoCE driver. bnxt fwctl is also
going to use an aux bus device that bnxt should create.
This requires some nomenclature changes and refactoring of
the existing bnxt aux dev functions.

Convert 'aux_priv' and 'edev' members of struct bnxt into
arrays where each element contains supported auxbus device's
data. Move struct bnxt_aux_priv from bnxt.h to ulp.h because
that is where it belongs. Make aux bus init/uninit/add/del
functions more generic which will accept aux device type as
a parameter. Make bnxt_ulp_start/stop functions (the only
other common functions applicable to any aux device) loop
through the aux devices to update their config and states.

Also, as an improvement in code, bnxt_register_dev() can skip
unnecessary dereferencing of edev from bp, instead use the
edev pointer from the function parameter.

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

Reviewed-by: Andy Gospodarek <redacted>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  29 ++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  13 +-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 238 ++++++++++--------
 include/linux/bnxt/ulp.h                      |  23 +-
 5 files changed, 181 insertions(+), 124 deletions(-)
<...>
quoted
-void bnxt_rdma_aux_device_uninit(struct bnxt *bp)
+void bnxt_aux_device_uninit(struct bnxt *bp, enum bnxt_auxdev_type idx)
 {
      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[idx])
              return;
<...>
quoted
-void bnxt_rdma_aux_device_del(struct bnxt *bp)
+void bnxt_aux_device_del(struct bnxt *bp, enum bnxt_auxdev_type idx)
 {
-     if (!bp->edev)
+     if (!bp->edev[idx])
              return;
You are not supposed to call these functions if you didn't initialize
auxdev for this idx first. Please don't use defensive programming style
for in-kernel API.
Sorry for late response, I started reworking the patches and wanted to
address this comment.
Without a map/list of active aux devs, we will have to check the
validity of the aux_priv or edev somewhere before. That won't change
much for the defensive programming concern.
To do away completely with these checks, I wish to handle this change
separately where we maintain bnxt's active auxdev by idx, in the newly
introduced struct bnxt_aux_device.
I hope that is fine.
I don't know. My preference is that you get things right from the
beginning. For reasons unknown to me, the Broadcom drivers are full of
random `if (.. == NULL)` checks, which makes the code hard to review, as
it's never clear whether the functions are re-entrant or not.

Thanks
quoted
Thanks
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help