Re: [PATCH rdma-next] bnxt_re: Rely on Kconfig to keep module dependency

[Leon Romanovsky <leon@kernel.org>]

On Wed, Mar 24, 2021 at 12:07:59PM -0300, Jason Gunthorpe wrote:
> On Wed, Mar 24, 2021 at 04:25:24PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> > 
> > Instead of manually messing with parent driver module reference
> > counting, rely on "depends on" keyword to ensure that proper
> > probe/remove chain is performed.
> 
> ?? kconfig doesn't impact module ordering.

Yeah, I was fast with the typing.

> 
> To have a proper remove chain there should be a symbol reference from
> bnxt_re to whatever the other module is

Right, they have probe_ulp() calls or something.

> 
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> >  drivers/infiniband/hw/bnxt_re/Kconfig |  4 +---
> >  drivers/infiniband/hw/bnxt_re/main.c  | 20 +++++---------------
> >  2 files changed, 6 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/infiniband/hw/bnxt_re/Kconfig b/drivers/infiniband/hw/bnxt_re/Kconfig
> > index 0feac5132ce1..b4779a6cd565 100644
> > +++ b/drivers/infiniband/hw/bnxt_re/Kconfig
> > @@ -2,9 +2,7 @@
> >  config INFINIBAND_BNXT_RE
> >  	tristate "Broadcom Netxtreme HCA support"
> >  	depends on 64BIT
> > -	depends on ETHERNET && NETDEVICES && PCI && INET && DCB
> > -	select NET_VENDOR_BROADCOM
> > -	select BNXT
> > +	depends on ETHERNET && NETDEVICES && PCI && INET && DCB && BNXT
> 
> Though this is correct, BNXT is a 'tristate' so it should be
> referenced with depends on select.
> 
> > diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> > index fdb8c2478258..a81adb07e5d9 100644
> > +++ b/drivers/infiniband/hw/bnxt_re/main.c
> > @@ -561,13 +561,6 @@ static struct bnxt_re_dev *bnxt_re_from_netdev(struct net_device *netdev)
> >  	return container_of(ibdev, struct bnxt_re_dev, ibdev);
> >  }
> >  
> > -static void bnxt_re_dev_unprobe(struct net_device *netdev,
> > -				struct bnxt_en_dev *en_dev)
> > -{
> > -	dev_put(netdev);
> > -	module_put(en_dev->pdev->driver->driver.owner);
> > -}
> 
> And you are right to be wondering WTF is this
> 
> Jason