| Message ID | 1412090950-20835-1-git-send-email-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tuesday, September 30, 2014 05:29:10 PM Lucas Stach wrote: > If the regulator connected to the CPU voltage plane doesn't > support an OPP specified voltage with the acceptable tolerance > it's better to just disable the OPP instead of constantly > failing the voltage scaling later on. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > resend: > - no functional change, split out from the imx5 cpufreq series > v2: > - rebase on top of pm/linux-next This makes sense to me, but I need ACKs from people who are more familiar with OPPs in general. > --- > drivers/cpufreq/cpufreq-cpu0.c | 66 +++++++++++++++++++++++++----------------- > 1 file changed, 40 insertions(+), 26 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c > index a5f8c5f5f4f4..5d73efbd0240 100644 > --- a/drivers/cpufreq/cpufreq-cpu0.c > +++ b/drivers/cpufreq/cpufreq-cpu0.c > @@ -185,6 +185,7 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) > struct device *cpu_dev; > struct regulator *cpu_reg; > struct clk *cpu_clk; > + unsigned long min_uV = ~0, max_uV = 0; > unsigned int transition_latency; > int ret; > > @@ -204,16 +205,10 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) > /* OPPs might be populated at runtime, don't check for error here */ > of_init_opp_table(cpu_dev); > > - ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table); > - if (ret) { > - dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret); > - goto out_put_node; > - } > - > priv = kzalloc(sizeof(*priv), GFP_KERNEL); > if (!priv) { > ret = -ENOMEM; > - goto out_free_table; > + goto out_put_node; > } > > of_property_read_u32(np, "voltage-tolerance", &priv->voltage_tolerance); > @@ -222,30 +217,49 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) > transition_latency = CPUFREQ_ETERNAL; > > if (!IS_ERR(cpu_reg)) { > - struct dev_pm_opp *opp; > - unsigned long min_uV, max_uV; > - int i; > - > /* > - * OPP is maintained in order of increasing frequency, and > - * freq_table initialised from OPP is therefore sorted in the > - * same order. > + * Disable any OPPs where the connected regulator isn't able to > + * provide the specified voltage and record minimum and maximum > + * voltage levels. > */ > - for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) > - ; > - rcu_read_lock(); > - opp = dev_pm_opp_find_freq_exact(cpu_dev, > - freq_table[0].frequency * 1000, true); > - min_uV = dev_pm_opp_get_voltage(opp); > - opp = dev_pm_opp_find_freq_exact(cpu_dev, > - freq_table[i-1].frequency * 1000, true); > - max_uV = dev_pm_opp_get_voltage(opp); > - rcu_read_unlock(); > + while (1) { > + struct dev_pm_opp *opp; > + unsigned long opp_freq = 0, opp_uV, tol_uV; > + > + rcu_read_lock(); > + opp = dev_pm_opp_find_freq_ceil(cpu_dev, &opp_freq); > + if (IS_ERR(opp)) { > + rcu_read_unlock(); > + break; > + } > + opp_uV = dev_pm_opp_get_voltage(opp); > + rcu_read_unlock(); > + > + tol_uV = opp_uV * priv->voltage_tolerance / 100; > + if (regulator_is_supported_voltage(cpu_reg, opp_uV, > + opp_uV + tol_uV)) { > + if (opp_uV < min_uV) > + min_uV = opp_uV; > + if (opp_uV > max_uV) > + max_uV = opp_uV; > + } else { > + dev_pm_opp_disable(cpu_dev, opp_freq); > + } > + > + opp_freq++; > + } > + > ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV); > if (ret > 0) > transition_latency += ret * 1000; > } > > + ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table); > + if (ret) { > + pr_err("failed to init cpufreq table: %d\n", ret); > + goto out_free_priv; > + } > + > /* > * For now, just loading the cooling device; > * thermal DT code takes care of matching them. > @@ -274,9 +288,9 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) > > out_cooling_unregister: > cpufreq_cooling_unregister(priv->cdev); > - kfree(priv); > -out_free_table: > dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table); > +out_free_priv: > + kfree(priv); > out_put_node: > of_node_put(np); > out_put_reg_clk: >
On 1 October 2014 01:23, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Tuesday, September 30, 2014 05:29:10 PM Lucas Stach wrote: >> If the regulator connected to the CPU voltage plane doesn't >> support an OPP specified voltage with the acceptable tolerance >> it's better to just disable the OPP instead of constantly >> failing the voltage scaling later on. >> >> Signed-off-by: Lucas Stach <l.stach@pengutronix.de> >> --- >> resend: >> - no functional change, split out from the imx5 cpufreq series >> v2: >> - rebase on top of pm/linux-next > > This makes sense to me, but I need ACKs from people who are more familiar > with OPPs in general. @Rafael: Where is this gone? http://permalink.gmane.org/gmane.linux.power-management.general/49384 I am quite sure you applied this.. Have you dropped this while playing with branches ? -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, October 01, 2014 08:59:17 AM Viresh Kumar wrote: > On 1 October 2014 01:23, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Tuesday, September 30, 2014 05:29:10 PM Lucas Stach wrote: > >> If the regulator connected to the CPU voltage plane doesn't > >> support an OPP specified voltage with the acceptable tolerance > >> it's better to just disable the OPP instead of constantly > >> failing the voltage scaling later on. > >> > >> Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > >> --- > >> resend: > >> - no functional change, split out from the imx5 cpufreq series > >> v2: > >> - rebase on top of pm/linux-next > > > > This makes sense to me, but I need ACKs from people who are more familiar > > with OPPs in general. > > @Rafael: Where is this gone? > > http://permalink.gmane.org/gmane.linux.power-management.general/49384 > > I am quite sure you applied this.. Yes, I did. > Have you dropped this while playing with branches ? I dropped it temporarily, because I wanted to fold https://patchwork.kernel.org/patch/4983431/ into it, but then forgot to re-apply it. Would you mind sending it again with the fix folded in for me? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2 October 2014 02:09, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> Would you mind sending it again with the fix folded in for me?
Done.
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Donnerstag, den 02.10.2014, 10:54 +0530 schrieb Viresh Kumar: > On 2 October 2014 02:09, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > Would you mind sending it again with the fix folded in for me? > > Done. I don't know how it makes sense to fold the fix into this patch. It has nothing to do with the rename. If you want to fold the fix in the relevant patch this would be "cpufreq: cpu0: Move per-cluster initialization code to ->init()" Regards, Lucas
On Thursday, October 02, 2014 01:57:55 PM Lucas Stach wrote: > Am Donnerstag, den 02.10.2014, 10:54 +0530 schrieb Viresh Kumar: > > On 2 October 2014 02:09, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > > Would you mind sending it again with the fix folded in for me? > > > > Done. > > I don't know how it makes sense to fold the fix into this patch. It has > nothing to do with the rename. > If you want to fold the fix in the relevant patch this would be > "cpufreq: cpu0: Move per-cluster initialization code to ->init()" OK, my confusion, sorry about that. I'll take the original version of the Viresh's patch and the fix on top of that separately, then. That said, it would help if the information about when things broke was there in the fix' chanelog.
On Wednesday, October 01, 2014 08:59:17 AM Viresh Kumar wrote: > On 1 October 2014 01:23, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Tuesday, September 30, 2014 05:29:10 PM Lucas Stach wrote: > >> If the regulator connected to the CPU voltage plane doesn't > >> support an OPP specified voltage with the acceptable tolerance > >> it's better to just disable the OPP instead of constantly > >> failing the voltage scaling later on. > >> > >> Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > >> --- > >> resend: > >> - no functional change, split out from the imx5 cpufreq series > >> v2: > >> - rebase on top of pm/linux-next > > > > This makes sense to me, but I need ACKs from people who are more familiar > > with OPPs in general. > > @Rafael: Where is this gone? > > http://permalink.gmane.org/gmane.linux.power-management.general/49384 OK, since this has been restored, what am I supposed to do with the $subject patch?
On 9 October 2014 04:06, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > OK, since this has been restored, what am I supposed to do with the > $subject patch? Yeah, so the patch looked fine to me. If this can still be applied without Lucas required to resend it, please add my Ack and apply it :) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, October 09, 2014 09:13:54 AM Viresh Kumar wrote: > On 9 October 2014 04:06, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > OK, since this has been restored, what am I supposed to do with the > > $subject patch? > > Yeah, so the patch looked fine to me. If this can still be applied without > Lucas required to resend it, please add my Ack and apply it :) No, it didn't apply for me cleanly. Lucas, can you please rebase this on top of the current Linus' tree and resend?
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index a5f8c5f5f4f4..5d73efbd0240 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -185,6 +185,7 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) struct device *cpu_dev; struct regulator *cpu_reg; struct clk *cpu_clk; + unsigned long min_uV = ~0, max_uV = 0; unsigned int transition_latency; int ret; @@ -204,16 +205,10 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) /* OPPs might be populated at runtime, don't check for error here */ of_init_opp_table(cpu_dev); - ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table); - if (ret) { - dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret); - goto out_put_node; - } - priv = kzalloc(sizeof(*priv), GFP_KERNEL); if (!priv) { ret = -ENOMEM; - goto out_free_table; + goto out_put_node; } of_property_read_u32(np, "voltage-tolerance", &priv->voltage_tolerance); @@ -222,30 +217,49 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) transition_latency = CPUFREQ_ETERNAL; if (!IS_ERR(cpu_reg)) { - struct dev_pm_opp *opp; - unsigned long min_uV, max_uV; - int i; - /* - * OPP is maintained in order of increasing frequency, and - * freq_table initialised from OPP is therefore sorted in the - * same order. + * Disable any OPPs where the connected regulator isn't able to + * provide the specified voltage and record minimum and maximum + * voltage levels. */ - for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) - ; - rcu_read_lock(); - opp = dev_pm_opp_find_freq_exact(cpu_dev, - freq_table[0].frequency * 1000, true); - min_uV = dev_pm_opp_get_voltage(opp); - opp = dev_pm_opp_find_freq_exact(cpu_dev, - freq_table[i-1].frequency * 1000, true); - max_uV = dev_pm_opp_get_voltage(opp); - rcu_read_unlock(); + while (1) { + struct dev_pm_opp *opp; + unsigned long opp_freq = 0, opp_uV, tol_uV; + + rcu_read_lock(); + opp = dev_pm_opp_find_freq_ceil(cpu_dev, &opp_freq); + if (IS_ERR(opp)) { + rcu_read_unlock(); + break; + } + opp_uV = dev_pm_opp_get_voltage(opp); + rcu_read_unlock(); + + tol_uV = opp_uV * priv->voltage_tolerance / 100; + if (regulator_is_supported_voltage(cpu_reg, opp_uV, + opp_uV + tol_uV)) { + if (opp_uV < min_uV) + min_uV = opp_uV; + if (opp_uV > max_uV) + max_uV = opp_uV; + } else { + dev_pm_opp_disable(cpu_dev, opp_freq); + } + + opp_freq++; + } + ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV); if (ret > 0) transition_latency += ret * 1000; } + ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table); + if (ret) { + pr_err("failed to init cpufreq table: %d\n", ret); + goto out_free_priv; + } + /* * For now, just loading the cooling device; * thermal DT code takes care of matching them. @@ -274,9 +288,9 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) out_cooling_unregister: cpufreq_cooling_unregister(priv->cdev); - kfree(priv); -out_free_table: dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table); +out_free_priv: + kfree(priv); out_put_node: of_node_put(np); out_put_reg_clk:
If the regulator connected to the CPU voltage plane doesn't support an OPP specified voltage with the acceptable tolerance it's better to just disable the OPP instead of constantly failing the voltage scaling later on. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- resend: - no functional change, split out from the imx5 cpufreq series v2: - rebase on top of pm/linux-next --- drivers/cpufreq/cpufreq-cpu0.c | 66 +++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 26 deletions(-)