Messages in this thread | | | Date | Fri, 2 Feb 2018 16:12:59 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH 8/8] thermal/drivers/cpu_cooling: Add the combo cpu cooling device |
| |
Hi Daniel,
I have gone through the other review comments, specially from Daniel T.. While I share some of his concerns, I have few more of mine.
On 23-01-18, 16:34, Daniel Lezcano wrote: > +late_initcall(cpu_cooling_init);
For example, this thing isn't going to fly nicely as you have assumed cpufreq and cpuidle drivers are going to be part of the kernel itself. What if they are modules and are inserted after late-init ? There are more cases like this where the cpufreq driver may unregister the cpufreq cooling device on the fly and then add it back. And so this stuff is a bit tricky.
Here is how I see the whole thing now:
- Yes we need individual support for both cpufreq and cpuidle cooling devices, and no one disagrees on that I believe.
- There is nothing in the thermal framework that disallows both cpufreq and cpuidle cooling devices to co-exist. Both would be part of the same thermal zone and so will get throttled with the same thermal sensor event. And so we will end up trying to cool down the SoC using both cpufreq and cpuidle technique.
- Now I am just wondering if we really need the "combo" functionality or not. Can we fine tune the DT cpu-cooling properties (existing ones) for a platform, so that it automatically acts as a combo cooling device? I am not 100% sure its gonna fly, but just wanted to make sure its not possible to work around with and then only try the combo device thing.
For example, suppose that with just cpufreq-cooling device we need to take the CPU down to 1 GHz from 2 GHz if we cross temperature 'X'. What if we can change this policy from DT and say the cpufreq-cooling device goes to 1.5 GHz and cpuidle-cooling device takes us to idle for 'y' us, and the effect of combination of these two is >= the effect of the 1 GHz for just the cpufreq-cooling device.
Is there any possibility of this to work ?
-- viresh
| |