This second set of patches to drivers/tty steps outside of the serial
dir, and an improved auditing finds two more serial drivers pretending
to be modular that really are not.
The reasoning for doing this is the same as the first set[1] of patches
and is largely copied below:
In the previous merge window, we made changes to allow better
delineation between modular and non-modular code in commit
0fd972a7d91d6e15393c449492a04d94c0b89351 ("module: relocate module_init
from init.h to module.h"). This allows us to now ensure module code
looks modular and non-modular code does not accidentally look modular
without suffering build breakage.
Here we target code that is, by nature of their Kconfig settings, only
available to be built-in, but implicitly presenting itself as being
possibly modular by way of using modular headers, macros, and functions.
The goal here is to remove that illusion of modularity from these
drivers, but in a way that leaves the actual runtime unchanged.
In doing so, we remove code that has never been tested and adds
no value to the tree. And we begin the process of expecting a
level of consistency between the Kconfig of a driver and the code
that the driver uses.
Build tested for allyesconfig on x86_64, and ARM for lpc81xx, and powerpc
for hvc_console and mpsc, layered onto tty/tty-next as a baseline.
Paul.
[1] https://lkml.kernel.org/r/[email protected]
--
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Joachim Eastwood <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Paul Gortmaker (5):
drivers/tty: make pty.c slightly more explicitly non-modular
drivers/tty: make sysrq.c slightly more explicitly non-modular
drivers/tty: make hvc_console.c explicitly non-modular
drivers/tty: make serial/mpsc.c driver explicitly non-modular
drivers/tty: make serial 8250_lpc18xx.c explicitly non-modular
drivers/tty/hvc/hvc_console.c | 18 +----------------
drivers/tty/pty.c | 7 +++++--
drivers/tty/serial/8250/8250_lpc18xx.c | 19 ++++--------------
drivers/tty/serial/mpsc.c | 36 +++-------------------------------
drivers/tty/sysrq.c | 6 +++++-
5 files changed, 18 insertions(+), 68 deletions(-)
--
2.5.0
The Kconfig currently controlling compilation of this code is:
drivers/tty/Kconfig:config LEGACY_PTYS
drivers/tty/Kconfig: bool "Legacy (BSD) PTY support"
...and:
drivers/tty/Kconfig:config UNIX98_PTYS
drivers/tty/Kconfig: bool "Unix98 PTY support" if EXPERT
combined with this:
obj-$(CONFIG_LEGACY_PTYS) += pty.o
obj-$(CONFIG_UNIX98_PTYS) += pty.o
...meaning that it currently is not being built as a module by anyone.
Lets remove the traces of modularity we can so that when reading the
driver there is less 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.
We don't delete the module.h include since other parts of the file are
using content from there.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/tty/pty.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 4d5937c185c1..a45660f62db5 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -7,7 +7,6 @@
*/
#include <linux/module.h>
-
#include <linux/errno.h>
#include <linux/interrupt.h>
#include <linux/tty.h>
@@ -501,6 +500,10 @@ static int pty_bsd_ioctl(struct tty_struct *tty,
}
static int legacy_count = CONFIG_LEGACY_PTY_COUNT;
+/*
+ * not really modular, but the easiest way to keep compat with existing
+ * bootargs behaviour is to continue using module_param here.
+ */
module_param(legacy_count, int, 0);
/*
@@ -877,4 +880,4 @@ static int __init pty_init(void)
unix98_pty_init();
return 0;
}
-module_init(pty_init);
+device_initcall(pty_init);
--
2.5.0
The Kconfig currently controlling compilation of this code is:
config.debug:config MAGIC_SYSRQ
bool "Magic SysRq key"
...meaning that it currently is not being built as a module by anyone.
Lets remove the traces of modularity we can so that when reading the
driver there is less 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.
We don't delete the module.h include since other parts of the file are
using content from there.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/tty/sysrq.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index b5b427888b24..dd2c435e223e 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -996,6 +996,10 @@ static const struct kernel_param_ops param_ops_sysrq_reset_seq = {
#define param_check_sysrq_reset_seq(name, p) \
__param_check(name, p, unsigned short)
+/*
+ * not really modular, but the easiest way to keep compat with existing
+ * bootargs behaviour is to continue using module_param here.
+ */
module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq,
&sysrq_reset_seq_len, 0644);
@@ -1112,4 +1116,4 @@ static int __init sysrq_init(void)
return 0;
}
-module_init(sysrq_init);
+device_initcall(sysrq_init);
--
2.5.0
The Kconfig currently controlling compilation of this code is:
drivers/tty/hvc/Kconfig:config HVC_DRIVER
drivers/tty/hvc/Kconfig: bool
...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, even
though someone bothered to comment that the code was not used.
Unlike other changes, this driver binds in w/o using module_init,
so we dont have init ordering concerns with this commit.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/tty/hvc/hvc_console.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 4e9c4cc9e1b5..9c30f67c802a 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -29,7 +29,7 @@
#include <linux/kernel.h>
#include <linux/kthread.h>
#include <linux/list.h>
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/major.h>
#include <linux/atomic.h>
#include <linux/sysrq.h>
@@ -1005,19 +1005,3 @@ put_tty:
out:
return err;
}
-
-/* This isn't particularly necessary due to this being a console driver
- * but it is nice to be thorough.
- */
-static void __exit hvc_exit(void)
-{
- if (hvc_driver) {
- kthread_stop(hvc_task);
-
- tty_unregister_driver(hvc_driver);
- /* return tty_struct instances allocated in hvc_init(). */
- put_tty_driver(hvc_driver);
- unregister_console(&hvc_console);
- }
-}
-module_exit(hvc_exit);
--
2.5.0
The Kconfig for this driver is currently:
config SERIAL_MPSC
bool "Marvell MPSC serial port 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.
Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.
We leave some tags like MODULE_AUTHOR for documentation purposes.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/tty/serial/mpsc.c | 36 +++---------------------------------
1 file changed, 3 insertions(+), 33 deletions(-)
diff --git a/drivers/tty/serial/mpsc.c b/drivers/tty/serial/mpsc.c
index 82bb6d1fe23b..edb32e3f1e84 100644
--- a/drivers/tty/serial/mpsc.c
+++ b/drivers/tty/serial/mpsc.c
@@ -55,8 +55,6 @@
#define SUPPORT_SYSRQ
#endif
-#include <linux/module.h>
-#include <linux/moduleparam.h>
#include <linux/tty.h>
#include <linux/tty_flip.h>
#include <linux/ioport.h>
@@ -2108,24 +2106,8 @@ static int mpsc_drv_probe(struct platform_device *dev)
return rc;
}
-static int mpsc_drv_remove(struct platform_device *dev)
-{
- pr_debug("mpsc_drv_exit: Removing MPSC %d\n", dev->id);
-
- if (dev->id < MPSC_NUM_CTLRS) {
- uart_remove_one_port(&mpsc_reg, &mpsc_ports[dev->id].port);
- mpsc_release_port((struct uart_port *)
- &mpsc_ports[dev->id].port);
- mpsc_drv_unmap_regs(&mpsc_ports[dev->id]);
- return 0;
- } else {
- return -ENODEV;
- }
-}
-
static struct platform_driver mpsc_driver = {
.probe = mpsc_drv_probe,
- .remove = mpsc_drv_remove,
.driver = {
.name = MPSC_CTLR_NAME,
},
@@ -2156,22 +2138,10 @@ static int __init mpsc_drv_init(void)
return rc;
}
+device_initcall(mpsc_drv_init);
-static void __exit mpsc_drv_exit(void)
-{
- platform_driver_unregister(&mpsc_driver);
- platform_driver_unregister(&mpsc_shared_driver);
- uart_unregister_driver(&mpsc_reg);
- memset(mpsc_ports, 0, sizeof(mpsc_ports));
- memset(&mpsc_shared_regs, 0, sizeof(mpsc_shared_regs));
-}
-
-module_init(mpsc_drv_init);
-module_exit(mpsc_drv_exit);
-
+/*
MODULE_AUTHOR("Mark A. Greer <[email protected]>");
MODULE_DESCRIPTION("Generic Marvell MPSC serial/UART driver");
-MODULE_VERSION(MPSC_VERSION);
MODULE_LICENSE("GPL");
-MODULE_ALIAS_CHARDEV_MAJOR(MPSC_MAJOR);
-MODULE_ALIAS("platform:" MPSC_CTLR_NAME);
+*/
--
2.5.0
The Kconfig currently controlling compilation of this code is:
8250/Kconfig:config SERIAL_8250_LPC18XX
8250/Kconfig: bool "NXP LPC18xx/43xx serial port 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.
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 leave some tags like MODULE_AUTHOR for documentation purposes.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Joachim Eastwood <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/tty/serial/8250/8250_lpc18xx.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index 99cd478851ff..f5112bee1ab6 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -15,7 +15,7 @@
#include <linux/clk.h>
#include <linux/io.h>
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/of.h>
#include <linux/platform_device.h>
@@ -198,33 +198,22 @@ dis_clk_reg:
return ret;
}
-static int lpc18xx_serial_remove(struct platform_device *pdev)
-{
- struct lpc18xx_uart_data *data = platform_get_drvdata(pdev);
-
- serial8250_unregister_port(data->line);
- clk_disable_unprepare(data->clk_uart);
- clk_disable_unprepare(data->clk_reg);
-
- return 0;
-}
-
static const struct of_device_id lpc18xx_serial_match[] = {
{ .compatible = "nxp,lpc1850-uart" },
{ },
};
-MODULE_DEVICE_TABLE(of, lpc18xx_serial_match);
static struct platform_driver lpc18xx_serial_driver = {
.probe = lpc18xx_serial_probe,
- .remove = lpc18xx_serial_remove,
.driver = {
.name = "lpc18xx-uart",
.of_match_table = lpc18xx_serial_match,
},
};
-module_platform_driver(lpc18xx_serial_driver);
+builtin_platform_driver(lpc18xx_serial_driver);
+/*
MODULE_AUTHOR("Joachim Eastwood <[email protected]>");
MODULE_DESCRIPTION("Serial port driver NXP LPC18xx/43xx devices");
MODULE_LICENSE("GPL v2");
+*/
--
2.5.0
On Sun, 2015-09-08 at 00:51:35 UTC, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
>
> drivers/tty/hvc/Kconfig:config HVC_DRIVER
> drivers/tty/hvc/Kconfig: bool
>
> ...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, even
> though someone bothered to comment that the code was not used.
>
> Unlike other changes, this driver binds in w/o using module_init,
> so we dont have init ordering concerns with this commit.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Jiri Slaby <[email protected]>
> Cc: [email protected]
> Signed-off-by: Paul Gortmaker <[email protected]>
Seems sane to me, possibly back in the deep dark past it could be a module, but
no point keeping the code around if it can't anymore.
Acked-by: Michael Ellerman <[email protected]>
cheers
Hi Paul,
On 9 August 2015 at 02:51, Paul Gortmaker <[email protected]> wrote:
> The Kconfig currently controlling compilation of this code is:
>
> 8250/Kconfig:config SERIAL_8250_LPC18XX
> 8250/Kconfig: bool "NXP LPC18xx/43xx serial port 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.
The bool in KConfig is more or less a copy-paste mistake on my side. I
really meant for this driver to be buildable as module and it builds
just fine with 'm'.
Want me send a patch changing it to tristate or will you handle it?
regards,
Joachim Eastwood
[Re: [PATCH 5/5] drivers/tty: make serial 8250_lpc18xx.c explicitly non-modular] On 10/08/2015 (Mon 20:42) Joachim Eastwood wrote:
> Hi Paul,
>
> On 9 August 2015 at 02:51, Paul Gortmaker <[email protected]> wrote:
> > The Kconfig currently controlling compilation of this code is:
> >
> > 8250/Kconfig:config SERIAL_8250_LPC18XX
> > 8250/Kconfig: bool "NXP LPC18xx/43xx serial port 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.
>
> The bool in KConfig is more or less a copy-paste mistake on my side. I
> really meant for this driver to be buildable as module and it builds
> just fine with 'm'.
>
> Want me send a patch changing it to tristate or will you handle it?
I will do a v2 in the next day or so with collected ACKs and this patch
replaced, unless Greg jumps in and indicates he'd like something
different.
Paul.
--
>
>
> regards, Joachim Eastwood
On Tue, Aug 11, 2015 at 02:05:44AM -0400, Paul Gortmaker wrote:
> [Re: [PATCH 5/5] drivers/tty: make serial 8250_lpc18xx.c explicitly non-modular] On 10/08/2015 (Mon 20:42) Joachim Eastwood wrote:
>
> > Hi Paul,
> >
> > On 9 August 2015 at 02:51, Paul Gortmaker <[email protected]> wrote:
> > > The Kconfig currently controlling compilation of this code is:
> > >
> > > 8250/Kconfig:config SERIAL_8250_LPC18XX
> > > 8250/Kconfig: bool "NXP LPC18xx/43xx serial port 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.
> >
> > The bool in KConfig is more or less a copy-paste mistake on my side. I
> > really meant for this driver to be buildable as module and it builds
> > just fine with 'm'.
> >
> > Want me send a patch changing it to tristate or will you handle it?
>
> I will do a v2 in the next day or so with collected ACKs and this patch
> replaced, unless Greg jumps in and indicates he'd like something
> different.
Nope, that would be fine with me, thanks.
greg k-h
[[PATCH 0/5] drivers/tty: make more bool drivers explicitly non-modular] On 18/10/2015 (Sun 18:21) Paul Gortmaker wrote:
> This next set of patches to drivers/tty fixes up more drivers that would
> perhaps appear modular when looking at the code, but are controlled by
> a bool Kconfig. We make sure they don't look modular and get rid
> of module related code in the drivers that would never be used.
[...]
> Paul Gortmaker (5):
> drivers/tty: make hvc/hvc_vio.c explicitly non-modular
> drivers/tty: make ehv_bytechan.c explicitly non-modular
> drivers/tty/serial: make serial/atmel_serial.c explicitly non-modular
> drivers/tty/serial: make msm_serial.c explicitly non-modular
> drivers/tty/serial: make 8250/8250_mtk.c explicitly non-modular
Hi Greg,
You've merged the #3 atmel patch and #5 the 8250_mtk patch; the msm_serial
patch (#4) is no longer applicable since the authors changed it to tristate.
That leaves the top two patches; both for PPC hardware. Did you skip
those because you wanted them pushed through the powerpc tree?
If so, that is fine, I can do that. Just let me know.
Thanks,
Paul.
--
>
> drivers/tty/ehv_bytechan.c | 40 +++------------------------------
> drivers/tty/hvc/hvc_vio.c | 29 ++++--------------------
> drivers/tty/serial/8250/8250_mtk.c | 35 ++++++-----------------------
> drivers/tty/serial/atmel_serial.c | 45 ++++----------------------------------
> drivers/tty/serial/msm_serial.c | 30 ++++---------------------
> 5 files changed, 22 insertions(+), 157 deletions(-)
>
> --
> 2.6.1
>