lkml.org 
[lkml]   [2023]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH V6 1/1] dmaengine: amd: qdma: Add AMD QDMA driver
From
Le 29/09/2023 à 19:24, Lizhi Hou a écrit :
> From: Nishad Saraf <nishads@amd.com>
>
> Adds driver to enable PCIe board which uses AMD QDMA (the Queue-based
> Direct Memory Access) subsystem. For example, Xilinx Alveo V70 AI
> Accelerator devices.
> https://www.xilinx.com/applications/data-center/v70.html
>
> The QDMA subsystem is used in conjunction with the PCI Express IP block
> to provide high performance data transfer between host memory and the
> card's DMA subsystem.
>

...

> +static int amd_qdma_probe(struct platform_device *pdev)
> +{
> + struct qdma_platdata *pdata = dev_get_platdata(&pdev->dev);
> + struct qdma_device *qdev;
> + struct resource *res;
> + void __iomem *regs;
> + int ret;
> +
> + qdev = devm_kzalloc(&pdev->dev, sizeof(*qdev), GFP_KERNEL);
> + if (!qdev)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, qdev);
> + qdev->pdev = pdev;
> + mutex_init(&qdev->ctxt_lock);

If this was done later, it could simplify some error handling below.

> +
> + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (!res) {
> + qdma_err(qdev, "Failed to get IRQ resource");
> + ret = -ENODEV;
> + goto failed;
> + }
> + qdev->err_irq_idx = pdata->irq_index;
> + qdev->queue_irq_start = res->start + 1;
> + qdev->queue_irq_num = res->end - res->start;
> +
> + regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> + if (IS_ERR(regs)) {
> + ret = PTR_ERR(regs);
> + qdma_err(qdev, "Failed to map IO resource, err %d", ret);
> + goto failed;
> + }
> +
> + qdev->regmap = devm_regmap_init_mmio(&pdev->dev, regs,
> + &qdma_regmap_config);
> + if (IS_ERR(qdev->regmap)) {
> + ret = PTR_ERR(qdev->regmap);
> + qdma_err(qdev, "Regmap init failed, err %d", ret);
> + goto failed;
> + }
> +
> + ret = qdma_device_verify(qdev);
> + if (ret)
> + goto failed;
> +
> + ret = qdma_get_hw_info(qdev);
> + if (ret)
> + goto failed;
> +
> + INIT_LIST_HEAD(&qdev->dma_dev.channels);
> +
> + ret = qdma_device_setup(qdev);
> + if (ret)
> + goto failed;
> +
> + ret = qdma_intr_init(qdev);
> + if (ret) {
> + qdma_err(qdev, "Failed to initialize IRQs %d", ret);
> + return ret;

Should it be "goto failed"?

> + }
> + qdev->status |= QDMA_DEV_STATUS_INTR_INIT;
> +
> + dma_cap_set(DMA_SLAVE, qdev->dma_dev.cap_mask);
> + dma_cap_set(DMA_PRIVATE, qdev->dma_dev.cap_mask);

...

> +struct qdma_device {
> + struct platform_device *pdev;
> + struct dma_device dma_dev;
> + struct regmap *regmap;
> + struct mutex ctxt_lock; /* protect ctxt registers */
> + const struct qdma_reg_field *rfields;
> + const struct qdma_reg *roffs;
> + struct qdma_queue *h2c_queues;
> + struct qdma_queue *c2h_queues;
> + struct qdma_intr_ring *qintr_rings;
> + u32 qintr_ring_num;
> + u32 qintr_ring_idx;
> + u32 chan_num;
> + u32 queue_irq_start;
> + u32 queue_irq_num;
> + u32 err_irq_idx;
> + u32 fid;
> + u32 status;

Using such a mechanism with this 'status' in the probe and
amd_qdma_remove(), apparently only to simplify the error handling of the
probe looks really unusual to me.

CJ

> +};

...

\
 
 \ /
  Last update: 2023-09-29 22:36    [W:0.047 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site