* [PATCH v2 00/22] mfd: demodularization of non-modular drivers
@ 2018-12-03 4:23 Paul Gortmaker
2018-12-03 4:23 ` [PATCH 15/22] mfd: tps65910: Make it explicitly non-modular Paul Gortmaker
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Paul Gortmaker @ 2018-12-03 4:23 UTC (permalink / raw)
To: Lee Jones
Cc: linux-kernel, Paul Gortmaker, Arnd Bergmann, Cory Maccarrone,
David Dajun Chen, Dong Aisheng, Eric Miao, Graeme Gregory,
Guennadi Liakhovetski, Haojian Zhuang, Jin Park,
Jorge Eduardo Candelaria, Laxman Dewangan, Linus Walleij,
Mark Brown, Mattias Nilsson, Michael Hennerich, Mike Rapoport,
Tony
[v1 --> v2: add some more commits as requested by Lee (MFD maintainer),
update the 00/NN text; re-do build and link testing on new linux-next. ]
This group of MFD drivers are all controlled by "bool" Kconfig settings,
but contain various amounts of largely pointless uses of infrastructure
related to modular operations, even though they can't be built modular.
We can easily remove/replace all of it. 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 downfalls this oversight 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 a lot of CPP overhead.
(4) it gets copied/replicated into other drivers and spreads quickly.
What we see in current drivers falls into one or more of the following
categories:
1) include of <linux/module.h> when it simply isn't needed
2) Use of MODULE_LICENSE, MODULE_DEVICE_TABLE, MODULE_AUTHOR etc.
macros that are no-ops for non-modular drivers.
3) Creation of a module_exit() function that will be compiled and
linked in but never actually called for non-modular drivers.
4) Addition of a platform_driver ".remove" function that is bound
into the struct but will never be called for non-module use cases.
Solution to #1 --> #3 is simple ; we just delete the related code.
The solution to #4 is similar - we delete the ".remove" function and
the binding into the platform_driver struct. However, since the same
".remove" function could also be triggered by an "unbind" (such as for
pass-through of a device to a guest instance) - so we also explicitly
disable any unbind for the driver.
The unbind mask allows us to ensure we will see if there was some odd
corner case out there that was relying on it. Typically it would be a
multi-port ethernet card passing a port through to a guest, so a
sensible use case in MFD drivers seems highly unlikely. This same
solution has already been used in multiple other mainline subsystems.
Build testing was done on drivers/mfd for allyesconfig on x86_64, ARM
and ARM-64 on a recent linux-next checkout.
Paul.
---
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Cory Maccarrone <darkstar6262@gmail.com>
Cc: David Dajun Chen <dchen@diasemi.com>
Cc: Dong Aisheng <dong.aisheng@linaro.org>
Cc: Eric Miao <eric.miao@marvell.com>
Cc: Graeme Gregory <gg@slimlogic.co.uk>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
Cc: Jin Park <jinyoungp@nvidia.com>
Cc: Jorge Eduardo Candelaria <jedu@slimlogic.co.uk>
Cc: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Mattias Nilsson <mattias.i.nilsson@stericsson.com>
Cc: Michael Hennerich <michael.hennerich@analog.com>
Cc: Mike Rapoport <mike@compulab.co.il>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Venu Byravarasu <vbyravarasu@nvidia.com>
Cc: linux-omap@vger.kernel.org
Cc: patches@opensource.cirrus.com
Cc: Support Opensource <support.opensource@diasemi.com>
Paul Gortmaker (22):
mfd: aat2870-core: Make it explicitly non-modular
mfd: adp5520: Make it explicitly non-modular
mfd: as3711: Make it explicitly non-modular
mfd: da903x: Make it explicitly non-modular
mfd: da9052-*: Make it explicitly non-modular
mfd: da9055-i2c: Make it explicitly non-modular
mfd: da9055-core: make it explicitly non-modular
mfd: db8500-prcmu: drop unused MODULE_ tags from non-modular code
mfd: htc-i2cpld: Make it explicitly non-modular
mfd: max8925-core: drop unused MODULE_ tags from non-modular code
mfd: rc5t583: Make it explicitly non-modular
mfd: sta2x11: drop unused MODULE_ tags from non-modular code
mfd: syscon: Make it explicitly non-modular
mfd: tps65090: Make it explicitly non-modular
mfd: tps65910: Make it explicitly non-modular
mfd: tps80031: Make it explicitly non-modular
mfd: wm831x-spi: Make it explicitly non-modular
mfd: wm831x-i2c: Make it explicitly non-modular
mfd: wm831x-core: drop unused MODULE_ tags from non-modular code
mfd: wm8350-i2c: Make it explicitly non-modular
mfd: wm8350-core: drop unused MODULE_ tags from non-modular code
mfd: wm8400-core: Make it explicitly non-modular
drivers/mfd/aat2870-core.c | 40 +++------------------------------------
drivers/mfd/adp5520.c | 30 +++++++----------------------
drivers/mfd/as3711.c | 14 --------------
drivers/mfd/da903x.c | 26 +++----------------------
drivers/mfd/da9052-core.c | 11 -----------
drivers/mfd/da9052-i2c.c | 22 ++-------------------
drivers/mfd/da9052-irq.c | 1 -
drivers/mfd/da9052-spi.c | 22 ++-------------------
drivers/mfd/da9055-core.c | 13 ++-----------
drivers/mfd/da9055-i2c.c | 24 ++---------------------
drivers/mfd/db8500-prcmu.c | 10 ++++------
drivers/mfd/htc-i2cpld.c | 18 +-----------------
drivers/mfd/max8925-core.c | 7 +------
drivers/mfd/rc5t583.c | 14 --------------
drivers/mfd/sta2x11-mfd.c | 10 ++++------
drivers/mfd/syscon.c | 12 +-----------
drivers/mfd/tps65090.c | 30 +++++------------------------
drivers/mfd/tps65910.c | 18 +-----------------
drivers/mfd/tps80031.c | 37 ++----------------------------------
drivers/mfd/wm831x-core.c | 7 ++-----
drivers/mfd/wm831x-i2c.c | 20 ++------------------
drivers/mfd/wm831x-spi.c | 24 ++---------------------
drivers/mfd/wm8350-core.c | 6 ++----
drivers/mfd/wm8350-i2c.c | 24 +----------------------
drivers/mfd/wm8400-core.c | 18 +++---------------
include/linux/mfd/da9052/da9052.h | 1 -
26 files changed, 52 insertions(+), 407 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 15/22] mfd: tps65910: Make it explicitly non-modular
2018-12-03 4:23 [PATCH v2 00/22] mfd: demodularization of non-modular drivers Paul Gortmaker
@ 2018-12-03 4:23 ` Paul Gortmaker
2018-12-05 11:35 ` [PATCH v2 00/22] mfd: demodularization of non-modular drivers Charles Keepax
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Paul Gortmaker @ 2018-12-03 4:23 UTC (permalink / raw)
To: Lee Jones
Cc: linux-kernel, Paul Gortmaker, Tony Lindgren, Graeme Gregory,
Jorge Eduardo Candelaria, linux-omap
The Kconfig currently controlling compilation of this code is:
drivers/mfd/Kconfig:config MFD_TPS65910
drivers/mfd/Kconfig- bool "TI TPS65910 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.
Since module_init was not in use by this code, the init ordering
remains unchanged with this commit.
We don't replace module.h with init.h since the file already has that.
We do delete an unused moduleparam.h include though.
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: Tony Lindgren <tony@atomide.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Graeme Gregory <gg@slimlogic.co.uk>
Cc: Jorge Eduardo Candelaria <jedu@slimlogic.co.uk>
Cc: linux-omap@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/mfd/tps65910.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index bf16cbe6fd88..aa3d472a10ff 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -1,5 +1,5 @@
/*
- * tps65910.c -- TI TPS6591x
+ * tps65910.c -- TI TPS6591x chip family multi-function driver
*
* Copyright 2010 Texas Instruments Inc.
*
@@ -13,8 +13,6 @@
*
*/
-#include <linux/module.h>
-#include <linux/moduleparam.h>
#include <linux/init.h>
#include <linux/err.h>
#include <linux/slab.h>
@@ -374,7 +372,6 @@ static const struct of_device_id tps65910_of_match[] = {
{ .compatible = "ti,tps65911", .data = (void *)TPS65911},
{ },
};
-MODULE_DEVICE_TABLE(of, tps65910_of_match);
static struct tps65910_board *tps65910_parse_dt(struct i2c_client *client,
unsigned long *chip_id)
@@ -527,8 +524,6 @@ static const struct i2c_device_id tps65910_i2c_id[] = {
{ "tps65911", TPS65911 },
{ }
};
-MODULE_DEVICE_TABLE(i2c, tps65910_i2c_id);
-
static struct i2c_driver tps65910_i2c_driver = {
.driver = {
@@ -545,14 +540,3 @@ static int __init tps65910_i2c_init(void)
}
/* init early so consumer devices can complete system boot */
subsys_initcall(tps65910_i2c_init);
-
-static void __exit tps65910_i2c_exit(void)
-{
- i2c_del_driver(&tps65910_i2c_driver);
-}
-module_exit(tps65910_i2c_exit);
-
-MODULE_AUTHOR("Graeme Gregory <gg@slimlogic.co.uk>");
-MODULE_AUTHOR("Jorge Eduardo Candelaria <jedu@slimlogic.co.uk>");
-MODULE_DESCRIPTION("TPS6591x chip family multi-function driver");
-MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
2018-12-03 4:23 [PATCH v2 00/22] mfd: demodularization of non-modular drivers Paul Gortmaker
2018-12-03 4:23 ` [PATCH 15/22] mfd: tps65910: Make it explicitly non-modular Paul Gortmaker
@ 2018-12-05 11:35 ` Charles Keepax
2018-12-05 11:48 ` Linus Walleij
2018-12-05 11:50 ` Linus Walleij
2018-12-05 12:01 ` Steve Twiss
3 siblings, 1 reply; 9+ messages in thread
From: Charles Keepax @ 2018-12-05 11:35 UTC (permalink / raw)
To: Paul Gortmaker
Cc: Lee Jones, linux-kernel, Arnd Bergmann, Cory Maccarrone,
David Dajun Chen, Dong Aisheng, Eric Miao, Graeme Gregory,
Guennadi Liakhovetski, Haojian Zhuang, Jin Park,
Jorge Eduardo Candelaria, Laxman Dewangan, Linus Walleij,
Mark Brown, Mattias Nilsson, Michael Hennerich, Mike Rapoport,
Tony Lindgren
On Sun, Dec 02, 2018 at 11:23:07PM -0500, Paul Gortmaker wrote:
> The solution to #4 is similar - we delete the ".remove" function and
> the binding into the platform_driver struct. However, since the same
> ".remove" function could also be triggered by an "unbind" (such as for
> pass-through of a device to a guest instance) - so we also explicitly
> disable any unbind for the driver.
>
> The unbind mask allows us to ensure we will see if there was some odd
> corner case out there that was relying on it. Typically it would be a
> multi-port ethernet card passing a port through to a guest, so a
> sensible use case in MFD drivers seems highly unlikely. This same
> solution has already been used in multiple other mainline subsystems.
>
I guess if this is a general direction thing, but it does seem
that module unload is not the only reason one might ever unbind a
driver. So are we sure we want to remove the option to unbind
these drivers? Certainly for testing it is sometimes useful.
Thanks,
Charles
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
2018-12-05 11:35 ` [PATCH v2 00/22] mfd: demodularization of non-modular drivers Charles Keepax
@ 2018-12-05 11:48 ` Linus Walleij
2018-12-05 13:40 ` Charles Keepax
0 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2018-12-05 11:48 UTC (permalink / raw)
To: Charles Keepax
Cc: Paul Gortmaker, Lee Jones, linux-kernel@vger.kernel.org,
Arnd Bergmann, Cory Maccarrone, David Dajun Chen, Dong Aisheng,
Eric Miao, Graeme Gregory, Guennadi Liakhovetski, Haojian Zhuang,
jinyoungp, Jorge Eduardo Candelaria, Laxman Dewangan, Mark Brown,
Mattias NILSSON, Michael Hennerich, Mike Rapoport
On Wed, Dec 5, 2018 at 12:36 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
> On Sun, Dec 02, 2018 at 11:23:07PM -0500, Paul Gortmaker wrote:
> > The solution to #4 is similar - we delete the ".remove" function and
> > the binding into the platform_driver struct. However, since the same
> > ".remove" function could also be triggered by an "unbind" (such as for
> > pass-through of a device to a guest instance) - so we also explicitly
> > disable any unbind for the driver.
> >
> > The unbind mask allows us to ensure we will see if there was some odd
> > corner case out there that was relying on it. Typically it would be a
> > multi-port ethernet card passing a port through to a guest, so a
> > sensible use case in MFD drivers seems highly unlikely. This same
> > solution has already been used in multiple other mainline subsystems.
> >
>
> I guess if this is a general direction thing, but it does seem
> that module unload is not the only reason one might ever unbind a
> driver. So are we sure we want to remove the option to unbind
> these drivers? Certainly for testing it is sometimes useful.
I personally never understood why these attributes are even
present on non-modular drivers.
If testing is about exercising unbind/bind to reintialize
the code through a new call to .probe(), why would the developer
not take it all the way through and make it a module?
It just looks like a half-measure.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
2018-12-03 4:23 [PATCH v2 00/22] mfd: demodularization of non-modular drivers Paul Gortmaker
2018-12-03 4:23 ` [PATCH 15/22] mfd: tps65910: Make it explicitly non-modular Paul Gortmaker
2018-12-05 11:35 ` [PATCH v2 00/22] mfd: demodularization of non-modular drivers Charles Keepax
@ 2018-12-05 11:50 ` Linus Walleij
2018-12-05 12:01 ` Steve Twiss
3 siblings, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2018-12-05 11:50 UTC (permalink / raw)
To: Paul Gortmaker
Cc: Lee Jones, linux-kernel@vger.kernel.org, Arnd Bergmann,
Cory Maccarrone, David Dajun Chen, Dong Aisheng, Eric Miao,
Graeme Gregory, Guennadi Liakhovetski, Haojian Zhuang, jinyoungp,
Jorge Eduardo Candelaria, Laxman Dewangan, Mark Brown,
Mattias NILSSON, Michael Hennerich, Mike Rapoport,
ext Tony Lindgren, Ve
On Mon, Dec 3, 2018 at 5:24 AM Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> [v1 --> v2: add some more commits as requested by Lee (MFD maintainer),
> update the 00/NN text; re-do build and link testing on new linux-next. ]
>
> This group of MFD drivers are all controlled by "bool" Kconfig settings,
> but contain various amounts of largely pointless uses of infrastructure
> related to modular operations, even though they can't be built modular.
>
> We can easily remove/replace all of it. 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 downfalls this oversight leads to are:
This series:
Acked-by: Linus Walleij <linus.walleij@linaro.org>
IMO it is clearly the right thing to do, moving a whole bunch
of clutter out of the way so we can see clearly and the effect
of getting rid of the <linux/module.h> is a substantial improvement
in its own right.
Yours.
Linus Walleij
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
2018-12-03 4:23 [PATCH v2 00/22] mfd: demodularization of non-modular drivers Paul Gortmaker
` (2 preceding siblings ...)
2018-12-05 11:50 ` Linus Walleij
@ 2018-12-05 12:01 ` Steve Twiss
2018-12-05 12:12 ` Steve Twiss
2018-12-05 18:08 ` Paul Gortmaker
3 siblings, 2 replies; 9+ messages in thread
From: Steve Twiss @ 2018-12-05 12:01 UTC (permalink / raw)
To: Paul Gortmaker, Lee Jones
Cc: linux-kernel@vger.kernel.org, Arnd Bergmann, Cory Maccarrone,
David Dajun Chen, Dong Aisheng, Eric Miao, Graeme Gregory,
Guennadi Liakhovetski, Haojian Zhuang, Jin Park,
Jorge Eduardo Candelaria, Laxman Dewangan, Linus Walleij,
Mark Brown, Mattias Nilsson, Michael Hennerich, Mike Rapoport
Hi Paul,
On 03 December 2018 04:23, Paul Gortmaker wrote:
> Subject: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
>
> [v1 --> v2: add some more commits as requested by Lee (MFD maintainer),
> update the 00/NN text; re-do build and link testing on new linux-next. ]
>
> This group of MFD drivers are all controlled by "bool" Kconfig settings,
> but contain various amounts of largely pointless uses of infrastructure
> related to modular operations, even though they can't be built modular.
>
> We can easily remove/replace all of it. We are trying to make driver
> code consistent with the Makefiles/Kconfigs that control them.
For the DA9052 and DA9055, changes:
- drivers/mfd/da9052-core.c | 11 -----------
- drivers/mfd/da9052-i2c.c | 22 ++-------------------
- drivers/mfd/da9052-irq.c | 1 -
- drivers/mfd/da9052-spi.c | 22 ++-------------------
- drivers/mfd/da9055-core.c | 13 ++-----------
- drivers/mfd/da9055-i2c.c | 24 ++---------------------
The responsibility can fall back to Dialog for these changes. We will
submit Kconfig patches for these and make them explicitly non-modular.
This will remove the ambiguity caused by the Kconfig bool options.
Regards,
Steve
> This
> means not using modular functions/macros for drivers that can never be
> built as a module. Some of the downfalls this oversight 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 a lot of CPP overhead.
> (4) it gets copied/replicated into other drivers and spreads quickly.
>
> What we see in current drivers falls into one or more of the following
> categories:
>
> 1) include of <linux/module.h> when it simply isn't needed
>
> 2) Use of MODULE_LICENSE, MODULE_DEVICE_TABLE, MODULE_AUTHOR etc.
> macros that are no-ops for non-modular drivers.
>
> 3) Creation of a module_exit() function that will be compiled and
> linked in but never actually called for non-modular drivers.
>
> 4) Addition of a platform_driver ".remove" function that is bound
> into the struct but will never be called for non-module use cases.
>
> Solution to #1 --> #3 is simple ; we just delete the related code.
>
> The solution to #4 is similar - we delete the ".remove" function and
> the binding into the platform_driver struct. However, since the same
> ".remove" function could also be triggered by an "unbind" (such as for
> pass-through of a device to a guest instance) - so we also explicitly
> disable any unbind for the driver.
>
> The unbind mask allows us to ensure we will see if there was some odd
> corner case out there that was relying on it. Typically it would be a
> multi-port ethernet card passing a port through to a guest, so a
> sensible use case in MFD drivers seems highly unlikely. This same
> solution has already been used in multiple other mainline subsystems.
>
> Build testing was done on drivers/mfd for allyesconfig on x86_64, ARM
> and ARM-64 on a recent linux-next checkout.
>
> Paul.
>
> ---
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Cory Maccarrone <darkstar6262@gmail.com>
> Cc: David Dajun Chen <dchen@diasemi.com>
> Cc: Dong Aisheng <dong.aisheng@linaro.org>
> Cc: Eric Miao <eric.miao@marvell.com>
> Cc: Graeme Gregory <gg@slimlogic.co.uk>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
> Cc: Jin Park <jinyoungp@nvidia.com>
> Cc: Jorge Eduardo Candelaria <jedu@slimlogic.co.uk>
> Cc: Laxman Dewangan <ldewangan@nvidia.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: Mattias Nilsson <mattias.i.nilsson@stericsson.com>
> Cc: Michael Hennerich <michael.hennerich@analog.com>
> Cc: Mike Rapoport <mike@compulab.co.il>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Venu Byravarasu <vbyravarasu@nvidia.com>
> Cc: linux-omap@vger.kernel.org
> Cc: patches@opensource.cirrus.com
> Cc: Support Opensource <support.opensource@diasemi.com>
>
>
> Paul Gortmaker (22):
> mfd: aat2870-core: Make it explicitly non-modular
> mfd: adp5520: Make it explicitly non-modular
> mfd: as3711: Make it explicitly non-modular
> mfd: da903x: Make it explicitly non-modular
> mfd: da9052-*: Make it explicitly non-modular
> mfd: da9055-i2c: Make it explicitly non-modular
> mfd: da9055-core: make it explicitly non-modular
> mfd: db8500-prcmu: drop unused MODULE_ tags from non-modular code
> mfd: htc-i2cpld: Make it explicitly non-modular
> mfd: max8925-core: drop unused MODULE_ tags from non-modular code
> mfd: rc5t583: Make it explicitly non-modular
> mfd: sta2x11: drop unused MODULE_ tags from non-modular code
> mfd: syscon: Make it explicitly non-modular
> mfd: tps65090: Make it explicitly non-modular
> mfd: tps65910: Make it explicitly non-modular
> mfd: tps80031: Make it explicitly non-modular
> mfd: wm831x-spi: Make it explicitly non-modular
> mfd: wm831x-i2c: Make it explicitly non-modular
> mfd: wm831x-core: drop unused MODULE_ tags from non-modular code
> mfd: wm8350-i2c: Make it explicitly non-modular
> mfd: wm8350-core: drop unused MODULE_ tags from non-modular code
> mfd: wm8400-core: Make it explicitly non-modular
>
> drivers/mfd/aat2870-core.c | 40 +++------------------------------------
> drivers/mfd/adp5520.c | 30 +++++++----------------------
> drivers/mfd/as3711.c | 14 --------------
> drivers/mfd/da903x.c | 26 +++----------------------
> drivers/mfd/da9052-core.c | 11 -----------
> drivers/mfd/da9052-i2c.c | 22 ++-------------------
> drivers/mfd/da9052-irq.c | 1 -
> drivers/mfd/da9052-spi.c | 22 ++-------------------
> drivers/mfd/da9055-core.c | 13 ++-----------
> drivers/mfd/da9055-i2c.c | 24 ++---------------------
> drivers/mfd/db8500-prcmu.c | 10 ++++------
> drivers/mfd/htc-i2cpld.c | 18 +-----------------
> drivers/mfd/max8925-core.c | 7 +------
> drivers/mfd/rc5t583.c | 14 --------------
> drivers/mfd/sta2x11-mfd.c | 10 ++++------
> drivers/mfd/syscon.c | 12 +-----------
> drivers/mfd/tps65090.c | 30 +++++------------------------
> drivers/mfd/tps65910.c | 18 +-----------------
> drivers/mfd/tps80031.c | 37 ++----------------------------------
> drivers/mfd/wm831x-core.c | 7 ++-----
> drivers/mfd/wm831x-i2c.c | 20 ++------------------
> drivers/mfd/wm831x-spi.c | 24 ++---------------------
> drivers/mfd/wm8350-core.c | 6 ++----
> drivers/mfd/wm8350-i2c.c | 24 +----------------------
> drivers/mfd/wm8400-core.c | 18 +++---------------
> include/linux/mfd/da9052/da9052.h | 1 -
> 26 files changed, 52 insertions(+), 407 deletions(-)
>
> --
> 2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
2018-12-05 12:01 ` Steve Twiss
@ 2018-12-05 12:12 ` Steve Twiss
2018-12-05 18:08 ` Paul Gortmaker
1 sibling, 0 replies; 9+ messages in thread
From: Steve Twiss @ 2018-12-05 12:12 UTC (permalink / raw)
To: Paul Gortmaker, Lee Jones
Cc: linux-kernel@vger.kernel.org, Arnd Bergmann, Cory Maccarrone,
David Dajun Chen, Dong Aisheng, Eric Miao, Graeme Gregory,
Guennadi Liakhovetski, Haojian Zhuang, Jin Park,
Jorge Eduardo Candelaria, Laxman Dewangan, Linus Walleij,
Mark Brown, Mattias Nilsson, Michael Hennerich, Mike Rapoport
Sorry typo.
I meant modular.
On 05 December 2018 12:02, Steve Twiss wrote:
> Subject: RE: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
>
> Hi Paul,
>
> On 03 December 2018 04:23, Paul Gortmaker wrote:
>
> > Subject: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
> >
> > [v1 --> v2: add some more commits as requested by Lee (MFD maintainer),
> > update the 00/NN text; re-do build and link testing on new linux-next. ]
> >
> > This group of MFD drivers are all controlled by "bool" Kconfig settings,
> > but contain various amounts of largely pointless uses of infrastructure
> > related to modular operations, even though they can't be built modular.
> >
> > We can easily remove/replace all of it. We are trying to make driver
> > code consistent with the Makefiles/Kconfigs that control them.
>
> For the DA9052 and DA9055, changes:
>
> - drivers/mfd/da9052-core.c | 11 -----------
> - drivers/mfd/da9052-i2c.c | 22 ++-------------------
> - drivers/mfd/da9052-irq.c | 1 -
> - drivers/mfd/da9052-spi.c | 22 ++-------------------
> - drivers/mfd/da9055-core.c | 13 ++-----------
> - drivers/mfd/da9055-i2c.c | 24 ++---------------------
>
> The responsibility can fall back to Dialog for these changes. We will
> submit Kconfig patches for these and make them explicitly non-modular.
Sorry. I meant modular.
> This will remove the ambiguity caused by the Kconfig bool options.
>
> Regards,
> Steve
>
> > This
> > means not using modular functions/macros for drivers that can never be
> > built as a module. Some of the downfalls this oversight 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 a lot of CPP overhead.
> > (4) it gets copied/replicated into other drivers and spreads quickly.
> >
> > What we see in current drivers falls into one or more of the following
> > categories:
> >
> > 1) include of <linux/module.h> when it simply isn't needed
> >
> > 2) Use of MODULE_LICENSE, MODULE_DEVICE_TABLE, MODULE_AUTHOR etc.
> > macros that are no-ops for non-modular drivers.
> >
> > 3) Creation of a module_exit() function that will be compiled and
> > linked in but never actually called for non-modular drivers.
> >
> > 4) Addition of a platform_driver ".remove" function that is bound
> > into the struct but will never be called for non-module use cases.
> >
> > Solution to #1 --> #3 is simple ; we just delete the related code.
> >
> > The solution to #4 is similar - we delete the ".remove" function and
> > the binding into the platform_driver struct. However, since the same
> > ".remove" function could also be triggered by an "unbind" (such as for
> > pass-through of a device to a guest instance) - so we also explicitly
> > disable any unbind for the driver.
> >
> > The unbind mask allows us to ensure we will see if there was some odd
> > corner case out there that was relying on it. Typically it would be a
> > multi-port ethernet card passing a port through to a guest, so a
> > sensible use case in MFD drivers seems highly unlikely. This same
> > solution has already been used in multiple other mainline subsystems.
> >
> > Build testing was done on drivers/mfd for allyesconfig on x86_64, ARM
> > and ARM-64 on a recent linux-next checkout.
> >
> > Paul.
> >
> > ---
> >
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Cory Maccarrone <darkstar6262@gmail.com>
> > Cc: David Dajun Chen <dchen@diasemi.com>
> > Cc: Dong Aisheng <dong.aisheng@linaro.org>
> > Cc: Eric Miao <eric.miao@marvell.com>
> > Cc: Graeme Gregory <gg@slimlogic.co.uk>
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
> > Cc: Jin Park <jinyoungp@nvidia.com>
> > Cc: Jorge Eduardo Candelaria <jedu@slimlogic.co.uk>
> > Cc: Laxman Dewangan <ldewangan@nvidia.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > Cc: Mattias Nilsson <mattias.i.nilsson@stericsson.com>
> > Cc: Michael Hennerich <michael.hennerich@analog.com>
> > Cc: Mike Rapoport <mike@compulab.co.il>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Venu Byravarasu <vbyravarasu@nvidia.com>
> > Cc: linux-omap@vger.kernel.org
> > Cc: patches@opensource.cirrus.com
> > Cc: Support Opensource <support.opensource@diasemi.com>
> >
> >
> > Paul Gortmaker (22):
> > mfd: aat2870-core: Make it explicitly non-modular
> > mfd: adp5520: Make it explicitly non-modular
> > mfd: as3711: Make it explicitly non-modular
> > mfd: da903x: Make it explicitly non-modular
> > mfd: da9052-*: Make it explicitly non-modular
> > mfd: da9055-i2c: Make it explicitly non-modular
> > mfd: da9055-core: make it explicitly non-modular
> > mfd: db8500-prcmu: drop unused MODULE_ tags from non-modular code
> > mfd: htc-i2cpld: Make it explicitly non-modular
> > mfd: max8925-core: drop unused MODULE_ tags from non-modular code
> > mfd: rc5t583: Make it explicitly non-modular
> > mfd: sta2x11: drop unused MODULE_ tags from non-modular code
> > mfd: syscon: Make it explicitly non-modular
> > mfd: tps65090: Make it explicitly non-modular
> > mfd: tps65910: Make it explicitly non-modular
> > mfd: tps80031: Make it explicitly non-modular
> > mfd: wm831x-spi: Make it explicitly non-modular
> > mfd: wm831x-i2c: Make it explicitly non-modular
> > mfd: wm831x-core: drop unused MODULE_ tags from non-modular code
> > mfd: wm8350-i2c: Make it explicitly non-modular
> > mfd: wm8350-core: drop unused MODULE_ tags from non-modular code
> > mfd: wm8400-core: Make it explicitly non-modular
> >
> > drivers/mfd/aat2870-core.c | 40 +++------------------------------------
> > drivers/mfd/adp5520.c | 30 +++++++----------------------
> > drivers/mfd/as3711.c | 14 --------------
> > drivers/mfd/da903x.c | 26 +++----------------------
> > drivers/mfd/da9052-core.c | 11 -----------
> > drivers/mfd/da9052-i2c.c | 22 ++-------------------
> > drivers/mfd/da9052-irq.c | 1 -
> > drivers/mfd/da9052-spi.c | 22 ++-------------------
> > drivers/mfd/da9055-core.c | 13 ++-----------
> > drivers/mfd/da9055-i2c.c | 24 ++---------------------
> > drivers/mfd/db8500-prcmu.c | 10 ++++------
> > drivers/mfd/htc-i2cpld.c | 18 +-----------------
> > drivers/mfd/max8925-core.c | 7 +------
> > drivers/mfd/rc5t583.c | 14 --------------
> > drivers/mfd/sta2x11-mfd.c | 10 ++++------
> > drivers/mfd/syscon.c | 12 +-----------
> > drivers/mfd/tps65090.c | 30 +++++------------------------
> > drivers/mfd/tps65910.c | 18 +-----------------
> > drivers/mfd/tps80031.c | 37 ++----------------------------------
> > drivers/mfd/wm831x-core.c | 7 ++-----
> > drivers/mfd/wm831x-i2c.c | 20 ++------------------
> > drivers/mfd/wm831x-spi.c | 24 ++---------------------
> > drivers/mfd/wm8350-core.c | 6 ++----
> > drivers/mfd/wm8350-i2c.c | 24 +----------------------
> > drivers/mfd/wm8400-core.c | 18 +++---------------
> > include/linux/mfd/da9052/da9052.h | 1 -
> > 26 files changed, 52 insertions(+), 407 deletions(-)
> >
> > --
> > 2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
2018-12-05 11:48 ` Linus Walleij
@ 2018-12-05 13:40 ` Charles Keepax
0 siblings, 0 replies; 9+ messages in thread
From: Charles Keepax @ 2018-12-05 13:40 UTC (permalink / raw)
To: Linus Walleij
Cc: Paul Gortmaker, Lee Jones, linux-kernel@vger.kernel.org,
Arnd Bergmann, Cory Maccarrone, David Dajun Chen, Dong Aisheng,
Eric Miao, Graeme Gregory, Guennadi Liakhovetski, Haojian Zhuang,
jinyoungp, Jorge Eduardo Candelaria, Laxman Dewangan, Mark Brown,
Mattias NILSSON, Michael Hennerich, Mike Rapoport
On Wed, Dec 05, 2018 at 12:48:47PM +0100, Linus Walleij wrote:
> On Wed, Dec 5, 2018 at 12:36 PM Charles Keepax
> <ckeepax@opensource.cirrus.com> wrote:
> > On Sun, Dec 02, 2018 at 11:23:07PM -0500, Paul Gortmaker wrote:
> > > The solution to #4 is similar - we delete the ".remove" function and
> > > the binding into the platform_driver struct. However, since the same
> > > ".remove" function could also be triggered by an "unbind" (such as for
> > > pass-through of a device to a guest instance) - so we also explicitly
> > > disable any unbind for the driver.
> > >
> > > The unbind mask allows us to ensure we will see if there was some odd
> > > corner case out there that was relying on it. Typically it would be a
> > > multi-port ethernet card passing a port through to a guest, so a
> > > sensible use case in MFD drivers seems highly unlikely. This same
> > > solution has already been used in multiple other mainline subsystems.
> > >
> >
> > I guess if this is a general direction thing, but it does seem
> > that module unload is not the only reason one might ever unbind a
> > driver. So are we sure we want to remove the option to unbind
> > these drivers? Certainly for testing it is sometimes useful.
>
> I personally never understood why these attributes are even
> present on non-modular drivers.
>
> If testing is about exercising unbind/bind to reintialize
> the code through a new call to .probe(), why would the developer
> not take it all the way through and make it a module?
> It just looks like a half-measure.
>
Well I guess in someways it is a half-measure. I vaguely seem to
remember some dependency nightmare that can make it really hard
to have the MFD allowed as a module in some cases, I can't
remember the exact details but probably why some of these are not
modules.
I certainly don't strongly object to removing the ability to
unbind these drivers, just wanted to make sure everyone is
aligned that it's a good thing to do. Does kinda remove a couple
of debugging options (debugging things like drivers interfering
with each other) and the last chance restart the driver and see
if that helps rescue something.
Thanks,
Charles
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
2018-12-05 12:01 ` Steve Twiss
2018-12-05 12:12 ` Steve Twiss
@ 2018-12-05 18:08 ` Paul Gortmaker
1 sibling, 0 replies; 9+ messages in thread
From: Paul Gortmaker @ 2018-12-05 18:08 UTC (permalink / raw)
To: Steve Twiss
Cc: Lee Jones, linux-kernel@vger.kernel.org, Arnd Bergmann,
Cory Maccarrone, David Dajun Chen, Dong Aisheng, Eric Miao,
Graeme Gregory, Guennadi Liakhovetski, Haojian Zhuang, Jin Park,
Jorge Eduardo Candelaria, Laxman Dewangan, Linus Walleij,
Mark Brown, Mattias Nilsson, Michael Hennerich, Mike Rapoport
[RE: [PATCH v2 00/22] mfd: demodularization of non-modular drivers] On 05/12/2018 (Wed 12:01) Steve Twiss wrote:
> Hi Paul,
>
> On 03 December 2018 04:23, Paul Gortmaker wrote:
>
> > Subject: [PATCH v2 00/22] mfd: demodularization of non-modular drivers
> >
> > [v1 --> v2: add some more commits as requested by Lee (MFD maintainer),
> > update the 00/NN text; re-do build and link testing on new linux-next. ]
> >
> > This group of MFD drivers are all controlled by "bool" Kconfig settings,
> > but contain various amounts of largely pointless uses of infrastructure
> > related to modular operations, even though they can't be built modular.
> >
> > We can easily remove/replace all of it. We are trying to make driver
> > code consistent with the Makefiles/Kconfigs that control them.
>
> For the DA9052 and DA9055, changes:
>
> - drivers/mfd/da9052-core.c | 11 -----------
> - drivers/mfd/da9052-i2c.c | 22 ++-------------------
> - drivers/mfd/da9052-irq.c | 1 -
> - drivers/mfd/da9052-spi.c | 22 ++-------------------
> - drivers/mfd/da9055-core.c | 13 ++-----------
> - drivers/mfd/da9055-i2c.c | 24 ++---------------------
>
> The responsibility can fall back to Dialog for these changes. We will
> submit Kconfig patches for these and make them explicitly non-modular.
> This will remove the ambiguity caused by the Kconfig bool options.
[typo noted: non-modular ---> modular]
Great, I'll look forward to seeing those patches to convert these
drivers to modular in the near future, and I'll not re-send mine.
Thanks,
P.
--
>
> Regards,
> Steve
>
> > This
> > means not using modular functions/macros for drivers that can never be
> > built as a module. Some of the downfalls this oversight 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 a lot of CPP overhead.
> > (4) it gets copied/replicated into other drivers and spreads quickly.
> >
> > What we see in current drivers falls into one or more of the following
> > categories:
> >
> > 1) include of <linux/module.h> when it simply isn't needed
> >
> > 2) Use of MODULE_LICENSE, MODULE_DEVICE_TABLE, MODULE_AUTHOR etc.
> > macros that are no-ops for non-modular drivers.
> >
> > 3) Creation of a module_exit() function that will be compiled and
> > linked in but never actually called for non-modular drivers.
> >
> > 4) Addition of a platform_driver ".remove" function that is bound
> > into the struct but will never be called for non-module use cases.
> >
> > Solution to #1 --> #3 is simple ; we just delete the related code.
> >
> > The solution to #4 is similar - we delete the ".remove" function and
> > the binding into the platform_driver struct. However, since the same
> > ".remove" function could also be triggered by an "unbind" (such as for
> > pass-through of a device to a guest instance) - so we also explicitly
> > disable any unbind for the driver.
> >
> > The unbind mask allows us to ensure we will see if there was some odd
> > corner case out there that was relying on it. Typically it would be a
> > multi-port ethernet card passing a port through to a guest, so a
> > sensible use case in MFD drivers seems highly unlikely. This same
> > solution has already been used in multiple other mainline subsystems.
> >
> > Build testing was done on drivers/mfd for allyesconfig on x86_64, ARM
> > and ARM-64 on a recent linux-next checkout.
> >
> > Paul.
> >
> > ---
> >
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Cory Maccarrone <darkstar6262@gmail.com>
> > Cc: David Dajun Chen <dchen@diasemi.com>
> > Cc: Dong Aisheng <dong.aisheng@linaro.org>
> > Cc: Eric Miao <eric.miao@marvell.com>
> > Cc: Graeme Gregory <gg@slimlogic.co.uk>
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
> > Cc: Jin Park <jinyoungp@nvidia.com>
> > Cc: Jorge Eduardo Candelaria <jedu@slimlogic.co.uk>
> > Cc: Laxman Dewangan <ldewangan@nvidia.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > Cc: Mattias Nilsson <mattias.i.nilsson@stericsson.com>
> > Cc: Michael Hennerich <michael.hennerich@analog.com>
> > Cc: Mike Rapoport <mike@compulab.co.il>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Venu Byravarasu <vbyravarasu@nvidia.com>
> > Cc: linux-omap@vger.kernel.org
> > Cc: patches@opensource.cirrus.com
> > Cc: Support Opensource <support.opensource@diasemi.com>
> >
> >
> > Paul Gortmaker (22):
> > mfd: aat2870-core: Make it explicitly non-modular
> > mfd: adp5520: Make it explicitly non-modular
> > mfd: as3711: Make it explicitly non-modular
> > mfd: da903x: Make it explicitly non-modular
> > mfd: da9052-*: Make it explicitly non-modular
> > mfd: da9055-i2c: Make it explicitly non-modular
> > mfd: da9055-core: make it explicitly non-modular
> > mfd: db8500-prcmu: drop unused MODULE_ tags from non-modular code
> > mfd: htc-i2cpld: Make it explicitly non-modular
> > mfd: max8925-core: drop unused MODULE_ tags from non-modular code
> > mfd: rc5t583: Make it explicitly non-modular
> > mfd: sta2x11: drop unused MODULE_ tags from non-modular code
> > mfd: syscon: Make it explicitly non-modular
> > mfd: tps65090: Make it explicitly non-modular
> > mfd: tps65910: Make it explicitly non-modular
> > mfd: tps80031: Make it explicitly non-modular
> > mfd: wm831x-spi: Make it explicitly non-modular
> > mfd: wm831x-i2c: Make it explicitly non-modular
> > mfd: wm831x-core: drop unused MODULE_ tags from non-modular code
> > mfd: wm8350-i2c: Make it explicitly non-modular
> > mfd: wm8350-core: drop unused MODULE_ tags from non-modular code
> > mfd: wm8400-core: Make it explicitly non-modular
> >
> > drivers/mfd/aat2870-core.c | 40 +++------------------------------------
> > drivers/mfd/adp5520.c | 30 +++++++----------------------
> > drivers/mfd/as3711.c | 14 --------------
> > drivers/mfd/da903x.c | 26 +++----------------------
> > drivers/mfd/da9052-core.c | 11 -----------
> > drivers/mfd/da9052-i2c.c | 22 ++-------------------
> > drivers/mfd/da9052-irq.c | 1 -
> > drivers/mfd/da9052-spi.c | 22 ++-------------------
> > drivers/mfd/da9055-core.c | 13 ++-----------
> > drivers/mfd/da9055-i2c.c | 24 ++---------------------
> > drivers/mfd/db8500-prcmu.c | 10 ++++------
> > drivers/mfd/htc-i2cpld.c | 18 +-----------------
> > drivers/mfd/max8925-core.c | 7 +------
> > drivers/mfd/rc5t583.c | 14 --------------
> > drivers/mfd/sta2x11-mfd.c | 10 ++++------
> > drivers/mfd/syscon.c | 12 +-----------
> > drivers/mfd/tps65090.c | 30 +++++------------------------
> > drivers/mfd/tps65910.c | 18 +-----------------
> > drivers/mfd/tps80031.c | 37 ++----------------------------------
> > drivers/mfd/wm831x-core.c | 7 ++-----
> > drivers/mfd/wm831x-i2c.c | 20 ++------------------
> > drivers/mfd/wm831x-spi.c | 24 ++---------------------
> > drivers/mfd/wm8350-core.c | 6 ++----
> > drivers/mfd/wm8350-i2c.c | 24 +----------------------
> > drivers/mfd/wm8400-core.c | 18 +++---------------
> > include/linux/mfd/da9052/da9052.h | 1 -
> > 26 files changed, 52 insertions(+), 407 deletions(-)
> >
> > --
> > 2.7.4
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-12-05 18:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-03 4:23 [PATCH v2 00/22] mfd: demodularization of non-modular drivers Paul Gortmaker
2018-12-03 4:23 ` [PATCH 15/22] mfd: tps65910: Make it explicitly non-modular Paul Gortmaker
2018-12-05 11:35 ` [PATCH v2 00/22] mfd: demodularization of non-modular drivers Charles Keepax
2018-12-05 11:48 ` Linus Walleij
2018-12-05 13:40 ` Charles Keepax
2018-12-05 11:50 ` Linus Walleij
2018-12-05 12:01 ` Steve Twiss
2018-12-05 12:12 ` Steve Twiss
2018-12-05 18:08 ` Paul Gortmaker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).