diff mbox

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

Message ID 1459113058-14340-4-git-send-email-paul.gortmaker@windriver.com
State New, archived
Headers show

Commit Message

Paul Gortmaker March 27, 2016, 9:10 p.m. UTC
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 <geert+renesas@glider.be>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/bus/simple-pm-bus.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

Comments

Geert Uytterhoeven March 28, 2016, 8:28 a.m. UTC | #1
Hi Paul,

On Sun, Mar 27, 2016 at 11:10 PM, Paul Gortmaker
<paul.gortmaker@windriver.com> 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 <geert+renesas@glider.be>
> Cc: Kevin Hilman <khilman@linaro.org>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

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 -- geert@linux-m68k.org

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
Paul Gortmaker March 28, 2016, 2:35 p.m. UTC | #2
[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
> <paul.gortmaker@windriver.com> 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 <geert+renesas@glider.be>
> > Cc: Kevin Hilman <khilman@linaro.org>
> > Cc: Simon Horman <horms+renesas@verge.net.au>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> 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 -- geert@linux-m68k.org
> 
> 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
diff mbox

Patch

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 <geert+renesas@glider.be>
+ *
  * 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 <geert+renesas@glider.be>");
-MODULE_LICENSE("GPL v2");
+builtin_platform_driver(simple_pm_bus_driver);