On Mon, 22 Jul 2019 at 17:38, Lorenzo Pieralisi <[email protected]> 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 <[email protected]> > Cc: Will Deacon <[email protected]> > Cc: Ulf Hansson <[email protected]> > Cc: Sudeep Holla <[email protected]> > Cc: Daniel Lezcano <[email protected]> > Cc: Catalin Marinas <[email protected]> > Cc: Mark Rutland <[email protected]> > Cc: "Rafael J. Wysocki" <[email protected]> > --- > arch/arm64/kernel/cpuidle.c | 47 +++++++++- > drivers/cpuidle/cpuidle-psci.c | 87 +++++++++++++++++- > drivers/firmware/psci/psci.c | 158 ++------------------------------- > include/linux/cpuidle.h | 17 +++- > include/linux/psci.h | 4 +- > 5 files changed, 153 insertions(+), 160 deletions(-) > > diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c > index 4bcd1bca0dfc..e4d6af2fdec7 100644 > --- a/arch/arm64/kernel/cpuidle.c > +++ b/arch/arm64/kernel/cpuidle.c > @@ -47,6 +47,44 @@ int arm_cpuidle_suspend(int index) > > #define ARM64_LPI_IS_RETENTION_STATE(arch_flags) (!(arch_flags)) > > +static int psci_acpi_cpu_init_idle(unsigned int cpu) > +{ > + int i, count; > + struct acpi_lpi_state *lpi; > + struct acpi_processor *pr = per_cpu(processors, cpu); > + > + /* > + * 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; > + > + if (unlikely(!pr || !pr->flags.has_lpi)) > + return -EINVAL; > + > + count = pr->power.count - 1; > + if (count <= 0) > + return -ENODEV; > + > + for (i = 0; i < count; i++) { > + u32 state; > + > + lpi = &pr->power.lpi_states[i + 1]; > + /* > + * Only bits[31:0] represent a PSCI power_state while > + * bits[63:32] must be 0x0 as per ARM ACPI FFH Specification > + */ > + state = lpi->address; > + if (!psci_power_state_is_valid(state)) { > + pr_warn("Invalid PSCI power state %#x\n", state); > + return -EINVAL; > + } > + } > + > + return 0; > +} > + > int acpi_processor_ffh_lpi_probe(unsigned int cpu) > { > return psci_acpi_cpu_init_idle(cpu); > @@ -54,10 +92,13 @@ int acpi_processor_ffh_lpi_probe(unsigned int cpu) > > int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi) > { > + u32 state = lpi->address; > + > if (ARM64_LPI_IS_RETENTION_STATE(lpi->arch_flags)) > - return CPU_PM_CPU_IDLE_ENTER_RETENTION(psci_cpu_suspend_enter, > - lpi->index); > + return > CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM(psci_cpu_suspend_enter, > + lpi->index, state); > else > - return CPU_PM_CPU_IDLE_ENTER(psci_cpu_suspend_enter, > lpi->index); > + return CPU_PM_CPU_IDLE_ENTER_PARAM(psci_cpu_suspend_enter, > + lpi->index, state); > }
I am not sure where the acpi+psci cpuidle code really belongs. Perhaps some code should be moved into separate acpi+psci cpuidle driver? In any case and whatever makes sense, it can be done on top of the current series. [...] Kind regards Uffe
