[PATCH 0/6] mfd: audit and demodule drivers/mfd/ab* drivers

[PATCH 0/6] mfd: audit and demodule drivers/mfd/ab* drivers

[Paul Gortmaker]

Nothing new here ; just another instance where we had modular remove
and exit functions -- but the Makefile/Kconfig limited the use case to
being always built-in; hence the code is useless and needs removing.

So we remove the dead code and the misleading hints of modularity from
these drivers, hoping that we manage to prevent some new drivers from
copying these same old mistakes into pending new drivers.

Paul.
---

Cc: Lee Jones <lee.jones@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mattias Wallin <mattias.wallin@stericsson.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: linux-arm-kernel@lists.infradead.org

Paul Gortmaker (6):
  mfd: ab3100-core: Make it explicitly non-modular
  mfd: ab8500-core: Make it explicitly non-modular
  mfd: ab8500-debugfs: Make it explicitly non-modular
  mfd: ab8500-gpadc: Make it explicitly non-modular
  mfd: ab8500: make sysctrl explicitly non-modular
  mfd: abx500-core: drop unused MODULE_ tags from non-modular code

 drivers/mfd/ab3100-core.c    | 39 +++------------------------------------
 drivers/mfd/ab8500-core.c    | 37 ++++++-------------------------------
 drivers/mfd/ab8500-debugfs.c | 21 ++-------------------
 drivers/mfd/ab8500-gpadc.c   | 19 ++-----------------
 drivers/mfd/ab8500-sysctrl.c |  9 ++++-----
 drivers/mfd/abx500-core.c    |  7 ++-----
 6 files changed, 19 insertions(+), 113 deletions(-)

-- 
2.10.1

[PATCH 1/6] mfd: ab3100-core: Make it explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config AB3100_CORE
drivers/mfd/Kconfig:    bool "ST-Ericsson AB3100 Mixed Signal Circuit core functions"

...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.  In doing
so, the debugfs unregister fcn becomes unused so we remove it too.

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_init was not in use by this code, 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: Linus Walleij <linus.walleij@linaro.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/mfd/ab3100-core.c | 39 +++------------------------------------
 1 file changed, 3 insertions(+), 36 deletions(-)

diff --git a/drivers/mfd/ab3100-core.c b/drivers/mfd/ab3100-core.c
index 6a5a98806cb8..099635bed188 100644
--- a/drivers/mfd/ab3100-core.c
+++ b/drivers/mfd/ab3100-core.c
@@ -12,7 +12,7 @@
 #include <linux/notifier.h>
 #include <linux/slab.h>
 #include <linux/err.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/device.h>
 #include <linux/interrupt.h>
@@ -628,20 +628,10 @@ static void ab3100_setup_debugfs(struct ab3100 *ab3100)
  exit_no_debugfs:
 	return;
 }
-static inline void ab3100_remove_debugfs(void)
-{
-	debugfs_remove(ab3100_set_reg_file);
-	debugfs_remove(ab3100_get_reg_file);
-	debugfs_remove(ab3100_reg_file);
-	debugfs_remove(ab3100_dir);
-}
 #else
 static inline void ab3100_setup_debugfs(struct ab3100 *ab3100)
 {
 }
-static inline void ab3100_remove_debugfs(void)
-{
-}
 #endif
 
 /*
@@ -949,45 +939,22 @@ static int ab3100_probe(struct i2c_client *client,
 	return err;
 }
 
-static int ab3100_remove(struct i2c_client *client)
-{
-	struct ab3100 *ab3100 = i2c_get_clientdata(client);
-
-	/* Unregister subdevices */
-	mfd_remove_devices(&client->dev);
-	ab3100_remove_debugfs();
-	i2c_unregister_device(ab3100->testreg_client);
-	return 0;
-}
-
 static const struct i2c_device_id ab3100_id[] = {
 	{ "ab3100", 0 },
 	{ }
 };
-MODULE_DEVICE_TABLE(i2c, ab3100_id);
 
 static struct i2c_driver ab3100_driver = {
 	.driver = {
-		.name	= "ab3100",
+		.name			= "ab3100",
+		.suppress_bind_attrs	= true,
 	},
 	.id_table	= ab3100_id,
 	.probe		= ab3100_probe,
-	.remove		= ab3100_remove,
 };
 
 static int __init ab3100_i2c_init(void)
 {
 	return i2c_add_driver(&ab3100_driver);
 }
-
-static void __exit ab3100_i2c_exit(void)
-{
-	i2c_del_driver(&ab3100_driver);
-}
-
 subsys_initcall(ab3100_i2c_init);
-module_exit(ab3100_i2c_exit);
-
-MODULE_AUTHOR("Linus Walleij <linus.walleij@stericsson.com>");
-MODULE_DESCRIPTION("AB3100 core driver");
-MODULE_LICENSE("GPL");
-- 
2.10.1

[PATCH 2/6] mfd: ab8500-core: Make it explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config AB8500_CORE
drivers/mfd/Kconfig:    bool "ST-Ericsson AB8500 Mixed Signal Power Management chip"

...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_init was not in use by this code, the init ordering
remains unchanged with this commit.

We replace module.h with moduleparam.h ; the latter since this file
was implicitly relying on getting it.

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: Linus Walleij <linus.walleij@linaro.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/mfd/ab8500-core.c | 37 ++++++-------------------------------
 1 file changed, 6 insertions(+), 31 deletions(-)

diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
index 589eebfc13df..6e00124cef01 100644
--- a/drivers/mfd/ab8500-core.c
+++ b/drivers/mfd/ab8500-core.c
@@ -14,7 +14,7 @@
 #include <linux/irqdomain.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
-#include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/abx500.h>
@@ -123,6 +123,10 @@ static DEFINE_SPINLOCK(on_stat_lock);
 static u8 turn_on_stat_mask = 0xFF;
 static u8 turn_on_stat_set;
 static bool no_bm; /* No battery management */
+/*
+ * not really modular, but the easiest way to keep compat with existing
+ * bootargs behaviour is to continue using module_param here.
+ */
 module_param(no_bm, bool, S_IRUGO);
 
 #define AB9540_MODEM_CTRL2_REG			0x23
@@ -1324,25 +1328,6 @@ static int ab8500_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int ab8500_remove(struct platform_device *pdev)
-{
-	struct ab8500 *ab8500 = platform_get_drvdata(pdev);
-
-	if (((is_ab8505(ab8500) || is_ab9540(ab8500)) &&
-			ab8500->chip_id >= AB8500_CUT2P0) || is_ab8540(ab8500))
-		sysfs_remove_group(&ab8500->dev->kobj, &ab9540_attr_group);
-	else
-		sysfs_remove_group(&ab8500->dev->kobj, &ab8500_attr_group);
-
-	if ((is_ab8505(ab8500) || is_ab9540(ab8500)) &&
-			ab8500->chip_id >= AB8500_CUT2P0)
-		sysfs_remove_group(&ab8500->dev->kobj, &ab8505_attr_group);
-
-	mfd_remove_devices(ab8500->dev);
-
-	return 0;
-}
-
 static const struct platform_device_id ab8500_id[] = {
 	{ "ab8500-core", AB8500_VERSION_AB8500 },
 	{ "ab8505-i2c", AB8500_VERSION_AB8505 },
@@ -1354,9 +1339,9 @@ static const struct platform_device_id ab8500_id[] = {
 static struct platform_driver ab8500_core_driver = {
 	.driver = {
 		.name = "ab8500-core",
+		.suppress_bind_attrs = true,
 	},
 	.probe	= ab8500_probe,
-	.remove	= ab8500_remove,
 	.id_table = ab8500_id,
 };
 
@@ -1364,14 +1349,4 @@ static int __init ab8500_core_init(void)
 {
 	return platform_driver_register(&ab8500_core_driver);
 }
-
-static void __exit ab8500_core_exit(void)
-{
-	platform_driver_unregister(&ab8500_core_driver);
-}
 core_initcall(ab8500_core_init);
-module_exit(ab8500_core_exit);
-
-MODULE_AUTHOR("Mattias Wallin, Srinidhi Kasagar, Rabin Vincent");
-MODULE_DESCRIPTION("AB8500 MFD core");
-MODULE_LICENSE("GPL v2");
-- 
2.10.1

[PATCH 3/6] mfd: ab8500-debugfs: Make it explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config AB8500_DEBUG
drivers/mfd/Kconfig:       bool "Enable debug info via debugfs"

...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_init was not in use by this code, 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: Linus Walleij <linus.walleij@linaro.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/mfd/ab8500-debugfs.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/ab8500-debugfs.c b/drivers/mfd/ab8500-debugfs.c
index acf6c00b14b9..c1c815241e02 100644
--- a/drivers/mfd/ab8500-debugfs.c
+++ b/drivers/mfd/ab8500-debugfs.c
@@ -74,7 +74,7 @@
 #include <linux/seq_file.h>
 #include <linux/uaccess.h>
 #include <linux/fs.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/debugfs.h>
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
@@ -3234,33 +3234,16 @@ static int ab8500_debug_probe(struct platform_device *plf)
 	return -ENOMEM;
 }
 
-static int ab8500_debug_remove(struct platform_device *plf)
-{
-	debugfs_remove_recursive(ab8500_dir);
-
-	return 0;
-}
-
 static struct platform_driver ab8500_debug_driver = {
 	.driver = {
 		.name = "ab8500-debug",
+		.suppress_bind_attrs = true,
 	},
 	.probe  = ab8500_debug_probe,
-	.remove = ab8500_debug_remove
 };
 
 static int __init ab8500_debug_init(void)
 {
 	return platform_driver_register(&ab8500_debug_driver);
 }
-
-static void __exit ab8500_debug_exit(void)
-{
-	platform_driver_unregister(&ab8500_debug_driver);
-}
 subsys_initcall(ab8500_debug_init);
-module_exit(ab8500_debug_exit);
-
-MODULE_AUTHOR("Mattias WALLIN <mattias.wallin@stericsson.com");
-MODULE_DESCRIPTION("AB8500 DEBUG");
-MODULE_LICENSE("GPL v2");
-- 
2.10.1

[PATCH 4/6] mfd: ab8500-gpadc: Make it explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config AB8500_GPADC
drivers/mfd/Kconfig:    bool "ST-Ericsson AB8500 GPADC 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.

Since module_init was not in use by this code, the init ordering
remains unchanged with this commit.

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: Linus Walleij <linus.walleij@linaro.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/mfd/ab8500-gpadc.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/mfd/ab8500-gpadc.c b/drivers/mfd/ab8500-gpadc.c
index 97dcadc8fa8b..f4e94869d612 100644
--- a/drivers/mfd/ab8500-gpadc.c
+++ b/drivers/mfd/ab8500-gpadc.c
@@ -5,9 +5,9 @@
  * Author: Arun R Murthy <arun.murthy@stericsson.com>
  * Author: Daniel Willerud <daniel.willerud@stericsson.com>
  * Author: Johan Palsson <johan.palsson@stericsson.com>
+ * Author: M'boumba Cedric Madianga
  */
 #include <linux/init.h>
-#include <linux/module.h>
 #include <linux/device.h>
 #include <linux/interrupt.h>
 #include <linux/spinlock.h>
@@ -1054,11 +1054,7 @@ static int __init ab8500_gpadc_init(void)
 {
 	return platform_driver_register(&ab8500_gpadc_driver);
 }
-
-static void __exit ab8500_gpadc_exit(void)
-{
-	platform_driver_unregister(&ab8500_gpadc_driver);
-}
+subsys_initcall_sync(ab8500_gpadc_init);
 
 /**
  * ab8540_gpadc_get_otp() - returns OTP values
@@ -1077,14 +1073,3 @@ void ab8540_gpadc_get_otp(struct ab8500_gpadc *gpadc,
 	*ibat_l  = gpadc->cal_data[ADC_INPUT_IBAT].otp_calib_lo;
 	*ibat_h  = gpadc->cal_data[ADC_INPUT_IBAT].otp_calib_hi;
 }
-
-subsys_initcall_sync(ab8500_gpadc_init);
-module_exit(ab8500_gpadc_exit);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Arun R Murthy");
-MODULE_AUTHOR("Daniel Willerud");
-MODULE_AUTHOR("Johan Palsson");
-MODULE_AUTHOR("M'boumba Cedric Madianga");
-MODULE_ALIAS("platform:ab8500_gpadc");
-MODULE_DESCRIPTION("AB8500 GPADC driver");
-- 
2.10.1

[PATCH 5/6] mfd: ab8500: make sysctrl explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config AB8500_CORE
drivers/mfd/Kconfig:    bool "ST-Ericsson AB8500 Mixed Signal Power Management chip"

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

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

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.

We replace module.h with init.h and export.h -- the latter since the file
does make use of EXPORT_SYMBOL.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/mfd/ab8500-sysctrl.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/ab8500-sysctrl.c b/drivers/mfd/ab8500-sysctrl.c
index 207cc497958a..80c0efa66ac1 100644
--- a/drivers/mfd/ab8500-sysctrl.c
+++ b/drivers/mfd/ab8500-sysctrl.c
@@ -1,11 +1,14 @@
 /*
+ * AB8500 system control driver
+ *
  * Copyright (C) ST-Ericsson SA 2010
  * Author: Mattias Nilsson <mattias.i.nilsson@stericsson.com> for ST Ericsson.
  * License terms: GNU General Public License (GPL) version 2
  */
 
 #include <linux/err.h>
-#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/export.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/reboot.h>
@@ -158,7 +161,3 @@ static int __init ab8500_sysctrl_init(void)
 	return platform_driver_register(&ab8500_sysctrl_driver);
 }
 arch_initcall(ab8500_sysctrl_init);
-
-MODULE_AUTHOR("Mattias Nilsson <mattias.i.nilsson@stericsson.com");
-MODULE_DESCRIPTION("AB8500 system control driver");
-MODULE_LICENSE("GPL v2");
-- 
2.10.1

[PATCH 6/6] mfd: abx500-core: drop unused MODULE_ tags from non-modular code

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config ABX500_CORE
drivers/mfd/Kconfig:    bool "ST-Ericsson ABX500 Mixed Signal Circuit register functions"

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

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

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

We replace module.h with init.h and export.h ; the latter since the
file does export some symbols.

Cc: Mattias Wallin <mattias.wallin@stericsson.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/mfd/abx500-core.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/abx500-core.c b/drivers/mfd/abx500-core.c
index fe418995108c..0d3846a4767c 100644
--- a/drivers/mfd/abx500-core.c
+++ b/drivers/mfd/abx500-core.c
@@ -8,7 +8,8 @@
 #include <linux/list.h>
 #include <linux/slab.h>
 #include <linux/err.h>
-#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/export.h>
 #include <linux/mfd/abx500.h>
 
 static LIST_HEAD(abx500_list);
@@ -150,7 +151,3 @@ int abx500_startup_irq_enabled(struct device *dev, unsigned int irq)
 		return -ENOTSUPP;
 }
 EXPORT_SYMBOL(abx500_startup_irq_enabled);
-
-MODULE_AUTHOR("Mattias Wallin <mattias.wallin@stericsson.com>");
-MODULE_DESCRIPTION("ABX500 core driver");
-MODULE_LICENSE("GPL");
-- 
2.10.1

Re: [PATCH 0/6] mfd: audit and demodule drivers/mfd/ab* drivers

[Linus Walleij]

On Sun, Oct 30, 2016 at 2:24 AM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:

> Nothing new here ; just another instance where we had modular remove
> and exit functions -- but the Makefile/Kconfig limited the use case to
> being always built-in; hence the code is useless and needs removing.
>
> So we remove the dead code and the misleading hints of modularity from
> these drivers, hoping that we manage to prevent some new drivers from
> copying these same old mistakes into pending new drivers.
(...)
> Paul Gortmaker (6):
>   mfd: ab3100-core: Make it explicitly non-modular
>   mfd: ab8500-core: Make it explicitly non-modular
>   mfd: ab8500-debugfs: Make it explicitly non-modular
>   mfd: ab8500-gpadc: Make it explicitly non-modular
>   mfd: ab8500: make sysctrl explicitly non-modular
>   mfd: abx500-core: drop unused MODULE_ tags from non-modular code

Acked-by: Linus Walleij <linus.walleij@linaro.org>

For the whole series.

Yours,
Linus Walleij

Re: [PATCH 0/6] mfd: audit and demodule drivers/mfd/ab* drivers

[Lee Jones]

On Sat, 29 Oct 2016, Paul Gortmaker wrote:

> Nothing new here ; just another instance where we had modular remove
> and exit functions -- but the Makefile/Kconfig limited the use case to
> being always built-in; hence the code is useless and needs removing.
> 
> So we remove the dead code and the misleading hints of modularity from
> these drivers, hoping that we manage to prevent some new drivers from
> copying these same old mistakes into pending new drivers.
> 
> Paul.
> ---
> 
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mattias Wallin <mattias.wallin@stericsson.com>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: linux-arm-kernel@lists.infradead.org
> 
> Paul Gortmaker (6):
>   mfd: ab3100-core: Make it explicitly non-modular
>   mfd: ab8500-core: Make it explicitly non-modular
>   mfd: ab8500-debugfs: Make it explicitly non-modular
>   mfd: ab8500-gpadc: Make it explicitly non-modular
>   mfd: ab8500: make sysctrl explicitly non-modular
>   mfd: abx500-core: drop unused MODULE_ tags from non-modular code

Applied the series with Linus' Ack.

>  drivers/mfd/ab3100-core.c    | 39 +++------------------------------------
>  drivers/mfd/ab8500-core.c    | 37 ++++++-------------------------------
>  drivers/mfd/ab8500-debugfs.c | 21 ++-------------------
>  drivers/mfd/ab8500-gpadc.c   | 19 ++-----------------
>  drivers/mfd/ab8500-sysctrl.c |  9 ++++-----
>  drivers/mfd/abx500-core.c    |  7 ++-----
>  6 files changed, 19 insertions(+), 113 deletions(-)
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog