On Tuesday, July 19, 2016 06:52:54 PM Sudeep Holla wrote:
> ACPI 6.0 introduced an optional object _LPI that provides an alternate
> method to describe Low Power Idle states. It defines the local power
> states for each node in a hierarchical processor topology. The OSPM can
> use _LPI object to select a local power state for each level of processor
> hierarchy in the system. They used to produce a composite power state
> request that is presented to the platform by the OSPM.
>
> Since multiple processors affect the idle state for any non-leaf hierarchy
> node, coordination of idle state requests between the processors is
> required. ACPI supports two different coordination schemes: Platform
> coordinated and OS initiated.
>
> This patch adds initial support for Platform coordination scheme of LPI.
>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Signed-off-by: Sudeep Holla <[email protected]>
> ---
> drivers/acpi/bus.c | 14 +-
> drivers/acpi/processor_driver.c | 2 +-
> drivers/acpi/processor_idle.c | 462
> +++++++++++++++++++++++++++++++++++-----
> include/acpi/processor.h | 24 ++-
> include/linux/acpi.h | 4 +
> 5 files changed, 446 insertions(+), 60 deletions(-)
>
[cut]
> +static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
> +{
> + int ret, i;
> + acpi_status status;
> + acpi_handle handle = pr->handle, pr_ahandle;
> + struct acpi_device *d = NULL;
> + struct acpi_lpi_states_array info[2], *tmp, *prev, *curr;
> +
> + if (!osc_pc_lpi_support_confirmed)
> + return -EOPNOTSUPP;
> +
> + if (!acpi_has_method(handle, "_LPI"))
> + return -EINVAL;
> +
> + flat_state_cnt = 0;
> + prev = &info[0];
> + curr = &info[1];
> + handle = pr->handle;
> + ret = acpi_processor_evaluate_lpi(handle, prev);
> + if (ret)
> + return ret;
> + flatten_lpi_states(pr, prev, NULL);
> +
> + while (ACPI_SUCCESS(status = acpi_get_parent(handle, &pr_ahandle))) {
I should have mentioned that earlier, but forgot, sorry about that.
Assignments under while () etc are generally discouraged as (a) error-prone
and (b) confusing to static analysis tools.
So I'd do
status = acpi_get_parent(handle, &pr_ahandle);
while (ACPI_SUCCESS(status)) {
> + acpi_bus_get_device(pr_ahandle, &d);
> + handle = pr_ahandle;
> +
> + if (strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID))
> + break;
> +
> + /* can be optional ? */
> + if (!acpi_has_method(handle, "_LPI"))
> + break;
> +
> + ret = acpi_processor_evaluate_lpi(handle, curr);
> + if (ret)
> + break;
> +
> + /* flatten all the LPI states in this level of hierarchy */
> + flatten_lpi_states(pr, curr, prev);
> +
> + tmp = prev, prev = curr, curr = tmp;
status = acpi_get_parent(handle, &pr_ahandle);
> + }
> +
Apart from this the patch looks OK to me, so please only update this one
and I'll queue up the series.
Thanks,
Rafael
