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

Groups > linux.kernel > #1436008 > unrolled thread

[PATCH 3/4] bus: tegra-aconnect: make it explicitly non-modular

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

  [PATCH 3/4] bus: tegra-aconnect: make it explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-07-03 19:40 +0200
    Re: [PATCH 3/4] bus: tegra-aconnect: make it explicitly non-modular Jon Hunter <jonathanh@nvidia.com> - 2016-07-04 11:20 +0200
      Re: [PATCH 3/4] bus: tegra-aconnect: make it explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-07-04 15:50 +0200
        Re: [PATCH 3/4] bus: tegra-aconnect: make it explicitly non-modular Jon Hunter <jonathanh@nvidia.com> - 2016-07-04 15:50 +0200

#1436008 — [PATCH 3/4] bus: tegra-aconnect: make it explicitly non-modular

The Kconfig currently controlling compilation of this code is:

drivers/bus/Kconfig:config TEGRA_ACONNECT
drivers/bus/Kconfig:    bool "Tegra ACONNECT Bus Driver"

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

We explicitly disallow a driver unbind, since that doesn't have a
sensible use case anyway, and it allows us to drop the ".remove"
code for non-modular drivers.

Since module_platform_driver() uses the same init level priority as
builtin_platform_driver() the init ordering remains unchanged with
this commit.

Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.

We also delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.

Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: linux-tegra@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/bus/tegra-aconnect.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/bus/tegra-aconnect.c b/drivers/bus/tegra-aconnect.c
index 7e4104b74fa8..f3982946ab8a 100644
--- a/drivers/bus/tegra-aconnect.c
+++ b/drivers/bus/tegra-aconnect.c
@@ -1,6 +1,8 @@
 /*
  * Tegra ACONNECT Bus Driver
  *
+ * Author: Jon Hunter <jonathanh@nvidia.com>
+ *
  * Copyright (C) 2016, NVIDIA CORPORATION.  All rights reserved.
  *
  * This file is subject to the terms and conditions of the GNU General Public
@@ -9,7 +11,7 @@
  */
 
 #include <linux/clk.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_clock.h>
@@ -66,15 +68,6 @@ clk_destroy:
 	return ret;
 }
 
-static int tegra_aconnect_remove(struct platform_device *pdev)
-{
-	pm_runtime_disable(&pdev->dev);
-
-	pm_clk_destroy(&pdev->dev);
-
-	return 0;
-}
-
 static int tegra_aconnect_runtime_resume(struct device *dev)
 {
 	return pm_clk_resume(dev);
@@ -94,19 +87,14 @@ static const struct of_device_id tegra_aconnect_of_match[] = {
 	{ .compatible = "nvidia,tegra210-aconnect", },
 	{ }
 };
-MODULE_DEVICE_TABLE(of, tegra_aconnect_of_match);
 
 static struct platform_driver tegra_aconnect_driver = {
 	.probe = tegra_aconnect_probe,
-	.remove = tegra_aconnect_remove,
 	.driver = {
 		.name = "tegra-aconnect",
+		.suppress_bind_attrs = true,
 		.of_match_table = tegra_aconnect_of_match,
 		.pm = &tegra_aconnect_pm_ops,
 	},
 };
-module_platform_driver(tegra_aconnect_driver);
-
-MODULE_DESCRIPTION("NVIDIA Tegra ACONNECT Bus Driver");
-MODULE_AUTHOR("Jon Hunter <jonathanh@nvidia.com>");
-MODULE_LICENSE("GPL v2");
+builtin_platform_driver(tegra_aconnect_driver);
-- 
2.8.4

[toc] | [next] | [standalone]

#1436360

Hi Paul,

On 03/07/16 18:30, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
> 
> drivers/bus/Kconfig:config TEGRA_ACONNECT
> drivers/bus/Kconfig:    bool "Tegra ACONNECT Bus Driver"
> 
> ...meaning that it currently is not being built as a module by anyone.
> 
> Lets remove the modular code that is essentially orphaned, so that
> when reading the driver there is no doubt it is builtin-only.
> 
> We explicitly disallow a driver unbind, since that doesn't have a
> sensible use case anyway, and it allows us to drop the ".remove"
> code for non-modular drivers.
> 
> Since module_platform_driver() uses the same init level priority as
> builtin_platform_driver() the init ordering remains unchanged with
> this commit.
> 
> Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
> 
> We also delete the MODULE_LICENSE tag etc. since all that information
> was (or is now) contained at the top of the file in the comments.

In version 3 of the aconnect series [0] I had made this a tristate
because we allowed it to be removed and you had submitted a patch to
export the PM_CLK APIs. However, when discussing with Thierry he said
that we were unable to merge with tristate because of the dependency on
your patch. So he suggested we merge with bool for now and then change
it back to tristate for v4.9.

I understand that we should not do this, but we do plan to make this
modular in the future.

Cheers
Jon

[0] http://marc.info/?l=linux-tegra&m=146616753627760&w=2

-- 
nvpublic

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

#1436515

[Re: [PATCH 3/4] bus: tegra-aconnect: make it explicitly non-modular] On 04/07/2016 (Mon 10:17) Jon Hunter wrote:

> Hi Paul,
> 
> On 03/07/16 18:30, Paul Gortmaker wrote:
> > The Kconfig currently controlling compilation of this code is:
> > 
> > drivers/bus/Kconfig:config TEGRA_ACONNECT
> > drivers/bus/Kconfig:    bool "Tegra ACONNECT Bus Driver"
> > 
> > ...meaning that it currently is not being built as a module by anyone.
> > 
> > Lets remove the modular code that is essentially orphaned, so that
> > when reading the driver there is no doubt it is builtin-only.
> > 
> > We explicitly disallow a driver unbind, since that doesn't have a
> > sensible use case anyway, and it allows us to drop the ".remove"
> > code for non-modular drivers.
> > 
> > Since module_platform_driver() uses the same init level priority as
> > builtin_platform_driver() the init ordering remains unchanged with
> > this commit.
> > 
> > Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
> > 
> > We also delete the MODULE_LICENSE tag etc. since all that information
> > was (or is now) contained at the top of the file in the comments.
> 
> In version 3 of the aconnect series [0] I had made this a tristate
> because we allowed it to be removed and you had submitted a patch to
> export the PM_CLK APIs. However, when discussing with Thierry he said
> that we were unable to merge with tristate because of the dependency on
> your patch. So he suggested we merge with bool for now and then change
> it back to tristate for v4.9.

Oh I see.  The exported clock syms were for the 210 DMA driver and it
never crossed my mind that this driver needed the same syms in order for
it to be tristate.  Guess the exports proved useful afterall.  :)

> 
> I understand that we should not do this, but we do plan to make this
> modular in the future.

No problem ; I will drop this patch in favor of a one line Kconfig patch
in my test queue so it doesn't trip the regular audit, just like I have
for the 210 below -- not yet submitted for the same dependency reason.

P.
---


 From fe5bdc6348828c157deb14e8d75c732abcc2ad2b Mon Sep 17 00:00:00 2001
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Fri, 20 May 2016 14:30:43 -0400
Subject: [PATCH] dma: tegra210-adma: convert TEGRA210_ADMA from bool to
 tristate

This driver currently uses modular infrastructure but is controlled
by a bool Kconfig.

There is a general consensus from the DMA reviewers and maintainers
that "if it can be modular, it should be modular" in order to keep
the bzImage size under control for multi platform kernels.

Build tested only.  Also needs some new pm_clk symbols exported
before this commit is applied to tree in order to avoid modpost
errors like:

  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!

Cc: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: dmaengine@vger.kernel.org
Cc: linux-tegra@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/dma/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 8c98779a12b1..866068d84ca2 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -468,7 +468,7 @@ config TEGRA20_APB_DMA
 	  or vice versa. It does not support memory to memory data transfer.
 
 config TEGRA210_ADMA
-	bool "NVIDIA Tegra210 ADMA support"
+	tristate "NVIDIA Tegra210 ADMA support"
 	depends on ARCH_TEGRA_210_SOC
 	select DMA_ENGINE
 	select DMA_VIRTUAL_CHANNELS
-- 
2.8.0

> 
> Cheers
> Jon
> 
> [0] http://marc.info/?l=linux-tegra&m=146616753627760&w=2
> 
> -- 
> nvpublic

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

#1436518

On 04/07/16 14:41, Paul Gortmaker wrote:
> [Re: [PATCH 3/4] bus: tegra-aconnect: make it explicitly non-modular] On 04/07/2016 (Mon 10:17) Jon Hunter wrote:
> 
>> Hi Paul,
>>
>> On 03/07/16 18:30, Paul Gortmaker wrote:
>>> The Kconfig currently controlling compilation of this code is:
>>>
>>> drivers/bus/Kconfig:config TEGRA_ACONNECT
>>> drivers/bus/Kconfig:    bool "Tegra ACONNECT Bus Driver"
>>>
>>> ...meaning that it currently is not being built as a module by anyone.
>>>
>>> Lets remove the modular code that is essentially orphaned, so that
>>> when reading the driver there is no doubt it is builtin-only.
>>>
>>> We explicitly disallow a driver unbind, since that doesn't have a
>>> sensible use case anyway, and it allows us to drop the ".remove"
>>> code for non-modular drivers.
>>>
>>> Since module_platform_driver() uses the same init level priority as
>>> builtin_platform_driver() the init ordering remains unchanged with
>>> this commit.
>>>
>>> Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
>>>
>>> We also delete the MODULE_LICENSE tag etc. since all that information
>>> was (or is now) contained at the top of the file in the comments.
>>
>> In version 3 of the aconnect series [0] I had made this a tristate
>> because we allowed it to be removed and you had submitted a patch to
>> export the PM_CLK APIs. However, when discussing with Thierry he said
>> that we were unable to merge with tristate because of the dependency on
>> your patch. So he suggested we merge with bool for now and then change
>> it back to tristate for v4.9.
> 
> Oh I see.  The exported clock syms were for the 210 DMA driver and it
> never crossed my mind that this driver needed the same syms in order for
> it to be tristate.  Guess the exports proved useful afterall.  :)

Yes indeed and I did learn my lesson after your previous catch for the
ADMA ;-)

>>
>> I understand that we should not do this, but we do plan to make this
>> modular in the future.
> 
> No problem ; I will drop this patch in favor of a one line Kconfig patch
> in my test queue so it doesn't trip the regular audit, just like I have
> for the 210 below -- not yet submitted for the same dependency reason.
> 
> P.
> ---
> 
> 
>  From fe5bdc6348828c157deb14e8d75c732abcc2ad2b Mon Sep 17 00:00:00 2001
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Date: Fri, 20 May 2016 14:30:43 -0400
> Subject: [PATCH] dma: tegra210-adma: convert TEGRA210_ADMA from bool to
>  tristate
> 
> This driver currently uses modular infrastructure but is controlled
> by a bool Kconfig.
> 
> There is a general consensus from the DMA reviewers and maintainers
> that "if it can be modular, it should be modular" in order to keep
> the bzImage size under control for multi platform kernels.
> 
> Build tested only.  Also needs some new pm_clk symbols exported
> before this commit is applied to tree in order to avoid modpost
> errors like:
> 
>   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!
> 
> Cc: Laxman Dewangan <ldewangan@nvidia.com>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: dmaengine@vger.kernel.org
> Cc: linux-tegra@vger.kernel.org
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  drivers/dma/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 8c98779a12b1..866068d84ca2 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -468,7 +468,7 @@ config TEGRA20_APB_DMA
>  	  or vice versa. It does not support memory to memory data transfer.
>  
>  config TEGRA210_ADMA
> -	bool "NVIDIA Tegra210 ADMA support"
> +	tristate "NVIDIA Tegra210 ADMA support"
>  	depends on ARCH_TEGRA_210_SOC
>  	select DMA_ENGINE
>  	select DMA_VIRTUAL_CHANNELS

Thanks! FWIW ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web