| Message ID | 1547404609-14161-1-git-send-email-paul.gortmaker@windriver.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mfd: demodularization of non-modular drivers | expand |
[...] > Paul Gortmaker (18): > mfd: aat2870-core: Make it explicitly non-modular > mfd: adp5520: Make it explicitly non-modular > mfd: as3711: 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 infrastructure from non-modular code > mfd: wm8350-i2c: Make it explicitly non-modular > mfd: wm8350-core: drop unused module infrastructure 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/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 | 15 ++------------- > drivers/mfd/wm831x-i2c.c | 20 ++------------------ > drivers/mfd/wm831x-spi.c | 24 ++---------------------- > drivers/mfd/wm8350-core.c | 30 ++---------------------------- > drivers/mfd/wm8350-i2c.c | 24 +----------------------- > drivers/mfd/wm8400-core.c | 18 +++--------------- > include/linux/mfd/wm831x/core.h | 1 - > include/linux/mfd/wm8350/core.h | 1 - > 20 files changed, 41 insertions(+), 332 deletions(-) All applied!
On Wed 2019-01-16 13:24:31, Lee Jones wrote: > [...] > > > Paul Gortmaker (18): > > mfd: aat2870-core: Make it explicitly non-modular > > mfd: adp5520: Make it explicitly non-modular > > mfd: as3711: 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 infrastructure from non-modular code > > mfd: wm8350-i2c: Make it explicitly non-modular > > mfd: wm8350-core: drop unused module infrastructure 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/db8500-prcmu.c | 10 ++++------ > > drivers/mfd/htc-i2cpld.c | 18 +----------------- > > 20 files changed, 41 insertions(+), 332 deletions(-) > > All applied! Is it good idea? We want distro kernels on ARM, too, which means people will eventually want these as a modules, no? Pavel
[Re: [PATCH v5 00/18] mfd: demodularization of non-modular drivers] On 07/03/2019 (Thu 00:10) Pavel Machek wrote: > On Wed 2019-01-16 13:24:31, Lee Jones wrote: > > [...] > > > > > Paul Gortmaker (18): > > > mfd: aat2870-core: Make it explicitly non-modular > > > mfd: adp5520: Make it explicitly non-modular > > > mfd: as3711: 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 infrastructure from non-modular code > > > mfd: wm8350-i2c: Make it explicitly non-modular > > > mfd: wm8350-core: drop unused module infrastructure 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/db8500-prcmu.c | 10 ++++------ > > > drivers/mfd/htc-i2cpld.c | 18 +----------------- > > > 20 files changed, 41 insertions(+), 332 deletions(-) > > > > All applied! > > Is it good idea? Pavel, I think yes it is good, and I hope you will allow me the chance to convince you of the same. It removes dead code, and removes the chance that people mistakenly believe any of these drivers were currently possible as modules, when they were really NOT at all modular. > We want distro kernels on ARM, too, which means people will eventually > want these as a modules, no? And at the risk of repeating something I've said a lot already, this is fine, and I 100% support people converting drivers to being modular, in the case where there is demand, and where people with the hardware who are willing to test that the modular use-case actually works. If people want it to be modular, then this work actually helps. You don't have drivers "hiding in the shadows" that pretend to be modules. Such drivers do not at all help with the "distro kernels" use case. If a driver author responds and says they intended to make their driver a module, I 100% support them, and will drop the code removal patch and also have supported them in making their work tristate. If the choice to convert to tristate happens a year or more from now, it is trivial to reclaim the unused-but-deleted code from git. But, again as I have said many times -- I can't know every detail of each driver to know if module/tristate makes any sense, as a use-case or if even technically possible (such as in DMA/IOMMU or similar low level core systems). So the right option is to remove the dead code and not impact the existing driver behaviour, and make it clear in the process to the authors and users, that the driver was never modular to begin with -- and in doing so, give them all a chance to comment and react. Pavel, I hope this more extended explanation makes sense to you, and that you simply have not seen me write these same details in the past. Thanks, Paul. -- > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Wed, 06 Mar 2019, Paul Gortmaker wrote: > [Re: [PATCH v5 00/18] mfd: demodularization of non-modular drivers] On 07/03/2019 (Thu 00:10) Pavel Machek wrote: > > > On Wed 2019-01-16 13:24:31, Lee Jones wrote: > > > [...] > > > > > > > Paul Gortmaker (18): > > > > mfd: aat2870-core: Make it explicitly non-modular > > > > mfd: adp5520: Make it explicitly non-modular > > > > mfd: as3711: 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 infrastructure from non-modular code > > > > mfd: wm8350-i2c: Make it explicitly non-modular > > > > mfd: wm8350-core: drop unused module infrastructure 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/db8500-prcmu.c | 10 ++++------ > > > > drivers/mfd/htc-i2cpld.c | 18 +----------------- > > > > 20 files changed, 41 insertions(+), 332 deletions(-) > > > > > > All applied! > > > > Is it good idea? > > Pavel, I think yes it is good, and I hope you will allow me the chance > to convince you of the same. It removes dead code, and removes the > chance that people mistakenly believe any of these drivers were > currently possible as modules, when they were really NOT at all modular. > > > We want distro kernels on ARM, too, which means people will eventually > > want these as a modules, no? > > And at the risk of repeating something I've said a lot already, this > is fine, and I 100% support people converting drivers to being modular, > in the case where there is demand, and where people with the hardware > who are willing to test that the modular use-case actually works. > > If people want it to be modular, then this work actually helps. You > don't have drivers "hiding in the shadows" that pretend to be modules. > Such drivers do not at all help with the "distro kernels" use case. > > If a driver author responds and says they intended to make their driver > a module, I 100% support them, and will drop the code removal patch and > also have supported them in making their work tristate. If the choice > to convert to tristate happens a year or more from now, it is trivial to > reclaim the unused-but-deleted code from git. > > But, again as I have said many times -- I can't know every detail of > each driver to know if module/tristate makes any sense, as a use-case or > if even technically possible (such as in DMA/IOMMU or similar low level > core systems). So the right option is to remove the dead code and not > impact the existing driver behaviour, and make it clear in the process > to the authors and users, that the driver was never modular to begin > with -- and in doing so, give them all a chance to comment and react. > > Pavel, I hope this more extended explanation makes sense to you, and > that you simply have not seen me write these same details in the past. Blimey. That's a really long winded way of saying: "Modular-ness is actually broken in these drivers; [Paul]'s patches make that point clear for all to see. If people (authors/distros) wish them to be modular, they need to fix them properly."
> > Pavel, I hope this more extended explanation makes sense to you, and > > that you simply have not seen me write these same details in the past. > > Blimey. That's a really long winded way of saying: > > "Modular-ness is actually broken in these drivers; [Paul]'s patches > make that point clear for all to see. If people (authors/distros) > wish them to be modular, they need to fix them properly." Has that been checked? I mean... when I saw the patch I thought "perhaps it should be doing bool->tristate instead"? Pavel
* Pavel Machek <pavel@ucw.cz> [190307 08:35]: > > > > Pavel, I hope this more extended explanation makes sense to you, and > > > that you simply have not seen me write these same details in the past. > > > > Blimey. That's a really long winded way of saying: > > > > "Modular-ness is actually broken in these drivers; [Paul]'s patches > > make that point clear for all to see. If people (authors/distros) > > wish them to be modular, they need to fix them properly." > > Has that been checked? > > I mean... when I saw the patch I thought "perhaps it should be doing > bool->tristate instead"? Thinking distors and loadable modules, for PMICs, only regulators need to be built-in in most cases. That's at least the case for example with drivers/mfd/motorola-cpcap.c. I'd say applying these patches makes sense unless we have people step up and fix their drivers. Regards, Tony
[v4 --> v5: add more acks, re-do build tests on newest linux-next after the 5.0 merge window closed.] [v3 --> v4: delete now unused exit fcn from wm835x core; add more acks now all in chrono order, re-test.] [v2 --> v3: drop diasemi commits as they will be modularized; delete now unused exit fcn from wm831x core; add more acks; re-test.] [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: Alessandro Rubini <rubini@gnudd.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Charles Keepax <ckeepax@opensource.cirrus.com> Cc: Cory Maccarrone <darkstar6262@gmail.com> Cc: Davide Ciminaghi <ciminaghi@gnudd.com> Cc: Dong Aisheng <dong.aisheng@linaro.org> 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: Tony Lindgren <tony@atomide.com> Cc: Venu Byravarasu <vbyravarasu@nvidia.com> Cc: linux-omap@vger.kernel.org Cc: patches@opensource.cirrus.com Paul Gortmaker (18): mfd: aat2870-core: Make it explicitly non-modular mfd: adp5520: Make it explicitly non-modular mfd: as3711: 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 infrastructure from non-modular code mfd: wm8350-i2c: Make it explicitly non-modular mfd: wm8350-core: drop unused module infrastructure 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/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 | 15 ++------------- drivers/mfd/wm831x-i2c.c | 20 ++------------------ drivers/mfd/wm831x-spi.c | 24 ++---------------------- drivers/mfd/wm8350-core.c | 30 ++---------------------------- drivers/mfd/wm8350-i2c.c | 24 +----------------------- drivers/mfd/wm8400-core.c | 18 +++--------------- include/linux/mfd/wm831x/core.h | 1 - include/linux/mfd/wm8350/core.h | 1 - 20 files changed, 41 insertions(+), 332 deletions(-)