RE: [UPDATE PATCH 4/4] ACPI: Add support to force header inclusion rules for .

2013-11-25 Thread Zheng, Lv
Hi, Rafael

> From: Rafael J. Wysocki [mailto:r...@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-a...@vger.kernel.org
> Subject: Re: [UPDATE PATCH 4/4] ACPI: Add support to force header inclusion 
> rules for .
> 
> On Saturday, November 23, 2013 07:54:18 AM Lv Zheng wrote:
> > From: Lv Zheng 
> >
> > (Update due to some typo fixes.)
> >
> > As there is only CONFIG_ACPI=n processing in the , it is not
> > safe to include ,  and 
> > 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 
> 
> 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  and  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__
> >
> > +/*  is not safe for CONFIG_ACPI=n environment */
> > +#ifndef _LINUX_ACPI_H
> > +#error "Please don't include  directly, include 
> >  instead."
> > +#endif
> 
> And this should either go into all of the  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



Re: [UPDATE PATCH 4/4] ACPI: Add support to force header inclusion rules for .

2013-11-25 Thread Rafael J. Wysocki
On Saturday, November 23, 2013 07:54:18 AM Lv Zheng wrote:
> From: Lv Zheng 
> 
> (Update due to some typo fixes.)
> 
> As there is only CONFIG_ACPI=n processing in the , it is not
> safe to include ,  and 
> 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 

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__
>  
> +/*  is not safe for CONFIG_ACPI=n environment */
> +#ifndef _LINUX_ACPI_H
> +#error "Please don't include  directly, include 
>  instead."
> +#endif

And this should either go into all of the  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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 4/4] ACPI: Add support to force header inclusion rules for .

2013-11-22 Thread Lv Zheng
From: Lv Zheng 

As there is only CONFIG_ACPI=n processing in the , it is not
safe to include ,  and 
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 
---
 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
 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__
 
+/*  is not safe for CONFIG_ACPI=n environment */
+#ifndef _LINUX_ACPI_H
+#error "Please don't include  direclty, including 
 instead."
+#endif
+
 #include 
 
 
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__
 
+/*  is not safe for CONFIG_ACPI=n environment */
+#ifndef _LINUX_ACPI_H
+#error "Please don't include  directly, including 
 instead."
+#endif
+
 
 #define ACPI_MAX_STRING80
 
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  directly, including  
instead."
+#endif
+
+#endif
+
 /* Common (in-kernel/user-space) ACPICA configuration */
 
 #define ACPI_USE_SYSTEM_CLIBRARY
-- 
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/