Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/3] Convert thermal bindings to yaml
@ 2020-03-05 12:56 Amit Kucheria
  2020-03-05 12:56 ` [PATCH v2 1/3] dt-bindings: thermal: Add yaml bindings for thermal sensors Amit Kucheria
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Amit Kucheria @ 2020-03-05 12:56 UTC (permalink / raw)
  To: linux-kernel, linux-arm-msm, swboyd, mka, daniel.lezcano,
	Amit Kucheria, Zhang Rui
  Cc: devicetree, linux-pm

Hi all,

Here is a series splitting up the thermal bindings into 3 separate bindings
in YAML, one each of the sensor, cooling-device and the thermal zones.
Since I was learning about YAML parsers while creating these bindings,
there are bound to be some issues.

Changes since v1:
- Addressed review comments from Rob
- Moved the license back to GPLv2, waiting for other authors to give
  permission to relicense to BSD-2-Clause as well
- Fixed up warnings thrown by dt_binding_check

I have to add that the bindings as they exist today, don't really follow
the "describe the hardware" model of devicetree. e.g. the entire
thermal-zone binding is a software abstraction to tie arbitrary,
board-specific trip points to cooling strategies. This doesn't fit well
into the model where the same SoC in two different form-factor devices e.g.
mobile and laptop, will have fairly different thermal profiles and might
benefit from different trip points and mitigation heuristics. I've started
some experiments with moving the thermal zone data to a board-specific
platform data that is used to initialise a "thermal zone driver".

In any case, if we ever move down that path, it'll probably end up being v2
of the binding, so this series is still relevant.

Please help review.

Regards,
Amit

Amit Kucheria (3):
  dt-bindings: thermal: Add yaml bindings for thermal sensors
  dt-bindings: thermal: Add yaml bindings for thermal cooling-devices
  dt-bindings: thermal: Add yaml bindings for thermal zones

 .../thermal/thermal-cooling-devices.yaml      | 114 ++++++
 .../bindings/thermal/thermal-sensor.yaml      |  72 ++++
 .../bindings/thermal/thermal-zones.yaml       | 325 ++++++++++++++++++
 3 files changed, 511 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal-sensor.yaml
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal-zones.yaml

-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/3] dt-bindings: thermal: Add yaml bindings for thermal sensors
  2020-03-05 12:56 [PATCH v2 0/3] Convert thermal bindings to yaml Amit Kucheria
@ 2020-03-05 12:56 ` Amit Kucheria
  2020-03-11  8:26   ` Vincent Guittot
  2020-03-05 12:56 ` [PATCH v2 2/3] dt-bindings: thermal: Add yaml bindings for thermal cooling-devices Amit Kucheria
  2020-03-05 12:56 ` [PATCH v2 3/3] dt-bindings: thermal: Add yaml bindings for thermal zones Amit Kucheria
  2 siblings, 1 reply; 11+ messages in thread
From: Amit Kucheria @ 2020-03-05 12:56 UTC (permalink / raw)
  To: linux-kernel, linux-arm-msm, swboyd, mka, daniel.lezcano,
	Amit Kucheria, Zhang Rui
  Cc: linux-pm, devicetree

As part of moving the thermal bindings to YAML, split it up into 3
bindings: thermal sensors, cooling devices and thermal zones.

The property #thermal-sensor-cells is required in each device that acts
as a thermal sensor. It is used to uniquely identify the instance of the
thermal sensor inside the system.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 .../bindings/thermal/thermal-sensor.yaml      | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal-sensor.yaml

diff --git a/Documentation/devicetree/bindings/thermal/thermal-sensor.yaml b/Documentation/devicetree/bindings/thermal/thermal-sensor.yaml
new file mode 100644
index 0000000000000..920ee7667591d
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/thermal-sensor.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0)
+# Copyright 2020 Linaro Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/thermal-sensor.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Thermal sensor binding
+
+maintainers:
+  - Amit Kucheria <amitk@kernel.org>
+
+description: |
+  Thermal management is achieved in devicetree by describing the sensor hardware
+  and the software abstraction of thermal zones required to take appropriate
+  action to mitigate thermal overloads.
+
+  The following node types are used to completely describe a thermal management
+  system in devicetree:
+   - thermal-sensor: device that measures temperature, has SoC-specific bindings
+   - cooling-device: device used to dissipate heat either passively or artively
+   - thermal-zones: a container of the following node types used to describe all
+     thermal data for the platform
+
+  This binding describes the thermal-sensor.
+
+  Thermal sensor devices provide temperature sensing capabilities on thermal
+  zones. Typical devices are I2C ADC converters and bandgaps. Thermal sensor
+  devices may control one or more internal sensors.
+
+properties:
+  "#thermal-sensor-cells":
+    description:
+      Used to uniquely identify a thermal sensor instance within an IC. Will be
+      0 on sensor nodes with only a single sensor and at least 1 on nodes
+      containing several internal sensors.
+    enum: [0, 1]
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    // Example 1: SDM845 TSENS
+    soc: soc@0 {
+            #address-cells = <2>;
+            #size-cells = <2>;
+
+            /* ... */
+
+            tsens0: thermal-sensor@c263000 {
+                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
+                    reg = <0 0x0c263000 0 0x1ff>, /* TM */
+                          <0 0x0c222000 0 0x1ff>; /* SROT */
+                    #qcom,sensors = <13>;
+                    interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>,
+                                 <GIC_SPI 508 IRQ_TYPE_LEVEL_HIGH>;
+                    interrupt-names = "uplow", "critical";
+                    #thermal-sensor-cells = <1>;
+            };
+
+            tsens1: thermal-sensor@c265000 {
+                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
+                    reg = <0 0x0c265000 0 0x1ff>, /* TM */
+                          <0 0x0c223000 0 0x1ff>; /* SROT */
+                    #qcom,sensors = <8>;
+                    interrupts = <GIC_SPI 507 IRQ_TYPE_LEVEL_HIGH>,
+                                 <GIC_SPI 509 IRQ_TYPE_LEVEL_HIGH>;
+                    interrupt-names = "uplow", "critical";
+                    #thermal-sensor-cells = <1>;
+            };
+    };
+...
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 2/3] dt-bindings: thermal: Add yaml bindings for thermal cooling-devices
  2020-03-05 12:56 [PATCH v2 0/3] Convert thermal bindings to yaml Amit Kucheria
  2020-03-05 12:56 ` [PATCH v2 1/3] dt-bindings: thermal: Add yaml bindings for thermal sensors Amit Kucheria
@ 2020-03-05 12:56 ` Amit Kucheria
  2020-03-12 19:01   ` Rob Herring
  2020-03-05 12:56 ` [PATCH v2 3/3] dt-bindings: thermal: Add yaml bindings for thermal zones Amit Kucheria
  2 siblings, 1 reply; 11+ messages in thread
From: Amit Kucheria @ 2020-03-05 12:56 UTC (permalink / raw)
  To: linux-kernel, linux-arm-msm, swboyd, mka, daniel.lezcano,
	Amit Kucheria, Zhang Rui
  Cc: linux-pm, devicetree

As part of moving the thermal bindings to YAML, split it up into 3
bindings: thermal sensors, cooling devices and thermal zones.

The property #cooling-cells is required in each device that acts as a
cooling device - whether active or passive. So any device that can
throttle its performance to passively reduce heat dissipation (e.g.
cpus, gpus) and any device that can actively dissipate heat at different
levels (e.g. fans) will contain this property.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 .../thermal/thermal-cooling-devices.yaml      | 114 ++++++++++++++++++
 1 file changed, 114 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml

diff --git a/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml b/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
new file mode 100644
index 0000000000000..4745ea4b41ae7
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: (GPL-2.0)
+# Copyright 2020 Linaro Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/thermal-cooling-devices.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Thermal cooling device binding
+
+maintainers:
+  - Amit Kucheria <amitk@kernel.org>
+
+description: |
+  Thermal management is achieved in devicetree by describing the sensor hardware
+  and the software abstraction of cooling devices and thermal zones required to
+  take appropriate action to mitigate thermal overload.
+
+  The following node types are used to completely describe a thermal management
+  system in devicetree:
+   - thermal-sensor: device that measures temperature, has SoC-specific bindings
+   - cooling-device: device used to dissipate heat either passively or artively
+   - thermal-zones: a container of the following node types used to describe all
+     thermal data for the platform
+
+  This binding describes the cooling devices.
+
+  There are essentially two ways to provide control on power dissipation:
+    - Passive cooling: by means of regulating device performance. A typical
+      passive cooling mechanism is a CPU that has dynamic voltage and frequency
+      scaling (DVFS), and uses lower frequencies as cooling states.
+    - Active cooling: by means of activating devices in order to remove the
+      dissipated heat, e.g. regulating fan speeds.
+
+  Any cooling device has a range of cooling states (i.e. different levels of
+  heat dissipation). They also have a way to determine the state of cooling in
+  which the device is. For example, a fan's cooling states correspond to the
+  different fan speeds possible. Cooling states are referred to by single
+  unsigned integers, where larger numbers mean greater heat dissipation. The
+  precise set of cooling states associated with a device should be defined in
+  a particular device's binding.
+
+properties:
+  "#cooling-cells":
+    description:
+        Must be 2, in order to specify minimum and maximum cooling state used in
+        the cooling-maps reference. The first cell is the minimum cooling state
+        and the second cell is the maximum cooling state requested.
+    const: 2
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/thermal/thermal.h>
+
+    // Example 1: Cpufreq cooling device on CPU0
+    cpus {
+            #address-cells = <2>;
+            #size-cells = <0>;
+
+            CPU0: cpu@0 {
+                    device_type = "cpu";
+                    compatible = "qcom,kryo385";
+                    reg = <0x0 0x0>;
+                    enable-method = "psci";
+                    cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+                                       &LITTLE_CPU_SLEEP_1
+                                       &CLUSTER_SLEEP_0>;
+                    capacity-dmips-mhz = <607>;
+                    dynamic-power-coefficient = <100>;
+                    qcom,freq-domain = <&cpufreq_hw 0>;
+                    #cooling-cells = <2>;
+                    next-level-cache = <&L2_0>;
+                    L2_0: l2-cache {
+                            compatible = "cache";
+                            next-level-cache = <&L3_0>;
+                            L3_0: l3-cache {
+                                    compatible = "cache";
+                            };
+                    };
+          };
+
+          /* ... */
+
+    };
+
+    /* ... */
+
+    thermal-zones {
+            cpu0-thermal {
+                    polling-delay-passive = <250>;
+                    polling-delay = <1000>;
+
+                    thermal-sensors = <&tsens0 1>;
+
+                    trips {
+                            cpu0_alert0: trip-point0 {
+                                    temperature = <90000>;
+                                    hysteresis = <2000>;
+                                    type = "passive";
+                            };
+                    };
+
+                    cooling-maps {
+                            map0 {
+                                    trip = <&cpu0_alert0>;
+                                    cooling-device = <&CPU0 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>;
+                            };
+                    };
+            };
+
+            /* ... */
+    };
+...
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 3/3] dt-bindings: thermal: Add yaml bindings for thermal zones
  2020-03-05 12:56 [PATCH v2 0/3] Convert thermal bindings to yaml Amit Kucheria
  2020-03-05 12:56 ` [PATCH v2 1/3] dt-bindings: thermal: Add yaml bindings for thermal sensors Amit Kucheria
  2020-03-05 12:56 ` [PATCH v2 2/3] dt-bindings: thermal: Add yaml bindings for thermal cooling-devices Amit Kucheria
@ 2020-03-05 12:56 ` Amit Kucheria
  2020-03-11 14:49   ` Rob Herring
  2 siblings, 1 reply; 11+ messages in thread
From: Amit Kucheria @ 2020-03-05 12:56 UTC (permalink / raw)
  To: linux-kernel, linux-arm-msm, swboyd, mka, daniel.lezcano,
	Amit Kucheria, Zhang Rui
  Cc: linux-pm, devicetree

As part of moving the thermal bindings to YAML, split it up into 3
bindings: thermal sensors, cooling devices and thermal zones.

The thermal-zone binding is a software abstraction to capture the
properties of each zone - how often they should be checked, the
temperature thresholds (trips) at which mitigation actions need to be
taken and the level of mitigation needed at those thresholds.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 .../bindings/thermal/thermal-zones.yaml       | 325 ++++++++++++++++++
 1 file changed, 325 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal-zones.yaml

diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
new file mode 100644
index 0000000000000..f8f3b72bc3119
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
@@ -0,0 +1,325 @@
+# SPDX-License-Identifier: (GPL-2.0)
+# Copyright 2020 Linaro Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/thermal-zones.yaml#
+$schema: http://devicetree.org/meta-schemas/base.yaml#
+
+title: Thermal zone binding
+
+maintainers:
+  - Amit Kucheria <amitk@kernel.org>
+
+description: |
+  Thermal management is achieved in devicetree by describing the sensor hardware
+  and the software abstraction of cooling devices and thermal zones required to
+  take appropriate action to mitigate thermal overloads.
+
+  The following node types are used to completely describe a thermal management
+  system in devicetree:
+   - thermal-sensor: device that measures temperature, has SoC-specific bindings
+   - cooling-device: device used to dissipate heat either passively or actively
+   - thermal-zones: a container of the following node types used to describe all
+     thermal data for the platform
+
+  This binding describes the thermal-zones.
+
+  The polling-delay properties of a thermal-zone are bound to the maximum dT/dt
+  (temperature derivative over time) in two situations for a thermal zone:
+    1. when passive cooling is activated (polling-delay-passive)
+    2. when the zone just needs to be monitored (polling-delay) or when
+       active cooling is activated.
+
+  The maximum dT/dt is highly bound to hardware power consumption and
+  dissipation capability. The delays should be chosen to account for said
+  max dT/dt, such that a device does not cross several trip boundaries
+  unexpectedly between polls. Choosing the right polling delays shall avoid
+  having the device in temperature ranges that may damage the silicon structures
+  and reduce silicon lifetime.
+
+properties:
+  thermal-zones:
+    type: object
+    description:
+      A /thermal-zones node is required in order to use the thermal framework to
+      manage input from the various thermal zones in the system in order to
+      mitigate thermal overload conditions. It does not represent a real device
+      in the system, but acts as a container to link thermal sensor devices,
+      platform-data regarding temperature thresholds and the mitigation actions
+      to take when the temperature crosses those thresholds.
+
+    properties:
+      $nodename:
+        pattern: "^[a-zA-Z][a-zA-Z0-9,\\-]{1,12}-thermal$"
+        type: object
+        description:
+          Each thermal zone node contains information about how frequently it
+          must be checked, the sensor responsible for reporting temperature for
+          this zone, one sub-node containing the various trip points for this
+          zone and one sub-node containing all the zone cooling-maps.
+
+        properties:
+          polling-delay:
+            $ref: /schemas/types.yaml#/definitions/uint32
+            minimum: 0
+            description:
+              The maximum number of milliseconds to wait between polls when
+              checking this thermal zone. Setting this to 0 disables the polling
+              timers setup by the thermal framework and assumes that the thermal
+              sensors in this zone support interrupts.
+
+          polling-delay-passive:
+            $ref: /schemas/types.yaml#/definitions/uint32
+            minimum: 0
+            description:
+              The maximum number of milliseconds to wait between polls when
+              checking this thermal zone while doing passive cooling. Setting
+              this to 0 disables the polling timers setup by the thermal
+              framework and assumes that the thermal sensors in this zone
+              support interrupts.
+
+          thermal-sensors:
+            $ref: /schemas/types.yaml#/definitions/phandle-array
+            description:
+              A list of thermal sensor phandles and sensor specifiers used to
+              monitor this thermal zone.
+
+          trips:
+            type: object
+            description:
+              This node describes a set of points in the temperature domain at
+              which the thermal framework needs to takes action. The actions to
+              be taken are defined in another node called cooling-maps.
+
+            patternProperties:
+              "^[a-zA-Z][a-zA-Z0-9,+\\._]{0,63}$":
+                type: object
+
+                properties:
+                  temperature:
+                    $ref: /schemas/types.yaml#/definitions/int32
+                    minimum: -273000
+                    maximum: 200000
+                    description:
+                      An integer expressing the trip temperature in millicelsius.
+
+                  hysteresis:
+                    $ref: /schemas/types.yaml#/definitions/uint32
+                    description:
+                      An unsigned integer expressing the hysteresis delta with
+                      respect to the trip temperature property above, also in
+                      millicelsius.
+
+                  type:
+                    enum:
+                        # active: enable active cooling e.g. fans
+                        - active
+                        # passive: enable passive cooling e.g. throttling cpu
+                        - passive
+                        # hot: send notification to driver if .notify
+                        #      callback registered
+                        - hot
+                        # critical: send notification to driver if .notify
+                        #           callback registered and trigger a shutdown
+                        - critical
+                    description: |
+                      There are four valid trip types: active, passive, hot,
+                      critical.
+
+                      The critical trip type is used to set the maximum
+                      temperature threshold above which the HW becomes
+                      unstable and underlying firmware might even trigger a
+                      reboot. Hitting the critical threshold triggers a system
+                      shutdown.
+
+                      The hot trip type can be used to send a notification to
+                      the thermal driver (if a .notify callback is registered).
+                      The action to be taken is left to the driver.
+
+                      The passive trip type can be used to slow down HW e.g. run
+                      the CPU, GPU, bus at a lower frequency.
+
+                      The active trip type can be used to control other HW to
+                      help in cooling e.g. fans can be sped up or slowed down
+
+                required:
+                  - temperature
+                  - hysteresis
+                  - type
+
+            additionalProperties: false
+
+          cooling-maps:
+            type: object
+            description:
+              This node describes the action to be taken when a thermal zone
+              crosses one of the temperature thresholds described in the trips
+              node. The action takes the form of a mapping relation between a
+              trip and the target cooling device state.
+
+            patternProperties:
+              "^map[0-9][-a-zA-Z0-9]*$":
+                type: object
+
+                properties:
+                  trip:
+                    $ref: /schemas/types.yaml#/definitions/phandle
+                    description:
+                      A phandle of a trip point node within this thermal zone.
+
+                  cooling-device:
+                    $ref: /schemas/types.yaml#/definitions/phandle-array
+                    description:
+                      A list of cooling device phandles along with the minimum
+                      and maximum cooling state specifiers for each cooling
+                      device. Using the THERMAL_NO_LIMIT (-1UL) constant in the
+                      cooling-device phandle limit specifier lets the framework
+                      use the minimum and maximum cooling state for that cooling
+                      device automatically.
+
+                  contribution:
+                    $ref: /schemas/types.yaml#/definitions/uint32
+                    minimum: 0
+                    maximum: 100
+                    description:
+                      The contribution of the cooling devices at the trip
+                      temperature, both referenced in this map, to this thermal
+                      zone as a percentage.
+
+                required:
+                  - trip
+                  - cooling-device
+
+            additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/thermal/thermal.h>
+
+    // Example 1: SDM845 TSENS
+    soc: soc@0 {
+            #address-cells = <2>;
+            #size-cells = <2>;
+
+            /* ... */
+
+            tsens0: thermal-sensor@c263000 {
+                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
+                    reg = <0 0x0c263000 0 0x1ff>, /* TM */
+                          <0 0x0c222000 0 0x1ff>; /* SROT */
+                    #qcom,sensors = <13>;
+                    interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>,
+                                 <GIC_SPI 508 IRQ_TYPE_LEVEL_HIGH>;
+                    interrupt-names = "uplow", "critical";
+                    #thermal-sensor-cells = <1>;
+            };
+
+            tsens1: thermal-sensor@c265000 {
+                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
+                    reg = <0 0x0c265000 0 0x1ff>, /* TM */
+                          <0 0x0c223000 0 0x1ff>; /* SROT */
+                    #qcom,sensors = <8>;
+                    interrupts = <GIC_SPI 507 IRQ_TYPE_LEVEL_HIGH>,
+                                 <GIC_SPI 509 IRQ_TYPE_LEVEL_HIGH>;
+                    interrupt-names = "uplow", "critical";
+                    #thermal-sensor-cells = <1>;
+            };
+    };
+
+    /* ... */
+
+    thermal-zones {
+            cpu0-thermal {
+                    polling-delay-passive = <250>;
+                    polling-delay = <1000>;
+
+                    thermal-sensors = <&tsens0 1>;
+
+                    trips {
+                            cpu0_alert0: trip-point0 {
+                                    temperature = <90000>;
+                                    hysteresis = <2000>;
+                                    type = "passive";
+                            };
+
+                            cpu0_alert1: trip-point1 {
+                                    temperature = <95000>;
+                                    hysteresis = <2000>;
+                                    type = "passive";
+                            };
+
+                            cpu0_crit: cpu_crit {
+                                    temperature = <110000>;
+                                    hysteresis = <1000>;
+                                    type = "critical";
+                            };
+                    };
+
+                    cooling-maps {
+                            map0 {
+                                    trip = <&cpu0_alert0>;
+                                    cooling-device = <&CPU0 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU1 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU2 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU3 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>;
+                            };
+
+                            map1 {
+                                    trip = <&cpu0_alert1>;
+                                    cooling-device = <&CPU0 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU1 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU2 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU3 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>;
+                            };
+                    };
+            };
+
+            /* ... */
+
+            cluster0-thermal {
+                    polling-delay-passive = <250>;
+                    polling-delay = <1000>;
+
+                    thermal-sensors = <&tsens0 5>;
+
+                    trips {
+                            cluster0_alert0: trip-point0 {
+                                    temperature = <90000>;
+                                    hysteresis = <2000>;
+                                    type = "hot";
+                            };
+                            cluster0_crit: cluster0_crit {
+                                    temperature = <110000>;
+                                    hysteresis = <2000>;
+                                    type = "critical";
+                            };
+                    };
+            };
+
+            /* ... */
+
+            gpu-thermal-top {
+                    polling-delay-passive = <250>;
+                    polling-delay = <1000>;
+
+                    thermal-sensors = <&tsens0 11>;
+
+                    trips {
+                            gpu1_alert0: trip-point0 {
+                                    temperature = <90000>;
+                                    hysteresis = <2000>;
+                                    type = "hot";
+                            };
+                    };
+            };
+    };
+...
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: thermal: Add yaml bindings for thermal sensors
  2020-03-05 12:56 ` [PATCH v2 1/3] dt-bindings: thermal: Add yaml bindings for thermal sensors Amit Kucheria
@ 2020-03-11  8:26   ` Vincent Guittot
  0 siblings, 0 replies; 11+ messages in thread
From: Vincent Guittot @ 2020-03-11  8:26 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: linux-kernel, linux-arm-msm, Stephen Boyd, Matthias Kaehlcke,
	Daniel Lezcano, Amit Kucheria, Zhang Rui, open list:THERMAL,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Thu, 5 Mar 2020 at 13:56, Amit Kucheria <amit.kucheria@linaro.org> wrote:
>
> As part of moving the thermal bindings to YAML, split it up into 3
> bindings: thermal sensors, cooling devices and thermal zones.
>
> The property #thermal-sensor-cells is required in each device that acts
> as a thermal sensor. It is used to uniquely identify the instance of the
> thermal sensor inside the system.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
>  .../bindings/thermal/thermal-sensor.yaml      | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/thermal-sensor.yaml
>
> diff --git a/Documentation/devicetree/bindings/thermal/thermal-sensor.yaml b/Documentation/devicetree/bindings/thermal/thermal-sensor.yaml
> new file mode 100644
> index 0000000000000..920ee7667591d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/thermal-sensor.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0)
> +# Copyright 2020 Linaro Ltd.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/thermal-sensor.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Thermal sensor binding
> +
> +maintainers:
> +  - Amit Kucheria <amitk@kernel.org>
> +
> +description: |
> +  Thermal management is achieved in devicetree by describing the sensor hardware
> +  and the software abstraction of thermal zones required to take appropriate
> +  action to mitigate thermal overloads.
> +
> +  The following node types are used to completely describe a thermal management
> +  system in devicetree:
> +   - thermal-sensor: device that measures temperature, has SoC-specific bindings
> +   - cooling-device: device used to dissipate heat either passively or artively

typo: s/artively/actively/

> +   - thermal-zones: a container of the following node types used to describe all
> +     thermal data for the platform
> +
> +  This binding describes the thermal-sensor.
> +
> +  Thermal sensor devices provide temperature sensing capabilities on thermal
> +  zones. Typical devices are I2C ADC converters and bandgaps. Thermal sensor
> +  devices may control one or more internal sensors.
> +
> +properties:
> +  "#thermal-sensor-cells":
> +    description:
> +      Used to uniquely identify a thermal sensor instance within an IC. Will be
> +      0 on sensor nodes with only a single sensor and at least 1 on nodes
> +      containing several internal sensors.
> +    enum: [0, 1]
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    // Example 1: SDM845 TSENS
> +    soc: soc@0 {
> +            #address-cells = <2>;
> +            #size-cells = <2>;
> +
> +            /* ... */
> +
> +            tsens0: thermal-sensor@c263000 {
> +                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> +                    reg = <0 0x0c263000 0 0x1ff>, /* TM */
> +                          <0 0x0c222000 0 0x1ff>; /* SROT */
> +                    #qcom,sensors = <13>;
> +                    interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>,
> +                                 <GIC_SPI 508 IRQ_TYPE_LEVEL_HIGH>;
> +                    interrupt-names = "uplow", "critical";
> +                    #thermal-sensor-cells = <1>;
> +            };
> +
> +            tsens1: thermal-sensor@c265000 {
> +                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> +                    reg = <0 0x0c265000 0 0x1ff>, /* TM */
> +                          <0 0x0c223000 0 0x1ff>; /* SROT */
> +                    #qcom,sensors = <8>;
> +                    interrupts = <GIC_SPI 507 IRQ_TYPE_LEVEL_HIGH>,
> +                                 <GIC_SPI 509 IRQ_TYPE_LEVEL_HIGH>;
> +                    interrupt-names = "uplow", "critical";
> +                    #thermal-sensor-cells = <1>;
> +            };
> +    };
> +...
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] dt-bindings: thermal: Add yaml bindings for thermal zones
  2020-03-05 12:56 ` [PATCH v2 3/3] dt-bindings: thermal: Add yaml bindings for thermal zones Amit Kucheria
@ 2020-03-11 14:49   ` Rob Herring
  2020-03-23 20:46     ` Amit Kucheria
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2020-03-11 14:49 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: linux-kernel, linux-arm-msm, swboyd, mka, daniel.lezcano,
	Amit Kucheria, Zhang Rui, linux-pm, devicetree

On Thu, Mar 05, 2020 at 06:26:43PM +0530, Amit Kucheria wrote:
> As part of moving the thermal bindings to YAML, split it up into 3
> bindings: thermal sensors, cooling devices and thermal zones.
> 
> The thermal-zone binding is a software abstraction to capture the
> properties of each zone - how often they should be checked, the
> temperature thresholds (trips) at which mitigation actions need to be
> taken and the level of mitigation needed at those thresholds.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
>  .../bindings/thermal/thermal-zones.yaml       | 325 ++++++++++++++++++
>  1 file changed, 325 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> 
> diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> new file mode 100644
> index 0000000000000..f8f3b72bc3119
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> @@ -0,0 +1,325 @@
> +# SPDX-License-Identifier: (GPL-2.0)
> +# Copyright 2020 Linaro Ltd.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/thermal-zones.yaml#
> +$schema: http://devicetree.org/meta-schemas/base.yaml#
> +
> +title: Thermal zone binding
> +
> +maintainers:
> +  - Amit Kucheria <amitk@kernel.org>
> +
> +description: |
> +  Thermal management is achieved in devicetree by describing the sensor hardware
> +  and the software abstraction of cooling devices and thermal zones required to
> +  take appropriate action to mitigate thermal overloads.
> +
> +  The following node types are used to completely describe a thermal management
> +  system in devicetree:
> +   - thermal-sensor: device that measures temperature, has SoC-specific bindings
> +   - cooling-device: device used to dissipate heat either passively or actively
> +   - thermal-zones: a container of the following node types used to describe all
> +     thermal data for the platform
> +
> +  This binding describes the thermal-zones.
> +
> +  The polling-delay properties of a thermal-zone are bound to the maximum dT/dt
> +  (temperature derivative over time) in two situations for a thermal zone:
> +    1. when passive cooling is activated (polling-delay-passive)
> +    2. when the zone just needs to be monitored (polling-delay) or when
> +       active cooling is activated.
> +
> +  The maximum dT/dt is highly bound to hardware power consumption and
> +  dissipation capability. The delays should be chosen to account for said
> +  max dT/dt, such that a device does not cross several trip boundaries
> +  unexpectedly between polls. Choosing the right polling delays shall avoid
> +  having the device in temperature ranges that may damage the silicon structures
> +  and reduce silicon lifetime.
> +
> +properties:
> +  thermal-zones:
> +    type: object
> +    description:
> +      A /thermal-zones node is required in order to use the thermal framework to
> +      manage input from the various thermal zones in the system in order to
> +      mitigate thermal overload conditions. It does not represent a real device
> +      in the system, but acts as a container to link thermal sensor devices,
> +      platform-data regarding temperature thresholds and the mitigation actions
> +      to take when the temperature crosses those thresholds.
> +

> +    properties:
> +      $nodename:
> +        pattern: "^[a-zA-Z][a-zA-Z0-9,\\-]{1,12}-thermal$"

These 3 lines should be:

patternProperties:
  "^[a-zA-Z][a-zA-Z0-9,\\-]{1,12}-thermal$":

Though we should drop ',' as well.

$nodename is only needed at the top level where it's kind of special.

> +        type: object
> +        description:
> +          Each thermal zone node contains information about how frequently it
> +          must be checked, the sensor responsible for reporting temperature for
> +          this zone, one sub-node containing the various trip points for this
> +          zone and one sub-node containing all the zone cooling-maps.
> +
> +        properties:
> +          polling-delay:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            minimum: 0

The type is unsigned, so the min is already 0.

> +            description:
> +              The maximum number of milliseconds to wait between polls when
> +              checking this thermal zone. Setting this to 0 disables the polling
> +              timers setup by the thermal framework and assumes that the thermal
> +              sensors in this zone support interrupts.
> +
> +          polling-delay-passive:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            minimum: 0
> +            description:
> +              The maximum number of milliseconds to wait between polls when
> +              checking this thermal zone while doing passive cooling. Setting
> +              this to 0 disables the polling timers setup by the thermal
> +              framework and assumes that the thermal sensors in this zone
> +              support interrupts.
> +
> +          thermal-sensors:
> +            $ref: /schemas/types.yaml#/definitions/phandle-array
> +            description:
> +              A list of thermal sensor phandles and sensor specifiers used to
> +              monitor this thermal zone.
> +
> +          trips:
> +            type: object
> +            description:
> +              This node describes a set of points in the temperature domain at
> +              which the thermal framework needs to takes action. The actions to
> +              be taken are defined in another node called cooling-maps.
> +
> +            patternProperties:
> +              "^[a-zA-Z][a-zA-Z0-9,+\\._]{0,63}$":

Drop ',', '+', '.', and ideally '_'. Probably need to add '-'.

> +                type: object
> +
> +                properties:
> +                  temperature:
> +                    $ref: /schemas/types.yaml#/definitions/int32
> +                    minimum: -273000
> +                    maximum: 200000
> +                    description:
> +                      An integer expressing the trip temperature in millicelsius.
> +
> +                  hysteresis:
> +                    $ref: /schemas/types.yaml#/definitions/uint32
> +                    description:
> +                      An unsigned integer expressing the hysteresis delta with
> +                      respect to the trip temperature property above, also in
> +                      millicelsius.
> +
> +                  type:

Needs a type reference (string).

> +                    enum:
> +                        # active: enable active cooling e.g. fans
> +                        - active

Wrong indentation. Should be 2 less.

Also, I think this would be more readable:

- active  # enable active cooling e.g. fans
  

> +                        # passive: enable passive cooling e.g. throttling cpu
> +                        - passive
> +                        # hot: send notification to driver if .notify
> +                        #      callback registered
> +                        - hot
> +                        # critical: send notification to driver if .notify
> +                        #           callback registered and trigger a shutdown
> +                        - critical
> +                    description: |
> +                      There are four valid trip types: active, passive, hot,
> +                      critical.
> +
> +                      The critical trip type is used to set the maximum
> +                      temperature threshold above which the HW becomes
> +                      unstable and underlying firmware might even trigger a
> +                      reboot. Hitting the critical threshold triggers a system
> +                      shutdown.
> +
> +                      The hot trip type can be used to send a notification to
> +                      the thermal driver (if a .notify callback is registered).
> +                      The action to be taken is left to the driver.
> +
> +                      The passive trip type can be used to slow down HW e.g. run
> +                      the CPU, GPU, bus at a lower frequency.
> +
> +                      The active trip type can be used to control other HW to
> +                      help in cooling e.g. fans can be sped up or slowed down
> +
> +                required:
> +                  - temperature
> +                  - hysteresis
> +                  - type
> +
> +            additionalProperties: false

You need 'additionalProperties' on the child nodes too, or are there 
vendor specific properties allowed?

> +
> +          cooling-maps:
> +            type: object
> +            description:
> +              This node describes the action to be taken when a thermal zone
> +              crosses one of the temperature thresholds described in the trips
> +              node. The action takes the form of a mapping relation between a
> +              trip and the target cooling device state.
> +
> +            patternProperties:
> +              "^map[0-9][-a-zA-Z0-9]*$":
> +                type: object
> +
> +                properties:
> +                  trip:
> +                    $ref: /schemas/types.yaml#/definitions/phandle
> +                    description:
> +                      A phandle of a trip point node within this thermal zone.
> +
> +                  cooling-device:
> +                    $ref: /schemas/types.yaml#/definitions/phandle-array
> +                    description:
> +                      A list of cooling device phandles along with the minimum
> +                      and maximum cooling state specifiers for each cooling
> +                      device. Using the THERMAL_NO_LIMIT (-1UL) constant in the
> +                      cooling-device phandle limit specifier lets the framework
> +                      use the minimum and maximum cooling state for that cooling
> +                      device automatically.
> +
> +                  contribution:
> +                    $ref: /schemas/types.yaml#/definitions/uint32
> +                    minimum: 0
> +                    maximum: 100
> +                    description:
> +                      The contribution of the cooling devices at the trip
> +                      temperature, both referenced in this map, to this thermal
> +                      zone as a percentage.
> +
> +                required:
> +                  - trip
> +                  - cooling-device
> +

'additionalProperties' for map* nodes?

> +            additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/thermal/thermal.h>
> +
> +    // Example 1: SDM845 TSENS
> +    soc: soc@0 {
> +            #address-cells = <2>;
> +            #size-cells = <2>;
> +
> +            /* ... */
> +
> +            tsens0: thermal-sensor@c263000 {
> +                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> +                    reg = <0 0x0c263000 0 0x1ff>, /* TM */
> +                          <0 0x0c222000 0 0x1ff>; /* SROT */
> +                    #qcom,sensors = <13>;
> +                    interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>,
> +                                 <GIC_SPI 508 IRQ_TYPE_LEVEL_HIGH>;
> +                    interrupt-names = "uplow", "critical";
> +                    #thermal-sensor-cells = <1>;
> +            };
> +
> +            tsens1: thermal-sensor@c265000 {
> +                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> +                    reg = <0 0x0c265000 0 0x1ff>, /* TM */
> +                          <0 0x0c223000 0 0x1ff>; /* SROT */
> +                    #qcom,sensors = <8>;
> +                    interrupts = <GIC_SPI 507 IRQ_TYPE_LEVEL_HIGH>,
> +                                 <GIC_SPI 509 IRQ_TYPE_LEVEL_HIGH>;
> +                    interrupt-names = "uplow", "critical";
> +                    #thermal-sensor-cells = <1>;
> +            };
> +    };
> +
> +    /* ... */
> +
> +    thermal-zones {
> +            cpu0-thermal {
> +                    polling-delay-passive = <250>;
> +                    polling-delay = <1000>;
> +
> +                    thermal-sensors = <&tsens0 1>;
> +
> +                    trips {
> +                            cpu0_alert0: trip-point0 {
> +                                    temperature = <90000>;
> +                                    hysteresis = <2000>;
> +                                    type = "passive";
> +                            };
> +
> +                            cpu0_alert1: trip-point1 {
> +                                    temperature = <95000>;
> +                                    hysteresis = <2000>;
> +                                    type = "passive";
> +                            };
> +
> +                            cpu0_crit: cpu_crit {
> +                                    temperature = <110000>;
> +                                    hysteresis = <1000>;
> +                                    type = "critical";
> +                            };
> +                    };
> +
> +                    cooling-maps {
> +                            map0 {
> +                                    trip = <&cpu0_alert0>;
> +                                    cooling-device = <&CPU0 THERMAL_NO_LIMIT
> +                                                            THERMAL_NO_LIMIT>,
> +                                                     <&CPU1 THERMAL_NO_LIMIT
> +                                                            THERMAL_NO_LIMIT>,
> +                                                     <&CPU2 THERMAL_NO_LIMIT
> +                                                            THERMAL_NO_LIMIT>,
> +                                                     <&CPU3 THERMAL_NO_LIMIT
> +                                                            THERMAL_NO_LIMIT>;
> +                            };
> +
> +                            map1 {
> +                                    trip = <&cpu0_alert1>;
> +                                    cooling-device = <&CPU0 THERMAL_NO_LIMIT
> +                                                            THERMAL_NO_LIMIT>,
> +                                                     <&CPU1 THERMAL_NO_LIMIT
> +                                                            THERMAL_NO_LIMIT>,
> +                                                     <&CPU2 THERMAL_NO_LIMIT
> +                                                            THERMAL_NO_LIMIT>,
> +                                                     <&CPU3 THERMAL_NO_LIMIT
> +                                                            THERMAL_NO_LIMIT>;
> +                            };
> +                    };
> +            };
> +
> +            /* ... */
> +
> +            cluster0-thermal {
> +                    polling-delay-passive = <250>;
> +                    polling-delay = <1000>;
> +
> +                    thermal-sensors = <&tsens0 5>;
> +
> +                    trips {
> +                            cluster0_alert0: trip-point0 {
> +                                    temperature = <90000>;
> +                                    hysteresis = <2000>;
> +                                    type = "hot";
> +                            };
> +                            cluster0_crit: cluster0_crit {
> +                                    temperature = <110000>;
> +                                    hysteresis = <2000>;
> +                                    type = "critical";
> +                            };
> +                    };
> +            };
> +
> +            /* ... */
> +
> +            gpu-thermal-top {

This one is not going to match (which should cause an error).
 
> +                    polling-delay-passive = <250>;
> +                    polling-delay = <1000>;
> +
> +                    thermal-sensors = <&tsens0 11>;
> +
> +                    trips {
> +                            gpu1_alert0: trip-point0 {
> +                                    temperature = <90000>;
> +                                    hysteresis = <2000>;
> +                                    type = "hot";
> +                            };
> +                    };
> +            };
> +    };
> +...
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] dt-bindings: thermal: Add yaml bindings for thermal cooling-devices
  2020-03-05 12:56 ` [PATCH v2 2/3] dt-bindings: thermal: Add yaml bindings for thermal cooling-devices Amit Kucheria
@ 2020-03-12 19:01   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2020-03-12 19:01 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: linux-kernel, linux-arm-msm, swboyd, mka, daniel.lezcano,
	Amit Kucheria, Zhang Rui, linux-pm, devicetree

On Thu, Mar 05, 2020 at 06:26:42PM +0530, Amit Kucheria wrote:
> As part of moving the thermal bindings to YAML, split it up into 3
> bindings: thermal sensors, cooling devices and thermal zones.
> 
> The property #cooling-cells is required in each device that acts as a
> cooling device - whether active or passive. So any device that can
> throttle its performance to passively reduce heat dissipation (e.g.
> cpus, gpus) and any device that can actively dissipate heat at different
> levels (e.g. fans) will contain this property.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
>  .../thermal/thermal-cooling-devices.yaml      | 114 ++++++++++++++++++
>  1 file changed, 114 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
> 
> diff --git a/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml b/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
> new file mode 100644
> index 0000000000000..4745ea4b41ae7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
> @@ -0,0 +1,114 @@
> +# SPDX-License-Identifier: (GPL-2.0)
> +# Copyright 2020 Linaro Ltd.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/thermal-cooling-devices.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Thermal cooling device binding
> +
> +maintainers:
> +  - Amit Kucheria <amitk@kernel.org>
> +
> +description: |
> +  Thermal management is achieved in devicetree by describing the sensor hardware
> +  and the software abstraction of cooling devices and thermal zones required to
> +  take appropriate action to mitigate thermal overload.
> +
> +  The following node types are used to completely describe a thermal management
> +  system in devicetree:
> +   - thermal-sensor: device that measures temperature, has SoC-specific bindings
> +   - cooling-device: device used to dissipate heat either passively or artively
> +   - thermal-zones: a container of the following node types used to describe all
> +     thermal data for the platform
> +
> +  This binding describes the cooling devices.
> +
> +  There are essentially two ways to provide control on power dissipation:
> +    - Passive cooling: by means of regulating device performance. A typical
> +      passive cooling mechanism is a CPU that has dynamic voltage and frequency
> +      scaling (DVFS), and uses lower frequencies as cooling states.
> +    - Active cooling: by means of activating devices in order to remove the
> +      dissipated heat, e.g. regulating fan speeds.
> +
> +  Any cooling device has a range of cooling states (i.e. different levels of
> +  heat dissipation). They also have a way to determine the state of cooling in
> +  which the device is. For example, a fan's cooling states correspond to the
> +  different fan speeds possible. Cooling states are referred to by single
> +  unsigned integers, where larger numbers mean greater heat dissipation. The
> +  precise set of cooling states associated with a device should be defined in
> +  a particular device's binding.
> +

This and patch 1 should have a 'select: true' so that this schema is 
applied to all nodes (it does nothing if #cooling-cells is not present).

> +properties:
> +  "#cooling-cells":
> +    description:
> +        Must be 2, in order to specify minimum and maximum cooling state used in
> +        the cooling-maps reference. The first cell is the minimum cooling state
> +        and the second cell is the maximum cooling state requested.
> +    const: 2
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/thermal/thermal.h>
> +
> +    // Example 1: Cpufreq cooling device on CPU0
> +    cpus {
> +            #address-cells = <2>;
> +            #size-cells = <0>;
> +
> +            CPU0: cpu@0 {
> +                    device_type = "cpu";
> +                    compatible = "qcom,kryo385";
> +                    reg = <0x0 0x0>;
> +                    enable-method = "psci";
> +                    cpu-idle-states = <&LITTLE_CPU_SLEEP_0
> +                                       &LITTLE_CPU_SLEEP_1
> +                                       &CLUSTER_SLEEP_0>;
> +                    capacity-dmips-mhz = <607>;
> +                    dynamic-power-coefficient = <100>;
> +                    qcom,freq-domain = <&cpufreq_hw 0>;
> +                    #cooling-cells = <2>;
> +                    next-level-cache = <&L2_0>;
> +                    L2_0: l2-cache {
> +                            compatible = "cache";
> +                            next-level-cache = <&L3_0>;
> +                            L3_0: l3-cache {
> +                                    compatible = "cache";
> +                            };
> +                    };
> +          };
> +
> +          /* ... */
> +
> +    };
> +
> +    /* ... */
> +
> +    thermal-zones {
> +            cpu0-thermal {
> +                    polling-delay-passive = <250>;
> +                    polling-delay = <1000>;
> +
> +                    thermal-sensors = <&tsens0 1>;
> +
> +                    trips {
> +                            cpu0_alert0: trip-point0 {
> +                                    temperature = <90000>;
> +                                    hysteresis = <2000>;
> +                                    type = "passive";
> +                            };
> +                    };
> +
> +                    cooling-maps {
> +                            map0 {
> +                                    trip = <&cpu0_alert0>;
> +                                    cooling-device = <&CPU0 THERMAL_NO_LIMIT
> +                                                            THERMAL_NO_LIMIT>;
> +                            };
> +                    };
> +            };
> +
> +            /* ... */
> +    };
> +...
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] dt-bindings: thermal: Add yaml bindings for thermal zones
  2020-03-11 14:49   ` Rob Herring
@ 2020-03-23 20:46     ` Amit Kucheria
  2020-03-23 21:16       ` Rob Herring
  0 siblings, 1 reply; 11+ messages in thread
From: Amit Kucheria @ 2020-03-23 20:46 UTC (permalink / raw)
  To: Rob Herring
  Cc: LKML, linux-arm-msm, Stephen Boyd, Matthias Kaehlcke,
	Daniel Lezcano, Zhang Rui, Linux PM list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Rob,

Thanks for the review.

On Wed, Mar 11, 2020 at 8:19 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Mar 05, 2020 at 06:26:43PM +0530, Amit Kucheria wrote:
> > As part of moving the thermal bindings to YAML, split it up into 3
> > bindings: thermal sensors, cooling devices and thermal zones.
> >
> > The thermal-zone binding is a software abstraction to capture the
> > properties of each zone - how often they should be checked, the
> > temperature thresholds (trips) at which mitigation actions need to be
> > taken and the level of mitigation needed at those thresholds.
> >
> > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> > ---
> >  .../bindings/thermal/thermal-zones.yaml       | 325 ++++++++++++++++++
> >  1 file changed, 325 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> > new file mode 100644
> > index 0000000000000..f8f3b72bc3119
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> > @@ -0,0 +1,325 @@
> > +# SPDX-License-Identifier: (GPL-2.0)
> > +# Copyright 2020 Linaro Ltd.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/thermal/thermal-zones.yaml#
> > +$schema: http://devicetree.org/meta-schemas/base.yaml#
> > +
> > +title: Thermal zone binding
> > +
> > +maintainers:
> > +  - Amit Kucheria <amitk@kernel.org>
> > +
> > +description: |
> > +  Thermal management is achieved in devicetree by describing the sensor hardware
> > +  and the software abstraction of cooling devices and thermal zones required to
> > +  take appropriate action to mitigate thermal overloads.
> > +
> > +  The following node types are used to completely describe a thermal management
> > +  system in devicetree:
> > +   - thermal-sensor: device that measures temperature, has SoC-specific bindings
> > +   - cooling-device: device used to dissipate heat either passively or actively
> > +   - thermal-zones: a container of the following node types used to describe all
> > +     thermal data for the platform
> > +
> > +  This binding describes the thermal-zones.
> > +
> > +  The polling-delay properties of a thermal-zone are bound to the maximum dT/dt
> > +  (temperature derivative over time) in two situations for a thermal zone:
> > +    1. when passive cooling is activated (polling-delay-passive)
> > +    2. when the zone just needs to be monitored (polling-delay) or when
> > +       active cooling is activated.
> > +
> > +  The maximum dT/dt is highly bound to hardware power consumption and
> > +  dissipation capability. The delays should be chosen to account for said
> > +  max dT/dt, such that a device does not cross several trip boundaries
> > +  unexpectedly between polls. Choosing the right polling delays shall avoid
> > +  having the device in temperature ranges that may damage the silicon structures
> > +  and reduce silicon lifetime.
> > +
> > +properties:
> > +  thermal-zones:
> > +    type: object
> > +    description:
> > +      A /thermal-zones node is required in order to use the thermal framework to
> > +      manage input from the various thermal zones in the system in order to
> > +      mitigate thermal overload conditions. It does not represent a real device
> > +      in the system, but acts as a container to link thermal sensor devices,
> > +      platform-data regarding temperature thresholds and the mitigation actions
> > +      to take when the temperature crosses those thresholds.
> > +
>
> > +    properties:
> > +      $nodename:
> > +        pattern: "^[a-zA-Z][a-zA-Z0-9,\\-]{1,12}-thermal$"
>
> These 3 lines should be:
>
> patternProperties:
>   "^[a-zA-Z][a-zA-Z0-9,\\-]{1,12}-thermal$":
>
> Though we should drop ',' as well.
>
> $nodename is only needed at the top level where it's kind of special.

Fixed.

> > +        type: object
> > +        description:
> > +          Each thermal zone node contains information about how frequently it
> > +          must be checked, the sensor responsible for reporting temperature for
> > +          this zone, one sub-node containing the various trip points for this
> > +          zone and one sub-node containing all the zone cooling-maps.
> > +
> > +        properties:
> > +          polling-delay:
> > +            $ref: /schemas/types.yaml#/definitions/uint32
> > +            minimum: 0
>
> The type is unsigned, so the min is already 0.

Dropped.

> > +            description:
> > +              The maximum number of milliseconds to wait between polls when
> > +              checking this thermal zone. Setting this to 0 disables the polling
> > +              timers setup by the thermal framework and assumes that the thermal
> > +              sensors in this zone support interrupts.
> > +
> > +          polling-delay-passive:
> > +            $ref: /schemas/types.yaml#/definitions/uint32
> > +            minimum: 0

Dropped.

> > +            description:
> > +              The maximum number of milliseconds to wait between polls when
> > +              checking this thermal zone while doing passive cooling. Setting
> > +              this to 0 disables the polling timers setup by the thermal
> > +              framework and assumes that the thermal sensors in this zone
> > +              support interrupts.
> > +
> > +          thermal-sensors:
> > +            $ref: /schemas/types.yaml#/definitions/phandle-array
> > +            description:
> > +              A list of thermal sensor phandles and sensor specifiers used to
> > +              monitor this thermal zone.
> > +
> > +          trips:
> > +            type: object
> > +            description:
> > +              This node describes a set of points in the temperature domain at
> > +              which the thermal framework needs to takes action. The actions to
> > +              be taken are defined in another node called cooling-maps.
> > +
> > +            patternProperties:
> > +              "^[a-zA-Z][a-zA-Z0-9,+\\._]{0,63}$":
>
> Drop ',', '+', '.', and ideally '_'. Probably need to add '-'.

Dropping underscore flags a lot of DTs in dtbs_check. Do you want me
to go fix them or can we live with the underscore. Is there some
document I should read on why underscore isn't desirable?

Fixed the rest.

>
> > +                type: object
> > +
> > +                properties:
> > +                  temperature:
> > +                    $ref: /schemas/types.yaml#/definitions/int32
> > +                    minimum: -273000
> > +                    maximum: 200000
> > +                    description:
> > +                      An integer expressing the trip temperature in millicelsius.
> > +
> > +                  hysteresis:
> > +                    $ref: /schemas/types.yaml#/definitions/uint32
> > +                    description:
> > +                      An unsigned integer expressing the hysteresis delta with
> > +                      respect to the trip temperature property above, also in
> > +                      millicelsius.
> > +
> > +                  type:
>
> Needs a type reference (string).

Fixed.

>
> > +                    enum:
> > +                        # active: enable active cooling e.g. fans
> > +                        - active
>
> Wrong indentation. Should be 2 less.
>
> Also, I think this would be more readable:
>
> - active  # enable active cooling e.g. fans

Yes, fixed.

>
> > +                        # passive: enable passive cooling e.g. throttling cpu
> > +                        - passive
> > +                        # hot: send notification to driver if .notify
> > +                        #      callback registered
> > +                        - hot
> > +                        # critical: send notification to driver if .notify
> > +                        #           callback registered and trigger a shutdown
> > +                        - critical
> > +                    description: |
> > +                      There are four valid trip types: active, passive, hot,
> > +                      critical.
> > +
> > +                      The critical trip type is used to set the maximum
> > +                      temperature threshold above which the HW becomes
> > +                      unstable and underlying firmware might even trigger a
> > +                      reboot. Hitting the critical threshold triggers a system
> > +                      shutdown.
> > +
> > +                      The hot trip type can be used to send a notification to
> > +                      the thermal driver (if a .notify callback is registered).
> > +                      The action to be taken is left to the driver.
> > +
> > +                      The passive trip type can be used to slow down HW e.g. run
> > +                      the CPU, GPU, bus at a lower frequency.
> > +
> > +                      The active trip type can be used to control other HW to
> > +                      help in cooling e.g. fans can be sped up or slowed down
> > +
> > +                required:
> > +                  - temperature
> > +                  - hysteresis
> > +                  - type
> > +
> > +            additionalProperties: false
>
> You need 'additionalProperties' on the child nodes too, or are there
> vendor specific properties allowed?

Done. No vendor-specific properties allowed here.

> > +
> > +          cooling-maps:
> > +            type: object
> > +            description:
> > +              This node describes the action to be taken when a thermal zone
> > +              crosses one of the temperature thresholds described in the trips
> > +              node. The action takes the form of a mapping relation between a
> > +              trip and the target cooling device state.
> > +
> > +            patternProperties:
> > +              "^map[0-9][-a-zA-Z0-9]*$":
> > +                type: object
> > +
> > +                properties:
> > +                  trip:
> > +                    $ref: /schemas/types.yaml#/definitions/phandle
> > +                    description:
> > +                      A phandle of a trip point node within this thermal zone.
> > +
> > +                  cooling-device:
> > +                    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +                    description:
> > +                      A list of cooling device phandles along with the minimum
> > +                      and maximum cooling state specifiers for each cooling
> > +                      device. Using the THERMAL_NO_LIMIT (-1UL) constant in the
> > +                      cooling-device phandle limit specifier lets the framework
> > +                      use the minimum and maximum cooling state for that cooling
> > +                      device automatically.
> > +
> > +                  contribution:
> > +                    $ref: /schemas/types.yaml#/definitions/uint32
> > +                    minimum: 0
> > +                    maximum: 100
> > +                    description:
> > +                      The contribution of the cooling devices at the trip
> > +                      temperature, both referenced in this map, to this thermal
> > +                      zone as a percentage.
> > +
> > +                required:
> > +                  - trip
> > +                  - cooling-device
> > +
>
> 'additionalProperties' for map* nodes?

Done.

> > +            additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/thermal/thermal.h>
> > +
> > +    // Example 1: SDM845 TSENS
> > +    soc: soc@0 {
> > +            #address-cells = <2>;
> > +            #size-cells = <2>;
> > +
> > +            /* ... */
> > +
> > +            tsens0: thermal-sensor@c263000 {
> > +                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> > +                    reg = <0 0x0c263000 0 0x1ff>, /* TM */
> > +                          <0 0x0c222000 0 0x1ff>; /* SROT */
> > +                    #qcom,sensors = <13>;
> > +                    interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>,
> > +                                 <GIC_SPI 508 IRQ_TYPE_LEVEL_HIGH>;
> > +                    interrupt-names = "uplow", "critical";
> > +                    #thermal-sensor-cells = <1>;
> > +            };
> > +
> > +            tsens1: thermal-sensor@c265000 {
> > +                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> > +                    reg = <0 0x0c265000 0 0x1ff>, /* TM */
> > +                          <0 0x0c223000 0 0x1ff>; /* SROT */
> > +                    #qcom,sensors = <8>;
> > +                    interrupts = <GIC_SPI 507 IRQ_TYPE_LEVEL_HIGH>,
> > +                                 <GIC_SPI 509 IRQ_TYPE_LEVEL_HIGH>;
> > +                    interrupt-names = "uplow", "critical";
> > +                    #thermal-sensor-cells = <1>;
> > +            };
> > +    };
> > +
> > +    /* ... */
> > +
> > +    thermal-zones {
> > +            cpu0-thermal {
> > +                    polling-delay-passive = <250>;
> > +                    polling-delay = <1000>;
> > +
> > +                    thermal-sensors = <&tsens0 1>;
> > +
> > +                    trips {
> > +                            cpu0_alert0: trip-point0 {
> > +                                    temperature = <90000>;
> > +                                    hysteresis = <2000>;
> > +                                    type = "passive";
> > +                            };
> > +
> > +                            cpu0_alert1: trip-point1 {
> > +                                    temperature = <95000>;
> > +                                    hysteresis = <2000>;
> > +                                    type = "passive";
> > +                            };
> > +
> > +                            cpu0_crit: cpu_crit {
> > +                                    temperature = <110000>;
> > +                                    hysteresis = <1000>;
> > +                                    type = "critical";
> > +                            };
> > +                    };
> > +
> > +                    cooling-maps {
> > +                            map0 {
> > +                                    trip = <&cpu0_alert0>;
> > +                                    cooling-device = <&CPU0 THERMAL_NO_LIMIT
> > +                                                            THERMAL_NO_LIMIT>,
> > +                                                     <&CPU1 THERMAL_NO_LIMIT
> > +                                                            THERMAL_NO_LIMIT>,
> > +                                                     <&CPU2 THERMAL_NO_LIMIT
> > +                                                            THERMAL_NO_LIMIT>,
> > +                                                     <&CPU3 THERMAL_NO_LIMIT
> > +                                                            THERMAL_NO_LIMIT>;
> > +                            };
> > +
> > +                            map1 {
> > +                                    trip = <&cpu0_alert1>;
> > +                                    cooling-device = <&CPU0 THERMAL_NO_LIMIT
> > +                                                            THERMAL_NO_LIMIT>,
> > +                                                     <&CPU1 THERMAL_NO_LIMIT
> > +                                                            THERMAL_NO_LIMIT>,
> > +                                                     <&CPU2 THERMAL_NO_LIMIT
> > +                                                            THERMAL_NO_LIMIT>,
> > +                                                     <&CPU3 THERMAL_NO_LIMIT
> > +                                                            THERMAL_NO_LIMIT>;
> > +                            };
> > +                    };
> > +            };
> > +
> > +            /* ... */
> > +
> > +            cluster0-thermal {
> > +                    polling-delay-passive = <250>;
> > +                    polling-delay = <1000>;
> > +
> > +                    thermal-sensors = <&tsens0 5>;
> > +
> > +                    trips {
> > +                            cluster0_alert0: trip-point0 {
> > +                                    temperature = <90000>;
> > +                                    hysteresis = <2000>;
> > +                                    type = "hot";
> > +                            };
> > +                            cluster0_crit: cluster0_crit {
> > +                                    temperature = <110000>;
> > +                                    hysteresis = <2000>;
> > +                                    type = "critical";
> > +                            };
> > +                    };
> > +            };
> > +
> > +            /* ... */
> > +
> > +            gpu-thermal-top {
>
> This one is not going to match (which should cause an error).

Good catch. Unfortunately, this isn't getting caught. Nor is the
12-char limitation before -thermal in the thermal zone name. I can't
figure out why.

> > +                    polling-delay-passive = <250>;
> > +                    polling-delay = <1000>;
> > +
> > +                    thermal-sensors = <&tsens0 11>;
> > +
> > +                    trips {
> > +                            gpu1_alert0: trip-point0 {
> > +                                    temperature = <90000>;
> > +                                    hysteresis = <2000>;
> > +                                    type = "hot";
> > +                            };
> > +                    };
> > +            };
> > +    };
> > +...
> > --
> > 2.20.1
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] dt-bindings: thermal: Add yaml bindings for thermal zones
  2020-03-23 20:46     ` Amit Kucheria
@ 2020-03-23 21:16       ` Rob Herring
  2020-03-24 10:33         ` Amit Kucheria
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2020-03-23 21:16 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: LKML, linux-arm-msm, Stephen Boyd, Matthias Kaehlcke,
	Daniel Lezcano, Zhang Rui, Linux PM list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Mon, Mar 23, 2020 at 2:46 PM Amit Kucheria <amit.kucheria@linaro.org> wrote:
>
> Hi Rob,
>
> Thanks for the review.
>
> On Wed, Mar 11, 2020 at 8:19 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Mar 05, 2020 at 06:26:43PM +0530, Amit Kucheria wrote:
> > > As part of moving the thermal bindings to YAML, split it up into 3
> > > bindings: thermal sensors, cooling devices and thermal zones.
> > >
> > > The thermal-zone binding is a software abstraction to capture the
> > > properties of each zone - how often they should be checked, the
> > > temperature thresholds (trips) at which mitigation actions need to be
> > > taken and the level of mitigation needed at those thresholds.

[...]

> > > +          trips:
> > > +            type: object
> > > +            description:
> > > +              This node describes a set of points in the temperature domain at
> > > +              which the thermal framework needs to takes action. The actions to
> > > +              be taken are defined in another node called cooling-maps.
> > > +
> > > +            patternProperties:
> > > +              "^[a-zA-Z][a-zA-Z0-9,+\\._]{0,63}$":
> >
> > Drop ',', '+', '.', and ideally '_'. Probably need to add '-'.
>
> Dropping underscore flags a lot of DTs in dtbs_check. Do you want me
> to go fix them or can we live with the underscore. Is there some
> document I should read on why underscore isn't desirable?

Just convention. dtc will warn (with W=2), but we probably should
document that better. Anyways, fine to leave '_' here.


> > > +            /* ... */
> > > +
> > > +            gpu-thermal-top {
> >
> > This one is not going to match (which should cause an error).
>
> Good catch. Unfortunately, this isn't getting caught. Nor is the
> 12-char limitation before -thermal in the thermal zone name. I can't
> figure out why.

That's because this schema has to be included by another schema which
matches on a parent node containing 'thermal-zones'. If
'thermal-zones' can be at the root node, then you should rework this
such that you have $nodename: {const: thermal-zones} as a top-level
property.

Rob

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] dt-bindings: thermal: Add yaml bindings for thermal zones
  2020-03-23 21:16       ` Rob Herring
@ 2020-03-24 10:33         ` Amit Kucheria
  2020-03-24 15:06           ` Rob Herring
  0 siblings, 1 reply; 11+ messages in thread
From: Amit Kucheria @ 2020-03-24 10:33 UTC (permalink / raw)
  To: Rob Herring
  Cc: LKML, linux-arm-msm, Stephen Boyd, Matthias Kaehlcke,
	Daniel Lezcano, Zhang Rui, Linux PM list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

[-- Attachment #1: Type: text/plain, Size: 3166 bytes --]

On Tue, Mar 24, 2020 at 2:46 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Mar 23, 2020 at 2:46 PM Amit Kucheria <amit.kucheria@linaro.org> wrote:
> >
> > Hi Rob,
> >
> > Thanks for the review.
> >
> > On Wed, Mar 11, 2020 at 8:19 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Thu, Mar 05, 2020 at 06:26:43PM +0530, Amit Kucheria wrote:
> > > > As part of moving the thermal bindings to YAML, split it up into 3
> > > > bindings: thermal sensors, cooling devices and thermal zones.
> > > >
> > > > The thermal-zone binding is a software abstraction to capture the
> > > > properties of each zone - how often they should be checked, the
> > > > temperature thresholds (trips) at which mitigation actions need to be
> > > > taken and the level of mitigation needed at those thresholds.
>

[...]

>
> > > > +            /* ... */
> > > > +
> > > > +            gpu-thermal-top {
> > >
> > > This one is not going to match (which should cause an error).
> >
> > Good catch. Unfortunately, this isn't getting caught. Nor is the
> > 12-char limitation before -thermal in the thermal zone name. I can't
> > figure out why.
>
> That's because this schema has to be included by another schema which
> matches on a parent node containing 'thermal-zones'. If
> 'thermal-zones' can be at the root node, then you should rework this
> such that you have $nodename: {const: thermal-zones} as a top-level
> property.

I've done all the change requested in the review(see attached patch),
including moving to

properties:
   $nodename:
       const: thermal-zones

but that generates a bunch of errors similar to:

/home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/zte.example.dt.yaml:
/: $nodename:0: 'thermal-zones' was expected
/home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/psci.example.dt.yaml:
/: $nodename:0: 'thermal-zones' was expected
/home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/sunxi.example.dt.yaml:
/: $nodename:0: 'thermal-zones' was expected
/home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/sprd/sprd.example.dt.yaml:
/: $nodename:0: 'thermal-zones' was expected
/home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/calxeda.example.dt.yaml:
/: $nodename:0: 'thermal-zones' was expected
/home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/ti/ti,davinci.example.dt.yaml:
/: $nodename:0: 'thermal-zones' was expected
/home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/spear.example.dt.yaml:
/: $nodename:0: 'thermal-zones' was expected
/home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/ti/nspire.example.dt.yaml:
/: $nodename:0: 'thermal-zones' was expected

It seems like dtc is expecting every node to have a thermal-zones node?

Looking at other root nodes such as cpus.yaml, the main difference I
noticed was the absence of the "select: true" property. However, if I
remove that, we go back to the schema not being applied.

You mentioned that the thermal-zones schema needs to included by
another schema. What did you mean by that?

Regards,
Amit

[-- Attachment #2: 0001-dt-bindings-thermal-Add-yaml-bindings-for-thermal-zo.patch --]
[-- Type: text/x-patch, Size: 14507 bytes --]

From 74dc9532a709259cfe89edf7d323b8a531c4c96d Mon Sep 17 00:00:00 2001
Message-Id: <74dc9532a709259cfe89edf7d323b8a531c4c96d.1585044912.git.amit.kucheria@linaro.org>
From: Amit Kucheria <amit.kucheria@linaro.org>
Date: Fri, 7 Feb 2020 00:32:26 +0530
Subject: [PATCH] dt-bindings: thermal: Add yaml bindings for thermal zones

As part of moving the thermal bindings to YAML, split it up into 3
bindings: thermal sensors, cooling devices and thermal zones.

The thermal-zone binding is a software abstraction to capture the
properties of each zone - how often they should be checked, the
temperature thresholds (trips) at which mitigation actions need to be
taken and the level of mitigation needed at those thresholds.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
 .../bindings/thermal/thermal-zones.yaml       | 321 ++++++++++++++++++
 1 file changed, 321 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal-zones.yaml

diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
new file mode 100644
index 000000000000..4840e69ab7ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
@@ -0,0 +1,321 @@
+# SPDX-License-Identifier: (GPL-2.0)
+# Copyright 2020 Linaro Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/thermal-zones.yaml#
+$schema: http://devicetree.org/meta-schemas/base.yaml#
+
+title: Thermal zone binding
+
+maintainers:
+  - Amit Kucheria <amitk@kernel.org>
+
+description: |
+  Thermal management is achieved in devicetree by describing the sensor hardware
+  and the software abstraction of cooling devices and thermal zones required to
+  take appropriate action to mitigate thermal overloads.
+
+  The following node types are used to completely describe a thermal management
+  system in devicetree:
+   - thermal-sensor: device that measures temperature, has SoC-specific bindings
+   - cooling-device: device used to dissipate heat either passively or actively
+   - thermal-zones: a container of the following node types used to describe all
+     thermal data for the platform
+
+  This binding describes the thermal-zones.
+
+  The polling-delay properties of a thermal-zone are bound to the maximum dT/dt
+  (temperature derivative over time) in two situations for a thermal zone:
+    1. when passive cooling is activated (polling-delay-passive)
+    2. when the zone just needs to be monitored (polling-delay) or when
+       active cooling is activated.
+
+  The maximum dT/dt is highly bound to hardware power consumption and
+  dissipation capability. The delays should be chosen to account for said
+  max dT/dt, such that a device does not cross several trip boundaries
+  unexpectedly between polls. Choosing the right polling delays shall avoid
+  having the device in temperature ranges that may damage the silicon structures
+  and reduce silicon lifetime.
+
+select: true
+
+properties:
+  $nodename:
+    const: thermal-zones
+    description:
+      A /thermal-zones node is required in order to use the thermal framework to
+      manage input from the various thermal zones in the system in order to
+      mitigate thermal overload conditions. It does not represent a real device
+      in the system, but acts as a container to link thermal sensor devices,
+      platform-data regarding temperature thresholds and the mitigation actions
+      to take when the temperature crosses those thresholds.
+
+patternProperties:
+ "^[a-zA-Z][a-zA-Z0-9\\-]{1,12}-thermal$":
+   type: object
+   description:
+     Each thermal zone node contains information about how frequently it
+     must be checked, the sensor responsible for reporting temperature for
+     this zone, one sub-node containing the various trip points for this
+     zone and one sub-node containing all the zone cooling-maps.
+
+   properties:
+     polling-delay:
+       $ref: /schemas/types.yaml#/definitions/uint32
+       description:
+         The maximum number of milliseconds to wait between polls when
+         checking this thermal zone. Setting this to 0 disables the polling
+         timers setup by the thermal framework and assumes that the thermal
+         sensors in this zone support interrupts.
+
+     polling-delay-passive:
+       $ref: /schemas/types.yaml#/definitions/uint32
+       description:
+         The maximum number of milliseconds to wait between polls when
+         checking this thermal zone while doing passive cooling. Setting
+         this to 0 disables the polling timers setup by the thermal
+         framework and assumes that the thermal sensors in this zone
+         support interrupts.
+
+     thermal-sensors:
+       $ref: /schemas/types.yaml#/definitions/phandle-array
+       description:
+         A list of thermal sensor phandles and sensor specifiers used to
+         monitor this thermal zone.
+
+     trips:
+       type: object
+       description:
+         This node describes a set of points in the temperature domain at
+         which the thermal framework needs to takes action. The actions to
+         be taken are defined in another node called cooling-maps.
+
+       patternProperties:
+         "^[a-zA-Z][a-zA-Z0-9\\-_]{0,63}$":
+           type: object
+
+           properties:
+             temperature:
+               $ref: /schemas/types.yaml#/definitions/int32
+               minimum: -273000
+               maximum: 200000
+               description:
+                 An integer expressing the trip temperature in millicelsius.
+
+             hysteresis:
+               $ref: /schemas/types.yaml#/definitions/uint32
+               description:
+                 An unsigned integer expressing the hysteresis delta with
+                 respect to the trip temperature property above, also in
+                 millicelsius.
+
+             type:
+               $ref: /schemas/types.yaml#/definitions/string
+               enum:
+                 - active   # enable active cooling e.g. fans
+                 - passive  # enable passive cooling e.g. throttling cpu
+                 - hot      # send notification to driver
+                 - critical # send notification to driver, trigger shutdown
+               description: |
+                 There are four valid trip types: active, passive, hot,
+                 critical.
+
+                 The critical trip type is used to set the maximum
+                 temperature threshold above which the HW becomes
+                 unstable and underlying firmware might even trigger a
+                 reboot. Hitting the critical threshold triggers a system
+                 shutdown.
+
+                 The hot trip type can be used to send a notification to
+                 the thermal driver (if a .notify callback is registered).
+                 The action to be taken is left to the driver.
+
+                 The passive trip type can be used to slow down HW e.g. run
+                 the CPU, GPU, bus at a lower frequency.
+
+                 The active trip type can be used to control other HW to
+                 help in cooling e.g. fans can be sped up or slowed down
+
+           required:
+             - temperature
+             - hysteresis
+             - type
+           additionalProperties: false
+
+       additionalProperties: false
+
+     cooling-maps:
+       type: object
+       description:
+         This node describes the action to be taken when a thermal zone
+         crosses one of the temperature thresholds described in the trips
+         node. The action takes the form of a mapping relation between a
+         trip and the target cooling device state.
+
+       patternProperties:
+         "^map[-a-zA-Z0-9]*$":
+           type: object
+
+           properties:
+             trip:
+               $ref: /schemas/types.yaml#/definitions/phandle
+               description:
+                 A phandle of a trip point node within this thermal zone.
+
+             cooling-device:
+               $ref: /schemas/types.yaml#/definitions/phandle-array
+               description:
+                 A list of cooling device phandles along with the minimum
+                 and maximum cooling state specifiers for each cooling
+                 device. Using the THERMAL_NO_LIMIT (-1UL) constant in the
+                 cooling-device phandle limit specifier lets the framework
+                 use the minimum and maximum cooling state for that cooling
+                 device automatically.
+
+             contribution:
+               $ref: /schemas/types.yaml#/definitions/uint32
+               minimum: 0
+               maximum: 100
+               description:
+                 The contribution of the cooling devices at the trip
+                 temperature, both referenced in this map, to this thermal
+                 zone as a percentage.
+
+           required:
+             - trip
+             - cooling-device
+           additionalProperties: false
+
+       additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/thermal/thermal.h>
+
+    // Example 1: SDM845 TSENS
+    soc: soc@0 {
+            #address-cells = <2>;
+            #size-cells = <2>;
+
+            /* ... */
+
+            tsens0: thermal-sensor@c263000 {
+                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
+                    reg = <0 0x0c263000 0 0x1ff>, /* TM */
+                          <0 0x0c222000 0 0x1ff>; /* SROT */
+                    #qcom,sensors = <13>;
+                    interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>,
+                                 <GIC_SPI 508 IRQ_TYPE_LEVEL_HIGH>;
+                    interrupt-names = "uplow", "critical";
+                    #thermal-sensor-cells = <1>;
+            };
+
+            tsens1: thermal-sensor@c265000 {
+                    compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
+                    reg = <0 0x0c265000 0 0x1ff>, /* TM */
+                          <0 0x0c223000 0 0x1ff>; /* SROT */
+                    #qcom,sensors = <8>;
+                    interrupts = <GIC_SPI 507 IRQ_TYPE_LEVEL_HIGH>,
+                                 <GIC_SPI 509 IRQ_TYPE_LEVEL_HIGH>;
+                    interrupt-names = "uplow", "critical";
+                    #thermal-sensor-cells = <1>;
+            };
+    };
+
+    /* ... */
+
+    thermal-zones {
+            cpu0-thermal {
+                    polling-delay-passive = <250>;
+                    polling-delay = <1000>;
+
+                    thermal-sensors = <&tsens0 1>;
+
+                    trips {
+                            cpu0_alert0: trip-point0 {
+                                    temperature = <90000>;
+                                    hysteresis = <2000>;
+                                    type = "passive";
+                            };
+
+                            cpu0_alert1: trip-point1 {
+                                    temperature = <95000>;
+                                    hysteresis = <2000>;
+                                    type = "passive";
+                            };
+
+                            cpu0_crit: cpu_crit {
+                                    temperature = <110000>;
+                                    hysteresis = <1000>;
+                                    type = "critical";
+                            };
+                    };
+
+                    cooling-maps {
+                            map0 {
+                                    trip = <&cpu0_alert0>;
+                                    cooling-device = <&CPU0 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU1 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU2 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU3 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>;
+                            };
+
+                            map1 {
+                                    trip = <&cpu0_alert1>;
+                                    cooling-device = <&CPU0 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU1 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU2 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>,
+                                                     <&CPU3 THERMAL_NO_LIMIT
+                                                            THERMAL_NO_LIMIT>;
+                            };
+                    };
+            };
+
+            /* ... */
+
+            cluster0-thermal {
+                    polling-delay-passive = <250>;
+                    polling-delay = <1000>;
+
+                    thermal-sensors = <&tsens0 5>;
+
+                    trips {
+                            cluster0_alert0: trip-point0 {
+                                    temperature = <90000>;
+                                    hysteresis = <2000>;
+                                    type = "hot";
+                            };
+                            cluster0_crit: cluster0_crit {
+                                    temperature = <110000>;
+                                    hysteresis = <2000>;
+                                    type = "critical";
+                            };
+                    };
+            };
+
+            /* ... */
+
+            gpu-thermal-top {
+                    polling-delay-passive = <250>;
+                    polling-delay = <1000>;
+
+                    thermal-sensors = <&tsens0 11>;
+
+                    trips {
+                            gpu1_alert0: trip-point0 {
+                                    temperature = <90000>;
+                                    hysteresis = <2000>;
+                                    type = "hot";
+                            };
+                    };
+            };
+    };
+...
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] dt-bindings: thermal: Add yaml bindings for thermal zones
  2020-03-24 10:33         ` Amit Kucheria
@ 2020-03-24 15:06           ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2020-03-24 15:06 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: LKML, linux-arm-msm, Stephen Boyd, Matthias Kaehlcke,
	Daniel Lezcano, Zhang Rui, Linux PM list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Tue, Mar 24, 2020 at 4:33 AM Amit Kucheria <amit.kucheria@linaro.org> wrote:
>
> On Tue, Mar 24, 2020 at 2:46 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Mar 23, 2020 at 2:46 PM Amit Kucheria <amit.kucheria@linaro.org> wrote:
> > >
> > > Hi Rob,
> > >
> > > Thanks for the review.
> > >
> > > On Wed, Mar 11, 2020 at 8:19 PM Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > On Thu, Mar 05, 2020 at 06:26:43PM +0530, Amit Kucheria wrote:
> > > > > As part of moving the thermal bindings to YAML, split it up into 3
> > > > > bindings: thermal sensors, cooling devices and thermal zones.
> > > > >
> > > > > The thermal-zone binding is a software abstraction to capture the
> > > > > properties of each zone - how often they should be checked, the
> > > > > temperature thresholds (trips) at which mitigation actions need to be
> > > > > taken and the level of mitigation needed at those thresholds.
> >
>
> [...]
>
> >
> > > > > +            /* ... */
> > > > > +
> > > > > +            gpu-thermal-top {
> > > >
> > > > This one is not going to match (which should cause an error).
> > >
> > > Good catch. Unfortunately, this isn't getting caught. Nor is the
> > > 12-char limitation before -thermal in the thermal zone name. I can't
> > > figure out why.
> >
> > That's because this schema has to be included by another schema which
> > matches on a parent node containing 'thermal-zones'. If
> > 'thermal-zones' can be at the root node, then you should rework this
> > such that you have $nodename: {const: thermal-zones} as a top-level
> > property.
>
> I've done all the change requested in the review(see attached patch),
> including moving to
>
> properties:
>    $nodename:
>        const: thermal-zones
>
> but that generates a bunch of errors similar to:
>
> /home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/zte.example.dt.yaml:
> /: $nodename:0: 'thermal-zones' was expected
> /home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/psci.example.dt.yaml:
> /: $nodename:0: 'thermal-zones' was expected
> /home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/sunxi.example.dt.yaml:
> /: $nodename:0: 'thermal-zones' was expected
> /home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/sprd/sprd.example.dt.yaml:
> /: $nodename:0: 'thermal-zones' was expected
> /home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/calxeda.example.dt.yaml:
> /: $nodename:0: 'thermal-zones' was expected
> /home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/ti/ti,davinci.example.dt.yaml:
> /: $nodename:0: 'thermal-zones' was expected
> /home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/spear.example.dt.yaml:
> /: $nodename:0: 'thermal-zones' was expected
> /home/amit/work/builds/build-aarch64/Documentation/devicetree/bindings/arm/ti/nspire.example.dt.yaml:
> /: $nodename:0: 'thermal-zones' was expected
>
> It seems like dtc is expecting every node to have a thermal-zones node?
>
> Looking at other root nodes such as cpus.yaml, the main difference I
> noticed was the absence of the "select: true" property. However, if I
> remove that, we go back to the schema not being applied.

'select: true' should be dropped. It will be applied to any
'thermal-zones' nodes. The generated 'select' will use $nodename if
compatible is not present for the schema.

I tested that putting an error in the example works.

> You mentioned that the thermal-zones schema needs to included by
> another schema. What did you mean by that?

Nevermind, I wasn't thinking that it's a top-level node. If it was a
child node, then you'd want to include it from the parent schemas.

Rob

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-05 12:56 [PATCH v2 0/3] Convert thermal bindings to yaml Amit Kucheria
2020-03-05 12:56 ` [PATCH v2 1/3] dt-bindings: thermal: Add yaml bindings for thermal sensors Amit Kucheria
2020-03-11  8:26   ` Vincent Guittot
2020-03-05 12:56 ` [PATCH v2 2/3] dt-bindings: thermal: Add yaml bindings for thermal cooling-devices Amit Kucheria
2020-03-12 19:01   ` Rob Herring
2020-03-05 12:56 ` [PATCH v2 3/3] dt-bindings: thermal: Add yaml bindings for thermal zones Amit Kucheria
2020-03-11 14:49   ` Rob Herring
2020-03-23 20:46     ` Amit Kucheria
2020-03-23 21:16       ` Rob Herring
2020-03-24 10:33         ` Amit Kucheria
2020-03-24 15:06           ` Rob Herring

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git