Re: [dpdk-dev] [PATCH v1] bus/auxiliary: introduce auxiliary bus

[Thomas Monjalon <thomas@monjalon.net>]

15/04/2021 09:55, Xueming(Steven) Li:
> From: Wang, Haiyue <haiyue.wang@intel.com>
> > From: Xueming(Steven) Li <xuemingl@nvidia.com>
> > > From: Wang, Haiyue <haiyue.wang@intel.com>
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Xueming Li
> > > > > +/**
> > > > > + * A structure describing an auxiliary driver.
> > > > > + */
> > > > > +struct rte_auxiliary_driver {
> > > > > +	TAILQ_ENTRY(rte_auxiliary_driver) next; /**< Next in list. */
> > > > > +	struct rte_driver driver;            /**< Inherit core driver. */
> > > > > +	struct rte_auxiliary_bus *bus;       /**< Auxiliary bus reference. */
> > > > > +	auxiliary_match_t *match;            /**< Device match function. */
> > > > > +	auxiliary_probe_t *probe;            /**< Device Probe function. */
> > > > > +	auxiliary_remove_t *remove;          /**< Device Remove function. */
> > > > > +	auxiliary_dma_map_t *dma_map;        /**< Device dma map function. */
> > > > > +	auxiliary_dma_unmap_t *dma_unmap;    /**< Device dma unmap function. */
> > > >
> > > > These API type can be pointer type defined, then no need "*":
> > > >
> > > > typedef int (*auxiliary_dma_unmap_t)(struct rte_auxiliary_device *dev,
> > > > 				    void *addr, uint64_t iova, size_t len);
> > > >
> > > > auxiliary_dma_unmap_t dma_unmap;
> > > >
> > > > Like:
> > > > https://patchwork.dpdk.org/project/dpdk/patch/20210331224547.2217759
> > > > -1-thomas@monjalon.net/
> > > >
> > > > typedef int (*rte_dev_dma_map_t)(struct rte_device *dev,
> > > > 				 void *addr, uint64_t iova, size_t len);
> > >
> > > Thanks, is there a reason to prefer pointer type?
> > 
> > Good practice to make code beautiful ? ;-)

Honestly, I am not sure which one is better,
having the pointer type hidden in the typedef or explicit?

> > > Thoma's patch looks good, will rebase on it once accepted.
> > 
> > I mean the function type is defined as pointer type.
> Yes, I'm talking another topic :)
> His patch makes dma map/unmap higher level to rte_bus, so that no need to define the api again here.

I think my patch will be abandoned.