On 10/4/22 09:43, Ilpo Järvinen wrote:
On Tue, 4 Oct 2022, Lizhi Hou wrote:
On 10/4/22 01:18, Ilpo Järvinen wrote:
On Wed, 28 Sep 2022, Lizhi Hou wrote:
Add driver to enable PCIe board which uses XDMA (the DMA/Bridge Subsystem
for PCI Express). For example, Xilinx Alveo PCIe devices.
https://www.xilinx.com/products/boards-and-kits/alveo.html
The XDMA engine support up to 4 Host to Card (H2C) and 4 Card to Host
(C2H)
channels. Memory transfers are specified on a per-channel basis in
descriptor linked lists, which the DMA fetches from host memory and
processes. Events such as descriptor completion and errors are signaled
using interrupts. The hardware detail is provided by
https://docs.xilinx.com/r/en-US/pg195-pcie-dma/Introduction
This driver implements dmaengine APIs.
- probe the available DMA channels
- use dma_slave_map for channel lookup
- use virtual channel to manage dmaengine tx descriptors
- implement device_prep_slave_sg callback to handle host scatter
gather
list
- implement device_config to config device address for DMA transfer
Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
Signed-off-by: Sonal Santan <sonal.santan@xxxxxxx>
Signed-off-by: Max Zhen <max.zhen@xxxxxxx>
Signed-off-by: Brian Xu <brian.xu@xxxxxxx>
---
+ *chans = devm_kzalloc(&xdev->pdev->dev, sizeof(**chans) * (*chan_num),
+ GFP_KERNEL);
+ if (!*chans)
+ return -ENOMEM;
+
+ for (i = 0, j = 0; i < pdata->max_dma_channels; i++) {
+ ret = xdma_read_reg(xdev, base + i * XDMA_CHAN_STRIDE,
+ XDMA_CHAN_IDENTIFIER, &identifier);
+ if (ret) {
+ xdma_err(xdev, "failed to read channel id: %d", ret);
+ return ret;
+ }
Is it ok to not rollback the allocation in case of an error occurs?
In this loop, the failures are returned by read/write registers. The
read/write register failure indicates serious hardware issue and the hardware
may not be rollback in this situation.
What I meant is that you allocated memory above (to *chans, see above).
Shouldn't that memory be free in case the hw is not working before you
return the error from this function?
Check also the other returns below for the same problemx.
The memory does not need to be freed immediately. And it should not be
memory leak because devm_* is used.
Thanks,
Lizhi
+
+ if (j == *chan_num) {
+ xdma_err(xdev, "invalid channel number");
+ return -EIO;
+ }
Same here?
+
+ /* init channel structure and hardware */
+ (*chans)[j].xdev_hdl = xdev;
+ (*chans)[j].base = base + i * XDMA_CHAN_STRIDE;
+ (*chans)[j].dir = dir;
+
+ ret = xdma_channel_init(&(*chans)[j]);
+ if (ret)
+ return ret;
And here.
+ (*chans)[j].vchan.desc_free = xdma_free_desc;
+ vchan_init(&(*chans)[j].vchan, &xdev->dma_dev);
+
+ j++;
+ }
+
+ dev_info(&xdev->pdev->dev, "configured %d %s channels", j,
+ (dir == DMA_MEM_TO_DEV) ? "H2C" : "C2H");
+
+ return 0;
+}
