All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] phy: remove needless usage of module header
@ 2018-12-10  0:34 Paul Gortmaker
  2018-12-10  0:34 ` [PATCH 1/3] phy: make phy-core explicitly non-modular Paul Gortmaker
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Paul Gortmaker @ 2018-12-10  0:34 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: linux-kernel, Paul Gortmaker, Andrew Lunn, Gregory CLEMENT

The most important thing to note here, is these clean-ups make no
changes to the final generated run-time.  The 1st commit removes an
unused function, otherwise the generated objects are also unchanged.

The work here represents a scan over the phy dir, looking for files
that have nothing to do with a modular use case, but are using modular
infrastructure regardless.

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.  This has been done in quite
a lot of other mainline subsystem dirs already.

Using modular infrastructure in non-modules might seem harmless, but some
of the 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 quickly.

As a data point for #3 above, an empty C file that just includes the
module.h header generates over 750kB of CPP output.  Repeating the same
experiment with init.h and the result is less than 12kB; with export.h
it is only about 1/2kB; with both it still is less than 12kB.

Build tested on x86-64 and ARM-64.

Paul.
---

Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>

Paul Gortmaker (3):
  phy: make phy-core explicitly non-modular
  phy: make phy-mvebu-sata explicitly non-modular
  phy: make phy-armada375-usb2 explicitly non-modular

 drivers/phy/marvell/phy-armada375-usb2.c |  8 +-------
 drivers/phy/marvell/phy-mvebu-sata.c     |  9 ++-------
 drivers/phy/phy-core.c                   | 12 +-----------
 3 files changed, 4 insertions(+), 25 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] phy: make phy-core explicitly non-modular
  2018-12-10  0:34 [PATCH 0/3] phy: remove needless usage of module header Paul Gortmaker
@ 2018-12-10  0:34 ` Paul Gortmaker
  2018-12-10  0:34 ` [PATCH 2/3] phy: make phy-mvebu-sata " Paul Gortmaker
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Paul Gortmaker @ 2018-12-10  0:34 UTC (permalink / raw)
  To: Kishon Vijay Abraham I; +Cc: linux-kernel, Paul Gortmaker

The Kconfig currently controlling compilation of this code is:

drivers/phy/Kconfig:config GENERIC_PHY
drivers/phy/Kconfig:    bool "PHY Core"

...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 don't remove module.h since the file is using other modular fcns
(to load other phy modules) even though the core support itself is
non-modular.

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: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/phy/phy-core.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index df3d4ba516ab..f10110676572 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -1048,14 +1048,4 @@ static int __init phy_core_init(void)
 
 	return 0;
 }
-module_init(phy_core_init);
-
-static void __exit phy_core_exit(void)
-{
-	class_destroy(phy_class);
-}
-module_exit(phy_core_exit);
-
-MODULE_DESCRIPTION("Generic PHY Framework");
-MODULE_AUTHOR("Kishon Vijay Abraham I <kishon@ti.com>");
-MODULE_LICENSE("GPL v2");
+device_initcall(phy_core_init);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] phy: make phy-mvebu-sata explicitly non-modular
  2018-12-10  0:34 [PATCH 0/3] phy: remove needless usage of module header Paul Gortmaker
  2018-12-10  0:34 ` [PATCH 1/3] phy: make phy-core explicitly non-modular Paul Gortmaker
@ 2018-12-10  0:34 ` Paul Gortmaker
  2018-12-10  0:34 ` [PATCH 3/3] phy: make phy-armada375-usb2 " Paul Gortmaker
  2019-02-04  9:55 ` [PATCH 0/3] phy: remove needless usage of module header Kishon Vijay Abraham I
  3 siblings, 0 replies; 6+ messages in thread
From: Paul Gortmaker @ 2018-12-10  0:34 UTC (permalink / raw)
  To: Kishon Vijay Abraham I; +Cc: linux-kernel, Paul Gortmaker, Andrew Lunn

The Kconfig currently controlling compilation of this code is:

drivers/phy/Kconfig:config PHY_MVEBU_SATA
drivers/phy/Kconfig:    def_bool y

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

Lets remove the couple of traces of modular infrastructure, 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 also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/phy/marvell/phy-mvebu-sata.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-sata.c b/drivers/phy/marvell/phy-mvebu-sata.c
index 768ce92e81ce..369fece2be7a 100644
--- a/drivers/phy/marvell/phy-mvebu-sata.c
+++ b/drivers/phy/marvell/phy-mvebu-sata.c
@@ -10,7 +10,7 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/clk.h>
 #include <linux/phy/phy.h>
 #include <linux/io.h>
@@ -122,7 +122,6 @@ static const struct of_device_id phy_mvebu_sata_of_match[] = {
 	{ .compatible = "marvell,mvebu-sata-phy" },
 	{ },
 };
-MODULE_DEVICE_TABLE(of, phy_mvebu_sata_of_match);
 
 static struct platform_driver phy_mvebu_sata_driver = {
 	.probe	= phy_mvebu_sata_probe,
@@ -131,8 +130,4 @@ static struct platform_driver phy_mvebu_sata_driver = {
 		.of_match_table	= phy_mvebu_sata_of_match,
 	}
 };
-module_platform_driver(phy_mvebu_sata_driver);
-
-MODULE_AUTHOR("Andrew Lunn <andrew@lunn.ch>");
-MODULE_DESCRIPTION("Marvell MVEBU SATA PHY driver");
-MODULE_LICENSE("GPL v2");
+builtin_platform_driver(phy_mvebu_sata_driver);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] phy: make phy-armada375-usb2 explicitly non-modular
  2018-12-10  0:34 [PATCH 0/3] phy: remove needless usage of module header Paul Gortmaker
  2018-12-10  0:34 ` [PATCH 1/3] phy: make phy-core explicitly non-modular Paul Gortmaker
  2018-12-10  0:34 ` [PATCH 2/3] phy: make phy-mvebu-sata " Paul Gortmaker
@ 2018-12-10  0:34 ` Paul Gortmaker
  2019-01-18 14:44   ` Gregory CLEMENT
  2019-02-04  9:55 ` [PATCH 0/3] phy: remove needless usage of module header Kishon Vijay Abraham I
  3 siblings, 1 reply; 6+ messages in thread
From: Paul Gortmaker @ 2018-12-10  0:34 UTC (permalink / raw)
  To: Kishon Vijay Abraham I; +Cc: linux-kernel, Paul Gortmaker, Gregory CLEMENT

The Kconfig currently controlling compilation of this code is:

drivers/phy/marvell/Kconfig:config ARMADA375_USBCLUSTER_PHY
drivers/phy/marvell/Kconfig:    def_bool y

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

Lets remove the couple of traces of modular infrastructure, 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 also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/phy/marvell/phy-armada375-usb2.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/phy/marvell/phy-armada375-usb2.c b/drivers/phy/marvell/phy-armada375-usb2.c
index 1a3db288c0a9..4f0295992500 100644
--- a/drivers/phy/marvell/phy-armada375-usb2.c
+++ b/drivers/phy/marvell/phy-armada375-usb2.c
@@ -18,7 +18,6 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
-#include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
@@ -142,7 +141,6 @@ static const struct of_device_id of_usb_cluster_table[] = {
 	{ .compatible = "marvell,armada-375-usb-cluster", },
 	{ /* end of list */ },
 };
-MODULE_DEVICE_TABLE(of, of_usb_cluster_table);
 
 static struct platform_driver armada375_usb_phy_driver = {
 	.probe	= armada375_usb_phy_probe,
@@ -151,8 +149,4 @@ static struct platform_driver armada375_usb_phy_driver = {
 		.name  = "armada-375-usb-cluster",
 	}
 };
-module_platform_driver(armada375_usb_phy_driver);
-
-MODULE_DESCRIPTION("Armada 375 USB cluster driver");
-MODULE_AUTHOR("Gregory CLEMENT <gregory.clement@free-electrons.com>");
-MODULE_LICENSE("GPL");
+builtin_platform_driver(armada375_usb_phy_driver);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] phy: make phy-armada375-usb2 explicitly non-modular
  2018-12-10  0:34 ` [PATCH 3/3] phy: make phy-armada375-usb2 " Paul Gortmaker
@ 2019-01-18 14:44   ` Gregory CLEMENT
  0 siblings, 0 replies; 6+ messages in thread
From: Gregory CLEMENT @ 2019-01-18 14:44 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: Kishon Vijay Abraham I, linux-kernel, Gregory CLEMENT

Hi Paul,
 
 On dim., déc. 09 2018, Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> The Kconfig currently controlling compilation of this code is:
>
> drivers/phy/marvell/Kconfig:config ARMADA375_USBCLUSTER_PHY
> drivers/phy/marvell/Kconfig:    def_bool y
>
> ...meaning that it currently is not being built as a module by anyone.
>
> Lets remove the couple of traces of modular infrastructure, 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 also delete the MODULE_LICENSE tag etc. since all that information
> is already contained at the top of the file in the comments.
>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Reviewed-by: Gregory CLEMENT <gregory.clement@bootlin.com>

Thanks,

Gregory

> ---
>  drivers/phy/marvell/phy-armada375-usb2.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/phy/marvell/phy-armada375-usb2.c b/drivers/phy/marvell/phy-armada375-usb2.c
> index 1a3db288c0a9..4f0295992500 100644
> --- a/drivers/phy/marvell/phy-armada375-usb2.c
> +++ b/drivers/phy/marvell/phy-armada375-usb2.c
> @@ -18,7 +18,6 @@
>  #include <linux/init.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> -#include <linux/module.h>
>  #include <linux/of_address.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> @@ -142,7 +141,6 @@ static const struct of_device_id of_usb_cluster_table[] = {
>  	{ .compatible = "marvell,armada-375-usb-cluster", },
>  	{ /* end of list */ },
>  };
> -MODULE_DEVICE_TABLE(of, of_usb_cluster_table);
>  
>  static struct platform_driver armada375_usb_phy_driver = {
>  	.probe	= armada375_usb_phy_probe,
> @@ -151,8 +149,4 @@ static struct platform_driver armada375_usb_phy_driver = {
>  		.name  = "armada-375-usb-cluster",
>  	}
>  };
> -module_platform_driver(armada375_usb_phy_driver);
> -
> -MODULE_DESCRIPTION("Armada 375 USB cluster driver");
> -MODULE_AUTHOR("Gregory CLEMENT <gregory.clement@free-electrons.com>");
> -MODULE_LICENSE("GPL");
> +builtin_platform_driver(armada375_usb_phy_driver);
> -- 
> 2.7.4
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/3] phy: remove needless usage of module header
  2018-12-10  0:34 [PATCH 0/3] phy: remove needless usage of module header Paul Gortmaker
                   ` (2 preceding siblings ...)
  2018-12-10  0:34 ` [PATCH 3/3] phy: make phy-armada375-usb2 " Paul Gortmaker
@ 2019-02-04  9:55 ` Kishon Vijay Abraham I
  3 siblings, 0 replies; 6+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-04  9:55 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: linux-kernel, Andrew Lunn, Gregory CLEMENT



On 10/12/18 6:04 AM, Paul Gortmaker wrote:
> The most important thing to note here, is these clean-ups make no
> changes to the final generated run-time.  The 1st commit removes an
> unused function, otherwise the generated objects are also unchanged.
> 
> The work here represents a scan over the phy dir, looking for files
> that have nothing to do with a modular use case, but are using modular
> infrastructure regardless.
> 
> 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.  This has been done in quite
> a lot of other mainline subsystem dirs already.
> 
> Using modular infrastructure in non-modules might seem harmless, but some
> of the 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 quickly.
> 
> As a data point for #3 above, an empty C file that just includes the
> module.h header generates over 750kB of CPP output.  Repeating the same
> experiment with init.h and the result is less than 12kB; with export.h
> it is only about 1/2kB; with both it still is less than 12kB.
> 
> Build tested on x86-64 and ARM-64.

merged, thanks!

-Kishon
> 
> Paul.
> ---
> 
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> 
> Paul Gortmaker (3):
>   phy: make phy-core explicitly non-modular
>   phy: make phy-mvebu-sata explicitly non-modular
>   phy: make phy-armada375-usb2 explicitly non-modular
> 
>  drivers/phy/marvell/phy-armada375-usb2.c |  8 +-------
>  drivers/phy/marvell/phy-mvebu-sata.c     |  9 ++-------
>  drivers/phy/phy-core.c                   | 12 +-----------
>  3 files changed, 4 insertions(+), 25 deletions(-)
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-02-04  9:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-10  0:34 [PATCH 0/3] phy: remove needless usage of module header Paul Gortmaker
2018-12-10  0:34 ` [PATCH 1/3] phy: make phy-core explicitly non-modular Paul Gortmaker
2018-12-10  0:34 ` [PATCH 2/3] phy: make phy-mvebu-sata " Paul Gortmaker
2018-12-10  0:34 ` [PATCH 3/3] phy: make phy-armada375-usb2 " Paul Gortmaker
2019-01-18 14:44   ` Gregory CLEMENT
2019-02-04  9:55 ` [PATCH 0/3] phy: remove needless usage of module header Kishon Vijay Abraham I

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.