Quantcast

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

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Gortmaker-3
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: [hidden email]
Cc: Paul Gortmaker <[hidden email]>
Cc: Rich Felker <[hidden email]>
Cc: Yoshinori Sato <[hidden email]>


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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Gortmaker-3
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 <[hidden email]>
Cc: Rich Felker <[hidden email]>
Cc: [hidden email]
Signed-off-by: Paul Gortmaker <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Gortmaker-3
In reply to this post by Paul Gortmaker-3
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 <[hidden email]>
Cc: Rich Felker <[hidden email]>
Cc: [hidden email]
Signed-off-by: Paul Gortmaker <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Gortmaker-3
In reply to this post by Paul Gortmaker-3
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 <[hidden email]>
Cc: Rich Felker <[hidden email]>
Cc: [hidden email]
Signed-off-by: Paul Gortmaker <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Gortmaker-3
In reply to this post by Paul Gortmaker-3
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 <[hidden email]>
Cc: Rich Felker <[hidden email]>
Cc: Paul Gortmaker <[hidden email]>
Cc: [hidden email]
Signed-off-by: Paul Gortmaker <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Gortmaker-3
In reply to this post by Paul Gortmaker-3
[[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: [hidden email]
> Cc: Paul Gortmaker <[hidden email]>
> Cc: Rich Felker <[hidden email]>
> Cc: Yoshinori Sato <[hidden email]>
>
>
> 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
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Rich Felker-2
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: [hidden email]
> > Cc: Paul Gortmaker <[hidden email]>
> > Cc: Rich Felker <[hidden email]>
> > Cc: Yoshinori Sato <[hidden email]>
> >
> >
> > 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 [hidden email]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Loading...