Hi Rob,
a gentle ping on the questions below.
On 13/01/2020 18:52, Daniel Lezcano wrote:
> On 13/01/2020 17:16, Rob Herring wrote:
>> On Sat, Jan 11, 2020 at 11:32 AM Daniel Lezcano
>> <daniel.lezcano@xxxxxxxxxx> wrote:
>>>
>>> Hi Rob,
>>>
>>>
>>> On Wed, 8 Jan 2020 at 15:03, Rob Herring <robh@xxxxxxxxxx> wrote:
>>>>
>>>> On Thu, Dec 19, 2019 at 11:19:27PM +0100, Daniel Lezcano wrote:
>>>>> Add DT documentation to add an idle state as a cooling device. The CPU
>>>>> is actually the cooling device but the definition is already used by
>>>>> frequency capping. As we need to make cpufreq capping and idle
>>>>> injection to co-exist together on the system in order to mitigate at
>>>>> different trip points, the CPU can not be used as the cooling device
>>>>> for idle injection. The idle state can be seen as an hardware feature
>>>>> and therefore as a component for the passive mitigation.
>>>>>
>>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
>>>>> ---
>>>>> Documentation/devicetree/bindings/arm/idle-states.txt | 11 +++++++++++
>>>>> 1 file changed, 11 insertions(+)
>>>>
>>>> This is now a schema in my tree. Can you rebase on that and I'll pick up
>>>> the binding change.
>>>
>>> Mmh, I'm now having some doubts about this binding because it will
>>> restrict any improvement of the cooling device for the future.
>>>
>>> It looks like adding a node to the CPU for the cooling device is more
>>> adequate.
>>> eg:
>>> CPU0: cpu@300 {
>>> device_type = "cpu";
>>> compatible = "arm,cortex-a9";
>>> reg = <0x300>;
>>> /* cpufreq controls */
>>> operating-points = <998400 0
>>> 800000 0
>>> 400000 0
>>> 200000 0>;
>>> clocks = <&prcmu_clk PRCMU_ARMSS>;
>>> clock-names = "cpu";
>>> clock-latency = <20000>;
>>> #cooling-cells = <2>;
>>> thermal-idle {
>>> #cooling-cells = <2>;
>>> };
>>> };
>>>
>>> [ ... ]
>>>
>>> cooling-device = <&{/cpus/cpu@300/thermal-idle}
>>> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>>
>>> A quick test with different configurations combination shows it is much
>>> more flexible and it is open for future changes.
>>>
>>> What do you think?
>>
>> Why do you need #cooling-cells in both cpu node and a child node?
>
> The cooling-cells in the CPU node is for the cpufreq cooling device and
> the one in the thermal-idle is for the idle cooling device. The first
> one is for backward compatibility. If no cpufreq cooling device exists
> then the first cooling-cells is not needed. May be we can define
> "thermal-dvfs" at the same time, so we do the change for both and
> prevent mixing the old and new bindings?
>
>> It's really only 1 device.
>
> The main problem is how the thermal framework is designed. When we
> register a cooling device we pass the node pointer and the core
> framework checks it has a #cooling-cells. Then cooling-maps must have a
> phandle to the node we registered before as a cooling device. This is
> when the thermal-zone <-> cooling device association is done.
>
> With the cpufreq cooling device, the "CPU slot" is now used and we can't
> point to it without ambiguity as we can have different cooling device
> strategies for the same CPU at different temperatures.
>
> Is it acceptable the following?
>
> CPU0: cpu@300 {
> [ ... ]
> thermal-idle {
> #cooling-cells = <2>;
> };
>
> thermal-dvfs {
> #cooling-cells = <2>;
> }
> };
>
> Or alternatively, can we define a passive-cooling node?
>
> thermal-cooling: passive0 {
> #cooling-cells = <2>;
> strategy="dvfs" | "idle"
> cooling-device=<&CPU0>
> };
>
> cooling-device = <&passive0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>
>> Maybe you could add another cell to contain an idle state node if that
> helps?
>
> (Assuming you are referring to a phandle to an idle state) The idle
> states are grouped per cluster because the CPUs belonging to the same
> cluster have the same idle states characteristics. Because of that, the
> phandle will point to the same node and it will be impossible to specify
> a per cpu cooling device, only per cluster.
>
>
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
