[PATCH 0/3] acpi: remove modular references from non modular code

[PATCH 0/3] acpi: remove modular references from non modular code

[Paul Gortmaker]

An audit of i386/x86_64 allmodconfig builds found these instances of 
non-modular ACPI code needlessly using the modular equivalents of 
their built-in counterparts.

In fixing that, we get rid of some dead code, make the mandate of
the remaining code more clear, and leave behind better examples
for future ACPI code to copy/emulate.

Two of them are trivial, in that there is no binary output change.
The other has dead code removal so the binary output will be
slightly smaller.   We've done a direct mapping of module_init to
device_initcall in each instance, so there is no initcall
reordering risks here.  Overall, in all cases, the runtime should
be completely unchanged.

Where others have deployed module_param() already, we have to
leave it, because there is no equivalent builtin_param (not yet)
and we have to remain compatible with existing bootarg users.

Paul.
---

Cc: Aaron Lu <aaron.lu@intel.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Len Brown <lenb@kernel.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linux-acpi@vger.kernel.org

Paul Gortmaker (3):
  drivers/acpi: make bgrt driver explicitly non-modular
  drivers/acpi: make apei/ghes.c more explicitly non-modular
  drivers/acpi: make pmic/intel_pmic_crc.c explicitly non-modular

 drivers/acpi/apei/ghes.c           | 23 +++++++----------------
 drivers/acpi/bgrt.c                | 10 +++-------
 drivers/acpi/pmic/intel_pmic_crc.c |  7 ++-----
 3 files changed, 12 insertions(+), 28 deletions(-)

-- 
2.6.1

[PATCH 1/3] drivers/acpi: make bgrt driver explicitly non-modular

[Paul Gortmaker]

The Kconfig for this driver is currently:

config ACPI_BGRT
    bool "Boottime Graphics Resource Table support"

...meaning that it currently is not being built as a module by anyone.
Lets remove all modular references, 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 also delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: linux-acpi@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/acpi/bgrt.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/bgrt.c b/drivers/acpi/bgrt.c
index a83e3c62c5a9..75f128e766a9 100644
--- a/drivers/acpi/bgrt.c
+++ b/drivers/acpi/bgrt.c
@@ -1,4 +1,6 @@
 /*
+ * BGRT boot graphic support
+ * Authors: Matthew Garrett, Josh Triplett <josh@joshtriplett.org>
  * Copyright 2012 Red Hat, Inc <mjg@redhat.com>
  * Copyright 2012 Intel Corporation
  *
@@ -8,7 +10,6 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/module.h>
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/sysfs.h>
@@ -103,9 +104,4 @@ out_kobject:
 	kobject_put(bgrt_kobj);
 	return ret;
 }
-
-module_init(bgrt_init);
-
-MODULE_AUTHOR("Matthew Garrett, Josh Triplett <josh@joshtriplett.org>");
-MODULE_DESCRIPTION("BGRT boot graphic support");
-MODULE_LICENSE("GPL");
+device_initcall(bgrt_init);
-- 
2.6.1

[PATCH 2/3] drivers/acpi: make apei/ghes.c more explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

config ACPI_APEI_GHES
      bool "APEI Generic Hardware Error Source"

...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 replace module.h with moduleparam.h as we are keeping the
pre-existing module_param that the file has, as currently that is
the easiest way to maintain compatibility with the existing boot
arg use cases.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Huang Ying <ying.huang@intel.com>
Cc: linux-acpi@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/acpi/apei/ghes.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 3dd9c462d22a..60746ef904e4 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -26,7 +26,7 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/init.h>
 #include <linux/acpi.h>
 #include <linux/io.h>
@@ -79,6 +79,11 @@
 	((struct acpi_hest_generic_status *)				\
 	 ((struct ghes_estatus_node *)(estatus_node) + 1))
 
+/*
+ * This driver isn't really modular, however for the time being,
+ * continuing to use module_param is the easiest way to remain
+ * compatible with existing boot arg use cases.
+ */
 bool ghes_disable;
 module_param_named(disable, ghes_disable, bool, 0);
 
@@ -1148,18 +1153,4 @@ err_ioremap_exit:
 err:
 	return rc;
 }
-
-static void __exit ghes_exit(void)
-{
-	platform_driver_unregister(&ghes_platform_driver);
-	ghes_estatus_pool_exit();
-	ghes_ioremap_exit();
-}
-
-module_init(ghes_init);
-module_exit(ghes_exit);
-
-MODULE_AUTHOR("Huang Ying");
-MODULE_DESCRIPTION("APEI Generic Hardware Error Source support");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:GHES");
+device_initcall(ghes_init);
-- 
2.6.1

[PATCH 3/3] drivers/acpi: make pmic/intel_pmic_crc.c explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

drivers/acpi/Kconfig:config CRC_PMIC_OPREGION
drivers/acpi/Kconfig:   bool "ACPI operation region support for CrystalCove PMIC"

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

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

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Aaron Lu <aaron.lu@intel.com>
Cc: linux-acpi@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/acpi/pmic/intel_pmic_crc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/pmic/intel_pmic_crc.c b/drivers/acpi/pmic/intel_pmic_crc.c
index 42df46a86c25..fcd1852dcdee 100644
--- a/drivers/acpi/pmic/intel_pmic_crc.c
+++ b/drivers/acpi/pmic/intel_pmic_crc.c
@@ -13,7 +13,7 @@
  * GNU General Public License for more details.
  */
 
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/acpi.h>
 #include <linux/mfd/intel_soc_pmic.h>
 #include <linux/regmap.h>
@@ -205,7 +205,4 @@ static int __init intel_crc_pmic_opregion_driver_init(void)
 {
 	return platform_driver_register(&intel_crc_pmic_opregion_driver);
 }
-module_init(intel_crc_pmic_opregion_driver_init);
-
-MODULE_DESCRIPTION("CrystalCove ACPI operation region driver");
-MODULE_LICENSE("GPL");
+device_initcall(intel_crc_pmic_opregion_driver_init);
-- 
2.6.1

Re: [PATCH 3/3] drivers/acpi: make pmic/intel_pmic_crc.c explicitly non-modular

[Aaron Lu]

On 02/15/2016 01:27 PM, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
> 
> drivers/acpi/Kconfig:config CRC_PMIC_OPREGION
> drivers/acpi/Kconfig:   bool "ACPI operation region support for CrystalCove PMIC"
> 
> ...meaning that it currently is not being built as a module by anyone.
> 
> Lets remove the couple modular references, 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 also delete the MODULE_LICENSE tag etc. since all that information
> is already contained at the top of the file in the comments.
> 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Aaron Lu <aaron.lu@intel.com>
> Cc: linux-acpi@vger.kernel.org
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Acked-by: Aaron Lu <aaron.lu@intel.com>

Initially I want to make it a module, but then people told me it's not
safe to remove operation region handler during runtime, so it became
always builtin. We can change it back to module once the above problem
is solved.

Re: [PATCH 0/3] acpi: remove modular references from non modular code

[Rafael J. Wysocki]

On Monday, February 15, 2016 12:27:48 AM Paul Gortmaker wrote:
> An audit of i386/x86_64 allmodconfig builds found these instances of 
> non-modular ACPI code needlessly using the modular equivalents of 
> their built-in counterparts.
> 
> In fixing that, we get rid of some dead code, make the mandate of
> the remaining code more clear, and leave behind better examples
> for future ACPI code to copy/emulate.
> 
> Two of them are trivial, in that there is no binary output change.
> The other has dead code removal so the binary output will be
> slightly smaller.   We've done a direct mapping of module_init to
> device_initcall in each instance, so there is no initcall
> reordering risks here.  Overall, in all cases, the runtime should
> be completely unchanged.
> 
> Where others have deployed module_param() already, we have to
> leave it, because there is no equivalent builtin_param (not yet)
> and we have to remain compatible with existing bootarg users.

All three applied.

Thanks,
Rafael

RE: [PATCH 0/3] acpi: remove modular references from non modular code

[Zheng, Lv]

Hi,

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Paul Gortmaker
> Sent: Monday, February 15, 2016 1:28 PM
> To: linux-kernel@vger.kernel.org
> Cc: Gortmaker, Paul (Wind River) <paul.gortmaker@windriver.com>; Lu, Aaron
> <aaron.lu@intel.com>; Huang, Ying <ying.huang@intel.com>; Josh Triplett
> <josh@joshtriplett.org>; Len Brown <lenb@kernel.org>; Matthew Garrett
> <mjg59@srcf.ucam.org>; Rafael J. Wysocki <rjw@rjwysocki.net>; linux-
> acpi@vger.kernel.org
> Subject: [PATCH 0/3] acpi: remove modular references from non modular code
> 
> An audit of i386/x86_64 allmodconfig builds found these instances of
> non-modular ACPI code needlessly using the modular equivalents of
> their built-in counterparts.
> 
> In fixing that, we get rid of some dead code, make the mandate of
> the remaining code more clear, and leave behind better examples
> for future ACPI code to copy/emulate.
> 
> Two of them are trivial, in that there is no binary output change.
> The other has dead code removal so the binary output will be
> slightly smaller.   We've done a direct mapping of module_init to
> device_initcall in each instance, so there is no initcall
> reordering risks here.  Overall, in all cases, the runtime should
> be completely unchanged.
> 
> Where others have deployed module_param() already, we have to
> leave it, because there is no equivalent builtin_param (not yet)
> and we have to remain compatible with existing bootarg users.
> 
> Paul.
> ---
> 
> Cc: Aaron Lu <aaron.lu@intel.com>
> Cc: Huang Ying <ying.huang@intel.com>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: linux-acpi@vger.kernel.org
> 
> Paul Gortmaker (3):
>   drivers/acpi: make bgrt driver explicitly non-modular
>   drivers/acpi: make apei/ghes.c more explicitly non-modular
>   drivers/acpi: make pmic/intel_pmic_crc.c explicitly non-modular
> 
>  drivers/acpi/apei/ghes.c           | 23 +++++++----------------
>  drivers/acpi/bgrt.c                | 10 +++-------
>  drivers/acpi/pmic/intel_pmic_crc.c |  7 ++-----
[Lv Zheng] 
Some drivers indeed can be built as modules.
They are not modules just because defects in the APIs that are provided by ACPICA.
And the defects will be no longer there sooner or later.

Thanks
-Lv

>  3 files changed, 12 insertions(+), 28 deletions(-)
> 
> --
> 2.6.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 3/3] drivers/acpi: make pmic/intel_pmic_crc.c explicitly non-modular

[Zheng, Lv]

Hi,

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Paul Gortmaker
> Subject: [PATCH 3/3] drivers/acpi: make pmic/intel_pmic_crc.c explicitly non-
> modular
> 
> The Kconfig currently controlling compilation of this code is:
> 
> drivers/acpi/Kconfig:config CRC_PMIC_OPREGION
> drivers/acpi/Kconfig:   bool "ACPI operation region support for CrystalCove
> PMIC"
> 
> ...meaning that it currently is not being built as a module by anyone.
> 
> Lets remove the couple modular references, 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 also delete the MODULE_LICENSE tag etc. since all that information
> is already contained at the top of the file in the comments.
[Lv Zheng] 
Especially, this can pass module test if it is built along with a workaround to fix the ACPICA defects.
The workaround is not upstreamed because it is just a workaround, real fix should be done inside of ACPICA.

Thanks and best regards
-Lv

> 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Aaron Lu <aaron.lu@intel.com>
> Cc: linux-acpi@vger.kernel.org
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  drivers/acpi/pmic/intel_pmic_crc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/pmic/intel_pmic_crc.c
> b/drivers/acpi/pmic/intel_pmic_crc.c
> index 42df46a86c25..fcd1852dcdee 100644
> --- a/drivers/acpi/pmic/intel_pmic_crc.c
> +++ b/drivers/acpi/pmic/intel_pmic_crc.c
> @@ -13,7 +13,7 @@
>   * GNU General Public License for more details.
>   */
> 
> -#include <linux/module.h>
> +#include <linux/init.h>
>  #include <linux/acpi.h>
>  #include <linux/mfd/intel_soc_pmic.h>
>  #include <linux/regmap.h>
> @@ -205,7 +205,4 @@ static int __init
> intel_crc_pmic_opregion_driver_init(void)
>  {
>  	return platform_driver_register(&intel_crc_pmic_opregion_driver);
>  }
> -module_init(intel_crc_pmic_opregion_driver_init);
> -
> -MODULE_DESCRIPTION("CrystalCove ACPI operation region driver");
> -MODULE_LICENSE("GPL");
> +device_initcall(intel_crc_pmic_opregion_driver_init);
> --
> 2.6.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html