2016-03-27 21:12:12

by Paul Gortmaker

[permalink] [raw]
Subject: [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 <[email protected]>
Cc: Brian Norris <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Gregory Fong <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Simon Horman <[email protected]>
Cc: Wolfram Sang <[email protected]>
Cc: [email protected]

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


2016-03-27 21:11:55

by Paul Gortmaker

[permalink] [raw]
Subject: [PATCH 1/4] drivers/bus: make brcmstb_gisb.c driver explicitly non-modular

The Kconfig for this driver is currently:

config BRCMSTB_GISB_ARB
bool "Broadcom STB GISB bus arbiter"

...meaning that it currently is not being built as a module by anyone.
Lets remove all modular references, so that when reading the driver
there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

Cc: Brian Norris <[email protected]>
Cc: Gregory Fong <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/bus/brcmstb_gisb.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index f364fa4d24eb..319104b22ca4 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -13,7 +13,6 @@

#include <linux/init.h>
#include <linux/types.h>
-#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/interrupt.h>
#include <linux/sysfs.h>
@@ -408,5 +407,4 @@ static int __init brcm_gisb_driver_init(void)
return platform_driver_probe(&brcmstb_gisb_arb_driver,
brcmstb_gisb_arb_probe);
}
-
-module_init(brcm_gisb_driver_init);
+device_initcall(brcm_gisb_driver_init);
--
2.6.1

2016-03-27 21:12:00

by Paul Gortmaker

[permalink] [raw]
Subject: [PATCH 3/4] drivers/bus: make simple-pm-bus.c explicitly non-modular

The Kconfig currently controlling compilation of this code is:

config SIMPLE_PM_BUS
bool "Simple Power-Managed 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_init translates to device_initcall in the non-modular
case, 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: Geert Uytterhoeven <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Simon Horman <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/bus/simple-pm-bus.c | 22 +++++-----------------
1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c
index c5eb46cbf388..e194ef4a7583 100644
--- a/drivers/bus/simple-pm-bus.c
+++ b/drivers/bus/simple-pm-bus.c
@@ -1,6 +1,8 @@
/*
* Simple Power-Managed Bus Driver
*
+ * Author: Geert Uytterhoeven <[email protected]>
+ *
* Copyright (C) 2014-2015 Glider bvba
*
* This file is subject to the terms and conditions of the GNU General Public
@@ -8,7 +10,7 @@
* for more details.
*/

-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
@@ -28,31 +30,17 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
return 0;
}

-static int simple_pm_bus_remove(struct platform_device *pdev)
-{
- dev_dbg(&pdev->dev, "%s\n", __func__);
-
- pm_runtime_disable(&pdev->dev);
- return 0;
-}
-
static const struct of_device_id simple_pm_bus_of_match[] = {
{ .compatible = "simple-pm-bus", },
{ /* sentinel */ }
};
-MODULE_DEVICE_TABLE(of, simple_pm_bus_of_match);

static struct platform_driver simple_pm_bus_driver = {
.probe = simple_pm_bus_probe,
- .remove = simple_pm_bus_remove,
.driver = {
.name = "simple-pm-bus",
.of_match_table = simple_pm_bus_of_match,
+ .suppress_bind_attrs = true,
},
};
-
-module_platform_driver(simple_pm_bus_driver);
-
-MODULE_DESCRIPTION("Simple Power-Managed Bus Driver");
-MODULE_AUTHOR("Geert Uytterhoeven <[email protected]>");
-MODULE_LICENSE("GPL v2");
+builtin_platform_driver(simple_pm_bus_driver);
--
2.6.1

2016-03-27 21:12:16

by Paul Gortmaker

[permalink] [raw]
Subject: [PATCH 2/4] drivers/bus: make imx-weim.c explicitly non-modular

The Kconfig currently controlling compilation of this code is:

drivers/bus/Kconfig:config IMX_WEIM
drivers/bus/Kconfig: bool "Freescale EIM 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_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
is already contained at the top of the file in the comments.

Cc: Shawn Guo <[email protected]>
Cc: Alison Chaiken <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Wolfram Sang <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/bus/imx-weim.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 1827fc4d15c1..d8592d9d2bc3 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -7,7 +7,7 @@
* License version 2. This program is licensed "as is" without any
* warranty of any kind, whether express or implied.
*/
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/clk.h>
#include <linux/io.h>
#include <linux/of_device.h>
@@ -57,7 +57,6 @@ static const struct of_device_id weim_id_table[] = {
{ .compatible = "fsl,imx51-weim", .data = &imx51_weim_devtype, },
{ }
};
-MODULE_DEVICE_TABLE(of, weim_id_table);

static int __init imx_weim_gpr_setup(struct platform_device *pdev)
{
@@ -210,8 +209,4 @@ static struct platform_driver weim_driver = {
.of_match_table = weim_id_table,
},
};
-module_platform_driver_probe(weim_driver, weim_probe);
-
-MODULE_AUTHOR("Freescale Semiconductor Inc.");
-MODULE_DESCRIPTION("i.MX EIM Controller Driver");
-MODULE_LICENSE("GPL");
+builtin_platform_driver_probe(weim_driver, weim_probe);
--
2.6.1

2016-03-27 21:11:54

by Paul Gortmaker

[permalink] [raw]
Subject: [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 <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
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 <[email protected]>
*/

#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 <[email protected]>");
-MODULE_LICENSE("GPL");
+device_initcall(arm_ccn_init);
--
2.6.1

2016-03-28 08:28:19

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 3/4] drivers/bus: make simple-pm-bus.c explicitly non-modular

Hi Paul,

On Sun, Mar 27, 2016 at 11:10 PM, Paul Gortmaker
<[email protected]> wrote:
> The Kconfig currently controlling compilation of this code is:
>
> config SIMPLE_PM_BUS
> bool "Simple Power-Managed 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.

Be prepared for the fallout. There are test farms running bind/unbind cycles
on random drivers.

> Since module_init translates to device_initcall in the non-modular
> case, 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: Geert Uytterhoeven <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: Simon Horman <[email protected]>
> Cc: [email protected]
> Signed-off-by: Paul Gortmaker <[email protected]>

NAK.

IIRC, I did test unbind.

The real and productive fix is to change "bool" to "tristate" in Kconfig.

All of these "make ... explicitly non-modular" may have to be reverted again
when our kernels become too big to boot.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2016-03-28 14:36:35

by Paul Gortmaker

[permalink] [raw]
Subject: Re: [PATCH 3/4] drivers/bus: make simple-pm-bus.c explicitly non-modular

[Re: [PATCH 3/4] drivers/bus: make simple-pm-bus.c explicitly non-modular] On 28/03/2016 (Mon 10:28) Geert Uytterhoeven wrote:

> Hi Paul,
>
> On Sun, Mar 27, 2016 at 11:10 PM, Paul Gortmaker
> <[email protected]> wrote:
> > The Kconfig currently controlling compilation of this code is:
> >
> > config SIMPLE_PM_BUS
> > bool "Simple Power-Managed 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.
>
> Be prepared for the fallout. There are test farms running bind/unbind cycles
> on random drivers.

If you say so. I find that a rather odd assertion. Can you point me at
some automated test results showing bind/unbind being cycled across
all drivers at random? I would expect many instances of runtime failures.

A while back even LinusW suggested in passing that a blanket blocking
unbind for built-in might make sense ; he was worried that bad things
would happen if people unbind drivers supplying core resources.[1] But
I noted the PCI pass through case is one valid use case for unbind while
built-in.

The point being that yes there are some valid use cases, but on the
whole they mostly don't make sense for an end user or for most drivers.
So we deal with it case by case currently.

>
> > Since module_init translates to device_initcall in the non-modular
> > case, 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: Geert Uytterhoeven <[email protected]>
> > Cc: Kevin Hilman <[email protected]>
> > Cc: Simon Horman <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Paul Gortmaker <[email protected]>
>
> NAK.
>
> IIRC, I did test unbind.
>
> The real and productive fix is to change "bool" to "tristate" in Kconfig.

Fine, I'll drop this patch and welcome your conversion to tristate. As
I've said several times in the past, authors and/or people with hardware
to test are welcome to convert to tristate, but I personally can't be
extending that functionality myself to all these drivers that are
currently limited to bool/built-in only, but misrepresenting as modular.

>
> All of these "make ... explicitly non-modular" may have to be reverted again
> when our kernels become too big to boot.

I think that is alarmist and not based on reality, but lets say for the
sake of argument that a handful of drivers do get converted to tristate
down the road. If that is done on demand, i.e. where the need and
testing is provided by someone who cares, then great! The code remains
consistent with the Makefile/Kconfig infrastructure in such a change.

But currently there is a significant disconnect between driver code and
the controlling Makefile/Kconfig -- and people just copy that disconnect
into their new driver without even thinking whether they want modular
support or not. We need to fix that, and we need to be asking as part
of the review of new drivers "Did you actually mean/want tristate here?"
We also should be asking if they expect a valid bind/unbind use case.

Paul.
--

[1] http://lkml.iu.edu/hypermail/linux/kernel/1506.0/02323.html

>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds

2016-03-29 11:38:38

by Will Deacon

[permalink] [raw]
Subject: 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

2016-03-29 11:45:06

by Will Deacon

[permalink] [raw]
Subject: 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

2016-03-29 11:53:13

by Pawel Moll

[permalink] [raw]
Subject: 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

2016-03-29 12:30:04

by Will Deacon

[permalink] [raw]
Subject: 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

2016-03-29 13:11:36

by Pawel Moll

[permalink] [raw]
Subject: 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ł

2016-03-29 13:15:21

by Paul Gortmaker

[permalink] [raw]
Subject: 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

2016-04-07 01:26:51

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH 1/4] drivers/bus: make brcmstb_gisb.c driver explicitly non-modular

On Sun, Mar 27, 2016 at 05:10:55PM -0400, Paul Gortmaker wrote:
> The Kconfig for this driver is currently:
>
> config BRCMSTB_GISB_ARB
> bool "Broadcom STB GISB bus arbiter"
>
> ...meaning that it currently is not being built as a module by anyone.
> Lets remove all modular references, so that when reading the driver
> there is no doubt it is builtin-only.
>
> Since module_init translates to device_initcall in the non-modular
> case, the init ordering remains unchanged with this commit.
>
> Cc: Brian Norris <[email protected]>
> Cc: Gregory Fong <[email protected]>
> Cc: Florian Fainelli <[email protected]>
> Cc: [email protected]
> Signed-off-by: Paul Gortmaker <[email protected]>

I think this driver probably doesn't make too much sense as a module
anyway (among other things, we can't hook the ARM fault handler beyond
init time, as it's marked __init). So:

Acked-by: Brian Norris <[email protected]>

Might be good to get Florian's ack though, as I'm not using this
platform any more.

Brian

> ---
> drivers/bus/brcmstb_gisb.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
> index f364fa4d24eb..319104b22ca4 100644
> --- a/drivers/bus/brcmstb_gisb.c
> +++ b/drivers/bus/brcmstb_gisb.c
> @@ -13,7 +13,6 @@
>
> #include <linux/init.h>
> #include <linux/types.h>
> -#include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/interrupt.h>
> #include <linux/sysfs.h>
> @@ -408,5 +407,4 @@ static int __init brcm_gisb_driver_init(void)
> return platform_driver_probe(&brcmstb_gisb_arb_driver,
> brcmstb_gisb_arb_probe);
> }
> -
> -module_init(brcm_gisb_driver_init);
> +device_initcall(brcm_gisb_driver_init);
> --
> 2.6.1
>

2016-04-07 02:58:30

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH 2/4] drivers/bus: make imx-weim.c explicitly non-modular

On Sun, Mar 27, 2016 at 05:10:56PM -0400, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
>
> drivers/bus/Kconfig:config IMX_WEIM
> drivers/bus/Kconfig: bool "Freescale EIM 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_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
> is already contained at the top of the file in the comments.
>
> Cc: Shawn Guo <[email protected]>

Acked-by: Shawn Guo <[email protected]>

> Cc: Alison Chaiken <[email protected]>
> Cc: Sascha Hauer <[email protected]>
> Cc: Wolfram Sang <[email protected]>
> Cc: [email protected]
> Signed-off-by: Paul Gortmaker <[email protected]>

2016-04-12 18:50:27

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 1/4] drivers/bus: make brcmstb_gisb.c driver explicitly non-modular

On 06/04/16 18:26, Brian Norris wrote:
> On Sun, Mar 27, 2016 at 05:10:55PM -0400, Paul Gortmaker wrote:
>> The Kconfig for this driver is currently:
>>
>> config BRCMSTB_GISB_ARB
>> bool "Broadcom STB GISB bus arbiter"
>>
>> ...meaning that it currently is not being built as a module by anyone.
>> Lets remove all modular references, so that when reading the driver
>> there is no doubt it is builtin-only.
>>
>> Since module_init translates to device_initcall in the non-modular
>> case, the init ordering remains unchanged with this commit.
>>
>> Cc: Brian Norris <[email protected]>
>> Cc: Gregory Fong <[email protected]>
>> Cc: Florian Fainelli <[email protected]>
>> Cc: [email protected]
>> Signed-off-by: Paul Gortmaker <[email protected]>
>
> I think this driver probably doesn't make too much sense as a module
> anyway (among other things, we can't hook the ARM fault handler beyond
> init time, as it's marked __init). So:
>
> Acked-by: Brian Norris <[email protected]>
>
> Might be good to get Florian's ack though, as I'm not using this
> platform any more.

I concur with Brian here, your changes look good:

Acked-by: Florian Fainelli <[email protected]>

Thanks Paul
--
Florian