| Message ID | 2930f1fabbb8319c11180c8db3d1693adc71aac1.1385163359.git.lv.zheng@intel.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Saturday, November 23, 2013 07:54:18 AM Lv Zheng wrote: > From: Lv Zheng <lv.zheng@intel.com> > > (Update due to some typo fixes.) > > As there is only CONFIG_ACPI=n processing in the <linux/acpi.h>, it is not > safe to include <acpi/acpi.h>, <acpi/acpi_drivers.h> and <acpi/acpi_bus.h> > directly for source out of Linux ACPI subsystems. > > This patch adds error messaging to warn developers of such wrong > inclusions. > > In order not to be bisected and reverted as a wrong commit, warning > messages are carefully split into a seperate patch other than the wrong > inclusion cleanups. > > Signed-off-by: Lv Zheng <lv.zheng@intel.com> I don't like this one to be honest. > --- > drivers/acpi/acpica/Makefile | 2 +- > include/acpi/acpi_bus.h | 5 +++++ > include/acpi/acpi_drivers.h | 5 +++++ > include/acpi/platform/aclinux.h | 10 ++++++++++ > 4 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/acpica/Makefile b/drivers/acpi/acpica/Makefile > index 4383040..7738677 100644 > --- a/drivers/acpi/acpica/Makefile > +++ b/drivers/acpi/acpica/Makefile > @@ -2,7 +2,7 @@ > # Makefile for ACPICA Core interpreter > # > > -ccflags-y := -Os > +ccflags-y := -Os -DLINUXIZED_ACPICA I don't like this (the naming and the way it is done). > ccflags-$(CONFIG_ACPI_DEBUG) += -DACPI_DEBUG_OUTPUT > > # use acpi.o to put all files here into acpi.o modparam namespace > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index 0af9667..0b1ea80 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -26,6 +26,11 @@ > #ifndef __ACPI_BUS_H__ > #define __ACPI_BUS_H__ > > +/* <acpi/acpi_bus.h> is not safe for CONFIG_ACPI=n environment */ > +#ifndef _LINUX_ACPI_H > +#error "Please don't include <acpi/acpi_bus.h> directly, include <linux/acpi.h> instead." > +#endif And this should either go into all of the <acpi/*.h> files, not only to the two of them you chose, or to none of them. I prefer none. Thanks, Rafael -- 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
Hi, Rafael > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net] > Sent: Tuesday, November 26, 2013 8:10 AM > To: Lv Zheng > Cc: Wysocki, Rafael J; Brown, Len; Zheng, Lv; linux-kernel@vger.kernel.org; linux-acpi@vger.kernel.org > Subject: Re: [UPDATE PATCH 4/4] ACPI: Add support to force header inclusion rules for <linux/acpi.h>. > > On Saturday, November 23, 2013 07:54:18 AM Lv Zheng wrote: > > From: Lv Zheng <lv.zheng@intel.com> > > > > (Update due to some typo fixes.) > > > > As there is only CONFIG_ACPI=n processing in the <linux/acpi.h>, it is not > > safe to include <acpi/acpi.h>, <acpi/acpi_drivers.h> and <acpi/acpi_bus.h> > > directly for source out of Linux ACPI subsystems. > > > > This patch adds error messaging to warn developers of such wrong > > inclusions. > > > > In order not to be bisected and reverted as a wrong commit, warning > > messages are carefully split into a seperate patch other than the wrong > > inclusion cleanups. > > > > Signed-off-by: Lv Zheng <lv.zheng@intel.com> > > I don't like this one to be honest. I generate this to prevent device driver writers to do stupid things before we can sort <acpi/acpi_bus.h> and <acpi/acpi_drivers.h> out. > > > --- > > drivers/acpi/acpica/Makefile | 2 +- > > include/acpi/acpi_bus.h | 5 +++++ > > include/acpi/acpi_drivers.h | 5 +++++ > > include/acpi/platform/aclinux.h | 10 ++++++++++ > > 4 files changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/acpi/acpica/Makefile b/drivers/acpi/acpica/Makefile > > index 4383040..7738677 100644 > > --- a/drivers/acpi/acpica/Makefile > > +++ b/drivers/acpi/acpica/Makefile > > @@ -2,7 +2,7 @@ > > # Makefile for ACPICA Core interpreter > > # > > > > -ccflags-y := -Os > > +ccflags-y := -Os -DLINUXIZED_ACPICA > > I don't like this (the naming and the way it is done). > > > ccflags-$(CONFIG_ACPI_DEBUG) += -DACPI_DEBUG_OUTPUT > > > > # use acpi.o to put all files here into acpi.o modparam namespace > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > > index 0af9667..0b1ea80 100644 > > --- a/include/acpi/acpi_bus.h > > +++ b/include/acpi/acpi_bus.h > > @@ -26,6 +26,11 @@ > > #ifndef __ACPI_BUS_H__ > > #define __ACPI_BUS_H__ > > > > +/* <acpi/acpi_bus.h> is not safe for CONFIG_ACPI=n environment */ > > +#ifndef _LINUX_ACPI_H > > +#error "Please don't include <acpi/acpi_bus.h> directly, include <linux/acpi.h> instead." > > +#endif > > And this should either go into all of the <acpi/*.h> files, not only to the two > of them you chose, or to none of them. I prefer none. OK, let it be done for now. So we need to open eyes on new ACPI commits. :-) I'll merge patch 2 and 3 and send it with your comments addressed and re-based. Thanks and best regards -Lv > > Thanks, > Rafael
diff --git a/drivers/acpi/acpica/Makefile b/drivers/acpi/acpica/Makefile index 4383040..7738677 100644 --- a/drivers/acpi/acpica/Makefile +++ b/drivers/acpi/acpica/Makefile @@ -2,7 +2,7 @@ # Makefile for ACPICA Core interpreter # -ccflags-y := -Os +ccflags-y := -Os -DLINUXIZED_ACPICA ccflags-$(CONFIG_ACPI_DEBUG) += -DACPI_DEBUG_OUTPUT # use acpi.o to put all files here into acpi.o modparam namespace diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 0af9667..0b1ea80 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -26,6 +26,11 @@ #ifndef __ACPI_BUS_H__ #define __ACPI_BUS_H__ +/* <acpi/acpi_bus.h> is not safe for CONFIG_ACPI=n environment */ +#ifndef _LINUX_ACPI_H +#error "Please don't include <acpi/acpi_bus.h> direclty, including <linux/acpi.h> instead." +#endif + #include <linux/device.h> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h index f3f1219..f5bed3a 100644 --- a/include/acpi/acpi_drivers.h +++ b/include/acpi/acpi_drivers.h @@ -26,6 +26,11 @@ #ifndef __ACPI_DRIVERS_H__ #define __ACPI_DRIVERS_H__ +/* <acpi/acpi_drivers.h> is not safe for CONFIG_ACPI=n environment */ +#ifndef _LINUX_ACPI_H +#error "Please don't include <acpi/acpi_drivers.h> directly, including <linux/acpi.h> instead." +#endif + #define ACPI_MAX_STRING 80 diff --git a/include/acpi/platform/aclinux.h b/include/acpi/platform/aclinux.h index 28f4f4d..0e05771 100644 --- a/include/acpi/platform/aclinux.h +++ b/include/acpi/platform/aclinux.h @@ -44,6 +44,16 @@ #ifndef __ACLINUX_H__ #define __ACLINUX_H__ +#ifdef __KERNEL__ + +/* ACPICA external files should not include ACPICA headers directly. */ + +#if !defined(LINUXIZED_ACPICA) && !defined(_LINUX_ACPI_H) +#error "Please don't include <acpi/acpi.h> directly, including <linux/acpi.h> instead." +#endif + +#endif + /* Common (in-kernel/user-space) ACPICA configuration */ #define ACPI_USE_SYSTEM_CLIBRARY