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

[Thomas Monjalon <thomas@monjalon.net>]

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.