Re: [RFC PATCH v5 17/27] qedn: Add qedn probe

[Leon Romanovsky <leon@kernel.org>]

On Wed, May 19, 2021 at 05:29:32PM +0300, Shai Malin wrote:
> On Wed, 19 May 2021 at 15:31, Leon Romanovsky wrote:
> > On Wed, May 19, 2021 at 02:13:30PM +0300, Shai Malin wrote:
> > > This patch introduces the functionality of loading and unloading
> > > physical function.
> > > qedn_probe() loads the offload device PF(physical function), and
> > > initialize the HW and the FW with the PF parameters using the
> > > HW ops->qed_nvmetcp_ops, which are similar to other "qed_*_ops" which
> > > are used by the qede, qedr, qedf and qedi device drivers.
> > > qedn_remove() unloads the offload device PF, re-initialize the HW and
> > > the FW with the PF parameters.
> > >
> > > The struct qedn_ctx is per PF container for PF-specific attributes and
> > > resources.
> > >
> > > Acked-by: Igor Russkikh <irusskikh@marvell.com>
> > > Signed-off-by: Dean Balandin <dbalandin@marvell.com>
> > > Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> > > Signed-off-by: Omkar Kulkarni <okulkarni@marvell.com>
> > > Signed-off-by: Michal Kalderon <mkalderon@marvell.com>
> > > Signed-off-by: Ariel Elior <aelior@marvell.com>
> > > Signed-off-by: Shai Malin <smalin@marvell.com>
> > > Reviewed-by: Hannes Reinecke <hare@suse.de>
> > > ---
> > >  drivers/nvme/hw/Kconfig          |   1 +
> > >  drivers/nvme/hw/qedn/qedn.h      |  35 +++++++
> > >  drivers/nvme/hw/qedn/qedn_main.c | 159 ++++++++++++++++++++++++++++++-
> > >  3 files changed, 190 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/nvme/hw/Kconfig b/drivers/nvme/hw/Kconfig
> > > index 374f1f9dbd3d..91b1bd6f07d8 100644
> > > --- a/drivers/nvme/hw/Kconfig
> > > +++ b/drivers/nvme/hw/Kconfig
> > > @@ -2,6 +2,7 @@
> > >  config NVME_QEDN
> > >       tristate "Marvell NVM Express over Fabrics TCP offload"
> > >       depends on NVME_TCP_OFFLOAD
> > > +     select QED_NVMETCP
> > >       help
> > >         This enables the Marvell NVMe TCP offload support (qedn).
> > >
> > > diff --git a/drivers/nvme/hw/qedn/qedn.h b/drivers/nvme/hw/qedn/qedn.h
> > > index bcd0748a10fd..f13073afbced 100644
> > > --- a/drivers/nvme/hw/qedn/qedn.h
> > > +++ b/drivers/nvme/hw/qedn/qedn.h
> > > @@ -6,14 +6,49 @@
> > >  #ifndef _QEDN_H_
> > >  #define _QEDN_H_
> > >
> > > +#include <linux/qed/qed_if.h>
> > > +#include <linux/qed/qed_nvmetcp_if.h>
> > > +
> > >  /* Driver includes */
> > >  #include "../../host/tcp-offload.h"
> > >
> > > +#define QEDN_MAJOR_VERSION           8
> > > +#define QEDN_MINOR_VERSION           62
> > > +#define QEDN_REVISION_VERSION                10
> > > +#define QEDN_ENGINEERING_VERSION     0
> > > +#define DRV_MODULE_VERSION __stringify(QEDE_MAJOR_VERSION) "."       \
> > > +             __stringify(QEDE_MINOR_VERSION) "."             \
> > > +             __stringify(QEDE_REVISION_VERSION) "."          \
> > > +             __stringify(QEDE_ENGINEERING_VERSION)
> > > +
> >
> > This driver module version is not used in this series and more
> > important the module version have no meaning in upstream at all
> > and the community strongly against addition of new such code.
> 
> Will be fixed.
> 
> >
> > >  #define QEDN_MODULE_NAME "qedn"
> >
> > And the general note, it will be great if you convert your probe/remove
> > flows to use auxiliary bus like other drivers that cross subsystems.
> 
> qedn is simply fitting in with the existing design of qed/qede/qedr/qedf/qedi.

I know.

> Changing the entire multi-protocol design to auxiliary bus is being studied.

It will be required to do at some point of time.

Thanks