Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






On 11/8/2015 3:47 PM, Andy Shevchenko wrote:
On Sun, Nov 8, 2015 at 6:53 AM, Sinan Kaya <okaya@xxxxxxxxxxxxxx> wrote:
This patch adds support for hidma engine. The driver
consists of two logical blocks. The DMA engine interface
and the low-level interface. The hardware only supports
memcpy/memset and this driver only support memcpy
interface. HW and driver doesn't support slave interface.

Make lines a bit longer.


OK

+       pm_runtime_set_autosuspend_delay(&pdev->dev, AUTOSUSPEND_TIMEOUT);
+       pm_runtime_use_autosuspend(&pdev->dev);
+       pm_runtime_set_active(&pdev->dev);
+       pm_runtime_enable(&pdev->dev);
+
+       trca_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);

+       if (!trca_resource) {
+               rc = -ENODEV;
+               goto bailout;
+       }

Why did you ignore my comment about this block?
Remove that condition entirely.

Removed these four lines above.

+
+       trca = devm_ioremap_resource(&pdev->dev, trca_resource);
+       if (IS_ERR(trca)) {
+               rc = -ENOMEM;
+               goto bailout;
+       }
+
+       evca_resource = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+       if (!evca_resource) {
+               rc = -ENODEV;
+               goto bailout;
+       }

Ditto.


done


+uninit:
+       hidma_debug_uninit(dmadev);
+       hidma_ll_uninit(dmadev->lldev);
+dmafree:
+       if (dmadev)
+               hidma_free(dmadev);
+bailout:
+       pm_runtime_disable(&pdev->dev);
+       pm_runtime_put_sync_suspend(&pdev->dev);

Are you sure this is appropriate sequence?

I think

pm_runtime_put();
pm_runtime_disable();

corrected, reordered and used pm_runtime_put_sync() instead.

will do the job.

+       return rc;
+}
+
+static int hidma_remove(struct platform_device *pdev)
+{
+       struct hidma_dev *dmadev = platform_get_drvdata(pdev);
+
+       dev_dbg(&pdev->dev, "removing\n");

Useless message.

Removed.

+       pm_runtime_get_sync(dmadev->ddev.dev);
+
+       dma_async_device_unregister(&dmadev->ddev);
+       hidma_debug_uninit(dmadev);
+       hidma_ll_uninit(dmadev->lldev);
+       hidma_free(dmadev);
+
+       dev_info(&pdev->dev, "HI-DMA engine removed\n");
+       pm_runtime_put_sync_suspend(&pdev->dev);
+       pm_runtime_disable(&pdev->dev);
+
+       return 0;
+}



--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux