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-amitpraka...@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?
