linux-kernel - Re: [PATCH fwctl 2/5] fwctl/bnxt_en: Refactor aux bus functions to be more generic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALs4sv1-rNOQvhUt28AZNabocx2q3z8np89M0-EyS5R=nOoSbg@mail.gmail.com>
Date: Sun, 25 Jan 2026 19:20:28 +0530
From: Pavan Chebbi <pavan.chebbi@...adcom.com>
To: Leon Romanovsky <leon@...nel.org>
Cc: jgg@...pe.ca, michael.chan@...adcom.com, linux-kernel@...r.kernel.org, 
	dave.jiang@...el.com, saeedm@...dia.com, Jonathan.Cameron@...wei.com, 
	gospo@...adcom.com, selvin.xavier@...adcom.com, 
	kalesh-anakkur.purayil@...adcom.com
Subject: Re: [PATCH fwctl 2/5] fwctl/bnxt_en: Refactor aux bus functions to be
 more generic

> > +     int                     auxdev_id;
> > +     atomic_t                auxdev_state[__BNXT_AUXDEV_MAX];
> > +#define      BNXT_ADEV_STATE_NONE    0
> > +#define      BNXT_ADEV_STATE_INIT    1
> > +#define      BNXT_ADEV_STATE_ADD     2
>
> This is indeed an incorrect use of atomic_t. This type only ensures that
> individual arithmetic operations are performed atomically. It does not
> eliminate the need for the locks that were removed.

Thanks for your review. Since the individual bp's auxdev state is
going to be set only during probe(), and is read-only elsewhere,
atomic operations are enough, no?

>
> >  };
> >
> >  #define BNXT_NUM_RX_RING_STATS                       8
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > index 8cad7b982664..064d7bc4ce8d 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > @@ -5100,7 +5100,7 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
>
> <...>
>
> >
> >  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;
> > +     int i;
> >
> > -     if (!edev || err)
>
> err is no longer used, and keeping it would alter the behavior of
> bnxt_resume() and related paths. In any case, callers should not invoke
> bnxt_ulp_start() when an error has already occurred.

Yes, this I can remove.

>
> Thanks
>
> > -             return;
> > +     for (i = 0; i < __BNXT_AUXDEV_MAX; i++) {
> > +             struct bnxt_aux_priv *aux_priv;
> > +             struct auxiliary_device *adev;
> > +             struct bnxt_en_dev *edev;

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5469 bytes)