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

["Zheng, Lv" <lv.zheng@xxxxxxxxx>]

Sorry for delayed reply.

> >>> 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.

Yes, you can try to use codes already there, but you still can find a way not 
introducing a new inclusion of <linux/acpi.h> in an ACPICA file.
Sorry for the inconvenience.

> >> > +        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);

ACPICA is a portable ACPI implementation, where there might be users using 
compilers not compatible with c99.

This difference is there for another reason:
It is exported as a sysfs entry, it could not be converted back to u8 without 
introducing new warnings during the current kernel compilation process.
At least for now, it is a committed difference.

Or you could wait until the "bool" portable layer to be implemented in the 
ACPICA.

> 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).

Yes, we may fix this ASAP.

Thanks
-Lv

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