Re: [PATCH V3 11/16] cpufreq: dt: Pass regulator name to the OPP core
On 18-07-18, 09:09, Geert Uytterhoeven wrote: > IMHO they should be documented somewhere, with a link to > Documentation/devicetree/bindings/regulator/regulator.txt. > How else can a DTS writer know he/she needs them? > Without cpu-supply (and clock!) properties, cpufreq can't work. > > Candidates: > - Documentation/devicetree/bindings/cpufreq/cpufreq-dt.txt > This has operating points without clocks nor cpu-supply, but with > clock-latency (for which clock? ;-) > - Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt > Again, operating points without clocks nor cpu-supply, but with > clock-latency. Sounds reasonable. Will cook patches for that soon. Thanks. -- viresh
Re: [PATCH V3 11/16] cpufreq: dt: Pass regulator name to the OPP core
Hi Viresh,
On Wed, Jul 18, 2018 at 7:50 AM Viresh Kumar wrote:
> On 17-07-18, 09:46, Geert Uytterhoeven wrote:
> > CC device-tree folks
> >
> > Replying to an old email, because that's the most accurate reference I
> > could find.
> >
> > On Tue, Feb 9, 2016 at 6:06 AM Viresh Kumar wrote:
> > > OPP core can handle the regulators by itself, and but it needs to know
> > > the name of the regulator to fetch. Add support for that.
> > >
> > > Signed-off-by: Viresh Kumar
> > > ---
> > > drivers/cpufreq/cpufreq-dt.c | 46
> > >
> > > 1 file changed, 46 insertions(+)
> > >
> > > diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> > > index 4c9f8a828f6f..2af75f8088bb 100644
> > > --- a/drivers/cpufreq/cpufreq-dt.c
> > > +++ b/drivers/cpufreq/cpufreq-dt.c
> >
> > > @@ -119,6 +120,30 @@ static int set_target(struct cpufreq_policy *policy,
> > > unsigned int index)
> > > return ret;
> > > }
> > >
> > > +/*
> > > + * An earlier version of opp-v1 bindings used to name the regulator
> > > + * "cpu0-supply", we still need to handle that for backwards
> > > compatibility.
> > > + */
> > > +static const char *find_supply_name(struct device *dev, struct
> > > device_node *np)
> > > +{
> > > + struct property *pp;
> > > + int cpu = dev->id;
> > > +
> > > + /* Try "cpu0" for older DTs */
> > > + if (!cpu) {
> > > + pp = of_find_property(np, "cpu0-supply", NULL);
> > > + if (pp)
> > > + return "cpu0";
> > > + }
> > > +
> > > + pp = of_find_property(np, "cpu-supply", NULL);
> > > + if (pp)
> > > + return "cpu";
> >
> > Despite the existence of lots of users of these properties, I couldn't find
> > both the "earlier version" and the "current version" of the opp-v1 bindings
> > documenting the "cpu0-supply" and "cpu-supply" properties?
>
> They are part of the device nodes and don't fall under the
> jurisdiction of OPP tables and so aren't defined there. We rely on the
> "-supply" property from the regulator bindings for the devices.
>
> > Even for opp-v2, they are not documented in
> > Documentation/devicetree/bindings/opp/opp.txt, but cpu-supply is used in
> > the examples?
>
> Same reasoning here as well.
Right, they are part of the cpu nodes, not of the operating-points properties
(which are part of the cpu nodes) or the opp_table nodes (for v2).
> > Can you please document these properties?
>
> I don't think we need to, do we ?
IMHO they should be documented somewhere, with a link to
Documentation/devicetree/bindings/regulator/regulator.txt.
How else can a DTS writer know he/she needs them?
Without cpu-supply (and clock!) properties, cpufreq can't work.
Candidates:
- Documentation/devicetree/bindings/cpufreq/cpufreq-dt.txt
This has operating points without clocks nor cpu-supply, but with
clock-latency (for which clock? ;-)
- Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
Again, operating points without clocks nor cpu-supply, but with
clock-latency.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH V3 11/16] cpufreq: dt: Pass regulator name to the OPP core
On 17-07-18, 09:46, Geert Uytterhoeven wrote:
> Hi Viresh,
>
> CC device-tree folks
>
> Replying to an old email, because that's the most accurate reference I
> could find.
>
> On Tue, Feb 9, 2016 at 6:06 AM Viresh Kumar wrote:
> > OPP core can handle the regulators by itself, and but it needs to know
> > the name of the regulator to fetch. Add support for that.
> >
> > Signed-off-by: Viresh Kumar
> > ---
> > drivers/cpufreq/cpufreq-dt.c | 46
> >
> > 1 file changed, 46 insertions(+)
> >
> > diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> > index 4c9f8a828f6f..2af75f8088bb 100644
> > --- a/drivers/cpufreq/cpufreq-dt.c
> > +++ b/drivers/cpufreq/cpufreq-dt.c
>
> > @@ -119,6 +120,30 @@ static int set_target(struct cpufreq_policy *policy,
> > unsigned int index)
> > return ret;
> > }
> >
> > +/*
> > + * An earlier version of opp-v1 bindings used to name the regulator
> > + * "cpu0-supply", we still need to handle that for backwards compatibility.
> > + */
> > +static const char *find_supply_name(struct device *dev, struct device_node
> > *np)
> > +{
> > + struct property *pp;
> > + int cpu = dev->id;
> > +
> > + /* Try "cpu0" for older DTs */
> > + if (!cpu) {
> > + pp = of_find_property(np, "cpu0-supply", NULL);
> > + if (pp)
> > + return "cpu0";
> > + }
> > +
> > + pp = of_find_property(np, "cpu-supply", NULL);
> > + if (pp)
> > + return "cpu";
>
> Despite the existence of lots of users of these properties, I couldn't find
> both the "earlier version" and the "current version" of the opp-v1 bindings
> documenting the "cpu0-supply" and "cpu-supply" properties?
They are part of the device nodes and don't fall under the
jurisdiction of OPP tables and so aren't defined there. We rely on the
"-supply" property from the regulator bindings for the devices.
> Even for opp-v2, they are not documented in
> Documentation/devicetree/bindings/opp/opp.txt, but cpu-supply is used in
> the examples?
Same reasoning here as well.
> For v2, I did find "[PATCH 01/16] PM / OPP: Add 'supply-names' binding"
> https://lore.kernel.org/lkml/2b87b162eabd1570ae2311e1ef8655acda72f678.1441972771.git.viresh.ku...@linaro.org/
> but presumably that's an even further evolution?
Yeah, that never made it to mainline is abandoned.
> Can you please document these properties?
I don't think we need to, do we ?
--
viresh
Re: [PATCH V3 11/16] cpufreq: dt: Pass regulator name to the OPP core
Hi Viresh,
CC device-tree folks
Replying to an old email, because that's the most accurate reference I
could find.
On Tue, Feb 9, 2016 at 6:06 AM Viresh Kumar wrote:
> OPP core can handle the regulators by itself, and but it needs to know
> the name of the regulator to fetch. Add support for that.
>
> Signed-off-by: Viresh Kumar
> ---
> drivers/cpufreq/cpufreq-dt.c | 46
>
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> index 4c9f8a828f6f..2af75f8088bb 100644
> --- a/drivers/cpufreq/cpufreq-dt.c
> +++ b/drivers/cpufreq/cpufreq-dt.c
> @@ -119,6 +120,30 @@ static int set_target(struct cpufreq_policy *policy,
> unsigned int index)
> return ret;
> }
>
> +/*
> + * An earlier version of opp-v1 bindings used to name the regulator
> + * "cpu0-supply", we still need to handle that for backwards compatibility.
> + */
> +static const char *find_supply_name(struct device *dev, struct device_node
> *np)
> +{
> + struct property *pp;
> + int cpu = dev->id;
> +
> + /* Try "cpu0" for older DTs */
> + if (!cpu) {
> + pp = of_find_property(np, "cpu0-supply", NULL);
> + if (pp)
> + return "cpu0";
> + }
> +
> + pp = of_find_property(np, "cpu-supply", NULL);
> + if (pp)
> + return "cpu";
Despite the existence of lots of users of these properties, I couldn't find
both the "earlier version" and the "current version" of the opp-v1 bindings
documenting the "cpu0-supply" and "cpu-supply" properties?
Even for opp-v2, they are not documented in
Documentation/devicetree/bindings/opp/opp.txt, but cpu-supply is used in
the examples?
For v2, I did find "[PATCH 01/16] PM / OPP: Add 'supply-names' binding"
https://lore.kernel.org/lkml/2b87b162eabd1570ae2311e1ef8655acda72f678.1441972771.git.viresh.ku...@linaro.org/
but presumably that's an even further evolution?
Can you please document these properties?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH V3 11/16] cpufreq: dt: Pass regulator name to the OPP core
On 02/09, Viresh Kumar wrote: > OPP core can handle the regulators by itself, and but it needs to know > the name of the regulator to fetch. Add support for that. > > Signed-off-by: Viresh Kumar > --- Reviewed-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH V3 11/16] cpufreq: dt: Pass regulator name to the OPP core
OPP core can handle the regulators by itself, and but it needs to know
the name of the regulator to fetch. Add support for that.
Signed-off-by: Viresh Kumar
---
drivers/cpufreq/cpufreq-dt.c | 46
1 file changed, 46 insertions(+)
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 4c9f8a828f6f..2af75f8088bb 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -34,6 +34,7 @@ struct private_data {
struct regulator *cpu_reg;
struct thermal_cooling_device *cdev;
unsigned int voltage_tolerance; /* in percentage */
+ const char *reg_name;
};
static struct freq_attr *cpufreq_dt_attr[] = {
@@ -119,6 +120,30 @@ static int set_target(struct cpufreq_policy *policy,
unsigned int index)
return ret;
}
+/*
+ * An earlier version of opp-v1 bindings used to name the regulator
+ * "cpu0-supply", we still need to handle that for backwards compatibility.
+ */
+static const char *find_supply_name(struct device *dev, struct device_node *np)
+{
+ struct property *pp;
+ int cpu = dev->id;
+
+ /* Try "cpu0" for older DTs */
+ if (!cpu) {
+ pp = of_find_property(np, "cpu0-supply", NULL);
+ if (pp)
+ return "cpu0";
+ }
+
+ pp = of_find_property(np, "cpu-supply", NULL);
+ if (pp)
+ return "cpu";
+
+ dev_dbg(dev, "no regulator for cpu%d\n", cpu);
+ return NULL;
+}
+
static int allocate_resources(int cpu, struct device **cdev,
struct regulator **creg, struct clk **cclk)
{
@@ -200,6 +225,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
unsigned long min_uV = ~0, max_uV = 0;
unsigned int transition_latency;
bool opp_v1 = false;
+ const char *name;
int ret;
ret = allocate_resources(policy->cpu, &cpu_dev, &cpu_reg, &cpu_clk);
@@ -229,6 +255,20 @@ static int cpufreq_init(struct cpufreq_policy *policy)
}
/*
+* OPP layer will be taking care of regulators now, but it needs to know
+* the name of the regulator first.
+*/
+ name = find_supply_name(cpu_dev, np);
+ if (name) {
+ ret = dev_pm_opp_set_regulator(cpu_dev, name);
+ if (ret) {
+ dev_err(cpu_dev, "Failed to set regulator for cpu%d:
%d\n",
+ policy->cpu, ret);
+ goto out_node_put;
+ }
+ }
+
+ /*
* Initialize OPP tables for all policy->cpus. They will be shared by
* all CPUs which have marked their CPUs shared with OPP bindings.
*
@@ -273,6 +313,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
goto out_free_opp;
}
+ priv->reg_name = name;
of_property_read_u32(np, "voltage-tolerance", &priv->voltage_tolerance);
transition_latency = dev_pm_opp_get_max_clock_latency(cpu_dev);
@@ -366,6 +407,8 @@ static int cpufreq_init(struct cpufreq_policy *policy)
kfree(priv);
out_free_opp:
dev_pm_opp_of_cpumask_remove_table(policy->cpus);
+ if (name)
+ dev_pm_opp_put_regulator(cpu_dev);
out_node_put:
of_node_put(np);
out_put_reg_clk:
@@ -383,6 +426,9 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
cpufreq_cooling_unregister(priv->cdev);
dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
dev_pm_opp_of_cpumask_remove_table(policy->related_cpus);
+ if (priv->reg_name)
+ dev_pm_opp_put_regulator(priv->cpu_dev);
+
clk_put(policy->clk);
if (!IS_ERR(priv->cpu_reg))
regulator_put(priv->cpu_reg);
--
2.7.1.370.gb2aa7f8
