Re: [PATCH v2] lib/dmadev: get DMA device using device ID

[fengchengwen <fengchengwen@huawei.com>]

Hi Thomas,


On 2024/2/9 0:25, Thomas Monjalon wrote:
> 19/12/2023 12:00, Amit Prakash Shukla:
>> +struct rte_dma_dev *
>> +rte_dma_pmd_get_dev_by_id(const int dev_id)
> const does not make sense here for an int parameter.


I think it could be OK with const even if the parameter is not pointer.

However, most DPDK APIs do not have const for simple types (e.g. 
int/uin16_t).

In this aspect, I think it's also OK to remove const of here for 
consistency.


>
>> +{
>> +	if (!rte_dma_is_valid(dev_id))
>> +		return NULL;
>> +
>> +	return &rte_dma_devices[dev_id];
>> +}
> [...]
>> +/**
>> + * @internal
>> + * Get the rte_dma_dev structure device pointer for the device id.
>> + *
>> + * @param dev_id
>> + *   Device ID value to select the device structure.
> This comment is not explanatory.
> What is an ID? Where does it come from?
> Where can we see such ID for DMA device?

This new API is used in the event-dma driver of cnxk [1]:

The rte_event_dma_adapter_vchan_add has parameter of dma_dev_id, and it then

invoke (*dev->dev_ops->dma_adapter_vchan_add)(dev, dma_dev_id, vchan, 
event),

at cnxk driver, this ops will check whether the DMA is 
cnxk_dmadev_pci_driver.

I think this is because the cnxk's event-and-dma implement has deep coupling

(because the cnxk's event device could interact with another vendor's 
dma device).


Maybe we should think of a better way to solve this kind of coupling 
problem.


Thanks


[1] 
https://patches.dpdk.org/project/dpdk/patch/20231208082835.2817601-3-amitprakashs@marvell.com/

>
>> + *
>> + * @return
>> + *   - rte_dma_dev structure pointer for the given device ID on success, NULL
>> + *   otherwise.
>> + */
>> +__rte_internal
>> +struct rte_dma_dev *rte_dma_pmd_get_dev_by_id(const int dev_id);
> Again, const does not make sense here.
>
> Chengwen, please can you comment this patch as you maintain dmadev?
>
>