[PATCH 0/4] sh: fix up modular use in non-modular code

[PATCH 0/4] sh: fix up modular use in non-modular code

[Paul Gortmaker]

For anyone new to the underlying goal of this cleanup, we are trying to
make kernel code consistent with the Makefiles/Kconfigs that control them.

This means not using modular functions/macros for code that can never
be built as a module.  Some of the other downfalls this 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 to CPP overhead.
 (4) it gets copied/replicated into other drivers and spreads like weeds.

Three of the four commits here are completely trivial with zero runtime
impact whatsoever.  The fourth has a deletion of a ".remove" function
and as we've done elsewhere, we block the sysfs ability to reach in and
manually execute that function, since there isn't a sane use case for
which doing that makes sense.

Build tested on today's linux-next ; full build fails on duplicate syms
from OF generic board, but that is a known and reported issue elsewhere;
it has nothing to do with the changes here.

Paul
--

Cc: linux-sh@vger.kernel.org
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Rich Felker <dalias@libc.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>


Paul Gortmaker (4):
  sh: make time.c explicitly non-modular
  sh: make mm/asids-debugfs explicitly non-modular
  sh: make board-secureedge5410 explicitly non-modular
  sh: make heartbeat driver explicitly non-modular

 arch/sh/boards/board-secureedge5410.c |  3 +--
 arch/sh/drivers/heartbeat.c           | 32 +++-----------------------------
 arch/sh/kernel/time.c                 |  3 +--
 arch/sh/mm/asids-debugfs.c            |  5 +----
 4 files changed, 6 insertions(+), 37 deletions(-)

-- 
2.8.0

[PATCH 1/4] sh: make time.c explicitly non-modular

[Paul Gortmaker]

The Makefile currently controlling compilation of this code is:

obj-y   := debugtraps.o dma-nommu.o dumpstack.o                 \
[...]
           syscalls_$(BITS).o time.o topology.o traps.o         \
           traps_$(BITS).o unwinder.o

...meaning that it currently is not being built as a module by anyone.

Lets remove the couple traces of modular code, 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: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: linux-sh@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/sh/kernel/time.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/sh/kernel/time.c b/arch/sh/kernel/time.c
index d6d0a986c6e9..bfe1de98d003 100644
--- a/arch/sh/kernel/time.c
+++ b/arch/sh/kernel/time.c
@@ -11,7 +11,6 @@
  * for more details.
  */
 #include <linux/kernel.h>
-#include <linux/module.h>
 #include <linux/init.h>
 #include <linux/profile.h>
 #include <linux/timex.h>
@@ -83,7 +82,7 @@ static int __init rtc_generic_init(void)
 
 	return PTR_ERR_OR_ZERO(pdev);
 }
-module_init(rtc_generic_init);
+device_initcall(rtc_generic_init);
 
 void (*board_time_init)(void);
 
-- 
2.8.0

[PATCH 2/4] sh: make mm/asids-debugfs explicitly non-modular

[Paul Gortmaker]

The Makefile/Kconfig currently controlling compilation of this code is:

obj-$(CONFIG_DEBUG_FS)          += $(debugfs-y)
debugfs-y                       := asids-debugfs.o

lib/Kconfig.debug:config DEBUG_FS
lib/Kconfig.debug:      bool "Debug Filesystem"

...meaning that it currently is not being built as a module by anyone.

Lets remove the couple traces of modular code, 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: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: linux-sh@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/sh/mm/asids-debugfs.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/sh/mm/asids-debugfs.c b/arch/sh/mm/asids-debugfs.c
index ecfc6b0c1da1..bf95fdaedd0c 100644
--- a/arch/sh/mm/asids-debugfs.c
+++ b/arch/sh/mm/asids-debugfs.c
@@ -17,7 +17,6 @@
  * for more details.
  */
 #include <linux/init.h>
-#include <linux/module.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
 #include <linux/spinlock.h>
@@ -70,6 +69,4 @@ static int __init asids_debugfs_init(void)
 
 	return PTR_ERR_OR_ZERO(asids_dentry);
 }
-module_init(asids_debugfs_init);
-
-MODULE_LICENSE("GPL v2");
+device_initcall(asids_debugfs_init);
-- 
2.8.0

[PATCH 3/4] sh: make board-secureedge5410 explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

config SH_SECUREEDGE5410
        bool "SecureEdge5410"

...meaning that it currently is not being built as a module by anyone.

Lets remove the couple traces of modularity 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 don't replace module.h with init.h since the file already has that.

Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: linux-sh@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/sh/boards/board-secureedge5410.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/sh/boards/board-secureedge5410.c b/arch/sh/boards/board-secureedge5410.c
index 98b36205aa7b..97ec67ffec2b 100644
--- a/arch/sh/boards/board-secureedge5410.c
+++ b/arch/sh/boards/board-secureedge5410.c
@@ -14,7 +14,6 @@
 #include <linux/interrupt.h>
 #include <linux/timer.h>
 #include <linux/delay.h>
-#include <linux/module.h>
 #include <linux/sched.h>
 #include <asm/machvec.h>
 #include <mach/secureedge5410.h>
@@ -49,7 +48,7 @@ static int __init eraseconfig_init(void)
 				irq);
 	return 0;
 }
-module_init(eraseconfig_init);
+device_initcall(eraseconfig_init);
 
 /*
  * Initialize IRQ setting
-- 
2.8.0

[PATCH 4/4] sh: make heartbeat driver explicitly non-modular

[Paul Gortmaker]

The Kconfig for this driver is currently:

config HEARTBEAT
        bool "Heartbeat LED"

...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 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.

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: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: linux-sh@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/sh/drivers/heartbeat.c | 32 +++-----------------------------
 1 file changed, 3 insertions(+), 29 deletions(-)

diff --git a/arch/sh/drivers/heartbeat.c b/arch/sh/drivers/heartbeat.c
index 7efc9c354fc7..49bace446a1a 100644
--- a/arch/sh/drivers/heartbeat.c
+++ b/arch/sh/drivers/heartbeat.c
@@ -19,7 +19,6 @@
  * for more details.
  */
 #include <linux/init.h>
-#include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/timer.h>
@@ -139,26 +138,11 @@ static int heartbeat_drv_probe(struct platform_device *pdev)
 	return mod_timer(&hd->timer, jiffies + 1);
 }
 
-static int heartbeat_drv_remove(struct platform_device *pdev)
-{
-	struct heartbeat_data *hd = platform_get_drvdata(pdev);
-
-	del_timer_sync(&hd->timer);
-	iounmap(hd->base);
-
-	platform_set_drvdata(pdev, NULL);
-
-	if (!pdev->dev.platform_data)
-		kfree(hd);
-
-	return 0;
-}
-
 static struct platform_driver heartbeat_driver = {
 	.probe		= heartbeat_drv_probe,
-	.remove		= heartbeat_drv_remove,
 	.driver		= {
-		.name	= DRV_NAME,
+		.name			= DRV_NAME,
+		.suppress_bind_attrs	= true,
 	},
 };
 
@@ -167,14 +151,4 @@ static int __init heartbeat_init(void)
 	printk(KERN_NOTICE DRV_NAME ": version %s loaded\n", DRV_VERSION);
 	return platform_driver_register(&heartbeat_driver);
 }
-
-static void __exit heartbeat_exit(void)
-{
-	platform_driver_unregister(&heartbeat_driver);
-}
-module_init(heartbeat_init);
-module_exit(heartbeat_exit);
-
-MODULE_VERSION(DRV_VERSION);
-MODULE_AUTHOR("Paul Mundt");
-MODULE_LICENSE("GPL v2");
+device_initcall(heartbeat_init);
-- 
2.8.0

Re: [PATCH 0/4] sh: fix up modular use in non-modular code

[Paul Gortmaker]

[[PATCH 0/4] sh: fix up modular use in non-modular code] On 22/04/2016 (Fri 14:07) Paul Gortmaker wrote:

> For anyone new to the underlying goal of this cleanup, we are trying to
> make kernel code consistent with the Makefiles/Kconfigs that control them.

Ping -- wondering if there are any issues here, or if I can expect these
to be merged upstream in the merge window.  I didn't see them loop
around via the linux-next tree but I wasn't sure if arch/sh has a repo
for linux-next.

Thanks,
Paul.
--

> 
> This means not using modular functions/macros for code that can never
> be built as a module.  Some of the other downfalls this 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 to CPP overhead.
>  (4) it gets copied/replicated into other drivers and spreads like weeds.
> 
> Three of the four commits here are completely trivial with zero runtime
> impact whatsoever.  The fourth has a deletion of a ".remove" function
> and as we've done elsewhere, we block the sysfs ability to reach in and
> manually execute that function, since there isn't a sane use case for
> which doing that makes sense.
> 
> Build tested on today's linux-next ; full build fails on duplicate syms
> from OF generic board, but that is a known and reported issue elsewhere;
> it has nothing to do with the changes here.
> 
> Paul
> --
> 
> Cc: linux-sh@vger.kernel.org
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Cc: Rich Felker <dalias@libc.org>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> 
> 
> Paul Gortmaker (4):
>   sh: make time.c explicitly non-modular
>   sh: make mm/asids-debugfs explicitly non-modular
>   sh: make board-secureedge5410 explicitly non-modular
>   sh: make heartbeat driver explicitly non-modular
> 
>  arch/sh/boards/board-secureedge5410.c |  3 +--
>  arch/sh/drivers/heartbeat.c           | 32 +++-----------------------------
>  arch/sh/kernel/time.c                 |  3 +--
>  arch/sh/mm/asids-debugfs.c            |  5 +----
>  4 files changed, 6 insertions(+), 37 deletions(-)
> 
> -- 
> 2.8.0
> 

Re: [PATCH 0/4] sh: fix up modular use in non-modular code

[Rich Felker]

On Thu, May 26, 2016 at 12:45:57PM -0400, Paul Gortmaker wrote:
> [[PATCH 0/4] sh: fix up modular use in non-modular code] On 22/04/2016 (Fri 14:07) Paul Gortmaker wrote:
> 
> > For anyone new to the underlying goal of this cleanup, we are trying to
> > make kernel code consistent with the Makefiles/Kconfigs that control them.
> 
> Ping -- wondering if there are any issues here, or if I can expect these
> to be merged upstream in the merge window.  I didn't see them loop
> around via the linux-next tree but I wasn't sure if arch/sh has a repo
> for linux-next.

I don't have experience with the affected code, but it looks ok unless
any of these "should" be modular and Kconfig was just wrong to
disallow it.

Sato-san, do you have an opinion on these? Should I go ahead and just
merge them?

Rich

> > This means not using modular functions/macros for code that can never
> > be built as a module.  Some of the other downfalls this 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 to CPP overhead.
> >  (4) it gets copied/replicated into other drivers and spreads like weeds.
> > 
> > Three of the four commits here are completely trivial with zero runtime
> > impact whatsoever.  The fourth has a deletion of a ".remove" function
> > and as we've done elsewhere, we block the sysfs ability to reach in and
> > manually execute that function, since there isn't a sane use case for
> > which doing that makes sense.
> > 
> > Build tested on today's linux-next ; full build fails on duplicate syms
> > from OF generic board, but that is a known and reported issue elsewhere;
> > it has nothing to do with the changes here.
> > 
> > Paul
> > --
> > 
> > Cc: linux-sh@vger.kernel.org
> > Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> > Cc: Rich Felker <dalias@libc.org>
> > Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> > 
> > 
> > Paul Gortmaker (4):
> >   sh: make time.c explicitly non-modular
> >   sh: make mm/asids-debugfs explicitly non-modular
> >   sh: make board-secureedge5410 explicitly non-modular
> >   sh: make heartbeat driver explicitly non-modular
> > 
> >  arch/sh/boards/board-secureedge5410.c |  3 +--
> >  arch/sh/drivers/heartbeat.c           | 32 +++-----------------------------
> >  arch/sh/kernel/time.c                 |  3 +--
> >  arch/sh/mm/asids-debugfs.c            |  5 +----
> >  4 files changed, 6 insertions(+), 37 deletions(-)
> > 
> > -- 
> > 2.8.0
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] sh: fix up modular use in non-modular code

[Yoshinori Sato]

On Fri, 27 May 2016 01:53:34 +0900,
Rich Felker wrote:
> 
> On Thu, May 26, 2016 at 12:45:57PM -0400, Paul Gortmaker wrote:
> > [[PATCH 0/4] sh: fix up modular use in non-modular code] On 22/04/2016 (Fri 14:07) Paul Gortmaker wrote:
> > 
> > > For anyone new to the underlying goal of this cleanup, we are trying to
> > > make kernel code consistent with the Makefiles/Kconfigs that control them.
> > 
> > Ping -- wondering if there are any issues here, or if I can expect these
> > to be merged upstream in the merge window.  I didn't see them loop
> > around via the linux-next tree but I wasn't sure if arch/sh has a repo
> > for linux-next.
> 
> I don't have experience with the affected code, but it looks ok unless
> any of these "should" be modular and Kconfig was just wrong to
> disallow it.
> 
> Sato-san, do you have an opinion on these? Should I go ahead and just
> merge them?

It looks no problem.
I think can merge it.

> Rich
> 
> > > This means not using modular functions/macros for code that can never
> > > be built as a module.  Some of the other downfalls this 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 to CPP overhead.
> > >  (4) it gets copied/replicated into other drivers and spreads like weeds.
> > > 
> > > Three of the four commits here are completely trivial with zero runtime
> > > impact whatsoever.  The fourth has a deletion of a ".remove" function
> > > and as we've done elsewhere, we block the sysfs ability to reach in and
> > > manually execute that function, since there isn't a sane use case for
> > > which doing that makes sense.
> > > 
> > > Build tested on today's linux-next ; full build fails on duplicate syms
> > > from OF generic board, but that is a known and reported issue elsewhere;
> > > it has nothing to do with the changes here.
> > > 
> > > Paul
> > > --
> > > 
> > > Cc: linux-sh@vger.kernel.org
> > > Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> > > Cc: Rich Felker <dalias@libc.org>
> > > Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> > > 
> > > 
> > > Paul Gortmaker (4):
> > >   sh: make time.c explicitly non-modular
> > >   sh: make mm/asids-debugfs explicitly non-modular
> > >   sh: make board-secureedge5410 explicitly non-modular
> > >   sh: make heartbeat driver explicitly non-modular
> > > 
> > >  arch/sh/boards/board-secureedge5410.c |  3 +--
> > >  arch/sh/drivers/heartbeat.c           | 32 +++-----------------------------
> > >  arch/sh/kernel/time.c                 |  3 +--
> > >  arch/sh/mm/asids-debugfs.c            |  5 +----
> > >  4 files changed, 6 insertions(+), 37 deletions(-)
> > > 
> > > -- 
> > > 2.8.0
> > > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

Re: [PATCH 0/4] sh: fix up modular use in non-modular code

[Paul Gortmaker]

[Re: [PATCH 0/4] sh: fix up modular use in non-modular code] On 02/06/2016 (Thu 15:54) Yoshinori Sato wrote:

> On Fri, 27 May 2016 01:53:34 +0900,
> Rich Felker wrote:
> > 
> > On Thu, May 26, 2016 at 12:45:57PM -0400, Paul Gortmaker wrote:
> > > [[PATCH 0/4] sh: fix up modular use in non-modular code] On 22/04/2016 (Fri 14:07) Paul Gortmaker wrote:
> > > 
> > > > For anyone new to the underlying goal of this cleanup, we are trying to
> > > > make kernel code consistent with the Makefiles/Kconfigs that control them.
> > > 
> > > Ping -- wondering if there are any issues here, or if I can expect these
> > > to be merged upstream in the merge window.  I didn't see them loop
> > > around via the linux-next tree but I wasn't sure if arch/sh has a repo
> > > for linux-next.
> > 
> > I don't have experience with the affected code, but it looks ok unless
> > any of these "should" be modular and Kconfig was just wrong to
> > disallow it.
> > 
> > Sato-san, do you have an opinion on these? Should I go ahead and just
> > merge them?
> 
> It looks no problem.
> I think can merge it.

Just checking in a month later; my queue that I test regularly against
the linux-next tree still has these:

sh-make-heartbeat-driver-explicitly-non-modular.patch
sh-make-board-secureedge5410.c-explicitly-non-m.patch
sh-make-mm-asids-debugfs-explicitly-non-modular.patch
sh-make-time.c-explicitly-non-modular.patch

Apologies in advance if you have a sh queue that is pending but for some
reason is not fed into the nightly linux-next tree generation.  If so,
let me know and I won't worry about looking for these until merge window.

Thanks,
Paul.
--

> 
> > Rich
> > 
> > > > This means not using modular functions/macros for code that can never
> > > > be built as a module.  Some of the other downfalls this 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 to CPP overhead.
> > > >  (4) it gets copied/replicated into other drivers and spreads like weeds.
> > > > 
> > > > Three of the four commits here are completely trivial with zero runtime
> > > > impact whatsoever.  The fourth has a deletion of a ".remove" function
> > > > and as we've done elsewhere, we block the sysfs ability to reach in and
> > > > manually execute that function, since there isn't a sane use case for
> > > > which doing that makes sense.
> > > > 
> > > > Build tested on today's linux-next ; full build fails on duplicate syms
> > > > from OF generic board, but that is a known and reported issue elsewhere;
> > > > it has nothing to do with the changes here.
> > > > 
> > > > Paul
> > > > --
> > > > 
> > > > Cc: linux-sh@vger.kernel.org
> > > > Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> > > > Cc: Rich Felker <dalias@libc.org>
> > > > Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> > > > 
> > > > 
> > > > Paul Gortmaker (4):
> > > >   sh: make time.c explicitly non-modular
> > > >   sh: make mm/asids-debugfs explicitly non-modular
> > > >   sh: make board-secureedge5410 explicitly non-modular
> > > >   sh: make heartbeat driver explicitly non-modular
> > > > 
> > > >  arch/sh/boards/board-secureedge5410.c |  3 +--
> > > >  arch/sh/drivers/heartbeat.c           | 32 +++-----------------------------
> > > >  arch/sh/kernel/time.c                 |  3 +--
> > > >  arch/sh/mm/asids-debugfs.c            |  5 +----
> > > >  4 files changed, 6 insertions(+), 37 deletions(-)
> > > > 
> > > > -- 
> > > > 2.8.0
> > > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> Yoshinori Sato
> <ysato@users.sourceforge.jp>