Skip to contentSkip to conversation

RFC: various ACPI core cleanups

PATCH Applied Bjorn Helgaas 3 participants 24 messages Nov 5, 2008 – Nov 6, 2008

Subsystems (8)
Documentation x86 ACPI PCI +4 more subsystems
lore
Share & tools
lore
#0 lore raw

cover RFC: various ACPI core cleanups

This series improves the ACPI debug documentation, improves modularity, makes Kconfig options a little more consistent, and removes some initcalls to reduce link order dependencies.

Comments welcome.

Bjorn

#1 lore raw

PATCH 1/11 ACPI: update debug parameter documentation

Fix the init, debug, info level definitions, which changed recently.

Reformat acpi.debug_layer and acpi.debug_level documentation so it's more readable, add some clues about how to figure out the mask bits that enable a specific ACPI_DEBUG_PRINT statement, and include some useful examples.

---
 Documentation/kernel-parameters.txt |   98 ++++++++++++++++++++++-------------
 drivers/acpi/Kconfig                |   12 +++-
 2 files changed, 70 insertions(+), 40 deletions(-)
Documentation/kernel-parameters.txt +61 -37
index 1bbcaa8..ddc791e 100644
@@ -198,42 +198,69 @@ and is between 256 and 4096 characters. It is defined in the file
 			that require a timer override, but don't have
 			HPET
 
-	acpi.debug_layer=	[HW,ACPI]
+	acpi.debug_*=	[HW,ACPI,ACPI_DEBUG]
+			CONFIG_ACPI_DEBUG must be enabled to produce any ACPI
+			debug output.  The type and amount of output is
+			controlled by debug_layer and debug_level parameters,
+			which can be in decimal or hex (prefixed with "0x").
+			Some values produce so much output that the system is
+			unusable.  The "log_buf_len" parameter may be useful
+			if you need to capture more output.
+
+			To see AML "Debug" output (stores to the Debug object
+			while interpreting AML (this is enabled by default)):
+			    acpi.debug_layer=0xffffffff acpi.debug_level=0x2
+			For debugging PCI/_PRT issues (PCI, info msgs):
+			    acpi.debug_layer=0x400000 acpi.debug_level=0x4
+			For ACPI hardware issues (hardware, all msgs):
+			    acpi.debug_layer=0x2 acpi.debug_level=0xffffffff
+
+	    acpi.debug_layer=	[HW,ACPI,ACPI_DEBUG]
 			Format: <int>
-			Each bit of the <int> indicates an ACPI debug layer,
-			1: enable, 0: disable. It is useful for boot time
-			debugging. After system has booted up, it can be set
-			via /sys/module/acpi/parameters/debug_layer.
-			CONFIG_ACPI_DEBUG must be enabled for this to produce any output.
-			Available bits (add the numbers together) to enable debug output
-			for specific parts of the ACPI subsystem:
-			0x01 utilities 0x02 hardware 0x04 events 0x08 tables
-			0x10 namespace 0x20 parser 0x40 dispatcher
-			0x80 executer 0x100 resources 0x200 acpica debugger
-			0x400 os services 0x800 acpica disassembler.
-			The number can be in decimal or prefixed with 0x in hex.
-			Warning: Many of these options can produce a lot of
-			output and make your system unusable. Be very careful.
-
-	acpi.debug_level=	[HW,ACPI]
+			Bits in <int> correspond to _COMPONENT definitions in
+			ACPI source files.  After boot, this mask can changed
+			on the fly, e.g., 
+			    echo 0x2 > /sys/module/acpi/parameters/debug_layer
+			The contents of debug_layer show the names of all the
+			defined bits and their current state.
+
+			The following are some of the global components
+			defined by the ACPI CA and the Linux OSPM:
+				0x01 utilities
+				0x02 hardware
+				0x04 events
+				0x08 tables
+				0x10 namespace
+				0x20 parser
+				0x40 dispatcher
+				0x80 executer
+			       0x100 resources
+			       0x200 ACPI CA debugger
+			       0x400 OS services
+			       0x800 ACPI CA disassembler
+			     0x40000 battery
+			     0x80000 button
+			    0x200000 fan
+			    0x400000 PCI
+			  0x10000000 bay
+
+			Many others, e.g., ACPI_BUS_COMPONENT and
+			ACPI_AC_COMPONENT, are defined by the Linux OSPM and
+			individual drivers.
+
+	    acpi.debug_level=	[HW,ACPI,ACPI_DEBUG]
 			Format: <int>
-			Each bit of the <int> indicates an ACPI debug level,
-			which corresponds to the level in an ACPI_DEBUG_PRINT
-			statement.  After system has booted up, this mask
-			can be set via /sys/module/acpi/parameters/debug_level.
-
-			CONFIG_ACPI_DEBUG must be enabled for this to produce
-			any output.  The number can be in decimal or prefixed
-			with 0x in hex.  Some of these options produce so much
-			output that the system is unusable.
-
-			The following global components are defined by the
-			ACPI CA:
-			       0x01 error
-			       0x02 warn
-			       0x04 init
-			       0x08 debug object
-			       0x10 info
+			Bits in <int> correspond to levels in ACPI_DEBUG_PRINT
+			statements.  After boot, this mask can be changed on
+			the fly, e.g.,
+			    echo 0x2 > /sys/module/acpi/parameters/debug_level
+			The contents of debug_level show the names of all the
+			defined bits and their current state.
+
+			The following levels are defined by the ACPI CA:
+			       0x01 init
+			       0x02 debug (AML stores to the Debug object)
+			       0x04 info
 			       0x20 init names
 			       0x40 parse
 			       0x80 load
@@ -248,9 +275,6 @@ and is between 256 and 4096 characters. It is defined in the file
 			    0x10000 resources
 			    0x20000 user requests
 			    0x40000 package
- 			The number can be in decimal or prefixed with 0x in hex.
- 			Warning: Many of these options can produce a lot of
- 			output and make your system unusable. Be very careful.
 
  	acpi.power_nocheck=	[HW,ACPI]
  			Format: 1/0 enable/disable the check of power state.
drivers/acpi/Kconfig +9 -3
index f4f6329..1c954a7 100644
@@ -312,9 +312,15 @@ config ACPI_DEBUG
 	bool "Debug Statements"
 	default n
 	help
-	  The ACPI driver can optionally report errors with a great deal
-	  of verbosity. Saying Y enables these statements. This will increase
-	  your kernel size by around 50K.
+	  The ACPI subsystem can produce debug output.  Saying Y enables this
+	  output and increases the kernel size by around 50K.
+
+	  Use the acpi.debug_layer and acpi.debug_level kernel command-line
+	  parameters documented in Documentation/kernel-parameters.txt to
+	  control the type and amount of debug output.
+
+	  When no parameters are supplied, the default settings cause AML
+	  stores to the "Debug" object to produce console output.
 
 config ACPI_DEBUG_FUNC_TRACE
 	bool "Additionally enable ACPI function tracing"
reviewer
#2 lore raw

Re: [PATCH 01/11] ACPI: update debug parameter documentation

Quoted 28 lines · Bjorn Helgaas

Fix the init, debug, info level definitions, which changed recently.

Reformat acpi.debug_layer and acpi.debug_level documentation so it's more readable, add some clues about how to figure out the mask bits that enable a specific ACPI_DEBUG_PRINT statement, and include some useful examples.

---
 Documentation/kernel-parameters.txt |   98 ++++++++++++++++++++++-------------
 drivers/acpi/Kconfig                |   12 +++-
 2 files changed, 70 insertions(+), 40 deletions(-)
Documentation/kernel-parameters.txt +6 -1
index 1bbcaa8..ddc791e 100644
@@ -198,42 +198,69 @@ and is between 256 and 4096 characters. It is defined in the file
 			that require a timer override, but don't have
 			HPET
 
-	acpi.debug_layer=	[HW,ACPI]
+	acpi.debug_*=	[HW,ACPI,ACPI_DEBUG]
+			CONFIG_ACPI_DEBUG must be enabled to produce any ACPI
+			debug output.  The type and amount of output is
+			controlled by debug_layer and debug_level parameters,
+			which can be in decimal or hex (prefixed with "0x").
+			Some values produce so much output that the system is

Documentation/kernel-parameters.txt and is between 256 and 4096 characters. It is defined in the file · line 216

Quoted 10 lines
Documentation/kernel-parameters.txt +10 -0
+			unusable.  The "log_buf_len" parameter may be useful
+			if you need to capture more output.
+
+			To see AML "Debug" output (stores to the Debug object
+			while interpreting AML (this is enabled by default)):
+			    acpi.debug_layer=0xffffffff acpi.debug_level=0x2
+			For debugging PCI/_PRT issues (PCI, info msgs):
+			    acpi.debug_layer=0x400000 acpi.debug_level=0x4
+			For ACPI hardware issues (hardware, all msgs):
+			    acpi.debug_layer=0x2 acpi.debug_level=0xffffffff

i like it.

Quoted 35 lines
Documentation/kernel-parameters.txt +18 -16
+
+	    acpi.debug_layer=	[HW,ACPI,ACPI_DEBUG]
 			Format: <int>
-			Each bit of the <int> indicates an ACPI debug layer,
-			1: enable, 0: disable. It is useful for boot time
-			debugging. After system has booted up, it can be set
-			via /sys/module/acpi/parameters/debug_layer.
-			CONFIG_ACPI_DEBUG must be enabled for this to produce any output.
-			Available bits (add the numbers together) to enable debug output
-			for specific parts of the ACPI subsystem:
-			0x01 utilities 0x02 hardware 0x04 events 0x08 tables
-			0x10 namespace 0x20 parser 0x40 dispatcher
-			0x80 executer 0x100 resources 0x200 acpica debugger
-			0x400 os services 0x800 acpica disassembler.
-			The number can be in decimal or prefixed with 0x in hex.
-			Warning: Many of these options can produce a lot of
-			output and make your system unusable. Be very careful.
-
-	acpi.debug_level=	[HW,ACPI]
+			Bits in <int> correspond to _COMPONENT definitions in
+			ACPI source files.  After boot, this mask can changed
+			on the fly, e.g., 
+			    echo 0x2 > /sys/module/acpi/parameters/debug_layer
+			The contents of debug_layer show the names of all the
+			defined bits and their current state.
+
+			The following are some of the global components
+			defined by the ACPI CA and the Linux OSPM:
+				0x01 utilities
+				0x02 hardware
+				0x04 events
+				0x08 tables
+				0x10 namespace
+				0x20 parser
+				0x40 dispatcher

Documentation/kernel-parameters.txt and is between 256 and 4096 characters. It is defined in the file · line 245

Quoted 10 lines
Documentation/kernel-parameters.txt +10 -0
+				0x80 executer
+			       0x100 resources
+			       0x200 ACPI CA debugger
+			       0x400 OS services
+			       0x800 ACPI CA disassembler
+			     0x40000 battery
+			     0x80000 button
+			    0x200000 fan
+			    0x400000 PCI
+			  0x10000000 bay

The (always current) bit definitions can always be had this way:

$ cat /sys/module/acpi/parameters/debug_layer
Description              	Hex        SET
ACPI_UTILITIES           	0x00000001 [*]
ACPI_HARDWARE            	0x00000002 [*]
ACPI_EVENTS              	0x00000004 [*]
ACPI_TABLES              	0x00000008 [*]
ACPI_NAMESPACE           	0x00000010 [*]
ACPI_PARSER              	0x00000020 [*]
ACPI_DISPATCHER          	0x00000040 [*]
ACPI_EXECUTER            	0x00000080 [*]
ACPI_RESOURCES           	0x00000100 [*]
ACPI_CA_DEBUGGER         	0x00000200 [*]
ACPI_OS_SERVICES         	0x00000400 [*]
ACPI_CA_DISASSEMBLER     	0x00000800 [*]
ACPI_COMPILER            	0x00001000 [*]
ACPI_TOOLS               	0x00002000 [*]
ACPI_ALL_DRIVERS         	0xFFFF0000 [*]

So I think just mentioning that is enough, rather than reproducing it here. However, I do think that highlighting the driver bits would be good, since they're not included above. bay isn't a good example, as it doesn't exist any more.

I think we've got the component definitions in the wrong place.

today we have

button.c
#define ACPI_BUTTON_COMPONENT 0x00080000
#define _COMPONENT ACPI_BUTTON_COMPONENT

but this should really be

acoutput.h
#define ACPI_BUTTON_COMPONENT 0x00080000

button.c
#define _COMPONENT ACPI_BUTTON_COMPONENT

and we should print all the driver bits in the module file, not just ACPI_ALL_DRIVERS

Quoted 39 lines
Documentation/kernel-parameters.txt +17 -17
+
+			Many others, e.g., ACPI_BUS_COMPONENT and
+			ACPI_AC_COMPONENT, are defined by the Linux OSPM and
+			individual drivers.
+
+	    acpi.debug_level=	[HW,ACPI,ACPI_DEBUG]
 			Format: <int>
-			Each bit of the <int> indicates an ACPI debug level,
-			which corresponds to the level in an ACPI_DEBUG_PRINT
-			statement.  After system has booted up, this mask
-			can be set via /sys/module/acpi/parameters/debug_level.
-
-			CONFIG_ACPI_DEBUG must be enabled for this to produce
-			any output.  The number can be in decimal or prefixed
-			with 0x in hex.  Some of these options produce so much
-			output that the system is unusable.
-
-			The following global components are defined by the
-			ACPI CA:
-			       0x01 error
-			       0x02 warn
-			       0x04 init
-			       0x08 debug object
-			       0x10 info
+			Bits in <int> correspond to levels in ACPI_DEBUG_PRINT
+			statements.  After boot, this mask can be changed on
+			the fly, e.g.,
+			    echo 0x2 > /sys/module/acpi/parameters/debug_level
+			The contents of debug_level show the names of all the
+			defined bits and their current state.
+
+			The following levels are defined by the ACPI CA:
+			       0x01 init
+			       0x02 debug (AML stores to the Debug object)
+			       0x04 info
 			       0x20 init names
 			       0x40 parse
 			       0x80 load

Documentation/kernel-parameters.txt and is between 256 and 4096 characters. It is defined in the file · line 253

Quoted 6 lines
Documentation/kernel-parameters.txt +0 -3
@@ -248,9 +275,6 @@ and is between 256 and 4096 characters. It is defined in the file
 			    0x10000 resources
 			    0x20000 user requests
 			    0x40000 package
- 			The number can be in decimal or prefixed with 0x in hex.
- 			Warning: Many of these options can produce a lot of
- 			output and make your system unusable. Be very careful.

actually, we have more than that:

$ cat /sys/module/acpi/parameters/debug_level
Description              	Hex        SET
ACPI_LV_INIT             	0x00000001 [*]
ACPI_LV_DEBUG_OBJECT     	0x00000002 [*]
ACPI_LV_INFO             	0x00000004 [ ]
ACPI_LV_INIT_NAMES       	0x00000020 [ ]
ACPI_LV_PARSE            	0x00000040 [ ]
ACPI_LV_LOAD             	0x00000080 [ ]
ACPI_LV_DISPATCH         	0x00000100 [ ]
ACPI_LV_EXEC             	0x00000200 [ ]
ACPI_LV_NAMES            	0x00000400 [ ]
ACPI_LV_OPREGION         	0x00000800 [ ]
ACPI_LV_BFIELD           	0x00001000 [ ]
ACPI_LV_TABLES           	0x00002000 [ ]
ACPI_LV_VALUES           	0x00004000 [ ]
ACPI_LV_OBJECTS          	0x00008000 [ ]
ACPI_LV_RESOURCES        	0x00010000 [ ]
ACPI_LV_USER_REQUESTS    	0x00020000 [ ]
ACPI_LV_PACKAGE          	0x00040000 [ ]
ACPI_LV_ALLOCATIONS      	0x00100000 [ ]
ACPI_LV_FUNCTIONS        	0x00200000 [ ]
ACPI_LV_OPTIMIZATIONS    	0x00400000 [ ]
ACPI_LV_MUTEX            	0x01000000 [ ]
ACPI_LV_THREADS          	0x02000000 [ ]
ACPI_LV_IO               	0x04000000 [ ]
ACPI_LV_INTERRUPTS       	0x08000000 [ ]
ACPI_LV_AML_DISASSEMBLE  	0x10000000 [ ]
ACPI_LV_VERBOSE_INFO     	0x20000000 [ ]
ACPI_LV_FULL_TABLES      	0x40000000 [ ]
ACPI_LV_EVENTS           	0x80000000 [ ]
Quoted 13 lines
Documentation/kernel-parameters.txt +0 -0
  	acpi.power_nocheck=	[HW,ACPI]
  			Format: 1/0 enable/disable the check of power state.
drivers/acpi/Kconfig +0 -2
index f4f6329..1c954a7 100644
@@ -312,9 +312,15 @@ config ACPI_DEBUG
 	bool "Debug Statements"
 	default n
 	help
-	  The ACPI driver can optionally report errors with a great deal
-	  of verbosity. Saying Y enables these statements. This will increase

drivers/acpi/Kconfig config ACPI_DEBUG · line 323

Quoted 10 lines
drivers/acpi/Kconfig +9 -1
-	  your kernel size by around 50K.
+	  The ACPI subsystem can produce debug output.  Saying Y enables this
+	  output and increases the kernel size by around 50K.
+
+	  Use the acpi.debug_layer and acpi.debug_level kernel command-line
+	  parameters documented in Documentation/kernel-parameters.txt to
+	  control the type and amount of debug output.
+
+	  When no parameters are supplied, the default settings cause AML
+	  stores to the "Debug" object to produce console output.

I think Thomas may have changed this at SuSE b/c they ship with CONFIG_ACPI_DEBUG=y, if so, maybe upstream should also?

thanks,
-Len

->

Quoted 8 lines
drivers/acpi/Kconfig +0 -1
 config ACPI_DEBUG_FUNC_TRACE
 	bool "Additionally enable ACPI function tracing"
 
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw

Re: [PATCH 01/11] ACPI: update debug parameter documentation

Quoted 41 lines · Len Brown
Quoted 9 lines · Bjorn Helgaas
Documentation/kernel-parameters.txt +9 -0
+			The following are some of the global components
+			defined by the ACPI CA and the Linux OSPM:
+				0x01 utilities
+				0x02 hardware
+				0x04 events
+				0x08 tables
+				0x10 namespace
+				0x20 parser
+				0x40 dispatcher

Documentation/kernel-parameters.txt and is between 256 and 4096 characters. It is defined in the file · line 245

Quoted 10 lines
Documentation/kernel-parameters.txt +10 -0
+				0x80 executer
+			       0x100 resources
+			       0x200 ACPI CA debugger
+			       0x400 OS services
+			       0x800 ACPI CA disassembler
+			     0x40000 battery
+			     0x80000 button
+			    0x200000 fan
+			    0x400000 PCI
+			  0x10000000 bay

The (always current) bit definitions can always be had this way:

$ cat /sys/module/acpi/parameters/debug_layer
Description              	Hex        SET
ACPI_UTILITIES           	0x00000001 [*]
ACPI_HARDWARE            	0x00000002 [*]
ACPI_EVENTS              	0x00000004 [*]
ACPI_TABLES              	0x00000008 [*]
ACPI_NAMESPACE           	0x00000010 [*]
ACPI_PARSER              	0x00000020 [*]
ACPI_DISPATCHER          	0x00000040 [*]
ACPI_EXECUTER            	0x00000080 [*]
ACPI_RESOURCES           	0x00000100 [*]
ACPI_CA_DEBUGGER         	0x00000200 [*]
ACPI_OS_SERVICES         	0x00000400 [*]
ACPI_CA_DISASSEMBLER     	0x00000800 [*]
ACPI_COMPILER            	0x00001000 [*]
ACPI_TOOLS               	0x00002000 [*]
ACPI_ALL_DRIVERS         	0xFFFF0000 [*]
Quoted 4 lines

So I think just mentioning that is enough, rather than reproducing it here. However, I do think that highlighting the driver bits would be good, since they're not included above. bay isn't a good example, as it doesn't exist any more.

I don't really like having the whole list in the doc either. One problem with using only a pointer to the sysfs file is that you have to have a booted machine with ACPI_DEBUG=y.

We could use a pointer to acoutput.h, especially if we moved the driver definitions there. It's still a minor irritation that the source code says "ACPI_DEBUG_PRINT((ACPI_DB_INFO ..." and you have to work back to the ACPI_LV_INFO definition before you have a mask value. But the benefit of having only a single copy of the list is probably worth that irritation.

If you're willing to see the component definitions in acoutput.h, would you also want to change acpi/debug.c so it prints the driver bits individually?

Quoted 5 lines

drivers/acpi/Kconfig config ACPI_DEBUG · line 323

Quoted 2 lines
drivers/acpi/Kconfig +2 -0
+	  When no parameters are supplied, the default settings cause AML
+	  stores to the "Debug" object to produce console output.

I think Thomas may have changed this at SuSE b/c they ship with CONFIG_ACPI_DEBUG=y, if so, maybe upstream should also?

Yeah, I don't really like the default mask being non-zero. I seem to have a bunch of machines that spew lots of debug output. Possibly these are protos with old firmware, but I suspect we shipped firmware that uses Debug.

I didn't change it because the default is currently set in the CA somewhere. I guess we could override it early in acpi_init() or something.

Bjorn

reviewer
#4 lore raw

Re: [PATCH 01/11] ACPI: update debug parameter documentation

Quoted 7 lines
Quoted 3 lines

$ cat /sys/module/acpi/parameters/debug_layer

I don't really like having the whole list in the doc either. One problem with using only a pointer to the sysfs file is that you have to have a booted machine with ACPI_DEBUG=y.

Good point.
Maybe in kernel-parameters.txt we should point to a Documedntation/acpi/debug.txt file with this stuff in it?

Quoted 6 lines

We could use a pointer to acoutput.h, especially if we moved the driver definitions there. It's still a minor irritation that the source code says "ACPI_DEBUG_PRINT((ACPI_DB_INFO ..." and you have to work back to the ACPI_LV_INFO definition before you have a mask value. But the benefit of having only a single copy of the list is probably worth that irritation.

agreed.

Quoted 3 lines

If you're willing to see the component definitions in acoutput.h, would you also want to change acpi/debug.c so it prints the driver bits individually?

re: acoutput.h
on second thought, putting them in acpi_drivers.h would be better, b/c that is linux and acouput is ACPICA.
I think having them together is better than having them spread around, but for readability and easy to verify by inspection that we don't have any conflicting definitions.

re: acpi/debug.c
Honestly, greg hates for us to print stuff in sysfs files, but we decided it was worth his wrath for this case b/c we had it in /proc and this documentation was so poorly documented. Making the print-out longer will not make him happier, but we're already going to burn in sysfs hell for our sins:-)

Quoted 6 lines
Quoted 5 lines

drivers/acpi/Kconfig config ACPI_DEBUG · line 323

Quoted 2 lines
drivers/acpi/Kconfig +2 -0
+	  When no parameters are supplied, the default settings cause AML
+	  stores to the "Debug" object to produce console output.

I think Thomas may have changed this at SuSE b/c they ship with CONFIG_ACPI_DEBUG=y, if so, maybe upstream should also?

Quoted 8 lines

Yeah, I don't really like the default mask being non-zero. I seem to have a bunch of machines that spew lots of debug output. Possibly these are protos with old firmware, but I suspect we shipped firmware that uses Debug.

I didn't change it because the default is currently set in the CA somewhere. I guess we could override it early in acpi_init() or something.

I'd apply a patch to change the default -- as long as we document it.

thanks,
-Len

#5 lore raw

Re: [PATCH 01/11] ACPI: update debug parameter documentation

Quoted 10 lines · Len Brown
Quoted 6 lines
Quoted 2 lines
$ cat /sys/module/acpi/parameters/debug_layer

I don't really like having the whole list in the doc either. One problem with using only a pointer to the sysfs file is that you have to have a booted machine with ACPI_DEBUG=y.

Good point.
Maybe in kernel-parameters.txt we should point to a Documedntation/acpi/debug.txt file with this stuff in it?

I think that's a good compromise. It should be available somewhere other than a source or header file IMO. I.e., some place in Documentation/[acpi].

~Randy

#6 lore raw

PATCH 2/11 ACPI: remove comments about debug layer/level to use

In reply to Bjorn Helgaas

I don't think there's any point in cluttering the code with these. Better to improve the documentation so *anybody* can figure out what layer & level to use.

---
 drivers/acpi/pci_root.c |    6 ------
 drivers/acpi/video.c    |    6 ------
 2 files changed, 0 insertions(+), 12 deletions(-)
drivers/acpi/pci_root.c +0 -6
index 1b8f67d..642554b 100644
@@ -376,15 +376,9 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 
 static int __init acpi_pci_root_init(void)
 {
-
 	if (acpi_pci_disabled)
 		return 0;
 
-	/* DEBUG:
-	   acpi_dbg_layer = ACPI_PCI_COMPONENT;
-	   acpi_dbg_level = 0xFFFFFFFF;
-	 */
-
 	if (acpi_bus_register_driver(&acpi_pci_root_driver) < 0)
 		return -ENODEV;
 
drivers/acpi/video.c +0 -6
index a29b0cc..bf0c26a 100644
@@ -2094,12 +2094,6 @@ static int __init acpi_video_init(void)
 {
 	int result = 0;
 
-
-	/*
-	   acpi_dbg_level = 0xFFFFFFFF;
-	   acpi_dbg_layer = 0x08000000;
-	 */
-
 	acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
 	if (!acpi_video_dir)
 		return -ENODEV;
reviewer Applied
#7 lore raw

Re: [PATCH 02/11] ACPI: remove comments about debug layer/level to use

applied.

thanks,
-len

Quoted 53 lines · Bjorn Helgaas

I don't think there's any point in cluttering the code with these. Better to improve the documentation so *anybody* can figure out what layer & level to use.

---
 drivers/acpi/pci_root.c |    6 ------
 drivers/acpi/video.c    |    6 ------
 2 files changed, 0 insertions(+), 12 deletions(-)
drivers/acpi/pci_root.c +0 -6
index 1b8f67d..642554b 100644
@@ -376,15 +376,9 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 
 static int __init acpi_pci_root_init(void)
 {
-
 	if (acpi_pci_disabled)
 		return 0;
 
-	/* DEBUG:
-	   acpi_dbg_layer = ACPI_PCI_COMPONENT;
-	   acpi_dbg_level = 0xFFFFFFFF;
-	 */
-
 	if (acpi_bus_register_driver(&acpi_pci_root_driver) < 0)
 		return -ENODEV;
 
drivers/acpi/video.c +0 -7
index a29b0cc..bf0c26a 100644
@@ -2094,12 +2094,6 @@ static int __init acpi_video_init(void)
 {
 	int result = 0;
 
-
-	/*
-	   acpi_dbg_level = 0xFFFFFFFF;
-	   acpi_dbg_layer = 0x08000000;
-	 */
-
 	acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
 	if (!acpi_video_dir)
 		return -ENODEV;
 
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw

PATCH 3/11 ACPI: SBS: remove useless acpi_cm_sbs_init() initcall

In reply to Bjorn Helgaas

acpi_cm_sbs_init() doesn't do anything, so we can just remove it.

---
 drivers/acpi/cm_sbs.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)
drivers/acpi/cm_sbs.c +0 -6
index 4441e84..80d5c88 100644
@@ -105,9 +105,3 @@ void acpi_unlock_battery_dir(struct proc_dir_entry *acpi_battery_dir_param)
 	return;
 }
 EXPORT_SYMBOL(acpi_unlock_battery_dir);
-
-static int __init acpi_cm_sbs_init(void)
-{
-	return 0;
-}
-subsys_initcall(acpi_cm_sbs_init);
reviewer Applied
#9 lore raw

Re: [PATCH 03/11] ACPI: SBS: remove useless acpi_cm_sbs_init() initcall

applied.

thanks,
-len

Quoted 27 lines · Bjorn Helgaas

acpi_cm_sbs_init() doesn't do anything, so we can just remove it.

---
 drivers/acpi/cm_sbs.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)
drivers/acpi/cm_sbs.c +0 -7
index 4441e84..80d5c88 100644
@@ -105,9 +105,3 @@ void acpi_unlock_battery_dir(struct proc_dir_entry *acpi_battery_dir_param)
 	return;
 }
 EXPORT_SYMBOL(acpi_unlock_battery_dir);
-
-static int __init acpi_cm_sbs_init(void)
-{
-	return 0;
-}
-subsys_initcall(acpi_cm_sbs_init);
 
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#10 lore raw

PATCH 4/11 ACPI: pci_link: remove acpi_irq_balance_set() interface

In reply to Bjorn Helgaas

This removes the acpi_irq_balance_set() interface from the PCI interrupt link driver.

x86 used acpi_irq_balance_set() to tell the PCI interrupt link driver to configure links to minimize IRQ sharing. But the link driver can easily figure out whether to turn on IRQ balancing based on the IRQ model (PIC/IOAPIC/etc), so we can get rid of that external interface.

It's better for the driver to figure this out at init-time. If we set it externally via the x86 code, the interface reduces modularity, and we depend on the fact that acpi_process_madt() happens before we process the kernel command line.

---
 arch/x86/include/asm/acpi.h |    1 -
 arch/x86/kernel/acpi/boot.c |    1 -
 drivers/acpi/pci_link.c     |   11 +++++++++--
 3 files changed, 9 insertions(+), 4 deletions(-)
arch/x86/include/asm/acpi.h +0 -1
index 8d676d8..9830681 100644
@@ -113,7 +113,6 @@ static inline void acpi_disable_pci(void)
 	acpi_pci_disabled = 1;
 	acpi_noirq_set();
 }
-extern int acpi_irq_balance_set(char *str);
 
 /* routines for saving/restoring kernel state */
 extern int acpi_save_state_mem(void);
arch/x86/kernel/acpi/boot.c +0 -1
index 8c1f76a..4c51a2f 100644
@@ -1343,7 +1343,6 @@ static void __init acpi_process_madt(void)
 			error = acpi_parse_madt_ioapic_entries();
 			if (!error) {
 				acpi_irq_model = ACPI_IRQ_MODEL_IOAPIC;
-				acpi_irq_balance_set(NULL);
 				acpi_ioapic = 1;
 
 				smp_found_config = 1;
drivers/acpi/pci_link.c +9 -2
index fcfdef7..e52ad91 100644
@@ -531,7 +531,7 @@ int __init acpi_irq_penalty_init(void)
 	return 0;
 }
 
-static int acpi_irq_balance;	/* 0: static, 1: balance */
+static int acpi_irq_balance = -1;	/* 0: static, 1: balance */
 
 static int acpi_pci_link_allocate(struct acpi_pci_link *link)
 {
@@ -950,10 +950,17 @@ device_initcall(irqrouter_init_sysfs);
 
 static int __init acpi_pci_link_init(void)
 {
-
 	if (acpi_noirq)
 		return 0;
 
+	if (acpi_irq_balance == -1) {
+		/* no command line switch: enable balancing in IOAPIC mode */
+		if (acpi_irq_model == ACPI_IRQ_MODEL_IOAPIC)
+			acpi_irq_balance = 1;
+		else
+			acpi_irq_balance = 0;
+	}
+
 	acpi_link.count = 0;
 	INIT_LIST_HEAD(&acpi_link.entries);
 
reviewer
#11 lore raw

Re: [PATCH 04/11] ACPI: pci_link: remove acpi_irq_balance_set() interface

applied to acpi-test

thanks,
-len

Quoted 83 lines · Bjorn Helgaas

This removes the acpi_irq_balance_set() interface from the PCI interrupt link driver.

x86 used acpi_irq_balance_set() to tell the PCI interrupt link driver to configure links to minimize IRQ sharing. But the link driver can easily figure out whether to turn on IRQ balancing based on the IRQ model (PIC/IOAPIC/etc), so we can get rid of that external interface.

It's better for the driver to figure this out at init-time. If we set it externally via the x86 code, the interface reduces modularity, and we depend on the fact that acpi_process_madt() happens before we process the kernel command line.

---
 arch/x86/include/asm/acpi.h |    1 -
 arch/x86/kernel/acpi/boot.c |    1 -
 drivers/acpi/pci_link.c     |   11 +++++++++--
 3 files changed, 9 insertions(+), 4 deletions(-)
arch/x86/include/asm/acpi.h +0 -1
index 8d676d8..9830681 100644
@@ -113,7 +113,6 @@ static inline void acpi_disable_pci(void)
 	acpi_pci_disabled = 1;
 	acpi_noirq_set();
 }
-extern int acpi_irq_balance_set(char *str);
 
 /* routines for saving/restoring kernel state */
 extern int acpi_save_state_mem(void);
arch/x86/kernel/acpi/boot.c +0 -1
index 8c1f76a..4c51a2f 100644
@@ -1343,7 +1343,6 @@ static void __init acpi_process_madt(void)
 			error = acpi_parse_madt_ioapic_entries();
 			if (!error) {
 				acpi_irq_model = ACPI_IRQ_MODEL_IOAPIC;
-				acpi_irq_balance_set(NULL);
 				acpi_ioapic = 1;
 
 				smp_found_config = 1;
drivers/acpi/pci_link.c +9 -3
index fcfdef7..e52ad91 100644
@@ -531,7 +531,7 @@ int __init acpi_irq_penalty_init(void)
 	return 0;
 }
 
-static int acpi_irq_balance;	/* 0: static, 1: balance */
+static int acpi_irq_balance = -1;	/* 0: static, 1: balance */
 
 static int acpi_pci_link_allocate(struct acpi_pci_link *link)
 {
@@ -950,10 +950,17 @@ device_initcall(irqrouter_init_sysfs);
 
 static int __init acpi_pci_link_init(void)
 {
-
 	if (acpi_noirq)
 		return 0;
 
+	if (acpi_irq_balance == -1) {
+		/* no command line switch: enable balancing in IOAPIC mode */
+		if (acpi_irq_model == ACPI_IRQ_MODEL_IOAPIC)
+			acpi_irq_balance = 1;
+		else
+			acpi_irq_balance = 0;
+	}
+
 	acpi_link.count = 0;
 	INIT_LIST_HEAD(&acpi_link.entries);
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#12 lore raw

PATCH 5/11 ACPI: remove CONFIG_ACPI_POWER

In reply to Bjorn Helgaas

Remove CONFIG_ACPI_POWER. It was always set the same as CONFIG_ACPI, and it had no menu label, so there was no way to set it to anything other than "y".

The interfaces under CONFIG_ACPI_POWER (acpi_device_sleep_wake(), acpi_power_transition(), etc) are called unconditionally from the ACPI core, so we already depend on it always being present.

---
 drivers/acpi/Kconfig        |    4 ----
 drivers/acpi/Makefile       |    2 +-
 include/acpi/acpi_drivers.h |    2 --
 3 files changed, 1 insertions(+), 7 deletions(-)
drivers/acpi/Kconfig +0 -4
index 1c954a7..20f4574 100644
@@ -347,10 +347,6 @@ config ACPI_PCI_SLOT
 	  help you correlate PCI bus addresses with the physical geography
 	  of your slots. If you are unsure, say N.
 
-config ACPI_POWER
-	bool
-	default y
-
 config ACPI_SYSTEM
 	bool
 	default y
drivers/acpi/Makefile +1 -1
index d91c027..8017f63 100644
@@ -51,7 +51,7 @@ 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-$(CONFIG_ACPI_POWER)	+= power.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
include/acpi/acpi_drivers.h +0 -2
index cf04c60..818215f 100644
@@ -86,7 +86,6 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_device *device, int domain,
                                   Power Resource
    -------------------------------------------------------------------------- */
 
-#ifdef CONFIG_ACPI_POWER
 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);
@@ -94,7 +93,6 @@ 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;
-#endif
 
 /* --------------------------------------------------------------------------
                                   Embedded Controller
reviewer Applied
#13 lore raw

Re: [PATCH 05/11] ACPI: remove CONFIG_ACPI_POWER

applied.

thanks,
-len

Quoted 69 lines · Bjorn Helgaas

Remove CONFIG_ACPI_POWER. It was always set the same as CONFIG_ACPI, and it had no menu label, so there was no way to set it to anything other than "y".

The interfaces under CONFIG_ACPI_POWER (acpi_device_sleep_wake(), acpi_power_transition(), etc) are called unconditionally from the ACPI core, so we already depend on it always being present.

---
 drivers/acpi/Kconfig        |    4 ----
 drivers/acpi/Makefile       |    2 +-
 include/acpi/acpi_drivers.h |    2 --
 3 files changed, 1 insertions(+), 7 deletions(-)
drivers/acpi/Kconfig +0 -4
index 1c954a7..20f4574 100644
@@ -347,10 +347,6 @@ config ACPI_PCI_SLOT
 	  help you correlate PCI bus addresses with the physical geography
 	  of your slots. If you are unsure, say N.
 
-config ACPI_POWER
-	bool
-	default y
-
 config ACPI_SYSTEM
 	bool
 	default y
drivers/acpi/Makefile +1 -1
index d91c027..8017f63 100644
@@ -51,7 +51,7 @@ 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-$(CONFIG_ACPI_POWER)	+= power.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
include/acpi/acpi_drivers.h +0 -3
index cf04c60..818215f 100644
@@ -86,7 +86,6 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_device *device, int domain,
                                   Power Resource
    -------------------------------------------------------------------------- */
 
-#ifdef CONFIG_ACPI_POWER
 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);
@@ -94,7 +93,6 @@ 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;
-#endif
 
 /* --------------------------------------------------------------------------
                                   Embedded Controller
 
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#14 lore raw

PATCH 6/11 ACPI: remove CONFIG_ACPI_EC

In reply to Bjorn Helgaas

Remove CONFIG_ACPI_EC. It was always set the same as CONFIG_ACPI, and it had no menu label, so there was no way to set it to anything other than "y".

Per section 6.5.4 of the ACPI 3.0b specification,

OSPM must make Embedded Controller operation regions, accessed via the Embedded Controllers described in ECDT, available before executing any control method.

The ECDT table is optional, but if it is present, the above text means that the EC it describes is a required part of the ACPI subsystem, so CONFIG_ACPI_EC=n wouldn't make sense.

---
 drivers/acpi/Kconfig        |    8 --------
 drivers/acpi/Makefile       |    2 +-
 drivers/acpi/bus.c          |    3 +--
 drivers/char/sonypi.c       |    4 ++--
 drivers/misc/Kconfig        |    4 ++--
 include/acpi/acpi_drivers.h |    2 --
 include/linux/acpi.h        |    4 ----
 7 files changed, 6 insertions(+), 21 deletions(-)
drivers/acpi/Kconfig +0 -8
index 20f4574..768bb07 100644
@@ -330,14 +330,6 @@ config ACPI_DEBUG_FUNC_TRACE
 	  ACPI Debug Statements slow down ACPI processing. Function trace
 	  is about half of the penalty and is rarely useful.
 
-config ACPI_EC
-	bool
-	default y
-	help
-	  This driver is required on some systems for the proper operation of
-	  the battery and thermal drivers.  If you are compiling for a 
-	  mobile system, say Y.
-
 config ACPI_PCI_SLOT
 	tristate "PCI slot detection driver"
 	default n
drivers/acpi/Makefile +1 -1
index 8017f63..fc62231 100644
@@ -39,7 +39,7 @@ obj-y				+= sleep/
 obj-y				+= bus.o glue.o
 obj-y				+= scan.o
 # Keep EC driver first. Initialization of others depend on it.
-obj-$(CONFIG_ACPI_EC)		+= ec.o
+obj-y				+= ec.o
 obj-$(CONFIG_ACPI_AC) 		+= ac.o
 obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
 obj-$(CONFIG_ACPI_BUTTON)	+= button.o
drivers/acpi/bus.c +1 -2
index c797c64..765fd1c 100644
@@ -774,7 +774,7 @@ static int __init acpi_bus_init(void)
 		       "Unable to initialize ACPI OS objects\n");
 		goto error1;
 	}
-#ifdef CONFIG_ACPI_EC
+
 	/*
 	 * ACPI 2.0 requires the EC driver to be loaded and work before
 	 * the EC device is found in the namespace (i.e. before acpi_initialize_objects()
@@ -785,7 +785,6 @@ static int __init acpi_bus_init(void)
 	 */
 	status = acpi_ec_ecdt_probe();
 	/* Ignore result. Not having an ECDT is not fatal. */
-#endif
 
 	status = acpi_initialize_objects(ACPI_FULL_INITIALIZATION);
 	if (ACPI_FAILURE(status)) {
drivers/char/sonypi.c +2 -2
index 2457b07..f437443 100644
@@ -523,7 +523,7 @@ static int acpi_driver_registered;
 
 static int sonypi_ec_write(u8 addr, u8 value)
 {
-#ifdef CONFIG_ACPI_EC
+#ifdef CONFIG_ACPI
 	if (SONYPI_ACPI_ACTIVE)
 		return ec_write(addr, value);
 #endif
@@ -539,7 +539,7 @@ static int sonypi_ec_write(u8 addr, u8 value)
 
 static int sonypi_ec_read(u8 addr, u8 *value)
 {
-#ifdef CONFIG_ACPI_EC
+#ifdef CONFIG_ACPI
 	if (SONYPI_ACPI_ACTIVE)
 		return ec_read(addr, value);
 #endif
drivers/misc/Kconfig +2 -2
index 9494400..4494ad2 100644
@@ -230,7 +230,7 @@ config HP_WMI
 config MSI_LAPTOP
         tristate "MSI Laptop Extras"
         depends on X86
-        depends on ACPI_EC
+        depends on ACPI
         depends on BACKLIGHT_CLASS_DEVICE
         ---help---
 	  This is a driver for laptops built by MSI (MICRO-STAR
@@ -260,7 +260,7 @@ config PANASONIC_LAPTOP
 config COMPAL_LAPTOP
 	tristate "Compal Laptop Extras"
 	depends on X86
-	depends on ACPI_EC
+	depends on ACPI
 	depends on BACKLIGHT_CLASS_DEVICE
 	---help---
 	  This is a driver for laptops built by Compal:
include/acpi/acpi_drivers.h +0 -2
index 818215f..b3c40dc 100644
@@ -97,10 +97,8 @@ extern int acpi_power_nocheck;
 /* --------------------------------------------------------------------------
                                   Embedded Controller
    -------------------------------------------------------------------------- */
-#ifdef CONFIG_ACPI_EC
 int acpi_ec_ecdt_probe(void);
 int acpi_boot_ec_enable(void);
-#endif
 
 /* --------------------------------------------------------------------------
                                     Processor
include/linux/acpi.h +0 -4
index fd6a452..d7846bd 100644
@@ -163,8 +163,6 @@ struct acpi_pci_driver {
 int acpi_pci_register_driver(struct acpi_pci_driver *driver);
 void acpi_pci_unregister_driver(struct acpi_pci_driver *driver);
 
-#ifdef CONFIG_ACPI_EC
-
 extern int ec_read(u8 addr, u8 *val);
 extern int ec_write(u8 addr, u8 val);
 extern int ec_transaction(u8 command,
@@ -172,8 +170,6 @@ extern int ec_transaction(u8 command,
                           u8 *rdata, unsigned rdata_len,
 			  int force_poll);
 
-#endif /*CONFIG_ACPI_EC*/
-
 #if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
 
 typedef void (*wmi_notify_handler) (u32 value, void *context);
reviewer Applied
#15 lore raw

Re: [PATCH 06/11] ACPI: remove CONFIG_ACPI_EC

applied.

thanks,
-Len

Quoted 165 lines · Bjorn Helgaas

Remove CONFIG_ACPI_EC. It was always set the same as CONFIG_ACPI, and it had no menu label, so there was no way to set it to anything other than "y".

Per section 6.5.4 of the ACPI 3.0b specification,

OSPM must make Embedded Controller operation regions, accessed via the Embedded Controllers described in ECDT, available before executing any control method.

The ECDT table is optional, but if it is present, the above text means that the EC it describes is a required part of the ACPI subsystem, so CONFIG_ACPI_EC=n wouldn't make sense.

---
 drivers/acpi/Kconfig        |    8 --------
 drivers/acpi/Makefile       |    2 +-
 drivers/acpi/bus.c          |    3 +--
 drivers/char/sonypi.c       |    4 ++--
 drivers/misc/Kconfig        |    4 ++--
 include/acpi/acpi_drivers.h |    2 --
 include/linux/acpi.h        |    4 ----
 7 files changed, 6 insertions(+), 21 deletions(-)
drivers/acpi/Kconfig +0 -8
index 20f4574..768bb07 100644
@@ -330,14 +330,6 @@ config ACPI_DEBUG_FUNC_TRACE
 	  ACPI Debug Statements slow down ACPI processing. Function trace
 	  is about half of the penalty and is rarely useful.
 
-config ACPI_EC
-	bool
-	default y
-	help
-	  This driver is required on some systems for the proper operation of
-	  the battery and thermal drivers.  If you are compiling for a 
-	  mobile system, say Y.
-
 config ACPI_PCI_SLOT
 	tristate "PCI slot detection driver"
 	default n
drivers/acpi/Makefile +1 -1
index 8017f63..fc62231 100644
@@ -39,7 +39,7 @@ obj-y				+= sleep/
 obj-y				+= bus.o glue.o
 obj-y				+= scan.o
 # Keep EC driver first. Initialization of others depend on it.
-obj-$(CONFIG_ACPI_EC)		+= ec.o
+obj-y				+= ec.o
 obj-$(CONFIG_ACPI_AC) 		+= ac.o
 obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
 obj-$(CONFIG_ACPI_BUTTON)	+= button.o
drivers/acpi/bus.c +1 -2
index c797c64..765fd1c 100644
@@ -774,7 +774,7 @@ static int __init acpi_bus_init(void)
 		       "Unable to initialize ACPI OS objects\n");
 		goto error1;
 	}
-#ifdef CONFIG_ACPI_EC
+
 	/*
 	 * ACPI 2.0 requires the EC driver to be loaded and work before
 	 * the EC device is found in the namespace (i.e. before acpi_initialize_objects()
@@ -785,7 +785,6 @@ static int __init acpi_bus_init(void)
 	 */
 	status = acpi_ec_ecdt_probe();
 	/* Ignore result. Not having an ECDT is not fatal. */
-#endif
 
 	status = acpi_initialize_objects(ACPI_FULL_INITIALIZATION);
 	if (ACPI_FAILURE(status)) {
drivers/char/sonypi.c +2 -2
index 2457b07..f437443 100644
@@ -523,7 +523,7 @@ static int acpi_driver_registered;
 
 static int sonypi_ec_write(u8 addr, u8 value)
 {
-#ifdef CONFIG_ACPI_EC
+#ifdef CONFIG_ACPI
 	if (SONYPI_ACPI_ACTIVE)
 		return ec_write(addr, value);
 #endif
@@ -539,7 +539,7 @@ static int sonypi_ec_write(u8 addr, u8 value)
 
 static int sonypi_ec_read(u8 addr, u8 *value)
 {
-#ifdef CONFIG_ACPI_EC
+#ifdef CONFIG_ACPI
 	if (SONYPI_ACPI_ACTIVE)
 		return ec_read(addr, value);
 #endif
drivers/misc/Kconfig +2 -2
index 9494400..4494ad2 100644
@@ -230,7 +230,7 @@ config HP_WMI
 config MSI_LAPTOP
         tristate "MSI Laptop Extras"
         depends on X86
-        depends on ACPI_EC
+        depends on ACPI
         depends on BACKLIGHT_CLASS_DEVICE
         ---help---
 	  This is a driver for laptops built by MSI (MICRO-STAR
@@ -260,7 +260,7 @@ config PANASONIC_LAPTOP
 config COMPAL_LAPTOP
 	tristate "Compal Laptop Extras"
 	depends on X86
-	depends on ACPI_EC
+	depends on ACPI
 	depends on BACKLIGHT_CLASS_DEVICE
 	---help---
 	  This is a driver for laptops built by Compal:
include/acpi/acpi_drivers.h +0 -2
index 818215f..b3c40dc 100644
@@ -97,10 +97,8 @@ extern int acpi_power_nocheck;
 /* --------------------------------------------------------------------------
                                   Embedded Controller
    -------------------------------------------------------------------------- */
-#ifdef CONFIG_ACPI_EC
 int acpi_ec_ecdt_probe(void);
 int acpi_boot_ec_enable(void);
-#endif
 
 /* --------------------------------------------------------------------------
                                     Processor
include/linux/acpi.h +0 -5
index fd6a452..d7846bd 100644
@@ -163,8 +163,6 @@ struct acpi_pci_driver {
 int acpi_pci_register_driver(struct acpi_pci_driver *driver);
 void acpi_pci_unregister_driver(struct acpi_pci_driver *driver);
 
-#ifdef CONFIG_ACPI_EC
-
 extern int ec_read(u8 addr, u8 *val);
 extern int ec_write(u8 addr, u8 val);
 extern int ec_transaction(u8 command,
@@ -172,8 +170,6 @@ extern int ec_transaction(u8 command,
                           u8 *rdata, unsigned rdata_len,
 			  int force_poll);
 
-#endif /*CONFIG_ACPI_EC*/
-
 #if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
 
 typedef void (*wmi_notify_handler) (u32 value, void *context);
 
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#16 lore raw

PATCH 7/11 ACPI: add CONFIG_ACPI_PCI

In reply to Bjorn Helgaas

Add CONFIG_ACPI_PCI. This makes it possible to omit the PCI-related ACPI drivers if we build with CONFIG_ACPI=y and CONFIG_PCI=n.

---
 drivers/acpi/Kconfig  |    3 +++
 drivers/acpi/Makefile |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)
drivers/acpi/Kconfig +3 -0
index 768bb07..7962bfc 100644
@@ -330,6 +330,9 @@ config ACPI_DEBUG_FUNC_TRACE
 	  ACPI Debug Statements slow down ACPI processing. Function trace
 	  is about half of the penalty and is rarely useful.
 
+config ACPI_PCI
+	def_bool y if PCI
+
 config ACPI_PCI_SLOT
 	tristate "PCI slot detection driver"
 	default n
drivers/acpi/Makefile +1 -1
index fc62231..d0ee4c5 100644
@@ -46,7 +46,7 @@ obj-$(CONFIG_ACPI_BUTTON)	+= button.o
 obj-$(CONFIG_ACPI_FAN)		+= fan.o
 obj-$(CONFIG_ACPI_DOCK)		+= dock.o
 obj-$(CONFIG_ACPI_VIDEO)	+= video.o
-obj-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o
+obj-$(CONFIG_ACPI_PCI)		+= pci_root.o pci_link.o pci_irq.o pci_bind.o
 obj-$(CONFIG_ACPI_PCI_SLOT)	+= pci_slot.o
 obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
 obj-$(CONFIG_ACPI_CONTAINER)	+= container.o
#17 lore raw

Re: [PATCH 07/11] ACPI: add CONFIG_ACPI_PCI

Quoted 2 lines · Bjorn Helgaas

Add CONFIG_ACPI_PCI. This makes it possible to omit the PCI-related ACPI drivers if we build with CONFIG_ACPI=y and CONFIG_PCI=n.

Len convinced me on IRC that even though there's no real logical reason to have ACPI depend on PCI or PM, there's also no real benefit to removing the dependency. Removing the dependencies would increase the number of possible configurations, which increases the maintenance effort.

So I withdraw patches 7-10 of this set.

I'm still interested in any comments on the rest.

Bjorn

Quoted 34 lines
---
 drivers/acpi/Kconfig  |    3 +++
 drivers/acpi/Makefile |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)
drivers/acpi/Kconfig +3 -0
index 768bb07..7962bfc 100644
@@ -330,6 +330,9 @@ config ACPI_DEBUG_FUNC_TRACE
 	  ACPI Debug Statements slow down ACPI processing. Function trace
 	  is about half of the penalty and is rarely useful.
 
+config ACPI_PCI
+	def_bool y if PCI
+
 config ACPI_PCI_SLOT
 	tristate "PCI slot detection driver"
 	default n
drivers/acpi/Makefile +1 -1
index fc62231..d0ee4c5 100644
@@ -46,7 +46,7 @@ obj-$(CONFIG_ACPI_BUTTON)	+= button.o
 obj-$(CONFIG_ACPI_FAN)		+= fan.o
 obj-$(CONFIG_ACPI_DOCK)		+= dock.o
 obj-$(CONFIG_ACPI_VIDEO)	+= video.o
-obj-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o
+obj-$(CONFIG_ACPI_PCI)		+= pci_root.o pci_link.o pci_irq.o pci_bind.o
 obj-$(CONFIG_ACPI_PCI_SLOT)	+= pci_slot.o
 obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
 obj-$(CONFIG_ACPI_CONTAINER)	+= container.o
#18 lore raw

PATCH 8/11 ACPI: remove ACPI dependency on PCI

In reply to Bjorn Helgaas

Allow ACPI to build without CONFIG_PCI.

---
 drivers/acpi/Kconfig           |    1 -
 drivers/acpi/osl.c             |    8 ++++++++
 drivers/acpi/reboot.c          |    7 +++++--
 drivers/pnp/pnpacpi/rsparser.c |    6 ++++--
 4 files changed, 17 insertions(+), 5 deletions(-)
drivers/acpi/Kconfig +0 -1
index 7962bfc..2e6f757 100644
@@ -6,7 +6,6 @@ menuconfig ACPI
 	bool "ACPI (Advanced Configuration and Power Interface) Support"
 	depends on !IA64_HP_SIM
 	depends on IA64 || X86
-	depends on PCI
 	depends on PM
 	select PNP
 	default y
drivers/acpi/osl.c +8 -0
index 4be2521..e162be5 100644
@@ -546,6 +546,7 @@ acpi_status
 acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 			       u32 *value, u32 width)
 {
+#ifdef CONFIG_PCI
 	int result, size;
 
 	if (!value)
@@ -570,12 +571,16 @@ acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 				reg, size, value);
 
 	return (result ? AE_ERROR : AE_OK);
+#else
+	return AE_ERROR;
+#endif
 }
 
 acpi_status
 acpi_os_write_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 				acpi_integer value, u32 width)
 {
+#ifdef CONFIG_PCI
 	int result, size;
 
 	switch (width) {
@@ -597,6 +602,9 @@ acpi_os_write_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 				reg, size, value);
 
 	return (result ? AE_ERROR : AE_OK);
+#else
+	return AE_ERROR;
+#endif
 }
 
 /* TODO: Change code to take advantage of driver model more */
drivers/acpi/reboot.c +5 -2
index 755baf2..a5af8b7 100644
@@ -6,9 +6,11 @@
 void acpi_reboot(void)
 {
 	struct acpi_generic_address *rr;
-	struct pci_bus *bus0;
 	u8 reset_value;
+#ifdef CONFIG_PCI
+	struct pci_bus *bus0;
 	unsigned int devfn;
+#endif
 
 	if (acpi_disabled)
 		return;
@@ -44,6 +46,7 @@ void acpi_reboot(void)
 	/* The reset register can only exist in I/O, Memory or PCI config space
 	 * on a device on bus 0. */
 	switch (rr->space_id) {
+#ifdef CONFIG_PCI
 	case ACPI_ADR_SPACE_PCI_CONFIG:
 		/* The reset register can only live on bus 0. */
 		bus0 = pci_find_bus(0, 0);
@@ -57,7 +60,7 @@ void acpi_reboot(void)
 		pci_bus_write_config_byte(bus0, devfn,
 				(rr->address & 0xffff), reset_value);
 		break;
-
+#endif
 	case ACPI_ADR_SPACE_SYSTEM_MEMORY:
 	case ACPI_ADR_SPACE_SYSTEM_IO:
 		printk(KERN_DEBUG "ACPI MEMORY or I/O RESET_REG.\n");
drivers/pnp/pnpacpi/rsparser.c +4 -2
index adf1785..96992db 100644
@@ -124,9 +124,11 @@ static void pnpacpi_parse_allocated_irqresource(struct pnp_dev *dev,
 
 	flags = irq_flags(triggering, polarity, shareable);
 	irq = acpi_register_gsi(gsi, triggering, polarity);
-	if (irq >= 0)
+	if (irq >= 0) {
+#ifdef CONFIG_PCI
 		pcibios_penalize_isa_irq(irq, 1);
-	else
+#endif
+	} else
 		flags |= IORESOURCE_DISABLED;
 
 	pnp_add_irq_resource(dev, irq, flags);
#19 lore raw

PATCH 9/11 ACPI: remove ACPI dependency on PM

In reply to Bjorn Helgaas

Allow ACPI to build without CONFIG_PM.

CONFIG_PM (power management) gives us things like suspend/resume support. The device discovery and configuration functionality of ACPI is still useful even without power management.

CC Robert P. J. Day <[email protected]>
---
 drivers/acpi/Kconfig |    1 -
 drivers/acpi/bus.c   |    2 ++
 2 files changed, 2 insertions(+), 1 deletions(-)
drivers/acpi/Kconfig +0 -1
index 2e6f757..9530192 100644
@@ -6,7 +6,6 @@ menuconfig ACPI
 	bool "ACPI (Advanced Configuration and Power Interface) Support"
 	depends on !IA64_HP_SIM
 	depends on IA64 || X86
-	depends on PM
 	select PNP
 	default y
 	---help---
drivers/acpi/bus.c +2 -0
index 765fd1c..b9cb6eb 100644
@@ -857,6 +857,7 @@ static int __init acpi_init(void)
 
 	if (!result) {
 		pci_mmcfg_late_init();
+#ifdef CONFIG_PM
 		if (!(pm_flags & PM_APM))
 			pm_flags |= PM_ACPI;
 		else {
@@ -865,6 +866,7 @@ static int __init acpi_init(void)
 			disable_acpi();
 			result = -ENODEV;
 		}
+#endif
 	} else
 		disable_acpi();
 	/*
#20 lore raw

PATCH 10/11 ia64: remove automatic PM selection

In reply to Bjorn Helgaas

Allow ia64 to build without CONFIG_PM.

The "select PM" was added here: 8140a90ec180192b202af086e7a582e5937c5580 to work around the fact that ia64 selected ACPI, which depended on PM. But kconfig select ignores dependencies, so allnoconfig could set CONFIG_PM=n and break the build.

We don't need that any longer because ACPI no longer depends on PM.

---
 arch/ia64/Kconfig |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
arch/ia64/Kconfig +0 -1
index 08559ee..333359c 100644
@@ -15,7 +15,6 @@ config IA64
 	bool
 	select PCI if (!IA64_HP_SIM)
 	select ACPI if (!IA64_HP_SIM)
-	select PM if (!IA64_HP_SIM)
 	select ARCH_SUPPORTS_MSI
 	select HAVE_IDE
 	select HAVE_OPROFILE
#21 lore raw

PATCH 11/11 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()
are all parts of the ACPI core. None is separately loadable. 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.

---
 drivers/acpi/Makefile |   15 ++++++---------
 drivers/acpi/acpi.h   |   15 +++++++++++++++
 drivers/acpi/bus.c    |   15 +++++++++++++++
 drivers/acpi/debug.c  |   16 ++++++++--------
 drivers/acpi/glue.c   |    8 +++-----
 drivers/acpi/scan.c   |   10 +++-------
 drivers/acpi/system.c |   10 ++--------
 7 files changed, 52 insertions(+), 37 deletions(-)
 create mode 100644 drivers/acpi/acpi.h
drivers/acpi/Makefile +6 -9
index d0ee4c5..ba5c1ea 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
@@ -51,14 +51,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.h new file +15 -0
new file mode 100644
index 0000000..8ef1ed5
@@ -0,0 +1,15 @@
+/*
+ * (c) Copyright 2008 Hewlett-Packard Development Company, L.P.
+ *	Bjorn Helgaas <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/* Functions internal to the ACPI core */
+
+int init_acpi_device_notify(void);
+int acpi_debug_init(void);
+int acpi_scan_init(void);
+int acpi_system_init(void);
drivers/acpi/bus.c +15 -0
index b9cb6eb..edabe0d 100644
@@ -39,6 +39,8 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
+#include "acpi.h"
+
 #define _COMPONENT		ACPI_BUS_COMPONENT
 ACPI_MODULE_NAME("bus");
 
@@ -853,6 +855,7 @@ static int __init acpi_init(void)
 		acpi_kobj = NULL;
 	}
 
+	init_acpi_device_notify();
 	result = acpi_bus_init();
 
 	if (!result) {
@@ -869,11 +872,23 @@ static int __init acpi_init(void)
 #endif
 	} 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;
+
+#ifdef CONFIG_ACPI_DEBUG
+	acpi_debug_init();
+#endif
+	acpi_scan_init();
+#ifdef CONFIG_ACPI_SYSTEM
+	acpi_system_init();
+#endif
 	return result;
 }
 
drivers/acpi/debug.c +8 -8
index abf36b4..5f642fa 100644
@@ -11,6 +11,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/acglobal.h>
 
+#include "acpi.h"
+
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
 ACPI_MODULE_NAME("debug");
 
@@ -283,17 +285,15 @@ acpi_system_write_debug(struct file *file,
 
 	return count;
 }
+#endif
 
-static int __init acpi_debug_init(void)
+int __init acpi_debug_init(void)
 {
+#ifdef CONFIG_ACPI_PROCFS
 	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 =
@@ -324,7 +324,7 @@ static int __init acpi_debug_init(void)
 	remove_proc_entry(ACPI_SYSTEM_FILE_DEBUG_LAYER, acpi_root_dir);
 	error = -ENODEV;
 	goto Done;
-}
-
-subsys_initcall(acpi_debug_init);
+#else
+	return 0;
 #endif
+}
drivers/acpi/glue.c +3 -5
index 24649ad..6f2dcdf 100644
@@ -12,6 +12,8 @@
 #include <linux/rwsem.h>
 #include <linux/acpi.h>
 
+#include "acpi.h"
+
 #define ACPI_GLUE_DEBUG	0
 #if ACPI_GLUE_DEBUG
 #define DBG(x...) printk(PREFIX x)
@@ -246,10 +248,8 @@ static int acpi_platform_notify_remove(struct device *dev)
 	return 0;
 }
 
-static int __init init_acpi_device_notify(void)
+int 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;
@@ -258,5 +258,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/scan.c +3 -7
index a9dda8e..1f605dd 100644
@@ -12,6 +12,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/acinterp.h>	/* for acpi_ex_eisa_id_to_string() */
 
+#include "acpi.h"
+
 #define _COMPONENT		ACPI_BUS_COMPONENT
 ACPI_MODULE_NAME("scan");
 #define STRUCT_TO_INT(s)	(*((int*)&s))
@@ -1566,15 +1568,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;
@@ -1607,5 +1605,3 @@ static int __init acpi_scan_init(void)
       Done:
 	return result;
 }
-
-subsys_initcall(acpi_scan_init);
drivers/acpi/system.c +2 -8
index 1d74171..8ade31c 100644
@@ -626,20 +626,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);
reviewer
#22 lore raw

Re: [PATCH 11/11] ACPI: call core init functions explicitly instead of using initcalls

+++ b/drivers/acpi/acpi.h
...
+/* Functions internal to the ACPI core */

I like the patch, except for the creation of drivers/acpi.h For we've already got too many files called 'acpi.h'... and Bob is working on a header clean-up as we speak...

Maybe the existing acpi/acpi_drivers.h would be better for these declarations?

thanks,
-Len

#23 lore raw

Re: [PATCH 11/11] ACPI: call core init functions explicitly instead of using initcalls

Quoted 6 lines · Len Brown

+++ b/drivers/acpi/acpi.h
...
+/* Functions internal to the ACPI core */

Quoted 6 lines

I like the patch, except for the creation of drivers/acpi.h For we've already got too many files called 'acpi.h'... and Bob is working on a header clean-up as we speak...

Maybe the existing acpi/acpi_drivers.h would be better for these declarations?

Sure, I can do that. I hesitate only slightly, because acpi_drivers.h is for, well, drivers, and these are core functions and definitely not related to drivers.

I created drivers/acpi/acpi.h following the example of drivers/pci/pci.h, which contains things intended for the PCI core only.

Bjorn