Re: [Xen-devel] [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.03.13 at 01:59, "Zheng, Lv" <lv.zheng@xxxxxxxxx> wrote:
>> > +#include <linux/acpi.h>
> 
> This line shouldn't appear in ACPICA codes (drivers/acpi/acpica).
> Please try to declare OSL interfaces via include/acpi/platform/aclinux.h.

I'm sorry, but why am I not permitted to follow code that's
already there:
- hwsleep.c already includes linux/acpi.h
- acpi_os_set_prepare_sleep() and acpi_os_prepare_sleep() are
  both declared/stubbed out in linux/acpi.h

I'm particularly not intending to do cleanup work on ACPICA just
so that I can subsequently fix incomplete code.

Rafael, with that in mind, your request to submit the ACPICA
changes separately is impossible: The hook functions just can't
have a declaration in the non-Linux ACPI headers (or else they
wouldn't currently live in linux/acpi.h). Hence breaking up the
patch in the way you asked me to will not even compile without
first cleaning up the existing code. And again - I'm sorry, no,
that's not something I was looking forward to do.

>> >
>> > +  status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
>> > +                                 acpi_gbl_sleep_type_b, true);
> 
> bool is not used in ACPICA, please try u8 instead.

Can do, but again - why am I not permitted to follow what's
already there? acglobal.h has

bool ACPI_INIT_GLOBAL(acpi_gbl_enable_aml_debug_object, FALSE);

The more that the declaration, as pointed out above, lives in
linux/acpi.h (and did so even _before_ this patch), i.e. isn't an
ACPICA one.

>> >    status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
>> > -                                 pm1b_control);
>> > +                                 pm1b_control, false);
> 
> Likewise.
> 
>> > -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
>> > -                            u32 pm1b_control)
>> > +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
>> > +                            bool extended)
> 
> Is this an ACPICA OSL interface? Then it should not include bool parameter.

Returning the question: Given the current placement of
declarations, it isn't. But if the current layout is wrong, than that
needs fixing first. Which in turn likely means that the inconsistency
will continue to persist for the next (or the next few) kernel
version(s).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel