Existing platforms, which do not support operating-points-v2, can
explicitly tell the opp core that some of the CPUs share opp tables,
with help of dev_pm_opp_set_sharing_cpus().
For such platforms, explicitly ask the opp core to provide list of CPUs
sharing the opp table with current cpu device, before falling back to
platform data.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/cpufreq/cpufreq-dt.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 5f8dbe640a20..aca9bec00f91 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -147,7 +147,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
struct clk *cpu_clk;
struct dev_pm_opp *suspend_opp;
unsigned int transition_latency;
- bool opp_v1 = false;
+ bool fallback = false;
const char *name;
int ret;
@@ -167,14 +167,16 @@ static int cpufreq_init(struct cpufreq_policy *policy)
/* Get OPP-sharing information from "operating-points-v2" bindings */
ret = dev_pm_opp_of_get_sharing_cpus(cpu_dev, policy->cpus);
if (ret) {
+ if (ret != -ENOENT)
+ goto out_put_clk;
+
/*
* operating-points-v2 not supported, fallback to old method of
- * finding shared-OPPs for backward compatibility.
+ * finding shared-OPPs for backward compatibility if the
+ * platform hasn't set sharing CPUs.
*/
- if (ret == -ENOENT)
- opp_v1 = true;
- else
- goto out_put_clk;
+ if (dev_pm_opp_get_sharing_cpus(cpu_dev, policy->cpus))
+ fallback = true;
}
/*
@@ -214,7 +216,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
goto out_free_opp;
}
- if (opp_v1) {
+ if (fallback) {
struct cpufreq_dt_platform_data *pd = cpufreq_get_driver_data();
if (!pd || !pd->independent_clocks)
--
2.7.1.410.g6faf27b
On 04/21, Viresh Kumar wrote:
> @@ -167,14 +167,16 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> /* Get OPP-sharing information from "operating-points-v2" bindings */
> ret = dev_pm_opp_of_get_sharing_cpus(cpu_dev, policy->cpus);
> if (ret) {
> + if (ret != -ENOENT)
> + goto out_put_clk;
> +
> /*
> * operating-points-v2 not supported, fallback to old method of
> - * finding shared-OPPs for backward compatibility.
> + * finding shared-OPPs for backward compatibility if the
> + * platform hasn't set sharing CPUs.
> */
> - if (ret == -ENOENT)
> - opp_v1 = true;
> - else
> - goto out_put_clk;
> + if (dev_pm_opp_get_sharing_cpus(cpu_dev, policy->cpus))
> + fallback = true;
I'm sort of lost, we make the same call twice here. Why would the
return value change between the first time and the second?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 22-04-16, 15:27, Stephen Boyd wrote:
> On 04/21, Viresh Kumar wrote:
> > @@ -167,14 +167,16 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> > /* Get OPP-sharing information from "operating-points-v2" bindings */
> > ret = dev_pm_opp_of_get_sharing_cpus(cpu_dev, policy->cpus);
> > if (ret) {
> > + if (ret != -ENOENT)
> > + goto out_put_clk;
> > +
> > /*
> > * operating-points-v2 not supported, fallback to old method of
> > - * finding shared-OPPs for backward compatibility.
> > + * finding shared-OPPs for backward compatibility if the
> > + * platform hasn't set sharing CPUs.
> > */
> > - if (ret == -ENOENT)
> > - opp_v1 = true;
> > - else
> > - goto out_put_clk;
> > + if (dev_pm_opp_get_sharing_cpus(cpu_dev, policy->cpus))
> > + fallback = true;
>
> I'm sort of lost, we make the same call twice here. Why would the
> return value change between the first time and the second?
Two different APIs, which look similar :)
The first one tries to find the sharing-cpus relation from DT, the
other one is for v1 bindings and finds it due to platform code
dev_pm_opp_set_sharing_cpus() call.
--
viresh
On 04/25, Viresh Kumar wrote:
> On 22-04-16, 15:27, Stephen Boyd wrote:
> > On 04/21, Viresh Kumar wrote:
> > > @@ -167,14 +167,16 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> > > /* Get OPP-sharing information from "operating-points-v2" bindings */
> > > ret = dev_pm_opp_of_get_sharing_cpus(cpu_dev, policy->cpus);
[..]
> > > + if (dev_pm_opp_get_sharing_cpus(cpu_dev, policy->cpus))
> > > + fallback = true;
> >
> > I'm sort of lost, we make the same call twice here. Why would the
> > return value change between the first time and the second?
>
> Two different APIs, which look similar :)
>
> The first one tries to find the sharing-cpus relation from DT, the
> other one is for v1 bindings and finds it due to platform code
> dev_pm_opp_set_sharing_cpus() call.
Ah thanks. My eyes glossed over the "of" part. Sounds fine.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Mon, Apr 25, 2016 at 11:45 PM, Stephen Boyd <[email protected]> wrote:
> On 04/25, Viresh Kumar wrote:
>> On 22-04-16, 15:27, Stephen Boyd wrote:
>> > On 04/21, Viresh Kumar wrote:
>> > > @@ -167,14 +167,16 @@ static int cpufreq_init(struct cpufreq_policy *policy)
>> > > /* Get OPP-sharing information from "operating-points-v2" bindings */
>> > > ret = dev_pm_opp_of_get_sharing_cpus(cpu_dev, policy->cpus);
> [..]
>> > > + if (dev_pm_opp_get_sharing_cpus(cpu_dev, policy->cpus))
>> > > + fallback = true;
>> >
>> > I'm sort of lost, we make the same call twice here. Why would the
>> > return value change between the first time and the second?
>>
>> Two different APIs, which look similar :)
>>
>> The first one tries to find the sharing-cpus relation from DT, the
>> other one is for v1 bindings and finds it due to platform code
>> dev_pm_opp_set_sharing_cpus() call.
>
> Ah thanks. My eyes glossed over the "of" part. Sounds fine.
So that would be an "ACK", right?
On 04/25, Rafael J. Wysocki wrote:
> On Mon, Apr 25, 2016 at 11:45 PM, Stephen Boyd <[email protected]> wrote:
> > On 04/25, Viresh Kumar wrote:
> >> On 22-04-16, 15:27, Stephen Boyd wrote:
> >> > On 04/21, Viresh Kumar wrote:
> >> > > @@ -167,14 +167,16 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> >> > > /* Get OPP-sharing information from "operating-points-v2" bindings */
> >> > > ret = dev_pm_opp_of_get_sharing_cpus(cpu_dev, policy->cpus);
> > [..]
> >> > > + if (dev_pm_opp_get_sharing_cpus(cpu_dev, policy->cpus))
> >> > > + fallback = true;
> >> >
> >> > I'm sort of lost, we make the same call twice here. Why would the
> >> > return value change between the first time and the second?
> >>
> >> Two different APIs, which look similar :)
> >>
> >> The first one tries to find the sharing-cpus relation from DT, the
> >> other one is for v1 bindings and finds it due to platform code
> >> dev_pm_opp_set_sharing_cpus() call.
> >
> > Ah thanks. My eyes glossed over the "of" part. Sounds fine.
>
> So that would be an "ACK", right?
Sure, I thought this was going for another round though.
I had to go back and re-read the patch once more, but you can
have my reviewed-by on this one too.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Monday, April 25, 2016 02:56:08 PM Stephen Boyd wrote:
> On 04/25, Rafael J. Wysocki wrote:
> > On Mon, Apr 25, 2016 at 11:45 PM, Stephen Boyd <[email protected]> wrote:
> > > On 04/25, Viresh Kumar wrote:
> > >> On 22-04-16, 15:27, Stephen Boyd wrote:
> > >> > On 04/21, Viresh Kumar wrote:
> > >> > > @@ -167,14 +167,16 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> > >> > > /* Get OPP-sharing information from "operating-points-v2" bindings */
> > >> > > ret = dev_pm_opp_of_get_sharing_cpus(cpu_dev, policy->cpus);
> > > [..]
> > >> > > + if (dev_pm_opp_get_sharing_cpus(cpu_dev, policy->cpus))
> > >> > > + fallback = true;
> > >> >
> > >> > I'm sort of lost, we make the same call twice here. Why would the
> > >> > return value change between the first time and the second?
> > >>
> > >> Two different APIs, which look similar :)
> > >>
> > >> The first one tries to find the sharing-cpus relation from DT, the
> > >> other one is for v1 bindings and finds it due to platform code
> > >> dev_pm_opp_set_sharing_cpus() call.
> > >
> > > Ah thanks. My eyes glossed over the "of" part. Sounds fine.
> >
> > So that would be an "ACK", right?
>
> Sure, I thought this was going for another round though.
OK
> I had to go back and re-read the patch once more, but you can
> have my reviewed-by on this one too.
Well, I'm still unsure what about the [6/10].
I have applied [1-5/10] for now and I'll be expecting updates or resends of
the rest.
Thanks,
Rafael