lkml.org 
[lkml]   [2015]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver
From
Date


On 11/8/2015 3:47 PM, Andy Shevchenko wrote:
> On Sun, Nov 8, 2015 at 6:53 AM, Sinan Kaya <okaya@codeaurora.org> 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


\
 
 \ /
  Last update: 2015-11-09 02:01    [W:0.099 / U:1.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site