08/10/2021 09:13, fengchengwen: > On 2021/10/6 18:26, Thomas Monjalon wrote: > > 24/09/2021 12:53, Chengwen Feng: > >> +++ b/lib/dmadev/rte_dmadev.h > >> + * The dmadev are dynamically allocated by rte_dma_pmd_allocate() during > >> the > >> + * PCI/SoC device probing phase performed at EAL initialization time. And > >> could > >> + * be released by rte_dma_pmd_release() during the PCI/SoC device removing > >> + * phase. > > > > I don't think this text has value, > > and we could imagine allocating a device ata later stage. > > Yes, we could remove the stage descriptor because it's a well-known > knowledge, but I > recommend keeping the rte_dma_pmd_allocate and rte_dma_pmd_release functions, > how about: > > * The dmadev are dynamically allocated by rte_dma_pmd_allocate(). And could > * be released by rte_dma_pmd_release().
These functions are for PMD. This file is for applications, so it is not appropriate. > > [...] > >> + * Configure the maximum number of dmadevs. > >> + * @note This function can be invoked before the primary process > >> rte_eal_init() > >> + * to change the maximum number of dmadevs. > > > > You should mention what is the default. > > Is the default exported to the app in this file? > > The default macro is RTE_DMADEV_DEFAULT_MAX_DEVS, and I place it at > rte_config.h. No we avoid adding thinds in rte_config.h. There should a static default which can be changed at runtime only. > I think it's better to focus on one place (rte_config.h) than to modify > config in multiple places (e.g. rte_dmadev.h/rte_xxx.h). Config is modified only in one place: the function. > >> + * > >> + * @param dev_max > >> + * maximum number of dmadevs. > >> + * > >> + * @return > >> + * 0 on success. Otherwise negative value is returned. > >> + */ > >> +__rte_experimental > >> +int rte_dma_dev_max(size_t dev_max); > > > > What about a function able to do more with the name rte_dma_init? > > It should allocate the inter-process shared memory, > > and do the lookup in case of secondary process. > > Yes, we defined dma_data_prepare() which do above thing, it's in 4th patch. > > Because we could not invoke some like allocate inter-process shared memory > before > rte_eal_init, so I think it's better keep rte_dma_dev_max as it is. Good point. > >> +++ b/lib/dmadev/rte_dmadev_core.h > >> +/** > >> + * @file > >> + * > >> + * DMA Device internal header. > >> + * > >> + * This header contains internal data types, that are used by the DMA > >> devices > >> + * in order to expose their ops to the class. > >> + * > >> + * Applications should not use these API directly. > > > > If it is not part of the API, it should not be exposed at all. > > Why not having all these stuff in a file dmadev_driver.h? > > Is it used by some inline functions? > > Yes, it's used by dataplane inline functions. OK, please give this reason in the description.
