Re: [dpdk-dev] [RFC PATCH 4/6] bus/mlx5_pci: add mlx5 PCI bus

[Parav Pandit <parav@mellanox.com>]

> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, June 17, 2020 1:44 PM
> To: Parav Pandit <parav@mellanox.com>; Gaëtan Rivet <grive@u256.net>
> Cc: dev@dpdk.org; ferruh.yigit@intel.com; Ori Kam <orika@mellanox.com>;
> Matan Azrad <matan@mellanox.com>
> Subject: Re: [dpdk-dev] [RFC PATCH 4/6] bus/mlx5_pci: add mlx5 PCI bus
> 
> 15/06/2020 23:00, Gaëtan Rivet:
> > On 10/06/20 17:17 +0000, Parav Pandit wrote:
> > > +# DEBUG which is usually provided on the command-line may enable #
> > > +CONFIG_RTE_LIBRTE_MLX5_DEBUG.
> > > +ifeq ($(DEBUG),1)
> > > +CONFIG_RTE_LIBRTE_MLX5_DEBUG := y
> > > +endif
> > > +
> > > +# User-defined CFLAGS.
> > > +ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DEBUG),y)
> > > +CFLAGS += -pedantic
> > > +ifneq ($(CONFIG_RTE_TOOLCHAIN_ICC),y) CFLAGS += -DPEDANTIC
> endif
> > > +AUTO_CONFIG_CFLAGS += -Wno-pedantic else CFLAGS += -UPEDANTIC
> endif
> > > +
> >
> > At this point why not define some
> > $(RTE_SDK)/drivers/common/mlx5/mlx5_common.mk
> >
> > That should be included by vdpa, mlx5, this one?
> > This would force-align flag behavior, this is becoming untidy.
> >
> > (Make is disappearing soon I heard, but still.)
> 
> Yes makefiles will be removed in 2 months.
> Please do not move makefiles at this point.
> 
> [...]
> > > +/**
> > > + * A structure describing a mlx5 pci driver.
> > > + */
> > > +struct rte_mlx5_pci_driver {
> >
> > A note on the namespace: rte_mlx5_pci seems heavy.
> > Do you expect other types of "super-driver", other than PCI?
> > Wouldn't rte_mlx5_driver be ok for example?
> >
> > > +	enum mlx5_class dev_class;    /**< Class of this driver */
> > > +	struct rte_driver driver;     /**< Inherit core driver. */
> > > +	pci_probe_t *probe;           /**< Class device probe function. */
> > > +	pci_remove_t *remove;         /**< Class device remove function. */
> > > +	pci_dma_map_t *dma_map;       /**< Class device dma map function.
> */
> > > +	pci_dma_unmap_t *dma_unmap;   /**< Class device dma unmap
> function. */
> > > +	TAILQ_ENTRY(rte_mlx5_pci_driver) next;
> > > +	const struct rte_pci_id *id_table; /**< ID table, NULL terminated.
> > > +*/
> >
> > At this point, why not inherit an rte_pci_driver instead of the core
> > rte_driver?
> 
> I agree we expect inheriting rte_pci_driver.
> 
Ok. I will attempt this.