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

Groups > linux.kernel > #1436718 > unrolled thread

[PATCH 0/6] mfd: trivial demodularization of non-modular drivers.

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] mfd: trivial demodularization of non-modular drivers. Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-07-05 03:30 +0200
    [PATCH 5/6] mfd: sun6i-prcm: Make it explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-07-05 03:30 +0200
      Re: [PATCH 5/6] mfd: sun6i-prcm: Make it explicitly non-modular Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-07-05 08:30 +0200
    [PATCH 2/6] mfd: as3722: Make it explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-07-05 03:30 +0200
      Re: [PATCH 2/6] mfd: as3722: Make it explicitly non-modular Laxman Dewangan <ldewangan@nvidia.com> - 2016-07-05 08:10 +0200
        Re: [PATCH 2/6] mfd: as3722: Make it explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-07-06 18:50 +0200

#1436718 — [PATCH 0/6] mfd: trivial demodularization of non-modular drivers.

I grouped this little series of mfd commits together because of the
common theme of their relatively trivial use of modular infrastructure
in these non-modular drivers.

What that means is there are no orphaned __exit functions to remove;
no unused ".remove" functions linked into the driver struct to be
deleted -- so in the end we have binary equivalence between the
pre-patched code and the post-patched code.

Easy to review and low risk as we approach the end of the development
window.  Seems like a good choice for sending at this point in time.

More specifically - the only thing we are doing here is mapping the
initcall that binds in the driver from the module variant to the
built-in variant ; something that CPP would do for us anyway -- and
then throwing away any MODULE_ tags which are no-ops in non-modular
builds (while preserving any author/desc. info they had in comments).
And in doing that, we can toss the include of module.h in favour of
init.h (asssuming it wasn't included already).

For anyone new to the underlying goal of this cleanup, we are trying to
make driver code consistent with the Makefiles/Kconfigs that control them.

This means not using modular functions/macros for drivers that can never
be built as a module.  Some of the other downfalls this leads to are:

 (1) it is easy to accidentally write unused module_exit and remove code
 (2) it can be misleading when reading the source, thinking it can be
     modular when the Makefile and/or Kconfig prohibit it
 (3) it requires the include of the module.h header file which in turn
     includes nearly everything else, thus adding to CPP overhead.
 (4) it gets copied/replicated into other drivers and spreads like weeds.

As usual, we can consider making some of these tristate if an author
has strong feelings about extending support into the modular realm,
but I won't be able to run-time test any of that.  And since I can't
even know if a modular use case makes sense, I won't extend the
functionality into the modular realm by default for anything.

Build tested on linux-next for arm, arm64 and x86-64 to ensure no typos
or similar issues crept in.

Paul.
---

Cc: Boris BREZILLON <boris.brezillon@free-electrons.com>
Cc: Florian Lobmaier <florian.lobmaier@ams.com>
Cc: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Sourav Poddar <sourav.poddar@ti.com>
Cc: Thor Thayer <tthayer@opensource.altera.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: linux-omap@vger.kernel.org

Paul Gortmaker (6):
  mfd: altera-a10sr: make it explicitly non-modular
  mfd: as3722: Make it explicitly non-modular
  mfd: intel_msic: Make it explicitly non-modular
  mfd: smsc-ece1099: Make it explicitly non-modular
  mfd: sun6i-prcm: Make it explicitly non-modular
  mfd: twl-core: Make it explicitly non-modular

 drivers/mfd/altera-a10sr.c | 14 ++++++--------
 drivers/mfd/as3722.c       | 12 ++----------
 drivers/mfd/intel_msic.c   |  9 ++-------
 drivers/mfd/smsc-ece1099.c | 11 ++---------
 drivers/mfd/sun6i-prcm.c   |  8 ++------
 drivers/mfd/twl-core.c     |  9 +--------
 6 files changed, 15 insertions(+), 48 deletions(-)

-- 
2.8.4

[toc] | [next] | [standalone]

#1436719 — [PATCH 5/6] mfd: sun6i-prcm: Make it explicitly non-modular

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config MFD_SUN6I_PRCM
drivers/mfd/Kconfig:    bool "Allwinner A31 PRCM controller"

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

Lets remove the couple traces of modularity so that when reading the
driver there is no doubt it is builtin-only.

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

We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Boris BREZILLON <boris.brezillon@free-electrons.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/mfd/sun6i-prcm.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
index 191173166d65..011fcc555945 100644
--- a/drivers/mfd/sun6i-prcm.c
+++ b/drivers/mfd/sun6i-prcm.c
@@ -9,7 +9,7 @@
  */
 
 #include <linux/mfd/core.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/of.h>
 
 struct prcm_data {
@@ -170,8 +170,4 @@ static struct platform_driver sun6i_prcm_driver = {
 	},
 	.probe = sun6i_prcm_probe,
 };
-module_platform_driver(sun6i_prcm_driver);
-
-MODULE_AUTHOR("Boris BREZILLON <boris.brezillon@free-electrons.com>");
-MODULE_DESCRIPTION("Allwinner sun6i PRCM driver");
-MODULE_LICENSE("GPL v2");
+builtin_platform_driver(sun6i_prcm_driver);
-- 
2.8.4

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

#1436791 — Re: [PATCH 5/6] mfd: sun6i-prcm: Make it explicitly non-modular

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jul 04, 2016 at 09:25:43PM -0400, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
> 
> drivers/mfd/Kconfig:config MFD_SUN6I_PRCM
> drivers/mfd/Kconfig:    bool "Allwinner A31 PRCM controller"
> 
> ...meaning that it currently is not being built as a module by anyone.
> 
> Lets remove the couple traces of modularity so that when reading the
> driver there is no doubt it is builtin-only.
> 
> Since module_platform_driver() uses the same init level priority as
> builtin_platform_driver() the init ordering remains unchanged with
> this commit.
> 
> We also delete the MODULE_LICENSE tag etc. since all that information
> is already contained at the top of the file in the comments.
> 
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Boris BREZILLON <boris.brezillon@free-electrons.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

#1436720 — [PATCH 2/6] mfd: as3722: Make it explicitly non-modular

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config MFD_AS3722
drivers/mfd/Kconfig:    bool "ams AS3722 Power Management IC"

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

Since module_i2c_driver() uses the same init level priority as
builtin_i2c_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
is already contained at the top of the file in the comments.

Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Florian Lobmaier <florian.lobmaier@ams.com>
Cc: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/mfd/as3722.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/as3722.c b/drivers/mfd/as3722.c
index f87342c211bc..c8a199d081ce 100644
--- a/drivers/mfd/as3722.c
+++ b/drivers/mfd/as3722.c
@@ -27,7 +27,7 @@
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/as3722.h>
 #include <linux/of.h>
@@ -439,13 +439,11 @@ static const struct of_device_id as3722_of_match[] = {
 	{ .compatible = "ams,as3722", },
 	{},
 };
-MODULE_DEVICE_TABLE(of, as3722_of_match);
 
 static const struct i2c_device_id as3722_i2c_id[] = {
 	{ "as3722", 0 },
 	{},
 };
-MODULE_DEVICE_TABLE(i2c, as3722_i2c_id);
 
 static const struct dev_pm_ops as3722_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(as3722_i2c_suspend, as3722_i2c_resume)
@@ -460,10 +458,4 @@ static struct i2c_driver as3722_i2c_driver = {
 	.probe = as3722_i2c_probe,
 	.id_table = as3722_i2c_id,
 };
-
-module_i2c_driver(as3722_i2c_driver);
-
-MODULE_DESCRIPTION("I2C support for AS3722 PMICs");
-MODULE_AUTHOR("Florian Lobmaier <florian.lobmaier@ams.com>");
-MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
-MODULE_LICENSE("GPL");
+builtin_i2c_driver(as3722_i2c_driver);
-- 
2.8.4

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

#1436781 — Re: [PATCH 2/6] mfd: as3722: Make it explicitly non-modular

On Tuesday 05 July 2016 06:55 AM, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
>
> drivers/mfd/Kconfig:config MFD_AS3722
> drivers/mfd/Kconfig:    bool "ams AS3722 Power Management IC"
>


Should we convert this to as "tristate" so that it can be built as 
module also.

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

#1437841 — Re: [PATCH 2/6] mfd: as3722: Make it explicitly non-modular

[Re: [PATCH 2/6] mfd: as3722: Make it explicitly non-modular] On 05/07/2016 (Tue 11:18) Laxman Dewangan wrote:

> 
> On Tuesday 05 July 2016 06:55 AM, Paul Gortmaker wrote:
> >The Kconfig currently controlling compilation of this code is:
> >
> >drivers/mfd/Kconfig:config MFD_AS3722
> >drivers/mfd/Kconfig:    bool "ams AS3722 Power Management IC"
> >
> 
> 
> Should we convert this to as "tristate" so that it can be built as module
> also.

Seems to compile and modpost w/o missing symbols as tristate:

paul@yow-builder:~/git/linux-head$ ls -l ../arm-build/drivers/mfd/as3722*
-rw-rw-r-- 1 paul paul 13010 Jul  6 12:30 ../arm-build/drivers/mfd/as3722.ko
-rw-rw-r-- 1 paul paul  1937 Jul  6 12:28 ../arm-build/drivers/mfd/as3722.mod.c
-rw-rw-r-- 1 paul paul  3820 Jul  6 12:29 ../arm-build/drivers/mfd/as3722.mod.o
-rw-rw-r-- 1 paul paul 11292 Jul  6 12:13 ../arm-build/drivers/mfd/as3722.o
paul@yow-builder:~/git/linux-head$ 

...so I'll drop this patch in favour of the one line tristate conversion in v2.

Paul.
--

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web