Re: [RFC PATCH v6 6/9] thermal: cpu_cooling: implement the power cooling device API

[Viresh Kumar]

On 8 December 2014 at 19:52, Javi Merino <javi.merino@xxxxxxx> wrote:
> Ok, changed it into:
>
>         cpu = cpumask_any(&cpufreq_device->allowed_cpus);
>         dev = get_cpu_device(cpu);
>         if (!dev) {
>                 dev_warn(&cpufreq_device->cool_dev->device,
>                         "No cpu device for cpu %d\n", cpu);
>                 ret = -EINVAL;
>                 goto unlock;
>         }
>
>         num_opps = dev_pm_opp_get_opp_count(dev);
>         if (num_opps <= 0) {
>                 ret = (num_opps < 0)? num_opps : -EINVAL;
>                 goto unlock;
>         }

And this might not work. This is what I said in the first reply.

So, a bit lengthy reply now :)

Every cpu has a device struct associated with it. When cpufreq
core initializes drivers, they ask for mapping (initializing) the opps.
At that point we pass policy->cpu to opp core. OPP core doesn't
know which cores share clock line (I am trying to solve that [1]) and
so it just initializes the OPPs for policy->cpu. Let us say it cpuX.

Now there will be few more CPUs which are going to share clock
line with it and hence will use the same OPPs. In thermal core,
you got clip_cpus which is exactly the masks of all these CPUs
sharing clock line.

If the OPP layer is good enough, then above code can work. But
because right now the OPPs are mapped to just cpuX, passing
any other cpu from clip_cpus will fail as it doesn't have any associated
OPPs.

Now what I asked you is to use the CPU for which
__cpufreq_cooling_register() is called. Normally we are calling
__cpufreq_cooling_register() for the CPU for which OPPs are
registered (but people might call it up for other CPUs as well)..

So, using that cpu *might* have worked here.

Now the earlier loop you used was good to get this information,
but it wasn't consistent and so I objected.

What you should do:

- Create another routine to find the cpu for which OPPs are bound
to
-  And save the cpu_dev for it in the global struct for cpu_cooling
- reuse it wherever required.

--
viresh

[1] https://www.marc.info/?t=141769172100002&r=1&w=2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/