Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1404689 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V3 0/2] Add support for Tegra210 ADMA Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-05-20 21:00 +0200
    Re: [PATCH V3 0/2] Add support for Tegra210 ADMA Vinod Koul <vinod.koul@intel.com> - 2016-05-24 05:50 +0200
      Re: [PATCH V3 0/2] Add support for Tegra210 ADMA Jon Hunter <jonathanh@nvidia.com> - 2016-05-24 13:30 +0200

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

On Fri, Oct 16, 2015 at 3:35 AM, Jon Hunter <jonathanh@nvidia.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!

...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.

Thanks,
Paul.
--

>
> V3 changes:
> - Updated DT binding per feedback from Mark and Stephen
> - Fixed up items mentioned by Vinod
>
> V2 changes:
> - Re-worked device-tree binding
>
> [0] https://lkml.org/lkml/2015/8/18/237
>
> Jon Hunter (2):
>   Documentation: DT: Add binding documentation for NVIDIA ADMA
>   dmaengine: tegra-adma: Add support for Tegra210 ADMA
>
>  .../devicetree/bindings/dma/tegra210-adma.txt      |  50 ++
>  drivers/dma/Kconfig                                |  13 +
>  drivers/dma/Makefile                               |   1 +
>  drivers/dma/tegra210-adma.c                        | 909 +++++++++++++++++++++
>  4 files changed, 973 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/tegra210-adma.txt
>  create mode 100644 drivers/dma/tegra210-adma.c
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1405815

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@nvidia.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!
> 
> ...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!

-- 
~Vinod

[toc] | [prev] | [next] | [standalone]

#1406088

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@nvidia.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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web