lkml.org 
[lkml]   [2013]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [UPDATE PATCH 4/4] ACPI: Add support to force header inclusion rules for <linux/acpi.h>.
Date
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

\
 
 \ /
  Last update: 2013-11-26 02:01    [W:0.063 / U:1.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site