RE: [PATCH 1/4] ACPICA: Add to remove mis-ordered inclusion of from .
Hi, Rafael
> From: Rafael J. Wysocki [mailto:r...@rjwysocki.net]
> Sent: Tuesday, April 22, 2014 7:16 PM
>
> On Tuesday, April 22, 2014 02:01:57 AM Zheng, Lv wrote:
> > Hi,
> >
> >
> >
> > > > +#define ACPI_NATIVE_INTERFACE_HEADER
> > >
> > > This is not good.
> > >
> > > We don't do things like this in the kernel, because they are confusing
> > > and hard
> > > to debug if necessary, so please find a different way to make this work.
> >
> > I use this extra header file to collect:
>
> I was not talking about the new header, which is basically OK, but about the
> #define above, which is just too confusing to live.
>
> Please use header file paths directly with #include.
OK.
>
> > 1. static inline OSL functions
> > 2. divergences of prototypes that haven't been back ported to ACPICA.
> > This file is useful for ACPICA release automation.
> >
> > There are the following concerns that lead to the use of this solution:
> > 1. for this extra header file itself
> > A. The new header file is OSPM specific, thus it needn't be upstreamed
> > to ACPICA;
> > B. Since it needn't be upstreamed to ACPICA, ACPICA needn't determine
> > the name of this extra header;
> > C. It has to be the last file included by .
> > 2. for the file that includes this extra header file
> > A. Currently there is no OSPM specific code in .
> > Thus I use a macro so that there is still no OSPM specific code in
> > and the name of the extra header can be determined
> by OSPM.
> >
> > If you want another solution, is the following acceptable?
> > 1. In
> > #define ACPI_INCLUDE_EXTRA_NATIVE_HEADER1
> > 2. In <- this is an ACPICA header file,
> > #ifdef ACPI_INCLUDE_EXTRA_NATIVE_HEADER
> > #include
> > #endif
> > Note that in this solution, the name of the extra header file will be
> > determined by ACPICA.
>
> I think I see what you're trying to do now. And I see that this
> ACPI_NATIVE_INTERFACE_HEADER
> thing is already there in the Linus' tree which is not good at all.
>
> I probably would create an extra ACPICA header, something like ,
> that would be empty for all hosts except for Linux and that would contain the
> stuff you want to put into acextra.h in Linux.
>
> That would be clean enough I suppose?
Yes, it is clear.
Thanks for the helping.
I'll update this patch and re-send this series.
Best regards
-Lv
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH 1/4] ACPICA: Add to remove mis-ordered inclusion of from .
On Tuesday, April 22, 2014 02:01:57 AM Zheng, Lv wrote: > Hi, > > > > > > +#define ACPI_NATIVE_INTERFACE_HEADER > > > > This is not good. > > > > We don't do things like this in the kernel, because they are confusing and > > hard > > to debug if necessary, so please find a different way to make this work. > > I use this extra header file to collect: I was not talking about the new header, which is basically OK, but about the #define above, which is just too confusing to live. Please use header file paths directly with #include. > 1. static inline OSL functions > 2. divergences of prototypes that haven't been back ported to ACPICA. > This file is useful for ACPICA release automation. > > There are the following concerns that lead to the use of this solution: > 1. for this extra header file itself > A. The new header file is OSPM specific, thus it needn't be upstreamed to > ACPICA; > B. Since it needn't be upstreamed to ACPICA, ACPICA needn't determine the > name of this extra header; > C. It has to be the last file included by . > 2. for the file that includes this extra header file > A. Currently there is no OSPM specific code in . > Thus I use a macro so that there is still no OSPM specific code in > and the name of the extra header can be determined by OSPM. > > If you want another solution, is the following acceptable? > 1. In > #define ACPI_INCLUDE_EXTRA_NATIVE_HEADER 1 > 2. In <- this is an ACPICA header file, > #ifdef ACPI_INCLUDE_EXTRA_NATIVE_HEADER > #include > #endif > Note that in this solution, the name of the extra header file will be > determined by ACPICA. I think I see what you're trying to do now. And I see that this ACPI_NATIVE_INTERFACE_HEADER thing is already there in the Linus' tree which is not good at all. I probably would create an extra ACPICA header, something like , that would be empty for all hosts except for Linux and that would contain the stuff you want to put into acextra.h in Linux. That would be clean enough I suppose? 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/
RE: [PATCH 1/4] ACPICA: Add to remove mis-ordered inclusion of from .
Hi,
> > +#define ACPI_NATIVE_INTERFACE_HEADER
>
> This is not good.
>
> We don't do things like this in the kernel, because they are confusing and
> hard
> to debug if necessary, so please find a different way to make this work.
I use this extra header file to collect:
1. static inline OSL functions
2. divergences of prototypes that haven't been back ported to ACPICA.
This file is useful for ACPICA release automation.
There are the following concerns that lead to the use of this solution:
1. for this extra header file itself
A. The new header file is OSPM specific, thus it needn't be upstreamed to
ACPICA;
B. Since it needn't be upstreamed to ACPICA, ACPICA needn't determine the
name of this extra header;
C. It has to be the last file included by .
2. for the file that includes this extra header file
A. Currently there is no OSPM specific code in .
Thus I use a macro so that there is still no OSPM specific code in
and the name of the extra header can be determined by OSPM.
If you want another solution, is the following acceptable?
1. In
#define ACPI_INCLUDE_EXTRA_NATIVE_HEADER1
2. In <- this is an ACPICA header file,
#ifdef ACPI_INCLUDE_EXTRA_NATIVE_HEADER
#include
#endif
Note that in this solution, the name of the extra header file will be
determined by ACPICA.
> And the name aclinuxxf.h is not one of my favourite.
Since this file needn't be upstreamed into ACPICA, could you help to determine
it if the acextra.h was still not acceptable?
Thanks and best regards
-Lv
>
> Thanks!
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH 1/4] ACPICA: Add to remove mis-ordered inclusion of from .
On Tuesday, April 08, 2014 03:56:44 PM Lv Zheng wrote:
> From ACPICA's perspective, should be included after
> inclusion of . But currently in Linux,
> included by has
> included to find ACPICA types for inline functions.
>
> This causes the following problem:
> 1. Redundant code in and :
>Linux must be careful to keep conditions for inclusion
>consistent with the conditions for inclusion.
>Which finally leads to the issue that we have to keep many useless macro
>definitions in or .
>Such conditions include:
> COMPILER_DEPENDENT_UINT64
> COMPILER_DEPENDENT_INT64
> ACPI_INLINE
> ACPI_SYSTEM_XFACE
> ACPI_EXTERNAL_XFACE
> ACPI_INTERNAL_XFACE
> ACPI_INTERNAL_VAR_XFACE
> ACPI_MUTEX_TYPE
> DEBUGGER_THREADING
> ACPI_ACQUIRE_GLOBAL_LOCK
> ACPI_RELEASE_GLOBAL_LOCK
> ACPI_FLUSH_CPU_CACHE
>They have default implementations in
>while Linux need to keep a copy in to avoid build errors.
>Typical Linux build error if we deletes COMPILER_DEPENDENT_x and
>ACPI_x_XFACE definitions from asm/acpi.h:
> CC init/main.o
> In file included from include/acpi/platform/aclinux.h:293:0,
> from include/acpi/platform/acenv.h:149,
> from include/acpi/acpi.h:56,
> from include/linux/acpi.h:41,
> from init/main.c:27:
> include/acpi/actypes.h:129:1: error: unknown type name
> 'COMPILER_DEPENDENT_UINT64'
> include/acpi/actypes.h:130:1: error: unknown type name
> 'COMPILER_DEPENDENT_INT64'
> In file included from include/acpi/platform/aclinux.h:293:0,
> from include/acpi/platform/acenv.h:149,
> from include/acpi/acpi.h:56,
> from include/linux/acpi.h:41,
> from init/main.c:27:
> include/acpi/actypes.h:1025:21: error: expected ')' before '*' token
> include/acpi/actypes.h:1028:21: error: expected ')' before '*' token
> In file included from include/acpi/acpi.h:63:0,
> from include/linux/acpi.h:41,
> from init/main.c:27:
> include/acpi/acpiosxf.h:240:7: error: unknown type name 'acpi_osd_handler'
> include/acpi/acpiosxf.h:247:6: error: unknown type name 'acpi_osd_handler'
> include/acpi/acpiosxf.h:260:3: error: unknown type name
> 'acpi_osd_exec_callback'
>
> This patch introduces to fix this issue by
> splitting conditions and declarations (most of them are inline functions)
> into 2 header files so that the wrong inclusion of can be
> removed from .
>
> Signed-off-by: Lv Zheng
> ---
> include/acpi/platform/aclinux.h | 101 --
> include/acpi/platform/aclinuxxf.h | 122
> +
> 2 files changed, 133 insertions(+), 90 deletions(-)
> create mode 100644 include/acpi/platform/aclinuxxf.h
>
> diff --git a/include/acpi/platform/aclinux.h b/include/acpi/platform/aclinux.h
> index f242909..e3ac5a5 100644
> --- a/include/acpi/platform/aclinux.h
> +++ b/include/acpi/platform/aclinux.h
> @@ -130,73 +130,6 @@
>
> #ifdef __KERNEL__
>
> -/*
> - * FIXME: Inclusion of actypes.h
> - * Linux kernel need this before defining inline OSL interfaces as
> - * actypes.h need to be included to find ACPICA type definitions.
> - * Since from ACPICA's perspective, the actypes.h should be included after
> - * acenv.h (aclinux.h), this leads to a inclusion mis-ordering issue.
> - */
> -#include
> -
> -/*
> - * Overrides for in-kernel ACPICA
> - */
> -acpi_status __init acpi_os_initialize(void);
> -#define ACPI_USE_ALTERNATE_PROTOTYPE_acpi_os_initialize
> -
> -acpi_status acpi_os_terminate(void);
> -#define ACPI_USE_ALTERNATE_PROTOTYPE_acpi_os_terminate
> -
> -/*
> - * Memory allocation/deallocation
> - */
> -
> -/*
> - * The irqs_disabled() check is for resume from RAM.
> - * Interrupts are off during resume, just like they are for boot.
> - * However, boot has (system_state != SYSTEM_RUNNING)
> - * to quiet __might_sleep() in kmalloc() and resume does not.
> - */
> -static inline void *acpi_os_allocate(acpi_size size)
> -{
> - return kmalloc(size, irqs_disabled()? GFP_ATOMIC : GFP_KERNEL);
> -}
> -
> -#define ACPI_USE_ALTERNATE_PROTOTYPE_acpi_os_allocate
> -
> -/* Use native linux version of acpi_os_allocate_zeroed */
> -
> -static inline void *acpi_os_allocate_zeroed(acpi_size size)
> -{
> - return kzalloc(size, irqs_disabled()? GFP_ATOMIC : GFP_KERNEL);
> -}
> -
> -#define ACPI_USE_ALTERNATE_PROTOTYPE_acpi_os_allocate_zeroed
> -#define USE_NATIVE_ALLOCATE_ZEROED
> -
> -static inline void acpi_os_free(void *memory)
> -{
> - kfree(memory);
> -}
> -
> -#define ACPI_USE_ALTERNATE_PROTOTYPE_acpi_os_free
> -
> -static inline void *acpi_os_acquire_object(acpi_cache_t * cache)
> -{
> - return kmem_cache_zalloc(cache,
> - irqs_disabled()? GFP_ATOMIC : GFP_KERNEL);
> -}
> -
> -#define
