Re: [PATCH v2 2/6] RDMA/bnxt_re: Use auxiliary driver interface

[Ajit Khaparde]

On Wed, Oct 26, 2022 at 2:24 AM Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> On Tue, Oct 25, 2022 at 10:31:06AM -0700, Ajit Khaparde wrote:
> > Use auxiliary driver interface for driver load, unload ROCE driver.
> > The driver does not need to register the interface using the netdev
> > notifier anymore. Removed the bnxt_re_dev_list which is not needed.
> > Currently probe, remove and shutdown ops have been implemented for
> > the auxiliary device.
> >
> > Signed-off-by: Ajit Khaparde <ajit.khaparde@xxxxxxxxxxxx>
> > Reviewed-by: Andy Gospodarek <andrew.gospodarek@xxxxxxxxxxxx>
> > ---
> >  drivers/infiniband/hw/bnxt_re/bnxt_re.h       |   9 +-
> >  drivers/infiniband/hw/bnxt_re/main.c          | 387 +++++++-----------
> >  drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  64 ---
> >  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c |  67 ++-
> >  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |   3 +
> >  5 files changed, 214 insertions(+), 316 deletions(-)
>
> <...>
>
> > -static struct bnxt_en_dev *bnxt_re_dev_probe(struct net_device *netdev)
> > +static struct bnxt_en_dev *bnxt_re_dev_probe(struct auxiliary_device *adev)
> >  {
> > -     struct bnxt_en_dev *en_dev;
> > +     struct bnxt_aux_dev *aux_dev =
> > +             container_of(adev, struct bnxt_aux_dev, aux_dev);
> > +     struct bnxt_en_dev *en_dev = NULL;
> >       struct pci_dev *pdev;
> >
> > -     en_dev = ((struct bnxt*)netdev_priv(netdev))->edev;
> > -     if (IS_ERR(en_dev))
> > -             return en_dev;
> > +     if (aux_dev)
> > +             en_dev = aux_dev->edev;
> > +
> > +     if (!en_dev)
> > +             return NULL;
>
> Thank you for working to convert this driver to auxiliary bus. I'm
> confident that it will be ready soon.
Thanks

>
> In order to effectively review this series, you need to structure
> patches in such way that you don't remove in patch X+1 code that you
> added in patch X.
Sure. Moving from v1 to v2, I surely ran into the situation and cleaned them.
I will clean up the rest in case I missed something.

>
> Also you should remove maze of redundant functions that do nothing, but
> just call to another function with useless checks.
ACK

>
> Auxiliary devices shouldn't be created if en_dev == NULL.
ACK

& Thanks

>
> Thanks

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature