lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 8 Nov 2015 19:31:35 -0500 From: Sinan Kaya <okaya@...eaurora.org> To: Andy Shevchenko <andy.shevchenko@...il.com> Cc: dmaengine <dmaengine@...r.kernel.org>, timur@...eaurora.org, cov@...eaurora.org, jcm@...hat.com, Andy Gross <agross@...eaurora.org>, linux-arm-msm@...r.kernel.org, linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>, Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>, Mark Rutland <mark.rutland@....com>, Ian Campbell <ijc+devicetree@...lion.org.uk>, Kumar Gala <galak@...eaurora.org>, Vinod Koul <vinod.koul@...el.com>, Dan Williams <dan.j.williams@...el.com>, devicetree <devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH V3 4/4] dma: add Qualcomm Technologies HIDMA channel driver On 11/8/2015 3:47 PM, Andy Shevchenko wrote: > On Sun, Nov 8, 2015 at 6:53 AM, Sinan Kaya <okaya@...eaurora.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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists