On Wed, Jun 25, 2014 at 03:10:17PM +0100, Lorenzo Pieralisi wrote:
> This patch implements the cpu_suspend cpu operations method through
> the PSCI CPU_SUSPEND API. The PSCI implementation translates the idle state
> index passed by the cpu_suspend core call into a valid PSCI state according to
> the PSCI states initialized at boot by the PSCI suspend backend.
>
> Entry point is set to cpu_resume physical address, that represents the
> default kernel execution address following a CPU reset.
>
> Idle state indices missing a DT node description are initialized to power
> state standby WFI so that if called by the idle driver they provide the
> default behaviour.
>
> Reviewed-by: Sebastian Capella <sebcape@xxxxxxxxx>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> ---
> arch/arm64/include/asm/psci.h | 4 ++
> arch/arm64/kernel/psci.c | 103 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 107 insertions(+)
[...]
> +static void psci_power_state_unpack(u32 power_state,
> + struct psci_power_state *state)
> +{
> + state->id = (power_state & PSCI_0_2_POWER_STATE_ID_MASK) >>
> + PSCI_0_2_POWER_STATE_ID_SHIFT;
> + state->type = (power_state & PSCI_0_2_POWER_STATE_TYPE_MASK) >>
> + PSCI_0_2_POWER_STATE_TYPE_SHIFT;
> + state->affinity_level =
> + (power_state & PSCI_0_2_POWER_STATE_AFFL_MASK) >>
> + PSCI_0_2_POWER_STATE_AFFL_SHIFT;
> +}
Is this valid for PSCI versions prior to 0.2?
> /*
> * The following two functions are invoked via the invoke_psci_fn pointer
> * and will not be inlined, allowing us to piggyback on the AAPCS.
> @@ -199,6 +216,77 @@ static int psci_migrate_info_type(void)
> return err;
> }
>
> +int __init psci_dt_register_idle_states(struct cpuidle_driver *drv,
> + struct device_node *state_nodes[])
> +{
> + int cpu, i;
Perhaps unsigned int? You print i with %u below.
> + for (i = 0; i < drv->state_count; i++) {
> + u32 psci_power_state;
> +
> + if (!state_nodes[i]) {
> + /*
> + * An index with a missing node pointer falls back to
> + * simple STANDBYWFI
> + */
> + psci_states[i].type = PSCI_POWER_STATE_TYPE_STANDBY;
> + continue;
> + }
Does this make sense? Are there any limitations on which state nodes
could be missing?
> +
> + if (of_property_read_u32(state_nodes[i], "entry-method-param",
> + &psci_power_state)) {
> + pr_warn(" * %s missing entry-method-param property\n",
> + state_nodes[i]->full_name);
> + /*
> + * If entry-method-param property is missing, fall
> + * back to STANDBYWFI state
> + */
> + psci_states[i].type = PSCI_POWER_STATE_TYPE_STANDBY;
> + continue;
Surely we want to throw away these states instead?
Otherwise we can get into a mess like:
psci_states[0] => low power state
psci_states[1] => lower power state
psci_states[2] => WFI / not low power
psci_states[3] => lowest power state
Where power usage and latency would jump around rather than follow
monotonic patterns.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
