| Message ID | 1499272696-28751-3-git-send-email-ego@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, 5 Jul 2017 22:08:13 +0530 "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> wrote: > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> > > In the current idle initialization code, if there are failures in > pnv_probe_idle_states, then no platform idle state is > enabled. However, since the error is not propagated to the top-level > function pnv_init_idle_states, we continue initialization in this > top-level function even though this will never be used. > > Hence change the the return type of pnv_probe_idle_states from void to > int and in case of failures, bail out early on in > pnv_init_idle_states. > > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com> Looks good to me. Reviewed-by: Nicholas Piggin <npiggin@gmail.com> I wonder if the warnings are strong enough here to let people know idle won't be used so power consumption will be high and performance significantly reduced on SMT machines?
Hello Nicholas, On Fri, Jul 07, 2017 at 01:01:49AM +1000, Nicholas Piggin wrote: > On Wed, 5 Jul 2017 22:08:13 +0530 > "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> wrote: > > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> > > > > In the current idle initialization code, if there are failures in > > pnv_probe_idle_states, then no platform idle state is > > enabled. However, since the error is not propagated to the top-level > > function pnv_init_idle_states, we continue initialization in this > > top-level function even though this will never be used. > > > > Hence change the the return type of pnv_probe_idle_states from void to > > int and in case of failures, bail out early on in > > pnv_init_idle_states. > > > > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > > Looks good to me. > > Reviewed-by: Nicholas Piggin <npiggin@gmail.com> > > I wonder if the warnings are strong enough here to let people know > idle won't be used so power consumption will be high and performance > significantly reduced on SMT machines? Good point. Will try to print an error message to this effect. -- Thanks and Regards gautham.
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c index b747bb5..a5990d9 100644 --- a/arch/powerpc/platforms/powernv/idle.c +++ b/arch/powerpc/platforms/powernv/idle.c @@ -813,26 +813,27 @@ static int __init pnv_idle_parse(struct device_node *np, int dt_idle_states) /* * Probe device tree for supported idle states */ -static void __init pnv_probe_idle_states(void) +static int __init pnv_probe_idle_states(void) { struct device_node *np; int dt_idle_states; - int i; + int i, rc; np = of_find_node_by_path("/ibm,opal/power-mgt"); if (!np) { pr_warn("opal: PowerMgmt Node not found\n"); - return; + return -ENODEV; } dt_idle_states = of_property_count_u32_elems(np, "ibm,cpu-idle-state-flags"); if (dt_idle_states < 0) { pr_warn("cpuidle-powernv: no idle states found in the DT\n"); - return; + return -ENOENT; } - if (pnv_idle_parse(np, dt_idle_states)) - return; + rc = pnv_idle_parse(np, dt_idle_states); + if (rc) + return rc; if (cpu_has_feature(CPU_FTR_ARCH_300)) pnv_power9_idle_init(); @@ -842,6 +843,8 @@ static void __init pnv_probe_idle_states(void) continue; supported_cpuidle_states |= pnv_idle.states[i].flags; } + + return 0; } static int __init pnv_init_idle_states(void) @@ -852,7 +855,8 @@ static int __init pnv_init_idle_states(void) if (cpuidle_disable != IDLE_NO_OVERRIDE) goto out; - pnv_probe_idle_states(); + if (pnv_probe_idle_states()) + goto out; if (!(supported_cpuidle_states & OPAL_PM_SLEEP_ENABLED_ER1)) { patch_instruction(