Quoting Lorenzo Pieralisi (2014-05-06 11:04:40)
> diff --git a/drivers/cpuidle/of_idle_states.c b/drivers/cpuidle/of_idle_states.c
> new file mode 100644
> index 0000000..360b7ad
> --- /dev/null
> +++ b/drivers/cpuidle/of_idle_states.c
> @@ -0,0 +1,293 @@
...
> +static int __init add_state_node(cpumask_t *cpumask,
> + struct device_node *state_node)
> +{
> + struct state_elem *el;
> + u32 tmp, val = 0;
> +
> + pr_debug(" * %s...\n", state_node->full_name);
> +
> + if (!state_cpus_valid(cpumask, state_node))
> + return -EINVAL;
> + /*
> + * Parse just the properties required to sort the states.
> + * Since we are missing a value defining the energy
> + * efficiency of a state, for now the sorting code uses
> + *
> + * min-residency-us+exit-latency-us
> + *
> + * as sorting rank.
> + */
> + if (of_property_read_u32(state_node, "min-residency-us",
> + &tmp)) {
> + pr_debug(" * %s missing min-residency-us property\n",
> + state_node->full_name);
> + return -EINVAL;
> + }
> +
> + val += tmp;
> +
> + if (of_property_read_u32(state_node, "exit-latency-us",
> + &tmp)) {
> + pr_debug(" * %s missing exit-latency-us property\n",
> + state_node->full_name);
> + return -EINVAL;
> + }
> +
> + val += tmp;
Sorry if i'm rehashing old stuff, but I prefer not to use the
min-residency + exit-latency to sort. I saw Rob's comment suggesting it
and your reply. I'm not sure when it was decided.
Would it be possible to sort instead based on the order in the
cpus->cpu-idle-states? If not, my preference would be to either use
index like you had before, or specify another sort order / rank value.
I think there's potential for us to create lower power states that
have lower min-residencies (reduced power consumption in the state,
allowing us to more quickly recover the higher entrance cost)
with higher exit latencies in such a way that the formula would not
sort as we expect. Having a separate value would allow us to control
the sorting in those cases.
> +
> +/*
For kernel-doc, I think you need a /** here, and a () after the
of_init_idle_driver below. Also maybe Return: instead of Returns:
and I think the return paragraph goes at the end, but am not positive.
kernel-doc-nano-HOWTO.txt
> + * of_init_idle_driver - Parse the DT idle states and initialize the
> + * idle driver states array
> + *
> + * @drv: Pointer to CPU idle driver to be initialized
> + * @state_nodes: Array of struct device_nodes to be initialized if
> + * init_nodes == true. Must be sized CPUIDLE_STATE_MAX
> + * @start_idx: First idle state index to be initialized
> + * @init_nodes: Boolean to request device nodes initialization
> + *
> + * Returns:
> + * 0 on success
> + * <0 on failure
> + *
> + * On success the states array in the cpuidle driver contains
> + * initialized entries in the states array, starting from index start_idx.
> + * If init_nodes == true, on success the state_nodes array is initialized
> + * with idle state DT node pointers, starting from index start_idx,
> + * in a 1:1 relation with the idle driver states array.
> + */
> +int __init of_init_idle_driver(struct cpuidle_driver *drv,
> + struct device_node *state_nodes[],
> + unsigned int start_idx, bool init_nodes)
> +{
Thanks!
Sebastian
--
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
