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

Groups > linux.kernel > #1365194 > unrolled thread

[PATCH 0/4] drivers/bus: remove unused modular code from non-modular drivers

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] drivers/bus: remove unused modular code from non-modular drivers Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-03-27 23:20 +0200
    [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-03-27 23:20 +0200
      Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly  non-modular Will Deacon <will.deacon@arm.com> - 2016-03-29 13:50 +0200
        Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly  non-modular Pawel Moll <pawel.moll@arm.com> - 2016-03-29 14:00 +0200
          Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly  non-modular Will Deacon <will.deacon@arm.com> - 2016-03-29 14:40 +0200
            Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly  non-modular Pawel Moll <pawel.moll@arm.com> - 2016-03-29 15:20 +0200
          Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly  non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-03-29 15:20 +0200
    Re: [PATCH 0/4] drivers/bus: remove unused modular code from  non-modular drivers Will Deacon <will.deacon@arm.com> - 2016-03-29 13:40 +0200

#1365194 — [PATCH 0/4] drivers/bus: remove unused modular code from non-modular drivers

The drivers/bus doesn't have a strict maintainer entry, but since
all the changes here are for ARM platforms, I'm Cc'ing arm-kernel
and hoping it makes sense to vector these few changes through the
arm-soc. [Olof, Will, Arnd? Seems you guys handle most of it...]

My ongoing audit looking for non-modular code that needlessly uses
modular macros (vs. built-in equivalents) and/or has dead code
relating to module unloading that can never be executed led to the
creation of these four commits.

Two are of the trivial kind, where we substitute in the non-modular
versions that CPP would have put in place anyway, resulting in no
actual changes, even at the binary output level.

The other two are of the kind where there was a ".remove" function
registered into the driver struct.  Being non-modular, these
functions will never be called via a normal module_exit path.

However, since it was possible (but largely pointless, and without
a real use case) to unbind these drivers via sysfs, we explicitly
disallow the unbind as part of the removal of the ".remove" itself.

For anyone new to the underlying goal of this cleanup, we are trying to
not use module support for code that can never be built as a module since:

 (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 code and spreads like weeds.

Build tested for arm and arm64 on v4.6-rc1 to ensure no silly typos
that would break compilation crept in.

---

Cc: Alison Chaiken <alison_chaiken@mentor.com>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Gregory Fong <gregory.0xf0@gmail.com>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-arm-kernel@lists.infradead.org

Paul Gortmaker (4):
  drivers/bus: make brcmstb_gisb.c driver explicitly non-modular
  drivers/bus: make imx-weim.c explicitly non-modular
  drivers/bus: make simple-pm-bus.c explicitly non-modular
  drivers/bus: make arm-ccn.c driver explicitly non-modular

 drivers/bus/arm-ccn.c       | 41 +++++------------------------------------
 drivers/bus/brcmstb_gisb.c  |  4 +---
 drivers/bus/imx-weim.c      |  9 ++-------
 drivers/bus/simple-pm-bus.c | 22 +++++-----------------
 4 files changed, 13 insertions(+), 63 deletions(-)

-- 
2.6.1

[toc] | [next] | [standalone]

#1365195 — [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly non-modular

The Kconfig for this driver is currently:

config ARM_CCN
        bool "ARM CCN driver support"

...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 translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We exchange module.h for moduleparam.h here since the driver uses
module_param_named, and for now the easiest way to remain compatible
with existing bootargs use cases is to leave this as-is.

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: Pawel Moll <pawel.moll@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/bus/arm-ccn.c | 41 +++++------------------------------------
 1 file changed, 5 insertions(+), 36 deletions(-)

diff --git a/drivers/bus/arm-ccn.c b/drivers/bus/arm-ccn.c
index 7082c7268845..45d2ba72ba8e 100644
--- a/drivers/bus/arm-ccn.c
+++ b/drivers/bus/arm-ccn.c
@@ -9,6 +9,8 @@
  * GNU General Public License for more details.
  *
  * Copyright (C) 2014 ARM Limited
+ *
+ * Author: Pawel Moll <pawel.moll@arm.com>
  */
 
 #include <linux/ctype.h>
@@ -16,7 +18,7 @@
 #include <linux/idr.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
-#include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/perf_event.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -1302,20 +1304,6 @@ error_cpu_notifier:
 	return err;
 }
 
-static void arm_ccn_pmu_cleanup(struct arm_ccn *ccn)
-{
-	int i;
-
-	irq_set_affinity(ccn->irq, cpu_possible_mask);
-	unregister_cpu_notifier(&ccn->dt.cpu_nb);
-	for (i = 0; i < ccn->num_xps; i++)
-		writel(0, ccn->xp[i].base + CCN_XP_DT_CONTROL);
-	writel(0, ccn->dt.base + CCN_DT_PMCR);
-	perf_pmu_unregister(&ccn->dt.pmu);
-	ida_simple_remove(&arm_ccn_pmu_ida, ccn->dt.id);
-}
-
-
 static int arm_ccn_for_each_valid_region(struct arm_ccn *ccn,
 		int (*callback)(struct arm_ccn *ccn, int region,
 		void __iomem *base, u32 type, u32 id))
@@ -1507,15 +1495,6 @@ static int arm_ccn_probe(struct platform_device *pdev)
 	return arm_ccn_pmu_init(ccn);
 }
 
-static int arm_ccn_remove(struct platform_device *pdev)
-{
-	struct arm_ccn *ccn = platform_get_drvdata(pdev);
-
-	arm_ccn_pmu_cleanup(ccn);
-
-	return 0;
-}
-
 static const struct of_device_id arm_ccn_match[] = {
 	{ .compatible = "arm,ccn-504", },
 	{},
@@ -1525,9 +1504,9 @@ static struct platform_driver arm_ccn_driver = {
 	.driver = {
 		.name = "arm-ccn",
 		.of_match_table = arm_ccn_match,
+		.suppress_bind_attrs = true,
 	},
 	.probe = arm_ccn_probe,
-	.remove = arm_ccn_remove,
 };
 
 static int __init arm_ccn_init(void)
@@ -1539,14 +1518,4 @@ static int __init arm_ccn_init(void)
 
 	return platform_driver_register(&arm_ccn_driver);
 }
-
-static void __exit arm_ccn_exit(void)
-{
-	platform_driver_unregister(&arm_ccn_driver);
-}
-
-module_init(arm_ccn_init);
-module_exit(arm_ccn_exit);
-
-MODULE_AUTHOR("Pawel Moll <pawel.moll@arm.com>");
-MODULE_LICENSE("GPL");
+device_initcall(arm_ccn_init);
-- 
2.6.1

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

#1366190 — Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly non-modular

On Sun, Mar 27, 2016 at 05:10:58PM -0400, Paul Gortmaker wrote:
> The Kconfig for this driver is currently:
> 
> config ARM_CCN
>         bool "ARM CCN driver support"
> 
> ...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 translates to device_initcall in the non-modular
> case, the init ordering remains unchanged with this commit.
> 
> We exchange module.h for moduleparam.h here since the driver uses
> module_param_named, and for now the easiest way to remain compatible
> with existing bootargs use cases is to leave this as-is.
> 
> 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.

I'd much rather fix the driver to build as a module, if at all possible.
Suzuki (CC'd) is taking a look at that, so please drop this patch for
now.

Will

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

#1366193 — Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly non-modular

Dnia 2016-03-29, Tue o godzinie 12:45 +0100, Will Deacon pisze:
> I'd much rather fix the driver to build as a module, if at all
> possible.
> Suzuki (CC'd) is taking a look at that, so please drop this patch for
> now.

There's no problem with building arm-ccn.c as a module - all it's
really doing today is providing a PMU driver. I don't even know why
have I made it bool-only in the first place...

Pawel

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

#1366223 — Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly non-modular

On Tue, Mar 29, 2016 at 12:53:06PM +0100, Pawel Moll wrote:
> Dnia 2016-03-29, Tue o godzinie 12:45 +0100, Will Deacon pisze:
> > I'd much rather fix the driver to build as a module, if at all
> > possible.
> > Suzuki (CC'd) is taking a look at that, so please drop this patch for
> > now.
> 
> There's no problem with building arm-ccn.c as a module - all it's
> really doing today is providing a PMU driver. I don't even know why
> have I made it bool-only in the first place...

Probably because it doesn't compile due to the irq_set_affinity call.

Will

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

#1366272 — Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly non-modular

On Tue, 2016-03-29 at 13:30 +0100, Will Deacon wrote:
> On Tue, Mar 29, 2016 at 12:53:06PM +0100, Pawel Moll wrote:
> > Dnia 2016-03-29, Tue o godzinie 12:45 +0100, Will Deacon pisze:
> > > I'd much rather fix the driver to build as a module, if at all
> > > possible.
> > > Suzuki (CC'd) is taking a look at that, so please drop this patch
> > > for
> > > now.
> > 
> > There's no problem with building arm-ccn.c as a module - all it's
> > really doing today is providing a PMU driver. I don't even know why
> > have I made it bool-only in the first place...
> 
> Probably because it doesn't compile due to the irq_set_affinity call.

The original arm-ccn.c did not call it at all. My guess is that I
copied ARM_CCI stanza without thinking. But I'm glad Suzuki will
straighten it out :-)

Paweł

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

#1366274 — Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly non-modular

[Re: [PATCH 4/4] drivers/bus: make arm-ccn.c driver explicitly non-modular] On 29/03/2016 (Tue 12:53) Pawel Moll wrote:

> Dnia 2016-03-29, Tue o godzinie 12:45 +0100, Will Deacon pisze:
> > I'd much rather fix the driver to build as a module, if at all
> > possible.
> > Suzuki (CC'd) is taking a look at that, so please drop this patch for
> > now.
> 
> There's no problem with building arm-ccn.c as a module - all it's
> really doing today is providing a PMU driver. I don't even know why
> have I made it bool-only in the first place...

My guess is that like many other instances, it originally starts out as
an innocent copy and paste during early driver development -- hence the
desire to get the code and Kconfigs consistent tree wide.

Anyway, thanks guys for tackling the conversion ; I'll shelf this patch
and watch for the conversion to hit linux-next.

Paul.
--

> 
> Pawel

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

#1366183 — Re: [PATCH 0/4] drivers/bus: remove unused modular code from non-modular drivers

On Sun, Mar 27, 2016 at 05:10:54PM -0400, Paul Gortmaker wrote:
> The drivers/bus doesn't have a strict maintainer entry, but since
> all the changes here are for ARM platforms, I'm Cc'ing arm-kernel
> and hoping it makes sense to vector these few changes through the
> arm-soc. [Olof, Will, Arnd? Seems you guys handle most of it...]

I plan to move the parts that are simply perf drivers under drivers/perf,
so that should remove some of the code from drivers/bus. There's a fiddly
part with the CCI, which has both perf code and bus initialisation code
that need prying apart.

Will

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web