On 2021/10/6 18:26, Thomas Monjalon wrote: > 24/09/2021 12:53, Chengwen Feng: >> The 'dmadevice' is a generic type of DMA device. > > Do you mean 'dmadev' ? > >> This patch introduce the 'dmadevice' device allocation APIs. >> >> The infrastructure is prepared to welcome drivers in drivers/dma/ > > Good > > [...] >> +The DMA library provides a DMA device framework for management and >> provisioning >> +of hardware and software DMA poll mode drivers, defining generic APIs which
[snip] > > [...] >> +++ 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(). > > [...] >> + * 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. 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). > >> + * >> + * @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. > >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice. >> + * >> + * Get the device identifier for the named DMA device. >> + * >> + * @param name >> + * DMA device name. >> + * >> + * @return >> + * Returns DMA device identifier on success. >> + * - <0: Failure to find named DMA device. >> + */ >> +__rte_experimental >> +int rte_dma_get_dev_id(const char *name); > > Should we add _by_name? > We could have a function to retrieve the ID by devargs as well. > >> +++ 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. [snip] > . > Thanks
