[PATCH 0/3] ACPI: trim acpi_drivers.h

[PATCH 0/3] ACPI: trim acpi_drivers.h

[Bjorn Helgaas]

The include/acpi/acpi_drivers.h file contains declarations
for several unrelated ACPI drivers, as well as things needed
only within the ACPI core (a.k.a. the "OSPM").

These patches move some of those declarations out into new
driver-specific and OSPM-specific header files where they
aren't visible to the world.

Comments welcome.

Bjorn

---

Bjorn Helgaas (3):
      ACPI: move internal core declarations to private header
      ACPI: processor: make new processor.h header local to driver
      ACPI: pci: make new pci.h header local to driver


 drivers/acpi/bus.c               |    2 ++
 drivers/acpi/ospm.h              |   23 +++++++++++++++++++
 drivers/acpi/pci.h               |   18 +++++++++++++++
 drivers/acpi/pci_bind.c          |    2 ++
 drivers/acpi/pci_irq.c           |    2 ++
 drivers/acpi/pci_root.c          |    2 ++
 drivers/acpi/processor.h         |    5 ++++
 drivers/acpi/processor_core.c    |    2 ++
 drivers/acpi/processor_thermal.c |    2 ++
 drivers/acpi/scan.c              |    2 ++
 drivers/acpi/sleep.c             |    1 +
 drivers/acpi/thermal.c           |    2 ++
 drivers/acpi/wakeup.c            |    1 +
 include/acpi/acpi_drivers.h      |   46 --------------------------------------
 14 files changed, 64 insertions(+), 46 deletions(-)
 create mode 100644 drivers/acpi/ospm.h
 create mode 100644 drivers/acpi/pci.h
 create mode 100644 drivers/acpi/processor.h

-- 
Signature

[PATCH 1/3] ACPI: pci: make new pci.h header local to driver

[Bjorn Helgaas]

The PCI-related declarations in acpi_drivers.h are used
only by the PCI root and related drivers.  This patch moves
those declarations to a new drivers/acpi/pci.h file.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 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

diff --git a/drivers/acpi/pci.h b/drivers/acpi/pci.h
new file mode 100644
index 0000000..b3512cf
--- /dev/null
+++ b/drivers/acpi/pci.h
@@ -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);
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 95650f8..5504c9d 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -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");
 
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index be6b909..c20c848 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -40,6 +40,8 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
+#include "pci.h"
+
 #define _COMPONENT		ACPI_PCI_COMPONENT
 ACPI_MODULE_NAME("pci_irq");
 
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 5b38a02..61e8979 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -36,6 +36,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"
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 5fc1bb0..9ea2684 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -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 */
 

[PATCH 2/3] ACPI: processor: make new processor.h header local to driver

[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.  This patch moves those declarations to a new
drivers/acpi/processor.h file.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 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

diff --git a/drivers/acpi/processor.h b/drivers/acpi/processor.h
new file mode 100644
index 0000000..51df8cb
--- /dev/null
+++ b/drivers/acpi/processor.h
@@ -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);
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 0cc2fd3..a744ad4 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -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"
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index b1eb376..a778ac2 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -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");
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 99e6f1f..23c5bc9 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -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"
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 9ea2684..4358917 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -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
   -------------------------------------------------------------------------- */

[PATCH 3/3] ACPI: move internal core declarations to private header

[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 "ospm.h"
inside drivers/acpi.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/bus.c          |    2 ++
 drivers/acpi/ospm.h         |   23 +++++++++++++++++++++++
 drivers/acpi/scan.c         |    2 ++
 drivers/acpi/sleep.c        |    1 +
 drivers/acpi/wakeup.c       |    1 +
 include/acpi/acpi_drivers.h |   23 -----------------------
 6 files changed, 29 insertions(+), 23 deletions(-)
 create mode 100644 drivers/acpi/ospm.h

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 765fd1c..0286607 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -39,6 +39,8 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
+#include "ospm.h"
+
 #define _COMPONENT		ACPI_BUS_COMPONENT
 ACPI_MODULE_NAME("bus");
 
diff --git a/drivers/acpi/ospm.h b/drivers/acpi/ospm.h
new file mode 100644
index 0000000..8e54c90
--- /dev/null
+++ b/drivers/acpi/ospm.h
@@ -0,0 +1,23 @@
+
+/* --------------------------------------------------------------------------
+                                  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
+  -------------------------------------------------------------------------- */
+extern int acpi_sleep_init(void);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index c54d7b6..2e00298 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -11,6 +11,8 @@
 
 #include <acpi/acpi_drivers.h>
 
+#include "ospm.h"
+
 #define _COMPONENT		ACPI_BUS_COMPONENT
 ACPI_MODULE_NAME("scan");
 #define STRUCT_TO_INT(s)	(*((int*)&s))
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 5192666..e841938 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -21,6 +21,7 @@
 
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
+#include "ospm.h"
 #include "sleep.h"
 
 u8 sleep_states[ACPI_S_STATE_COUNT];
diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
index 2d34806..58c1f91 100644
--- a/drivers/acpi/wakeup.c
+++ b/drivers/acpi/wakeup.c
@@ -8,6 +8,7 @@
 #include <acpi/acpi_drivers.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
+#include "ospm.h"
 #include "sleep.h"
 
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 4358917..f237ec2 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -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__*/

Re: [PATCH 2/3] ACPI: processor: make new processor.h header local to driver

[Len Brown]

we already have include/acpi/processor.h,
i'd rather not also have a drivers/acpi/processor.h

--
Len Brown, Intel Open Source Technology Center

On Thu, 19 Feb 2009, Bjorn Helgaas wrote:

> The processor-related declarations in acpi_drivers.h are used
> only by the processor driver, which happens to be split across
> several files.  This patch moves those declarations to a new
> drivers/acpi/processor.h file.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> ---
>  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
> 
> diff --git a/drivers/acpi/processor.h b/drivers/acpi/processor.h
> new file mode 100644
> index 0000000..51df8cb
> --- /dev/null
> +++ b/drivers/acpi/processor.h
> @@ -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);
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index 0cc2fd3..a744ad4 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -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"
> diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> index b1eb376..a778ac2 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -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");
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 99e6f1f..23c5bc9 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -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"
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index 9ea2684..4358917 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -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
>    -------------------------------------------------------------------------- */
> 
> --
> 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] ACPI: move internal core declarations to private header

[Len Brown]

On Thu, 19 Feb 2009, Bjorn Helgaas wrote:

> 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 "ospm.h"
> inside drivers/acpi.

I'm not sure that we're using the same terminology.

In ACPI I usually use the word "core" to refer to everything
that is now under drivers/acpi/acpica/
eg. the ACPI interpreter and the OS agnostic code in ACPICA.

That is why dmesg prints this:

ACPI: Core revision 20090123

I use the word "Linux/ACPI" to refer to everything in
drivers/acpi/* and the ACPI stuff under arch/ etc.

OSPM is supposed to be a generic concept of the OS
controlling power management, and ACPI is supposed to
be a component that allows OSPM to work
on (ACPI-compliant) systems.

I think that the ACPI guys probably invented the term OSPM.
I have heard it used outside an ACPI context only once,
and that was mostly to describe what a non-ACPI system
does to deal with its lack of ACPI...

Sometimes in the context of ACPICA, the OS code is called
the "host OS" (and I generally cringe when this term is used
rather than just saying "Linux") and sometimes it is called OSPM --
ie where the OS policy etc are.

We havn't used the term OSPM in the Linux context much.
We do use the term PM, as in CONFIG_PM though...
I'm not immediately wild about the name ospm.h for some reason.

Anyway, to the content of the patch...

We are sort of in transition WRT hiding private information.

We just finished re-factoring all the ACPICA-specific header
files, resulting in 21 private headers under drivers/acpi/acpica/
and out of include/acpi/

I'm not fond of include/acpi/

Currently it is a bucket for include/acpi/acpi.h
to pick up files, and that is in-turn included
by linux/acpi.h -- which is the file that most of
the kernel should include to talk to ACPI.

I do want things that are private to drivers/acpi/ files
to be in drivers/acpi/, and not in include/acpi/

so i guess I like what you're doing here,
but for some reason i'm not excited about the name "ospm.h",
maybe I"ll sleep on it and it will sound excellent tomorrow...

Len Brown, Intel Open Source Technology Center


> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> ---
>  drivers/acpi/bus.c          |    2 ++
>  drivers/acpi/ospm.h         |   23 +++++++++++++++++++++++
>  drivers/acpi/scan.c         |    2 ++
>  drivers/acpi/sleep.c        |    1 +
>  drivers/acpi/wakeup.c       |    1 +
>  include/acpi/acpi_drivers.h |   23 -----------------------
>  6 files changed, 29 insertions(+), 23 deletions(-)
>  create mode 100644 drivers/acpi/ospm.h
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 765fd1c..0286607 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -39,6 +39,8 @@
>  #include <acpi/acpi_bus.h>
>  #include <acpi/acpi_drivers.h>
>  
> +#include "ospm.h"
> +
>  #define _COMPONENT		ACPI_BUS_COMPONENT
>  ACPI_MODULE_NAME("bus");
>  
> diff --git a/drivers/acpi/ospm.h b/drivers/acpi/ospm.h
> new file mode 100644
> index 0000000..8e54c90
> --- /dev/null
> +++ b/drivers/acpi/ospm.h
> @@ -0,0 +1,23 @@
> +
> +/* --------------------------------------------------------------------------
> +                                  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
> +  -------------------------------------------------------------------------- */
> +extern int acpi_sleep_init(void);
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index c54d7b6..2e00298 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -11,6 +11,8 @@
>  
>  #include <acpi/acpi_drivers.h>
>  
> +#include "ospm.h"
> +
>  #define _COMPONENT		ACPI_BUS_COMPONENT
>  ACPI_MODULE_NAME("scan");
>  #define STRUCT_TO_INT(s)	(*((int*)&s))
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 5192666..e841938 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -21,6 +21,7 @@
>  
>  #include <acpi/acpi_bus.h>
>  #include <acpi/acpi_drivers.h>
> +#include "ospm.h"
>  #include "sleep.h"
>  
>  u8 sleep_states[ACPI_S_STATE_COUNT];
> diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> index 2d34806..58c1f91 100644
> --- a/drivers/acpi/wakeup.c
> +++ b/drivers/acpi/wakeup.c
> @@ -8,6 +8,7 @@
>  #include <acpi/acpi_drivers.h>
>  #include <linux/kernel.h>
>  #include <linux/types.h>
> +#include "ospm.h"
>  #include "sleep.h"
>  
>  #define _COMPONENT		ACPI_SYSTEM_COMPONENT
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index 4358917..f237ec2 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -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__*/
> 
> --
> 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 1/3] ACPI: pci: make new pci.h header local to driver

[Len Brown]

I like this patch, but not wild about the name "pci.h"

there are already about 20 pci.h files in the tree.
sure, they are context specific, but still i don't like it.

(and I don't like having 11 acpi.h files in the tree either...)

how about acpi_pci.h?

yeah, i know it would be redundant, as it lives in drivers/acpi/ 
but as basically everything it the file starts with "acpi_pci..."
maybe a little redundancy can be seen as consistent?

thanks,
--
Len Brown, Intel Open Source Technology Center

On Thu, 19 Feb 2009, Bjorn Helgaas wrote:

> The PCI-related declarations in acpi_drivers.h are used
> only by the PCI root and related drivers.  This patch moves
> those declarations to a new drivers/acpi/pci.h file.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> ---
>  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
> 
> diff --git a/drivers/acpi/pci.h b/drivers/acpi/pci.h
> new file mode 100644
> index 0000000..b3512cf
> --- /dev/null
> +++ b/drivers/acpi/pci.h
> @@ -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);
> diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
> index 95650f8..5504c9d 100644
> --- a/drivers/acpi/pci_bind.c
> +++ b/drivers/acpi/pci_bind.c
> @@ -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");
>  
> diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
> index be6b909..c20c848 100644
> --- a/drivers/acpi/pci_irq.c
> +++ b/drivers/acpi/pci_irq.c
> @@ -40,6 +40,8 @@
>  #include <acpi/acpi_bus.h>
>  #include <acpi/acpi_drivers.h>
>  
> +#include "pci.h"
> +
>  #define _COMPONENT		ACPI_PCI_COMPONENT
>  ACPI_MODULE_NAME("pci_irq");
>  
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 5b38a02..61e8979 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -36,6 +36,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"
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index 5fc1bb0..9ea2684 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -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 */
>  
> 
> --
> 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] ACPI: move internal core declarations to private header

[Bjorn Helgaas]

On Saturday 21 February 2009 09:21:14 pm Len Brown wrote:
> On Thu, 19 Feb 2009, Bjorn Helgaas wrote:
> 
> > 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 "ospm.h"
> > inside drivers/acpi.
> 
> I'm not sure that we're using the same terminology.

I'm sure we're not :-)

> In ACPI I usually use the word "core" to refer to everything
> that is now under drivers/acpi/acpica/
> eg. the ACPI interpreter and the OS agnostic code in ACPICA.
> 
> That is why dmesg prints this:
> 
> ACPI: Core revision 20090123
> 
> I use the word "Linux/ACPI" to refer to everything in
> drivers/acpi/* and the ACPI stuff under arch/ etc.

I think the common Linux usage of "core" is more along the lines
of "bus infrastructure and implementation of stuff in the driver/
kernel interface," e.g., 
    PCI core -> drivers/pci/*
    PNP core -> drivers/pnp/*
So by analogy, I think the "ACPI core" should refer to the stuff
in drivers/acpi.  The stuff in drivers/acpi/acpica is something
else that doesn't really appear in other subsystems, so using "core"
to refer to the ACPICA makes ACPI unnecessarily different.

But this is tangential, and I'll gladly remove "core" from the
changelog!

> ...
> I do want things that are private to drivers/acpi/ files
> to be in drivers/acpi/, and not in include/acpi/
> 
> so i guess I like what you're doing here,
> but for some reason i'm not excited about the name "ospm.h",
> maybe I"ll sleep on it and it will sound excellent tomorrow...

I'm not married to "ospm.h", and you're right that it's not very
descriptive and doesn't have much history in Linux.  I tried "acpi.h"
and "acpi_core.h" earlier, but those were duds as well.  Here
are some similar files in other parts of Linux:

    block/blk.h
    crypto/internal.h
    drivers/base/base.h
    drivers/pci/pci.h
    drivers/pnp/base.h
    drivers/scsi/scsi_priv.h
    fs/internal.h
    mm/internal.h

What would you think of "drivers/acpi/internal.h"?

Bjorn

Re: [PATCH 1/3] ACPI: pci: make new pci.h header local to driver

[Bjorn Helgaas]

On Saturday 21 February 2009 09:28:31 pm Len Brown wrote:
> I like this patch, but not wild about the name "pci.h"
> 
> there are already about 20 pci.h files in the tree.
> sure, they are context specific, but still i don't like it.
> 
> (and I don't like having 11 acpi.h files in the tree either...)
> 
> how about acpi_pci.h?
> 
> yeah, i know it would be redundant, as it lives in drivers/acpi/ 
> but as basically everything it the file starts with "acpi_pci..."
> maybe a little redundancy can be seen as consistent?

Personally, I *like* the fact that we have a hierarchy where the
context provides useful information.

"ls drivers/acpi" already looks goofy because most of the names are
nice and simple, but we have the "acpi_memhotplug.c" weird thing.
I think it would be better to see "pci.h" next to the PCI-related
drivers and "processor.h" next to the processor-related drivers
rather than having "acpi_pci.h" and "acpi_processor.h" off by
themselves.

But I want the stuff out of include/acpi/acpi_drivers.h more than
I care about the new names, so if these names:

    drivers/acpi/acpi_pci.h
    drivers/acpi/acpi_processor.h
    drivers/acpi/internal.h

work for you, I'll update the patches.

Bjorn

> On Thu, 19 Feb 2009, Bjorn Helgaas wrote:
> 
> > The PCI-related declarations in acpi_drivers.h are used
> > only by the PCI root and related drivers.  This patch moves
> > those declarations to a new drivers/acpi/pci.h file.
> > 
> > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> > ---
> >  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
> > 
> > diff --git a/drivers/acpi/pci.h b/drivers/acpi/pci.h
> > new file mode 100644
> > index 0000000..b3512cf
> > --- /dev/null
> > +++ b/drivers/acpi/pci.h
> > @@ -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);
> > diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
> > index 95650f8..5504c9d 100644
> > --- a/drivers/acpi/pci_bind.c
> > +++ b/drivers/acpi/pci_bind.c
> > @@ -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");
> >  
> > diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
> > index be6b909..c20c848 100644
> > --- a/drivers/acpi/pci_irq.c
> > +++ b/drivers/acpi/pci_irq.c
> > @@ -40,6 +40,8 @@
> >  #include <acpi/acpi_bus.h>
> >  #include <acpi/acpi_drivers.h>
> >  
> > +#include "pci.h"
> > +
> >  #define _COMPONENT		ACPI_PCI_COMPONENT
> >  ACPI_MODULE_NAME("pci_irq");
> >  
> > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> > index 5b38a02..61e8979 100644
> > --- a/drivers/acpi/pci_root.c
> > +++ b/drivers/acpi/pci_root.c
> > @@ -36,6 +36,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"
> > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> > index 5fc1bb0..9ea2684 100644
> > --- a/include/acpi/acpi_drivers.h
> > +++ b/include/acpi/acpi_drivers.h
> > @@ -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 */
> >  
> > 
> > --
> > 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 1/3] ACPI: pci: make new pci.h header local to driver

[Bjorn Helgaas]

On Tuesday 24 February 2009 12:06:07 pm Bjorn Helgaas wrote:
> On Saturday 21 February 2009 09:28:31 pm Len Brown wrote:
> > I like this patch, but not wild about the name "pci.h"
> > 
> > there are already about 20 pci.h files in the tree.
> > sure, they are context specific, but still i don't like it.
> > 
> > (and I don't like having 11 acpi.h files in the tree either...)
> > 
> > how about acpi_pci.h?
> > 
> > yeah, i know it would be redundant, as it lives in drivers/acpi/ 
> > but as basically everything it the file starts with "acpi_pci..."
> > maybe a little redundancy can be seen as consistent?
> 
> Personally, I *like* the fact that we have a hierarchy where the
> context provides useful information.

P.S.  My opinion here might be influenced by the fact that I always use
"cscope -p4".  Cscope users who use the default (show only one path
component) would benefit a lot more from having redundant information
in the filename.

Bjorn

Re: [PATCH 3/3] ACPI: move internal core declarations to private header

[Len Brown]

On Tue, 24 Feb 2009, Bjorn Helgaas wrote:

> On Saturday 21 February 2009 09:21:14 pm Len Brown wrote:
> > On Thu, 19 Feb 2009, Bjorn Helgaas wrote:
> > 
> > > 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 "ospm.h"
> > > inside drivers/acpi.
> > 
> > I'm not sure that we're using the same terminology.
> 
> I'm sure we're not :-)
> 
> > In ACPI I usually use the word "core" to refer to everything
> > that is now under drivers/acpi/acpica/
> > eg. the ACPI interpreter and the OS agnostic code in ACPICA.
> > 
> > That is why dmesg prints this:
> > 
> > ACPI: Core revision 20090123
> > 
> > I use the word "Linux/ACPI" to refer to everything in
> > drivers/acpi/* and the ACPI stuff under arch/ etc.
> 
> I think the common Linux usage of "core" is more along the lines
> of "bus infrastructure and implementation of stuff in the driver/
> kernel interface," e.g., 
>     PCI core -> drivers/pci/*
>     PNP core -> drivers/pnp/*
> So by analogy, I think the "ACPI core" should refer to the stuff
> in drivers/acpi.  The stuff in drivers/acpi/acpica is something
> else that doesn't really appear in other subsystems, so using "core"
> to refer to the ACPICA makes ACPI unnecessarily different.
> 
> But this is tangential, and I'll gladly remove "core" from the
> changelog!

Okay, lets stay away from the word core,
since it has two common conflicting uses.

> > I do want things that are private to drivers/acpi/ files
> > to be in drivers/acpi/, and not in include/acpi/
> > 
> > so i guess I like what you're doing here,
> > but for some reason i'm not excited about the name "ospm.h",
> > maybe I"ll sleep on it and it will sound excellent tomorrow...
> 
> I'm not married to "ospm.h", and you're right that it's not very
> descriptive and doesn't have much history in Linux.  I tried "acpi.h"
> and "acpi_core.h" earlier, but those were duds as well.  Here
> are some similar files in other parts of Linux:
> 
>     block/blk.h
>     crypto/internal.h
>     drivers/base/base.h
>     drivers/pci/pci.h
>     drivers/pnp/base.h
>     drivers/scsi/scsi_priv.h
>     fs/internal.h
>     mm/internal.h
> 
> What would you think of "drivers/acpi/internal.h"?

better.

I do like internal.h better than ospm.h because internal.h
sounds like something that drivers should not include,
while ospm.h sounds like something they should include.

The murkyness right now I think is because we still have two
different types of code in drivers/acpi/

acpi/ seems to be where we have some infrastructre code,
plus some "acpi drivers", such as battery.c that use the
infrastructure to implement OS policy.  Maybe we should
have the infrastructure and the drivers in two different
directories?

acpi/acpica/ is a single place for the ACPICA C-code
	and has its own private headers down there, finally.

include/linux/acpi.h
	this is what other parts of the kernel use to talk
	to the acpi sub-system

include/acpi/
	this is here so that the above can include <acpi/acpi.h>
	which bundles various public ACPICA headers
	there is probably a more elegant way to do this.

drivers/platform/x86/
	platform specific drivers that use acpi
	rather than implement acpi, finally moved out
	of the drivers/acpi/ directory

thanks,
-Len

Re: [PATCH 3/3] ACPI: move internal core declarations to private header

[Bjorn Helgaas]

On Thursday 26 February 2009 02:00:01 pm Len Brown wrote:
> On Tue, 24 Feb 2009, Bjorn Helgaas wrote:
> > What would you think of "drivers/acpi/internal.h"?
> 
> better.
> 
> I do like internal.h better than ospm.h because internal.h
> sounds like something that drivers should not include,
> while ospm.h sounds like something they should include.

Yep, I can agree with that.

> The murkyness right now I think is because we still have two
> different types of code in drivers/acpi/
> 
> acpi/ seems to be where we have some infrastructre code,
> plus some "acpi drivers", such as battery.c that use the
> infrastructure to implement OS policy.  Maybe we should
> have the infrastructure and the drivers in two different
> directories?

Yes, maybe so.  We certainly don't have drivers under drivers/pci
just because they happen to call pci_register_driver().

drivers/platform/x86 obviously isn't the right place for generic
things like battery.c, fan.c, etc, but I'd be happy if they were
moved out of drivers/acpi someday.

Bjorn

> acpi/acpica/ is a single place for the ACPICA C-code
> 	and has its own private headers down there, finally.
> 
> include/linux/acpi.h
> 	this is what other parts of the kernel use to talk
> 	to the acpi sub-system
> 
> include/acpi/
> 	this is here so that the above can include <acpi/acpi.h>
> 	which bundles various public ACPICA headers
> 	there is probably a more elegant way to do this.
> 
> drivers/platform/x86/
> 	platform specific drivers that use acpi
> 	rather than implement acpi, finally moved out
> 	of the drivers/acpi/ directory