linux-kernel - Re: [PATCH V3 0/2] Add support for Tegra210 ADMA

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 24 May 2016 12:19:54 +0100
From:	Jon Hunter <jonathanh@...dia.com>
To:	Vinod Koul <vinod.koul@...el.com>,
	Paul Gortmaker <paul.gortmaker@...driver.com>
CC:	Laxman Dewangan <ldewangan@...dia.com>,
	Stephen Warren <swarren@...dotorg.org>,
	Thierry Reding <thierry.reding@...il.com>,
	"Alexandre Courbot" <gnurou@...il.com>,
	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>, <dmaengine@...r.kernel.org>,
	<linux-tegra@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V3 0/2] Add support for Tegra210 ADMA

Hi Paul,

On 24/05/16 04:53, Vinod Koul wrote:
> On Fri, May 20, 2016 at 02:51:14PM -0400, Paul Gortmaker wrote:
>> On Fri, Oct 16, 2015 at 3:35 AM, Jon Hunter <jonathanh@...dia.com> wrote:
>>> Add support for the Tegra210 Audio DMA (ADMA) controller. This was
>>> originally distributed as an RFC [0] based upon the existing tegra
>>> APB-DMA driver. Since then the driver has been significantly
>>> re-worked to remove a lot of the unused/unnecessary functionality
>>> that was carried over from the APB-DMA. This version is no longer
>>> derived from the APB-DMA driver and has been updated to use the
>>> virt-dma helpers.
>>
>> Does this driver make sense to support modular use?  I ask because
>> it inherits the inconsistency of being a bool Kconfig but using some
>> modular infrastructure from the APB driver.
>>
>> Sometime ago  when I sent patches to dma/* to "demodularize" other
>> bools, the general consensus of reviewers/maintainers was that
>> (to paraphrase) "if it can be modular, it should be modular" -- in
>> order to keep the multi platform bzImage size as small as can be.
>>
>> I did a quick test with it tristate, but that fails as follows:
>>
>> ERROR: "pm_clk_add_clk" [drivers/dma/tegra210-adma.ko] undefined!
>> ERROR: "pm_clk_create" [drivers/dma/tegra210-adma.ko] undefined!
>> ERROR: "pm_clk_destroy" [drivers/dma/tegra210-adma.ko] undefined!
>> ERROR: "pm_clk_suspend" [drivers/dma/tegra210-adma.ko] undefined!
>> ERROR: "pm_clk_resume" [drivers/dma/tegra210-adma.ko] undefined!

Thanks for reporting.

>> ...since drivers/base/power/clock_ops.c does not currently have any of
>> those fcns as EXPORT_SYMBOL.  Not a show-stopper, but I don't
>> want to blindly export those fcns if it doesn't make sense.
> 
> Sure, patch is welcome!

I don't have strong feeling either way. I think that long-term it would
be nice to add EXPORTS because I could see a lot of drivers starting to
use these. However, seeing that the ADMA is currently the only one it is
fine to demodularise the ADMA driver for now. Let me know if you want me
to send a patch or not.

Cheers
Jon

-- 
nvpublic