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

[Jon Hunter]

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@xxxxxxxxxx> 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