Skip to contentSkip to conversation

ACPI: improve modularity and remove initcalls

PATCH New Bjorn Helgaas 1 participant 5 messages Nov 14, 2008

Subsystems (4)
ACPI PCI ACPI component architecture (acpica) ACPI thermal driver
lore
Share & tools
lore

Bjorn Helgaas

author
#0 lore raw

cover ACPI: improve modularity and remove initcalls

I'm on a quest to improve modularity and remove initcalls.

When possible, we should avoid initcalls because the order they happen depends on the module link order, so innocuous makefile changes can break things.

Yakui, I think I addressed your comments about sysfs duplicate filenames (I think the link order dependency was removed by commit 9b473de87209fa86eb421b23386693b461612f30). But if not, let me know.

Len, instead of making drivers/acpi/acpi.h, I made a drivers/acpi/acpi_core.h. The things in there shouldn't be visible outside of drivers/acpi, so I don't really want to put them in include/acpi/acpi_drivers.h.

I also added new pci.h and processor.h header files in drivers/acpi for those drivers. You mentioned Bob's ongoing header clean-up, but I assume he's concentrating on the ACPI CA, whereas the ones I added are for Linux drivers.

Comments welcome.

Bjorn

author
#1 lore raw

PATCH 1/4 ACPI: pci: make new pci.h header local to driver

The pci-related declarations in acpi_drivers.h are used only by the PCI root and related drivers. The usual way to do this is to have a per-driver header, so I created such a header and moved these declarations to it.

Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/acpi/pci.h          |   18 ++++++++++++++++++
 drivers/acpi/pci_bind.c     |    2 ++
 drivers/acpi/pci_irq.c      |    2 ++
 drivers/acpi/pci_root.c     |    2 ++
 include/acpi/acpi_drivers.h |   13 -------------
 5 files changed, 24 insertions(+), 13 deletions(-)
 create mode 100644 drivers/acpi/pci.h
drivers/acpi/pci.h new file +18 -0
new file mode 100644
index 0000000..b3512cf
@@ -0,0 +1,18 @@
+/* ACPI PCI Interrupt Link (pci_link.c) */
+
+int acpi_pci_link_allocate_irq(acpi_handle handle, int index, int *triggering,
+			       int *polarity, char **name);
+int acpi_pci_link_free_irq(acpi_handle handle);
+
+/* ACPI PCI Interrupt Routing (pci_irq.c) */
+
+int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus);
+void acpi_pci_irq_del_prt(int segment, int bus);
+
+/* ACPI PCI Device Binding (pci_bind.c) */
+
+struct pci_bus;
+
+int acpi_pci_bind(struct acpi_device *device);
+int acpi_pci_bind_root(struct acpi_device *device, struct acpi_pci_id *id,
+		       struct pci_bus *bus);
drivers/acpi/pci_bind.c +2 -0
index 4b252ea..676d0a4 100644
@@ -35,6 +35,8 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
+#include "pci.h"
+
 #define _COMPONENT		ACPI_PCI_COMPONENT
 ACPI_MODULE_NAME("pci_bind");
drivers/acpi/pci_irq.c +2 -0
index 11acaee..a5ba18c 100644
@@ -38,6 +38,8 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
+#include "pci.h"
+
 #define _COMPONENT		ACPI_PCI_COMPONENT
 ACPI_MODULE_NAME("pci_irq");
drivers/acpi/pci_root.c +2 -0
index 642554b..5a8fc37 100644
@@ -35,6 +35,8 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
+#include "pci.h"
+
 #define _COMPONENT		ACPI_PCI_COMPONENT
 ACPI_MODULE_NAME("pci_root");
 #define ACPI_PCI_ROOT_CLASS		"pci_bridge"
include/acpi/acpi_drivers.h +0 -13
index 5fc1bb0..9ea2684 100644
@@ -75,23 +75,10 @@
 /* ACPI PCI Interrupt Link (pci_link.c) */
 
 int acpi_irq_penalty_init(void);
-int acpi_pci_link_allocate_irq(acpi_handle handle, int index, int *triggering,
-			       int *polarity, char **name);
-int acpi_pci_link_free_irq(acpi_handle handle);
-
-/* ACPI PCI Interrupt Routing (pci_irq.c) */
-
-int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus);
-void acpi_pci_irq_del_prt(int segment, int bus);
 
 /* ACPI PCI Device Binding (pci_bind.c) */
 
-struct pci_bus;
-
 acpi_status acpi_get_pci_id(acpi_handle handle, struct acpi_pci_id *id);
-int acpi_pci_bind(struct acpi_device *device);
-int acpi_pci_bind_root(struct acpi_device *device, struct acpi_pci_id *id,
-		       struct pci_bus *bus);
 
 /* Arch-defined function to add a bus to the system */
author
#2 lore raw

PATCH 2/4 ACPI: processor: make new processor.h header local to driver

In reply to Bjorn Helgaas

The processor-related declarations in acpi_drivers.h are used only by the processor driver, which happens to be split across several files. The usual way to do this is to have a per-driver header, so I created such a header and moved these declarations to it.

Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/acpi/processor.h         |    5 +++++
 drivers/acpi/processor_core.c    |    2 ++
 drivers/acpi/processor_thermal.c |    2 ++
 drivers/acpi/thermal.c           |    2 ++
 include/acpi/acpi_drivers.h      |   10 ----------
 5 files changed, 11 insertions(+), 10 deletions(-)
 create mode 100644 drivers/acpi/processor.h
drivers/acpi/processor.h new file +5 -0
new file mode 100644
index 0000000..51df8cb
@@ -0,0 +1,5 @@
+#define ACPI_PROCESSOR_LIMIT_NONE	0x00
+#define ACPI_PROCESSOR_LIMIT_INCREMENT	0x01
+#define ACPI_PROCESSOR_LIMIT_DECREMENT	0x02
+
+int acpi_processor_set_thermal_limit(acpi_handle handle, int type);
drivers/acpi/processor_core.c +2 -0
index 3494836..e4a5956 100644
@@ -59,6 +59,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/processor.h>
 
+#include "processor.h"
+
 #define ACPI_PROCESSOR_CLASS		"processor"
 #define ACPI_PROCESSOR_DEVICE_NAME	"Processor"
 #define ACPI_PROCESSOR_FILE_INFO	"info"
drivers/acpi/processor_thermal.c +2 -0
index b1eb376..a778ac2 100644
@@ -40,6 +40,8 @@
 #include <acpi/processor.h>
 #include <acpi/acpi_drivers.h>
 
+#include "processor.h"
+
 #define ACPI_PROCESSOR_CLASS            "processor"
 #define _COMPONENT              ACPI_PROCESSOR_COMPONENT
 ACPI_MODULE_NAME("processor_thermal");
drivers/acpi/thermal.c +2 -0
index 073ff09..6baaa09 100644
@@ -47,6 +47,8 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
+#include "processor.h"
+
 #define ACPI_THERMAL_CLASS		"thermal_zone"
 #define ACPI_THERMAL_DEVICE_NAME	"Thermal Zone"
 #define ACPI_THERMAL_FILE_STATE		"state"
include/acpi/acpi_drivers.h +0 -10
index 9ea2684..4358917 100644
@@ -103,16 +103,6 @@ extern int acpi_power_nocheck;
 int acpi_ec_ecdt_probe(void);
 int acpi_boot_ec_enable(void);
 
-/* --------------------------------------------------------------------------
-                                    Processor
-   -------------------------------------------------------------------------- */
-
-#define ACPI_PROCESSOR_LIMIT_NONE	0x00
-#define ACPI_PROCESSOR_LIMIT_INCREMENT	0x01
-#define ACPI_PROCESSOR_LIMIT_DECREMENT	0x02
-
-int acpi_processor_set_thermal_limit(acpi_handle handle, int type);
-
 /*--------------------------------------------------------------------------
                                   Dock Station
   -------------------------------------------------------------------------- */
author
#3 lore raw

PATCH 3/4 ACPI: move internal core declaration to private header

In reply to Bjorn Helgaas

A number of things that shouldn't be exposed outside the ACPI core were declared in include/acpi/acpi_drivers.h, where anybody can see them. This patch moves those declarations to a new "acpi_core.h" inside drivers/acpi.

Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/acpi/acpi_core.h    |   22 ++++++++++++++++++++++
 drivers/acpi/bus.c          |    2 ++
 drivers/acpi/scan.c         |    2 ++
 drivers/acpi/sleep/main.c   |    1 +
 drivers/acpi/sleep/wakeup.c |    1 +
 include/acpi/acpi_drivers.h |   23 -----------------------
 6 files changed, 28 insertions(+), 23 deletions(-)
 create mode 100644 drivers/acpi/acpi_core.h
drivers/acpi/acpi_core.h new file +22 -0
new file mode 100644
index 0000000..1f98674
@@ -0,0 +1,22 @@
+/* --------------------------------------------------------------------------
+                                  Power Resource
+   -------------------------------------------------------------------------- */
+
+int acpi_device_sleep_wake(struct acpi_device *dev,
+                           int enable, int sleep_state, int dev_state);
+int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state);
+int acpi_disable_wakeup_device_power(struct acpi_device *dev);
+int acpi_power_get_inferred_state(struct acpi_device *device);
+int acpi_power_transition(struct acpi_device *device, int state);
+extern int acpi_power_nocheck;
+
+/* --------------------------------------------------------------------------
+                                  Embedded Controller
+   -------------------------------------------------------------------------- */
+int acpi_ec_ecdt_probe(void);
+int acpi_boot_ec_enable(void);
+
+/*--------------------------------------------------------------------------
+                                  Suspend/Resume
+  -------------------------------------------------------------------------- */
+int acpi_sleep_init(void);
drivers/acpi/bus.c +2 -0
index 765fd1c..bd2d60f 100644
@@ -39,6 +39,8 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
+#include "acpi_core.h"
+
 #define _COMPONENT		ACPI_BUS_COMPONENT
 ACPI_MODULE_NAME("bus");
drivers/acpi/scan.c +2 -0
index bd5253e..0193e9c 100644
@@ -12,6 +12,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/acinterp.h>	/* for acpi_ex_eisa_id_to_string() */
 
+#include "acpi_core.h"
+
 #define _COMPONENT		ACPI_BUS_COMPONENT
 ACPI_MODULE_NAME("scan");
 #define STRUCT_TO_INT(s)	(*((int*)&s))
drivers/acpi/sleep/main.c +1 -0
index 80c0868..58e1949 100644
@@ -21,6 +21,7 @@
 
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
+#include "../acpi_core.h"
 #include "sleep.h"
 
 u8 sleep_states[ACPI_S_STATE_COUNT];
drivers/acpi/sleep/wakeup.c +1 -0
index dea4c23..2debac5 100644
@@ -9,6 +9,7 @@
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <acpi/acevents.h>
+#include "../acpi_core.h"
 #include "sleep.h"
 
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
include/acpi/acpi_drivers.h +0 -23
index 4358917..f237ec2 100644
@@ -85,24 +85,6 @@ acpi_status acpi_get_pci_id(acpi_handle handle, struct acpi_pci_id *id);
 struct pci_bus *pci_acpi_scan_root(struct acpi_device *device, int domain,
 				   int bus);
 
-/* --------------------------------------------------------------------------
-                                  Power Resource
-   -------------------------------------------------------------------------- */
-
-int acpi_device_sleep_wake(struct acpi_device *dev,
-                           int enable, int sleep_state, int dev_state);
-int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state);
-int acpi_disable_wakeup_device_power(struct acpi_device *dev);
-int acpi_power_get_inferred_state(struct acpi_device *device);
-int acpi_power_transition(struct acpi_device *device, int state);
-extern int acpi_power_nocheck;
-
-/* --------------------------------------------------------------------------
-                                  Embedded Controller
-   -------------------------------------------------------------------------- */
-int acpi_ec_ecdt_probe(void);
-int acpi_boot_ec_enable(void);
-
 /*--------------------------------------------------------------------------
                                   Dock Station
   -------------------------------------------------------------------------- */

@@ -142,9 +124,4 @@ static inline void unregister_hotplug_dock_device(acpi_handle handle)
 }
 #endif
 
-/*--------------------------------------------------------------------------
-                                  Suspend/Resume
-  -------------------------------------------------------------------------- */
-extern int acpi_sleep_init(void);
-
 #endif /*__ACPI_DRIVERS_H__*/
author
#4 lore raw

PATCH 4/4 ACPI: call core init functions explicitly instead of using initcalls

In reply to Bjorn Helgaas

These functions:
init_acpi_device_notify()
acpi_debug_init()
acpi_scan_init()
acpi_system_init()
acpi_power_init()
are all parts of the ACPI core. None can be compiled or loaded separately. So we might as well call them directly from acpi_init() rather than using initcalls, which depend on module link order to make things happen in the correct order.

Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/acpi/Makefile    |   15 ++++++---------
 drivers/acpi/acpi_core.h |   19 +++++++++++++++++++
 drivers/acpi/bus.c       |   10 ++++++++++
 drivers/acpi/debug.c     |   10 +++-------
 drivers/acpi/glue.c      |    8 +++-----
 drivers/acpi/power.c     |    8 +-------
 drivers/acpi/scan.c      |    8 +-------
 drivers/acpi/system.c    |   10 ++--------
 8 files changed, 45 insertions(+), 43 deletions(-)
drivers/acpi/Makefile +6 -9
index 3c0c933..9ab533e 100644
@@ -19,7 +19,7 @@ obj-y				+= tables.o
 obj-$(CONFIG_X86)		+= blacklist.o
 
 #
-# ACPI Core Subsystem (Interpreter)
+# ACPI CA Core Subsystem (Interpreter)
 #
 obj-y				+= osl.o utils.o reboot.o\
 				   dispatcher/ events/ executer/ hardware/ \

@@ -36,10 +36,10 @@ processor-objs	+= processor_perflib.o
 endif
 
 obj-y				+= sleep/
-obj-y				+= bus.o glue.o
-obj-y				+= scan.o
-# Keep EC driver first. Initialization of others depend on it.
-obj-y				+= ec.o
+obj-y				+= bus.o scan.o ec.o power.o
+obj-y				+= glue.o
+obj-$(CONFIG_ACPI_DEBUG)	+= debug.o
+
 obj-$(CONFIG_ACPI_AC) 		+= ac.o
 obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
 obj-$(CONFIG_ACPI_BUTTON)	+= button.o

@@ -55,14 +55,11 @@ obj-$(CONFIG_ACPI_PCI_SLOT)	+= pci_slot.o
 obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
 obj-$(CONFIG_ACPI_CONTAINER)	+= container.o
 obj-$(CONFIG_ACPI_THERMAL)	+= thermal.o
-obj-y				+= power.o
 obj-$(CONFIG_ACPI_SYSTEM)	+= system.o event.o
-obj-$(CONFIG_ACPI_DEBUG)	+= debug.o
 obj-$(CONFIG_ACPI_NUMA)		+= numa.o
 obj-$(CONFIG_ACPI_WMI)		+= wmi.o
 obj-$(CONFIG_ACPI_ASUS)		+= asus_acpi.o
 obj-$(CONFIG_ACPI_TOSHIBA)	+= toshiba_acpi.o
 obj-$(CONFIG_ACPI_HOTPLUG_MEMORY)	+= acpi_memhotplug.o
 obj-$(CONFIG_ACPI_PROCFS_POWER)	+= cm_sbs.o
-obj-$(CONFIG_ACPI_SBS)		+= sbshc.o
-obj-$(CONFIG_ACPI_SBS)		+= sbs.o
+obj-$(CONFIG_ACPI_SBS)		+= sbshc.o sbs.o
drivers/acpi/acpi_core.h +19 -0
index 1f98674..dbce692 100644
@@ -1,3 +1,22 @@
+/* Functions internal to the Linux OSPM (the "ACPI core" in Linux terms) */
+
+int init_acpi_device_notify(void);
+int acpi_scan_init(void);
+int acpi_power_init(void);
+
+#if defined(CONFIG_ACPI_DEBUG) && defined(CONFIG_ACPI_PROCFS)
+int acpi_debug_init(void);
+#else
+static inline int acpi_debug_init(void) { return 0; }
+#endif
+
+
+#ifdef CONFIG_ACPI_SYSTEM
+int acpi_system_init(void);
+#else
+static inline int acpi_system_init(void) { return 0; }
+#endif
+
 /* --------------------------------------------------------------------------
                                   Power Resource
    -------------------------------------------------------------------------- */
drivers/acpi/bus.c +10 -0
index bd2d60f..8c78949 100644
@@ -855,6 +855,7 @@ static int __init acpi_init(void)
 		acpi_kobj = NULL;
 	}
 
+	init_acpi_device_notify();
 	result = acpi_bus_init();
 
 	if (!result) {

@@ -869,11 +870,20 @@ static int __init acpi_init(void)
 		}
 	} else
 		disable_acpi();
+
 	/*
 	 * If the laptop falls into the DMI check table, the power state check
 	 * will be disabled in the course of device power transistion.
 	 */
 	dmi_check_system(power_nocheck_dmi_table);
+
+	if (acpi_disabled)
+		return result;
+
+	acpi_debug_init();
+	acpi_scan_init();
+	acpi_system_init();
+	acpi_power_init();
 	return result;
 }
drivers/acpi/debug.c +3 -7
index c483968..224c3d1 100644
@@ -11,6 +11,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/acglobal.h>
 
+#include "acpi_core.h"
+
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
 ACPI_MODULE_NAME("debug");
 

@@ -299,16 +301,12 @@ acpi_system_write_debug(struct file *file,
 	return count;
 }
 
-static int __init acpi_debug_init(void)
+int __init acpi_debug_init(void)
 {
 	struct proc_dir_entry *entry;
 	int error = 0;
 	char *name;
 
-
-	if (acpi_disabled)
-		return 0;
-
 	/* 'debug_layer' [R/W] */
 	name = ACPI_SYSTEM_FILE_DEBUG_LAYER;
 	entry =

@@ -340,6 +338,4 @@ static int __init acpi_debug_init(void)
 	error = -ENODEV;
 	goto Done;
 }
-
-subsys_initcall(acpi_debug_init);
 #endif
drivers/acpi/glue.c +3 -5
index adec3d1..bd3b5c9 100644
@@ -12,6 +12,8 @@
 #include <linux/rwsem.h>
 #include <linux/acpi.h>
 
+#include "acpi_core.h"
+
 #define ACPI_GLUE_DEBUG	0
 #if ACPI_GLUE_DEBUG
 #define DBG(x...) printk(PREFIX x)

@@ -286,10 +288,8 @@ static int acpi_platform_notify_remove(struct device *dev)
 	return 0;
 }
 
-static int __init init_acpi_device_notify(void)
+int __init init_acpi_device_notify(void)
 {
-	if (acpi_disabled)
-		return 0;
 	if (platform_notify || platform_notify_remove) {
 		printk(KERN_ERR PREFIX "Can't use platform_notify\n");
 		return 0;

@@ -298,5 +298,3 @@ static int __init init_acpi_device_notify(void)
 	platform_notify_remove = acpi_platform_notify_remove;
 	return 0;
 }
-
-arch_initcall(init_acpi_device_notify);
drivers/acpi/power.c +1 -7
index bb7d50d..99ce34a 100644
@@ -769,14 +769,10 @@ static int acpi_power_resume(struct acpi_device *device)
 	return 0;
 }
 
-static int __init acpi_power_init(void)
+int __init acpi_power_init(void)
 {
 	int result = 0;
 
-
-	if (acpi_disabled)
-		return 0;
-
 	INIT_LIST_HEAD(&acpi_power_resource_list);
 
 	acpi_power_dir = proc_mkdir(ACPI_POWER_CLASS, acpi_root_dir);

@@ -791,5 +787,3 @@ static int __init acpi_power_init(void)
 
 	return 0;
 }
-
-subsys_initcall(acpi_power_init);
drivers/acpi/scan.c +1 -7
index 0193e9c..642a301 100644
@@ -1538,15 +1538,11 @@ static int acpi_bus_scan_fixed(struct acpi_device *root)
 }
 
 
-static int __init acpi_scan_init(void)
+int __init acpi_scan_init(void)
 {
 	int result;
 	struct acpi_bus_ops ops;
 
-
-	if (acpi_disabled)
-		return 0;
-
 	memset(&ops, 0, sizeof(ops));
 	ops.acpi_op_add = 1;
 	ops.acpi_op_start = 1;

@@ -1579,5 +1575,3 @@ static int __init acpi_scan_init(void)
       Done:
 	return result;
 }
-
-subsys_initcall(acpi_scan_init);
drivers/acpi/system.c +2 -8
index 6e4107f..d2a5d2c 100644
@@ -630,20 +630,14 @@ static int acpi_system_procfs_init(void)
 }
 #endif
 
-static int __init acpi_system_init(void)
+int __init acpi_system_init(void)
 {
-	int result = 0;
-
-	if (acpi_disabled)
-		return 0;
+	int result;
 
 	result = acpi_system_procfs_init();
 	if (result)
 		return result;
 
 	result = acpi_system_sysfs_init();
-
 	return result;
 }
-
-subsys_initcall(acpi_system_init);