On Tue, 2015-20-01 at 11:26:49 UTC, Preeti U Murthy wrote:
> @@ -177,34 +178,39 @@ static int powernv_add_idle_states(void)
> return nr_idle_states;
> }
>
> - idle_state_latency = of_get_property(power_mgt,
> - "ibm,cpu-idle-state-latencies-ns", NULL);
> - if (!idle_state_latency) {
> + dt_idle_states = len_flags / sizeof(u32);
> +
> + latency_ns = kzalloc(sizeof(*latency_ns) * dt_idle_states, GFP_KERNEL);
> + rc = of_property_read_u32(power_mgt,
> + "ibm,cpu-idle-state-latencies-ns", latency_ns);
That's only reading the first value.
If you want to read the full property you need the _array version.
> + if (rc) {
> pr_warn("DT-PowerMgmt: missing
> ibm,cpu-idle-state-latencies-ns\n");
You missed my hint that "DT-PowerMgmt" is a weird and ugly prefix. Can you use
"cpuidle-powernv:" instead?
>
> - dt_idle_states = len_flags / sizeof(u32);
> + residency_ns = kzalloc(sizeof(*residency_ns) * dt_idle_states,
> GFP_KERNEL);
> + rc = of_property_read_u32(power_mgt,
> + "ibm,cpu-idle-state-residency-ns", residency_ns);
> + if (rc) {
> + pr_warn("DT-PowerMgmt: missing
> ibm,cpu-idle-state-residency-ns\n");
> + pr_warn("Falling back to default values\n");
I don't think this is worth a warning seeing as we know there are firmwares out
there which do not have the property.
> for (i = 0; i < dt_idle_states; i++) {
>
> flags = be32_to_cpu(idle_state_flags[i]);
> -
> - /* Cpuidle accepts exit_latency in us and we estimate
> - * target residency to be 10x exit_latency
> + /*
> + * Cpuidle accepts exit_latency and target_residency in us.
> + * Use default target_residency values if f/w does not expose
> it.
> */
> - latency_ns = be32_to_cpu(idle_state_latency[i]);
> if (flags & OPAL_PM_NAP_ENABLED) {
> /* Add NAP state */
> strcpy(powernv_states[nr_idle_states].name, "Nap");
> strcpy(powernv_states[nr_idle_states].desc, "Nap");
> powernv_states[nr_idle_states].flags = 0;
> - powernv_states[nr_idle_states].exit_latency =
> - ((unsigned int)latency_ns) / 1000;
> - powernv_states[nr_idle_states].target_residency =
> - ((unsigned int)latency_ns / 100);
> + powernv_states[nr_idle_states].target_residency = 100;
> powernv_states[nr_idle_states].enter = &nap_loop;
> - nr_idle_states++;
That looks wrong? Or do you mean to do that?
cheers
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
