On 11/08/2023 05:48, Guo Mengqi wrote:
> This patch adds a driver for HiSilicon Ascend SDMA engine.
>
> The DMA controller can do transfers between device and memory
> or memory to memory. Currently, the controller only support
> single copy. Drives can pass a substreamid to the DMA engine,
> which will enable transfers in user-space addresses.
...
> +
> +static int sdma_device_probe(struct platform_device *pdev)
> +{
> + int ret;
> + struct device *dev;
> + struct sdma_dev *sdev;
> + struct sdma_hardware_info info;
> +
> + dev = &pdev->dev;
> +
> + if (!pdev->dev.bus) {
> + pr_debug("the sdma dev bus is NULL\n");
How is this possible?
> + return -EPROBE_DEFER;
> + }
> +
> + if (!pdev->dev.bus->iommu_ops) {
> + pr_debug("defer probe sdma device\n");
Anyway, no pr_xxx, but dev_xxx. Is it really, really possible?
> + return -EPROBE_DEFER;> + }
> +
> + sdev = devm_kzalloc(dev, sizeof(*sdev), GFP_KERNEL);
> + if (!sdev) {
> + pr_err("alloc sdma_device failed\n");
> + return -ENOMEM;
> + }
> +
> + sdev->pdev = pdev;
> + dev_set_drvdata(&pdev->dev, sdev);
Come on, you just stored pdev->dev under dev!
> +
> + ret = of_sdma_collect_info(pdev, &info);
> + if (ret < 0) {
> + pr_err("collect device info failed, %d\n", ret);
dev_err
Please work on this driver to start looking like other kernel drivers.
> + return ret;
> + }
> +
> + sdev->io_base = ioremap(info.base_addr, SDMA_IOMEM_SIZE);
> + if (!sdev->io_base) {
> + pr_err("ioremap failed\n");
> + ret = -ENOMEM;
> + return ret;
> + }
> +
> + /* Fill in dmaengine */
> + sdma_init_dma_device(&sdev->dma_dev, dev);
> +
> + ret = sdma_init_channels(sdev, &info);
> + if (ret < 0)
> + goto unmap_iobase;
> +
> + ret = iommu_dev_enable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
> + if (ret) {
> + pr_err("iommu failed to init iopf, %d\n", ret);
> + goto destroy_channels;
> + }
> +
> + ret = iommu_dev_enable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
> + if (ret) {
> + pr_err("iommu failed to init sva, %d\n", ret);
> + goto disable_iopf;
> + }
> +
> + sdev->streamid = pdev->dev.iommu->fwspec->ids[0];
> +
> + snprintf(sdev->name, SDMA_DEVICE_NAME_LENGTH_MAX, "sdma%d", sdev->idx);
> + pr_info("%s device probe success\n", sdev->name);
No, drop.
> +
> + ret = dma_async_device_register(&sdev->dma_dev);
> + if (ret) {
> + dev_err(dev, "failed to register slave DMA engine: %d\n", ret);
> + goto disable_sva;
> + }
> +
> + return 0;
> +
> +disable_sva:
> + iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
> +disable_iopf:
> + iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
> +destroy_channels:
> + sdma_destroy_channels(sdev);
> +unmap_iobase:
> + iounmap(sdev->io_base);
> + return ret;
> +}
> +
> +static int sdma_device_remove(struct platform_device *pdev)
> +{
> + struct sdma_dev *psdma_dev = dev_get_drvdata(&pdev->dev);
> +
> + dma_async_device_unregister(&psdma_dev->dma_dev);
> +
> + iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
> + iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
> +
> + sdma_destroy_channels(psdma_dev);
> +
> + iounmap(psdma_dev->io_base);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sdma_of_match[] = {
> + { .compatible = "hisilicon,sdma" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, sdma_of_match);
> +
> +static struct platform_driver sdma_driver = {
> + .probe = sdma_device_probe,
> + .remove = sdma_device_remove,
> + .driver = {
> + .name = SDMA_DEVICE_NAME,
> + .of_match_table = sdma_of_match,
> + },
> +};
> +
> +static int __init sdma_driver_init(void)
> +{
> + return platform_driver_register(&sdma_driver);
> +}
> +module_init(sdma_driver_init);
> +
> +static void sdma_driver_exit(void)
> +{
> + platform_driver_unregister(&sdma_driver);
> +}
> +module_exit(sdma_driver_exit);
module_platform_driver
Best regards,
Krzysztof
