Re: [PATCH 6/6] PSCI: cpuidle: Refactor CPU suspend power_state parameter handling

[Sudeep Holla]

On Mon, Jul 22, 2019 at 04:37:45PM +0100, Lorenzo Pieralisi wrote:
> Current PSCI code handles idle state entry through the
> psci_cpu_suspend_enter() API, that takes an idle state index as a
> parameter and convert the index into a previously initialized
> power_state parameter before calling the PSCI.CPU_SUSPEND() with it.
>
> This is unwieldly, since it forces the PSCI firmware layer to keep track
> of power_state parameter for every idle state so that the
> index->power_state conversion can be made in the PSCI firmware layer
> instead of the CPUidle driver implementations.
>
> Move the power_state handling out of drivers/firmware/psci
> into the respective ACPI/DT PSCI CPUidle backends and convert
> the psci_cpu_suspend_enter() API to get the power_state
> parameter as input, which makes it closer to its firmware
> interface PSCI.CPU_SUSPEND() API.
>
> A notable side effect is that the PSCI ACPI/DT CPUidle backends
> now can directly handle (and if needed update) power_state
> parameters before handing them over to the PSCI firmware
> interface to trigger PSCI.CPU_SUSPEND() calls.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Cc: Sudeep Holla <sudeep.holla@xxxxxxx>

Reviewed-by: Sudeep Holla <sudeep.holla@xxxxxxx>

> +static __init int psci_cpu_init_idle(unsigned int cpu)
> +{
> +	struct device_node *cpu_node;
> +	int ret;
> +
> +	/*
> +	 * If the PSCI cpu_suspend function hook has not been initialized
> +	 * idle states must not be enabled, so bail out
> +	 */
> +	if (!psci_ops.cpu_suspend)
> +		return -EOPNOTSUPP;
> +
> +	cpu_node = of_get_cpu_node(cpu, NULL);

[nit] You could use of_cpu_device_node_get in linux/of_device.h as
it may avoid parsing if used later during the boot(i.e. after
cpu->of_node is populated). I think there's another instance in
psci_idle_init_cpu

--
Regards,
Sudeep