diff mbox series

[RFC,1/2] dt-bindings: thermal: sprd: Add virtual thermal documentation

Message ID 1606466112-31584-1-git-send-email-gao.yunxiao6@gmail.com
State Changes Requested, archived
Headers show
Series [RFC,1/2] dt-bindings: thermal: sprd: Add virtual thermal documentation | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema fail build log

Commit Message

gao yunxiao Nov. 27, 2020, 8:35 a.m. UTC 
From: "jeson.gao" <jeson.gao@unisoc.com>

virtual thermal node definition description in dts file

Signed-off-by: jeson.gao <jeson.gao@unisoc.com>
---
 .../thermal/sprd-virtual-thermal.yaml         | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.yaml

Comments

Lukasz Luba Nov. 27, 2020, 9:27 a.m. UTC | #1 
On 11/27/20 8:35 AM, gao.yunxiao6@gmail.com wrote:
> From: "jeson.gao" <jeson.gao@unisoc.com>
> 
> virtual thermal node definition description in dts file
> 
> Signed-off-by: jeson.gao <jeson.gao@unisoc.com>
> ---
>   .../thermal/sprd-virtual-thermal.yaml         | 38 +++++++++++++++++++
>   1 file changed, 38 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.yaml
> 
> diff --git a/Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.yaml b/Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.yaml
> new file mode 100644
> index 000000000000..3e3d2282e2a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/sprd-virtual-thermal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Spreadtrum virtual thermal driver bindings
> +
> +maintainers:
> +  - Yunxiao Gao <gao.yunxiao6@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: sprd,virtual-thermal
> +
> +  reg:
> +    description: specify the virtual sensor id.
> +    maxItems: 1
> +
> +  thmzone-names:
> +    description: specify per-core thermal zone name.
> +
> +required:
> +  - compatible
> +  - reg
> +  - thmzone-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    virtual_sensor: virtual-sensor@1 {
> +      compatible = "sprd,virtual-thermal";
> +      reg = <1>;
> +      thmzone-names = "ank0-thmzone","ank1-thmzone","ank2-thmzone",
> +                      "ank3-thmzone","ank4-thmzone","ank5-thmzone","prometheus6-tzone0",
> +                      "prometheus6-tzone1","prometheus7-thmzone";
> +    };
> 

It's coming back. There were attempts to solve this problem.
Javi tried to solved this using hierarchical thermal zones [1].
It was even agreed (IIRC during LPC) but couldn't continue. Then Eduardo
was going to continue this (last message at [3]). Unfortunately, 
development stopped.

I also have out-of-tree similar implementation for my Odroid-xu4,
which does no have an 'SoC' sensor, but have CPU sensors and needs
some aggregation function to get temperature.

I can pick up Javi's patches and continue 'hierarchical thermal zones'
approach.

Javi, Daniel, Rui what do you think?

Regards,
Lukasz

[1] https://lwn.net/Articles/666015/
[2] 
https://patchwork.kernel.org/project/linux-pm/patch/1448464186-26289-2-git-send-email-javi.merino@arm.com/
[3] 
https://patchwork.kernel.org/project/linux-pm/patch/1448464186-26289-3-git-send-email-javi.merino@arm.com/
[4] 
https://patchwork.kernel.org/project/linux-pm/patch/1448464186-26289-4-git-send-email-javi.merino@arm.com/
[5] 
https://patchwork.kernel.org/project/linux-pm/patch/1448464186-26289-5-git-send-email-javi.merino@arm.com/
Daniel Lezcano Nov. 27, 2020, 1:26 p.m. UTC | #2 
Hi Lukasz,

On 27/11/2020 10:27, Lukasz Luba wrote:
> 
> 
> On 11/27/20 8:35 AM, gao.yunxiao6@gmail.com wrote:
>> From: "jeson.gao" <jeson.gao@unisoc.com>
>>
>> virtual thermal node definition description in dts file
>>
>> Signed-off-by: jeson.gao <jeson.gao@unisoc.com>
>> ---

[ ... ]

> It's coming back. There were attempts to solve this problem.
> Javi tried to solved this using hierarchical thermal zones [1].
> It was even agreed (IIRC during LPC) but couldn't continue. Then Eduardo
> was going to continue this (last message at [3]). Unfortunately,
> development stopped.
> 
> I also have out-of-tree similar implementation for my Odroid-xu4,
> which does no have an 'SoC' sensor, but have CPU sensors and needs
> some aggregation function to get temperature.
> 
> I can pick up Javi's patches and continue 'hierarchical thermal zones'
> approach.
> 
> Javi, Daniel, Rui what do you think?

I already worked on the hierarchical thermal zones and my opinion is
that fits not really well.

We want to define a new feature because the thermal framework is built
on the 1:1 relationship between a governor and a thermal zone.

Practically speaking, we want to mitigate two thermal zones from one
governor, especially here the IPA governor.

The DTPM framework is being implemented to solve that by providing an
automatic power rebalancing between the power manageable capable devices.

In our case, the IPA would stick on the 'sustainable-power' resulting on
the aggregation of the two performance domains and set the power limit
on the parent node. The automatic power rebalancing will ensure maximum
throughput between the two performance domains instead of capping the whole.
Lukasz Luba Nov. 27, 2020, 1:58 p.m. UTC | #3 
On 11/27/20 1:26 PM, Daniel Lezcano wrote:
> 
> Hi Lukasz,
> 
> On 27/11/2020 10:27, Lukasz Luba wrote:
>>
>>
>> On 11/27/20 8:35 AM, gao.yunxiao6@gmail.com wrote:
>>> From: "jeson.gao" <jeson.gao@unisoc.com>
>>>
>>> virtual thermal node definition description in dts file
>>>
>>> Signed-off-by: jeson.gao <jeson.gao@unisoc.com>
>>> ---
> 
> [ ... ]
> 
>> It's coming back. There were attempts to solve this problem.
>> Javi tried to solved this using hierarchical thermal zones [1].
>> It was even agreed (IIRC during LPC) but couldn't continue. Then Eduardo
>> was going to continue this (last message at [3]). Unfortunately,
>> development stopped.
>>
>> I also have out-of-tree similar implementation for my Odroid-xu4,
>> which does no have an 'SoC' sensor, but have CPU sensors and needs
>> some aggregation function to get temperature.
>>
>> I can pick up Javi's patches and continue 'hierarchical thermal zones'
>> approach.
>>
>> Javi, Daniel, Rui what do you think?
> 
> I already worked on the hierarchical thermal zones and my opinion is
> that fits not really well.
> 
> We want to define a new feature because the thermal framework is built
> on the 1:1 relationship between a governor and a thermal zone.
> 
> Practically speaking, we want to mitigate two thermal zones from one
> governor, especially here the IPA governor.
> 
> The DTPM framework is being implemented to solve that by providing an
> automatic power rebalancing between the power manageable capable devices.
> 
> In our case, the IPA would stick on the 'sustainable-power' resulting on
> the aggregation of the two performance domains and set the power limit
> on the parent node. The automatic power rebalancing will ensure maximum
> throughput between the two performance domains instead of capping the whole.
> 
> 

Make sense. Thank you for sharing valuable opinion.

Regards,
Lukasz
gao yunxiao Nov. 30, 2020, 9:03 a.m. UTC | #4 
Hi Daniel

Thank you for your the new information

I have a question trouble to you
We should choose which per-core thermal zone as the IPA's input
reference temperature in the current kernel version? thank you.

On 27/11/2020, Lukasz Luba <lukasz.luba@arm.com> wrote:
>
>
> On 11/27/20 1:26 PM, Daniel Lezcano wrote:
>>
>> Hi Lukasz,
>>
>> On 27/11/2020 10:27, Lukasz Luba wrote:
>>>
>>>
>>> On 11/27/20 8:35 AM, gao.yunxiao6@gmail.com wrote:
>>>> From: "jeson.gao" <jeson.gao@unisoc.com>
>>>>
>>>> virtual thermal node definition description in dts file
>>>>
>>>> Signed-off-by: jeson.gao <jeson.gao@unisoc.com>
>>>> ---
>>
>> [ ... ]
>>
>>> It's coming back. There were attempts to solve this problem.
>>> Javi tried to solved this using hierarchical thermal zones [1].
>>> It was even agreed (IIRC during LPC) but couldn't continue. Then Eduardo
>>> was going to continue this (last message at [3]). Unfortunately,
>>> development stopped.
>>>
>>> I also have out-of-tree similar implementation for my Odroid-xu4,
>>> which does no have an 'SoC' sensor, but have CPU sensors and needs
>>> some aggregation function to get temperature.
>>>
>>> I can pick up Javi's patches and continue 'hierarchical thermal zones'
>>> approach.
>>>
>>> Javi, Daniel, Rui what do you think?
>>
>> I already worked on the hierarchical thermal zones and my opinion is
>> that fits not really well.
>>
>> We want to define a new feature because the thermal framework is built
>> on the 1:1 relationship between a governor and a thermal zone.
>>
>> Practically speaking, we want to mitigate two thermal zones from one
>> governor, especially here the IPA governor.
>>
>> The DTPM framework is being implemented to solve that by providing an
>> automatic power rebalancing between the power manageable capable devices.
>>
>> In our case, the IPA would stick on the 'sustainable-power' resulting on
>> the aggregation of the two performance domains and set the power limit
>> on the parent node. The automatic power rebalancing will ensure maximum
>> throughput between the two performance domains instead of capping the
>> whole.
>>
>>
>
> Make sense. Thank you for sharing valuable opinion.
>
> Regards,
> Lukasz
>
Daniel Lezcano Nov. 30, 2020, 9:40 a.m. UTC | #5 
On 30/11/2020 10:03, gao yunxiao wrote:
> Hi Daniel
> 
> Thank you for your the new information
> 
> I have a question trouble to you
> We should choose which per-core thermal zone as the IPA's input
> reference temperature in the current kernel version? thank you.

Can you give a pointer to a DT describing your hardware ?



> On 27/11/2020, Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>>
>> On 11/27/20 1:26 PM, Daniel Lezcano wrote:
>>>
>>> Hi Lukasz,
>>>
>>> On 27/11/2020 10:27, Lukasz Luba wrote:
>>>>
>>>>
>>>> On 11/27/20 8:35 AM, gao.yunxiao6@gmail.com wrote:
>>>>> From: "jeson.gao" <jeson.gao@unisoc.com>
>>>>>
>>>>> virtual thermal node definition description in dts file
>>>>>
>>>>> Signed-off-by: jeson.gao <jeson.gao@unisoc.com>
>>>>> ---
>>>
>>> [ ... ]
>>>
>>>> It's coming back. There were attempts to solve this problem.
>>>> Javi tried to solved this using hierarchical thermal zones [1].
>>>> It was even agreed (IIRC during LPC) but couldn't continue. Then Eduardo
>>>> was going to continue this (last message at [3]). Unfortunately,
>>>> development stopped.
>>>>
>>>> I also have out-of-tree similar implementation for my Odroid-xu4,
>>>> which does no have an 'SoC' sensor, but have CPU sensors and needs
>>>> some aggregation function to get temperature.
>>>>
>>>> I can pick up Javi's patches and continue 'hierarchical thermal zones'
>>>> approach.
>>>>
>>>> Javi, Daniel, Rui what do you think?
>>>
>>> I already worked on the hierarchical thermal zones and my opinion is
>>> that fits not really well.
>>>
>>> We want to define a new feature because the thermal framework is built
>>> on the 1:1 relationship between a governor and a thermal zone.
>>>
>>> Practically speaking, we want to mitigate two thermal zones from one
>>> governor, especially here the IPA governor.
>>>
>>> The DTPM framework is being implemented to solve that by providing an
>>> automatic power rebalancing between the power manageable capable devices.
>>>
>>> In our case, the IPA would stick on the 'sustainable-power' resulting on
>>> the aggregation of the two performance domains and set the power limit
>>> on the parent node. The automatic power rebalancing will ensure maximum
>>> throughput between the two performance domains instead of capping the
>>> whole.
>>>
>>>
>>
>> Make sense. Thank you for sharing valuable opinion.
>>
>> Regards,
>> Lukasz
>>
gao yunxiao Nov. 30, 2020, 10:57 a.m. UTC | #6 
Hi Daniel

Defined per-core thermal zone in DTS file as the following. thanks

prometheus6_tzone0: prometheus6-tzone0 {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm0 0>;
};

prometheus6_tzone1: prometheus6-tzone1 {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm0 1>;
};

prometheus7_thmzone: prometheus7-thmzone {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm0 2>;
};

ank0_thmzone: ank0-thmzone {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm0 3>;
};

ank1_thmzone: ank1-thmzone {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm0 4>;
};

gpu_thmzone: gpu-thmzone {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm1 0>;
};

ank2_thmzone: ank2-thmzone {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm1 1>;
};

ank3_thmzone: ank3-thmzone {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm1 2>;
};

ank4_thmzone: ank4-thmzone {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm1 3>;
};

ank5_thmzone: ank5-thmzone {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm1 4>;
};

cputop_thmzone: cputop-thmzone {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm1 5>;
};

gpuank2_thmzone: gpuank2-thmzone {
			polling-delay-passive = <0>;
			polling-delay = <0>;
			thermal-sensors = <&ap_thm2 0>;
};

On 30/11/2020, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> On 30/11/2020 10:03, gao yunxiao wrote:
>> Hi Daniel
>>
>> Thank you for your the new information
>>
>> I have a question trouble to you
>> We should choose which per-core thermal zone as the IPA's input
>> reference temperature in the current kernel version? thank you.
>
> Can you give a pointer to a DT describing your hardware ?
>
>
>
>> On 27/11/2020, Lukasz Luba <lukasz.luba@arm.com> wrote:
>>>
>>>
>>> On 11/27/20 1:26 PM, Daniel Lezcano wrote:
>>>>
>>>> Hi Lukasz,
>>>>
>>>> On 27/11/2020 10:27, Lukasz Luba wrote:
>>>>>
>>>>>
>>>>> On 11/27/20 8:35 AM, gao.yunxiao6@gmail.com wrote:
>>>>>> From: "jeson.gao" <jeson.gao@unisoc.com>
>>>>>>
>>>>>> virtual thermal node definition description in dts file
>>>>>>
>>>>>> Signed-off-by: jeson.gao <jeson.gao@unisoc.com>
>>>>>> ---
>>>>
>>>> [ ... ]
>>>>
>>>>> It's coming back. There were attempts to solve this problem.
>>>>> Javi tried to solved this using hierarchical thermal zones [1].
>>>>> It was even agreed (IIRC during LPC) but couldn't continue. Then
>>>>> Eduardo
>>>>> was going to continue this (last message at [3]). Unfortunately,
>>>>> development stopped.
>>>>>
>>>>> I also have out-of-tree similar implementation for my Odroid-xu4,
>>>>> which does no have an 'SoC' sensor, but have CPU sensors and needs
>>>>> some aggregation function to get temperature.
>>>>>
>>>>> I can pick up Javi's patches and continue 'hierarchical thermal zones'
>>>>> approach.
>>>>>
>>>>> Javi, Daniel, Rui what do you think?
>>>>
>>>> I already worked on the hierarchical thermal zones and my opinion is
>>>> that fits not really well.
>>>>
>>>> We want to define a new feature because the thermal framework is built
>>>> on the 1:1 relationship between a governor and a thermal zone.
>>>>
>>>> Practically speaking, we want to mitigate two thermal zones from one
>>>> governor, especially here the IPA governor.
>>>>
>>>> The DTPM framework is being implemented to solve that by providing an
>>>> automatic power rebalancing between the power manageable capable
>>>> devices.
>>>>
>>>> In our case, the IPA would stick on the 'sustainable-power' resulting
>>>> on
>>>> the aggregation of the two performance domains and set the power limit
>>>> on the parent node. The automatic power rebalancing will ensure maximum
>>>> throughput between the two performance domains instead of capping the
>>>> whole.
>>>>
>>>>
>>>
>>> Make sense. Thank you for sharing valuable opinion.
>>>
>>> Regards,
>>> Lukasz
>>>
>
>
> --
> <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
>
Rob Herring Nov. 30, 2020, 5:36 p.m. UTC | #7 
On Fri, 27 Nov 2020 16:35:12 +0800, gao.yunxiao6@gmail.com wrote:
> From: "jeson.gao" <jeson.gao@unisoc.com>
> 
> virtual thermal node definition description in dts file
> 
> Signed-off-by: jeson.gao <jeson.gao@unisoc.com>
> ---
>  .../thermal/sprd-virtual-thermal.yaml         | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.example.dts:21.11-21: Warning (reg_format): /example-0/virtual-sensor@1:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.example.dt.yaml: example-0: virtual-sensor@1:reg:0: [1] is too short
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml


See https://patchwork.ozlabs.org/patch/1407041

The base for the patch is generally the last rc1. Any dependencies
should be noted.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.yaml b/Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.yaml
new file mode 100644
index 000000000000..3e3d2282e2a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/sprd-virtual-thermal.yaml
@@ -0,0 +1,38 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/sprd-virtual-thermal.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Spreadtrum virtual thermal driver bindings
+
+maintainers:
+  - Yunxiao Gao <gao.yunxiao6@gmail.com>
+
+properties:
+  compatible:
+    const: sprd,virtual-thermal
+
+  reg:
+    description: specify the virtual sensor id.
+    maxItems: 1
+
+  thmzone-names:
+    description: specify per-core thermal zone name.
+
+required:
+  - compatible
+  - reg
+  - thmzone-names
+
+additionalProperties: false
+
+examples:
+  - |
+    virtual_sensor: virtual-sensor@1 {
+      compatible = "sprd,virtual-thermal";
+      reg = <1>;
+      thmzone-names = "ank0-thmzone","ank1-thmzone","ank2-thmzone",
+                      "ank3-thmzone","ank4-thmzone","ank5-thmzone","prometheus6-tzone0",
+                      "prometheus6-tzone1","prometheus7-thmzone";
+    };