[PATCH v1 0/9] imx: thermal: Allow trip point configuration from DT

[PATCH v1 0/9] imx: thermal: Allow trip point configuration from DT

[Francesco Dolcini]

This series allows to specify the imx thermal drivers trip point from the device tree,
without this change the threshold are hard-coded and this might not be correct given the
thermal design of the final system.

This change is backward compatible with the existing device tree, and even
with this change in by default the thresholds are the same as before.

Toradex board are also updated to use a system-specific thresholds.

Discussion on the current design is here:
https://lore.kernel.org/all/4ba1d7d2-3e8c-ba60-37fd-9598f415c076@linaro.org/

One side note, after this change the dtbs checker starts complaining with this message

```
linux/arch/arm/boot/dts/imx6dl-alti6p.dtb: tempmon: '#thermal-sensor-cells' does not match any of the regexes: '^(automotive|commercial|extended-commercial|industrial)-thermal$', 'pinctrl-[0-9]+'
	From schema: linux/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
```

to my understanding this is just a side effect, '#thermal-sensor-cells' is not changed in
any way by this series. I can fix that, I wonder if I should remove the property from the
imx dtsi files or add it to the binding yaml definition, not sure about it.
Anybody can advise?

Changes in v2:
 - fix build error without CONFIG_THERMAL_OF
 - more verbose error reporting in case the dts is not correct
 - additional comment on the threshold fixup in case the passive threshold is
   higher than critical
 - while parsing the dts thermal, return immediately if the node is not there


Francesco Dolcini (9):
  dt-bindings: thermal: Define trips node in $defs
  thermal: thermal: Export OF trip helper function
  dt-bindings: thermal: imx: Add trips point
  imx: thermal: Configure trip point from DT
  ARM: dts: imx[67]: Add trips points
  ARM: dts: imx6qdl-apalis: Set CPU critical trip point
  ARM: dts: imx7-colibri: Set CPU critical trip point
  ARM: dts: imx6ull-colibri: Set CPU critical trip point
  ARM: dts: imx6qdl-colibri: Set CPU critical trip point

 .../bindings/thermal/imx-thermal.yaml         |  27 ++++
 .../bindings/thermal/thermal-zones.yaml       | 130 +++++++++---------
 arch/arm/boot/dts/imx-thermal.dtsi            |  61 ++++++++
 arch/arm/boot/dts/imx6qdl-apalis.dtsi         |  12 ++
 arch/arm/boot/dts/imx6qdl-colibri.dtsi        |  12 ++
 arch/arm/boot/dts/imx6qdl.dtsi                |   2 +
 arch/arm/boot/dts/imx6sl.dtsi                 |   2 +
 arch/arm/boot/dts/imx6sll.dtsi                |   2 +
 arch/arm/boot/dts/imx6sx.dtsi                 |   2 +
 arch/arm/boot/dts/imx6ul.dtsi                 |   2 +
 arch/arm/boot/dts/imx6ull-colibri.dtsi        |  12 ++
 arch/arm/boot/dts/imx7-colibri.dtsi           |  12 ++
 arch/arm/boot/dts/imx7s.dtsi                  |   2 +
 drivers/thermal/imx_thermal.c                 |  58 ++++++++
 drivers/thermal/thermal_core.h                |   7 +
 drivers/thermal/thermal_of.c                  |   5 +-
 16 files changed, 283 insertions(+), 65 deletions(-)
 create mode 100644 arch/arm/boot/dts/imx-thermal.dtsi

-- 
2.25.1

[PATCH v1 0/9] imx: thermal: Allow trip point configuration from DT

[Francesco Dolcini]

This series allows to specify the imx thermal drivers trip point from the device tree,
without this change the threshold are hard-coded and this might not be correct given the
thermal design of the final system.

This change is backward compatible with the existing device tree, and even
with this change in by default the thresholds are the same as before.

Toradex board are also updated to use a system-specific thresholds.

Discussion on the current design is here:
https://lore.kernel.org/all/4ba1d7d2-3e8c-ba60-37fd-9598f415c076@linaro.org/

One side note, after this change the dtbs checker starts complaining with this message

```
linux/arch/arm/boot/dts/imx6dl-alti6p.dtb: tempmon: '#thermal-sensor-cells' does not match any of the regexes: '^(automotive|commercial|extended-commercial|industrial)-thermal$', 'pinctrl-[0-9]+'
	From schema: linux/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
```

to my understanding this is just a side effect, '#thermal-sensor-cells' is not changed in
any way by this series. I can fix that, I wonder if I should remove the property from the
imx dtsi files or add it to the binding yaml definition, not sure about it.
Anybody can advise?

Changes in v2:
 - fix build error without CONFIG_THERMAL_OF
 - more verbose error reporting in case the dts is not correct
 - additional comment on the threshold fixup in case the passive threshold is
   higher than critical
 - while parsing the dts thermal, return immediately if the node is not there


Francesco Dolcini (9):
  dt-bindings: thermal: Define trips node in $defs
  thermal: thermal: Export OF trip helper function
  dt-bindings: thermal: imx: Add trips point
  imx: thermal: Configure trip point from DT
  ARM: dts: imx[67]: Add trips points
  ARM: dts: imx6qdl-apalis: Set CPU critical trip point
  ARM: dts: imx7-colibri: Set CPU critical trip point
  ARM: dts: imx6ull-colibri: Set CPU critical trip point
  ARM: dts: imx6qdl-colibri: Set CPU critical trip point

 .../bindings/thermal/imx-thermal.yaml         |  27 ++++
 .../bindings/thermal/thermal-zones.yaml       | 130 +++++++++---------
 arch/arm/boot/dts/imx-thermal.dtsi            |  61 ++++++++
 arch/arm/boot/dts/imx6qdl-apalis.dtsi         |  12 ++
 arch/arm/boot/dts/imx6qdl-colibri.dtsi        |  12 ++
 arch/arm/boot/dts/imx6qdl.dtsi                |   2 +
 arch/arm/boot/dts/imx6sl.dtsi                 |   2 +
 arch/arm/boot/dts/imx6sll.dtsi                |   2 +
 arch/arm/boot/dts/imx6sx.dtsi                 |   2 +
 arch/arm/boot/dts/imx6ul.dtsi                 |   2 +
 arch/arm/boot/dts/imx6ull-colibri.dtsi        |  12 ++
 arch/arm/boot/dts/imx7-colibri.dtsi           |  12 ++
 arch/arm/boot/dts/imx7s.dtsi                  |   2 +
 drivers/thermal/imx_thermal.c                 |  58 ++++++++
 drivers/thermal/thermal_core.h                |   7 +
 drivers/thermal/thermal_of.c                  |   5 +-
 16 files changed, 283 insertions(+), 65 deletions(-)
 create mode 100644 arch/arm/boot/dts/imx-thermal.dtsi

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Francesco Dolcini]

Move `trips` definition to `#/$defs/trips-base` and just reference it
from the trips node. This allows to easily re-use this binding from
another binding file.

No functional changes expected.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 .../bindings/thermal/thermal-zones.yaml       | 130 +++++++++---------
 1 file changed, 67 insertions(+), 63 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
index 2d34f3ccb257..ba84233d20b7 100644
--- a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
+++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
@@ -10,6 +10,72 @@ title: Thermal zone binding
 maintainers:
   - Amit Kucheria <amitk@kernel.org>
 
+$defs:
+  trips-base:
+    type: object
+    description:
+      This node describes a set of points in the temperature domain at
+      which the thermal framework needs to take 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. Any cooling action initiated by the framework is
+              maintained until the temperature falls below
+              (trip temperature - hysteresis). This potentially prevents a
+              situation where the trip gets constantly triggered soon after
+              cooling action is removed.
+
+          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
+
 description: |
   Thermal management is achieved in devicetree by describing the sensor hardware
   and the software abstraction of cooling devices and thermal zones required to
@@ -105,69 +171,7 @@ patternProperties:
           10-inch tablet is around 4500mW.
 
       trips:
-        type: object
-        description:
-          This node describes a set of points in the temperature domain at
-          which the thermal framework needs to take 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. Any cooling action initiated by the framework is
-                  maintained until the temperature falls below
-                  (trip temperature - hysteresis). This potentially prevents a
-                  situation where the trip gets constantly triggered soon after
-                  cooling action is removed.
-
-              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
+        $ref: "#/$defs/trips-base"
 
       cooling-maps:
         type: object
-- 
2.25.1

[PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Francesco Dolcini]

Move `trips` definition to `#/$defs/trips-base` and just reference it
from the trips node. This allows to easily re-use this binding from
another binding file.

No functional changes expected.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 .../bindings/thermal/thermal-zones.yaml       | 130 +++++++++---------
 1 file changed, 67 insertions(+), 63 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
index 2d34f3ccb257..ba84233d20b7 100644
--- a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
+++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
@@ -10,6 +10,72 @@ title: Thermal zone binding
 maintainers:
   - Amit Kucheria <amitk@kernel.org>
 
+$defs:
+  trips-base:
+    type: object
+    description:
+      This node describes a set of points in the temperature domain at
+      which the thermal framework needs to take 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. Any cooling action initiated by the framework is
+              maintained until the temperature falls below
+              (trip temperature - hysteresis). This potentially prevents a
+              situation where the trip gets constantly triggered soon after
+              cooling action is removed.
+
+          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
+
 description: |
   Thermal management is achieved in devicetree by describing the sensor hardware
   and the software abstraction of cooling devices and thermal zones required to
@@ -105,69 +171,7 @@ patternProperties:
           10-inch tablet is around 4500mW.
 
       trips:
-        type: object
-        description:
-          This node describes a set of points in the temperature domain at
-          which the thermal framework needs to take 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. Any cooling action initiated by the framework is
-                  maintained until the temperature falls below
-                  (trip temperature - hysteresis). This potentially prevents a
-                  situation where the trip gets constantly triggered soon after
-                  cooling action is removed.
-
-              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
+        $ref: "#/$defs/trips-base"
 
       cooling-maps:
         type: object
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Krzysztof Kozlowski]

On 17/06/2022 00:08, Francesco Dolcini wrote:
> Move `trips` definition to `#/$defs/trips-base` and just reference it
> from the trips node. This allows to easily re-use this binding from
> another binding file.
> 
> No functional changes expected.

If you want to re-use trips, they should be rather moved to separate
YAML file... but anyway this should not be done per-driver bindings, but
in more general way. Either the problem - using one DTS for different
temperature grades - looks generic or is wrong at the core. In the first
option, the generic bindings should be fixed. In the second case - using
same DTS for different HW is not correct approach and why only thermal
should be specific? I can imagine that cooling devices might have
different settings, regulator voltages for DVFS could be a bit different...

Best regards,
Krzysztof

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Krzysztof Kozlowski]

On 17/06/2022 00:08, Francesco Dolcini wrote:
> Move `trips` definition to `#/$defs/trips-base` and just reference it
> from the trips node. This allows to easily re-use this binding from
> another binding file.
> 
> No functional changes expected.

If you want to re-use trips, they should be rather moved to separate
YAML file... but anyway this should not be done per-driver bindings, but
in more general way. Either the problem - using one DTS for different
temperature grades - looks generic or is wrong at the core. In the first
option, the generic bindings should be fixed. In the second case - using
same DTS for different HW is not correct approach and why only thermal
should be specific? I can imagine that cooling devices might have
different settings, regulator voltages for DVFS could be a bit different...

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Francesco Dolcini]

Hello Krzysztof,
thanks for your comment, let me try to provide you some additional
background to better understand this change.

On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote:
> On 17/06/2022 00:08, Francesco Dolcini wrote:
> > Move `trips` definition to `#/$defs/trips-base` and just reference it
> > from the trips node. This allows to easily re-use this binding from
> > another binding file.
> > 
> > No functional changes expected.
> 
> If you want to re-use trips, they should be rather moved to separate
> YAML file...

Fine, this should not be a big deal to achieve. Let's agree on the rest
first, however.

> but anyway this should not be done per-driver bindings, but
> in more general way. Either the problem - using one DTS for different
> temperature grades - looks generic or is wrong at the core. In the first
> option, the generic bindings should be fixed. In the second case - using
> same DTS for different HW is not correct approach and why only thermal
> should be specific? I can imagine that cooling devices might have
> different settings, regulator voltages for DVFS could be a bit different...

Let me try to explain the problem I am trying to solve here.

Currently the imx-thermal driver harcode the critical trip threshold,
this trip point is read-only as it is considered a system property that
should not be changed and it is set to a value that is less than the
actual SoC maximum temperature. NO thermal_of driver used.

Because of that there are systems that cannot work on some valid
temperature range.

We are currently looking at a solution that would be backward compatible
with old device tree.

I proposed the following:
1- just increase the threshold to the actual max value allowed according
   to the SoC thermal grade. 

   As easy as 

-	data->temp_critical = data->temp_max - (1000 * 5);
+	data->temp_critical = data->temp_max;
   
   in drivers/thermal/imx_thermal.c 

   https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/

   It was not considered good enough by Lucas since this is a overall
   system design question, therefore should be configurable.

2- make the critical trip write-able from userspace/sysfs.

   Daniel is against this since critical trip point is a system
   property, not something the user should be allowed to change.

3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@toradex.com/

   Initially proposed by Daniel, but Marco did not like the idea.

4- New device tree property, fsl,tempmon-critical-offset, ditched also
   by Marco

5- The current solution in this patch, with the existing trip points
   that are hardcoded in the code exposed in the device tree as trips.


Ideally one could just implement the imx6/7 thermal sensor reading and
just make use of the thermal_of driver, however that would break
compatibility with a lot of existing system ... to me this is just a
no-go.

Adding only one set of thermal trip point in the dts (no thermal-grade
specific set) could work in some specific scenario, however it does not
work for me since I have the same dts files using different temperature
grade SoC. I would need to update this in the firmware before starting
Linux.

Krzysztof, what do you think? I would not mind to get back to one of
the more simpler approach I proposed.

Lucas, are you really that against the simple working solution I
proposed initially [1]? I feel like I am running in circles ...

Francesco

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Francesco Dolcini]

Hello Krzysztof,
thanks for your comment, let me try to provide you some additional
background to better understand this change.

On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote:
> On 17/06/2022 00:08, Francesco Dolcini wrote:
> > Move `trips` definition to `#/$defs/trips-base` and just reference it
> > from the trips node. This allows to easily re-use this binding from
> > another binding file.
> > 
> > No functional changes expected.
> 
> If you want to re-use trips, they should be rather moved to separate
> YAML file...

Fine, this should not be a big deal to achieve. Let's agree on the rest
first, however.

> but anyway this should not be done per-driver bindings, but
> in more general way. Either the problem - using one DTS for different
> temperature grades - looks generic or is wrong at the core. In the first
> option, the generic bindings should be fixed. In the second case - using
> same DTS for different HW is not correct approach and why only thermal
> should be specific? I can imagine that cooling devices might have
> different settings, regulator voltages for DVFS could be a bit different...

Let me try to explain the problem I am trying to solve here.

Currently the imx-thermal driver harcode the critical trip threshold,
this trip point is read-only as it is considered a system property that
should not be changed and it is set to a value that is less than the
actual SoC maximum temperature. NO thermal_of driver used.

Because of that there are systems that cannot work on some valid
temperature range.

We are currently looking at a solution that would be backward compatible
with old device tree.

I proposed the following:
1- just increase the threshold to the actual max value allowed according
   to the SoC thermal grade. 

   As easy as 

-	data->temp_critical = data->temp_max - (1000 * 5);
+	data->temp_critical = data->temp_max;
   
   in drivers/thermal/imx_thermal.c 

   https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/

   It was not considered good enough by Lucas since this is a overall
   system design question, therefore should be configurable.

2- make the critical trip write-able from userspace/sysfs.

   Daniel is against this since critical trip point is a system
   property, not something the user should be allowed to change.

3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@toradex.com/

   Initially proposed by Daniel, but Marco did not like the idea.

4- New device tree property, fsl,tempmon-critical-offset, ditched also
   by Marco

5- The current solution in this patch, with the existing trip points
   that are hardcoded in the code exposed in the device tree as trips.


Ideally one could just implement the imx6/7 thermal sensor reading and
just make use of the thermal_of driver, however that would break
compatibility with a lot of existing system ... to me this is just a
no-go.

Adding only one set of thermal trip point in the dts (no thermal-grade
specific set) could work in some specific scenario, however it does not
work for me since I have the same dts files using different temperature
grade SoC. I would need to update this in the firmware before starting
Linux.

Krzysztof, what do you think? I would not mind to get back to one of
the more simpler approach I proposed.

Lucas, are you really that against the simple working solution I
proposed initially [1]? I feel like I am running in circles ...

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Krzysztof Kozlowski]

On 20/06/2022 17:48, Francesco Dolcini wrote:
> Hello Krzysztof,
> thanks for your comment, let me try to provide you some additional
> background to better understand this change.
> 
> On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote:
>> On 17/06/2022 00:08, Francesco Dolcini wrote:
>>> Move `trips` definition to `#/$defs/trips-base` and just reference it
>>> from the trips node. This allows to easily re-use this binding from
>>> another binding file.
>>>
>>> No functional changes expected.
>>
>> If you want to re-use trips, they should be rather moved to separate
>> YAML file...
> 
> Fine, this should not be a big deal to achieve. Let's agree on the rest
> first, however.
> 
>> but anyway this should not be done per-driver bindings, but
>> in more general way. Either the problem - using one DTS for different
>> temperature grades - looks generic or is wrong at the core. In the first
>> option, the generic bindings should be fixed. In the second case - using
>> same DTS for different HW is not correct approach and why only thermal
>> should be specific? I can imagine that cooling devices might have
>> different settings, regulator voltages for DVFS could be a bit different...
> 
> Let me try to explain the problem I am trying to solve here.
> 
> Currently the imx-thermal driver harcode the critical trip threshold,
> this trip point is read-only as it is considered a system property that
> should not be changed and it is set to a value that is less than the
> actual SoC maximum temperature. NO thermal_of driver used.
> 
> Because of that there are systems that cannot work on some valid
> temperature range.
> 
> We are currently looking at a solution that would be backward compatible
> with old device tree.
> 
> I proposed the following:
> 1- just increase the threshold to the actual max value allowed according
>    to the SoC thermal grade. 
> 
>    As easy as 
> 
> -	data->temp_critical = data->temp_max - (1000 * 5);
> +	data->temp_critical = data->temp_max;
>    
>    in drivers/thermal/imx_thermal.c 
> 
>    https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/
> 
>    It was not considered good enough by Lucas since this is a overall
>    system design question, therefore should be configurable.
> 
> 2- make the critical trip write-able from userspace/sysfs.
> 
>    Daniel is against this since critical trip point is a system
>    property, not something the user should be allowed to change.
> 
> 3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@toradex.com/
> 
>    Initially proposed by Daniel, but Marco did not like the idea.
> 
> 4- New device tree property, fsl,tempmon-critical-offset, ditched also
>    by Marco
> 
> 5- The current solution in this patch, with the existing trip points
>    that are hardcoded in the code exposed in the device tree as trips.

Thanks for the explanation, I see the problem.

> 
> Ideally one could just implement the imx6/7 thermal sensor reading and
> just make use of the thermal_of driver, however that would break
> compatibility with a lot of existing system ... to me this is just a
> no-go.

This I did not understand...  What is not implemented in thermal sensor
which would solve the issue? And why it cannot be implemented in
backwards compatible way?
Your change is also not backwards friendly, which means existing boards
(old DTS) will not receive the update.

> Adding only one set of thermal trip point in the dts (no thermal-grade
> specific set) could work in some specific scenario, however it does not
> work for me since I have the same dts files using different temperature
> grade SoC. I would need to update this in the firmware before starting
> Linux.

Usually the bootloader loads the overlay and this is recommended
approach to runtime tweaking DTB for some variant.

> 
> Krzysztof, what do you think? I would not mind to get back to one of
> the more simpler approach I proposed.

As I said, I see the problem, but I am not sure that solution is
correct. I can also rephrase the solution to a such one: "I want to
support iMX6 and iMX7 with one DTS, so I will embed all properties from
both DTS into one DTS and then during boot I will read soc-id register
and choose some subset of the properties"

No way...

I also brought in previous reply trouble with regulator voltages or some
other electric-properties. I don't want several duplicated properties
per different variants of the same SoC.

1. If the devices are fully compatible, use one DTS. If you can squeeze
different variants into the same DTS without any duplication so that
entire DTS is used 100% by both variants - sure no, problem, less code.

2. If the devices are in general compatible but have discoverable
differences, use one DTS, discover the differences and apply them
dynamically via driver (e.g. read the temperature offset from some
nvmem/OTP).

3. If the devices are partially the same but have differences, you can
use overlays for that differences. This is quite flexible and clean
solution as it also clearly documents the hardware in DTS and its overlays.

4. In all other options devices are different, so I expect different DTS.

We had similar cases already in the past - some SoC versions could work
on higher frequencies with higher voltages
(arch/arm/boot/dts/exynos4412-prime.dtsi). These variants identified as
exactly the same SoC as the earlier/slower one. You cannot use one DTS
for them. We have chosen different DTS.

For Samsung other case, same SoC comes in different bins with slightly
different voltages for CPU/memory. Same frequencies but different
voltages. This might be the closest to your problem. This was detectable
runtime, so we had one DTS and we adjusted the voltages based on static
tables in the driver (exynos5422-asv.c).

> Lucas, are you really that against the simple working solution I
> proposed initially [1]? I feel like I am running in circles ...

Yes, because it is not generic and skips other similar cases, like
regulator voltages or battery properties. I can easily imagine that next
week someone comes with duplicated opp tables, then duplicated voltages,
then duplicated CPU nodes and finally we have one DTS for imx6 and imx7
but everything is in multiple variants. :)
Also I am against because DTS describes one hardware, not multiple
different variants.

Best regards,
Krzysztof

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Krzysztof Kozlowski]

On 20/06/2022 17:48, Francesco Dolcini wrote:
> Hello Krzysztof,
> thanks for your comment, let me try to provide you some additional
> background to better understand this change.
> 
> On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote:
>> On 17/06/2022 00:08, Francesco Dolcini wrote:
>>> Move `trips` definition to `#/$defs/trips-base` and just reference it
>>> from the trips node. This allows to easily re-use this binding from
>>> another binding file.
>>>
>>> No functional changes expected.
>>
>> If you want to re-use trips, they should be rather moved to separate
>> YAML file...
> 
> Fine, this should not be a big deal to achieve. Let's agree on the rest
> first, however.
> 
>> but anyway this should not be done per-driver bindings, but
>> in more general way. Either the problem - using one DTS for different
>> temperature grades - looks generic or is wrong at the core. In the first
>> option, the generic bindings should be fixed. In the second case - using
>> same DTS for different HW is not correct approach and why only thermal
>> should be specific? I can imagine that cooling devices might have
>> different settings, regulator voltages for DVFS could be a bit different...
> 
> Let me try to explain the problem I am trying to solve here.
> 
> Currently the imx-thermal driver harcode the critical trip threshold,
> this trip point is read-only as it is considered a system property that
> should not be changed and it is set to a value that is less than the
> actual SoC maximum temperature. NO thermal_of driver used.
> 
> Because of that there are systems that cannot work on some valid
> temperature range.
> 
> We are currently looking at a solution that would be backward compatible
> with old device tree.
> 
> I proposed the following:
> 1- just increase the threshold to the actual max value allowed according
>    to the SoC thermal grade. 
> 
>    As easy as 
> 
> -	data->temp_critical = data->temp_max - (1000 * 5);
> +	data->temp_critical = data->temp_max;
>    
>    in drivers/thermal/imx_thermal.c 
> 
>    https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/
> 
>    It was not considered good enough by Lucas since this is a overall
>    system design question, therefore should be configurable.
> 
> 2- make the critical trip write-able from userspace/sysfs.
> 
>    Daniel is against this since critical trip point is a system
>    property, not something the user should be allowed to change.
> 
> 3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@toradex.com/
> 
>    Initially proposed by Daniel, but Marco did not like the idea.
> 
> 4- New device tree property, fsl,tempmon-critical-offset, ditched also
>    by Marco
> 
> 5- The current solution in this patch, with the existing trip points
>    that are hardcoded in the code exposed in the device tree as trips.

Thanks for the explanation, I see the problem.

> 
> Ideally one could just implement the imx6/7 thermal sensor reading and
> just make use of the thermal_of driver, however that would break
> compatibility with a lot of existing system ... to me this is just a
> no-go.

This I did not understand...  What is not implemented in thermal sensor
which would solve the issue? And why it cannot be implemented in
backwards compatible way?
Your change is also not backwards friendly, which means existing boards
(old DTS) will not receive the update.

> Adding only one set of thermal trip point in the dts (no thermal-grade
> specific set) could work in some specific scenario, however it does not
> work for me since I have the same dts files using different temperature
> grade SoC. I would need to update this in the firmware before starting
> Linux.

Usually the bootloader loads the overlay and this is recommended
approach to runtime tweaking DTB for some variant.

> 
> Krzysztof, what do you think? I would not mind to get back to one of
> the more simpler approach I proposed.

As I said, I see the problem, but I am not sure that solution is
correct. I can also rephrase the solution to a such one: "I want to
support iMX6 and iMX7 with one DTS, so I will embed all properties from
both DTS into one DTS and then during boot I will read soc-id register
and choose some subset of the properties"

No way...

I also brought in previous reply trouble with regulator voltages or some
other electric-properties. I don't want several duplicated properties
per different variants of the same SoC.

1. If the devices are fully compatible, use one DTS. If you can squeeze
different variants into the same DTS without any duplication so that
entire DTS is used 100% by both variants - sure no, problem, less code.

2. If the devices are in general compatible but have discoverable
differences, use one DTS, discover the differences and apply them
dynamically via driver (e.g. read the temperature offset from some
nvmem/OTP).

3. If the devices are partially the same but have differences, you can
use overlays for that differences. This is quite flexible and clean
solution as it also clearly documents the hardware in DTS and its overlays.

4. In all other options devices are different, so I expect different DTS.

We had similar cases already in the past - some SoC versions could work
on higher frequencies with higher voltages
(arch/arm/boot/dts/exynos4412-prime.dtsi). These variants identified as
exactly the same SoC as the earlier/slower one. You cannot use one DTS
for them. We have chosen different DTS.

For Samsung other case, same SoC comes in different bins with slightly
different voltages for CPU/memory. Same frequencies but different
voltages. This might be the closest to your problem. This was detectable
runtime, so we had one DTS and we adjusted the voltages based on static
tables in the driver (exynos5422-asv.c).

> Lucas, are you really that against the simple working solution I
> proposed initially [1]? I feel like I am running in circles ...

Yes, because it is not generic and skips other similar cases, like
regulator voltages or battery properties. I can easily imagine that next
week someone comes with duplicated opp tables, then duplicated voltages,
then duplicated CPU nodes and finally we have one DTS for imx6 and imx7
but everything is in multiple variants. :)
Also I am against because DTS describes one hardware, not multiple
different variants.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Francesco Dolcini]

On Mon, Jun 20, 2022 at 06:44:23PM +0200, Krzysztof Kozlowski wrote:
> On 20/06/2022 17:48, Francesco Dolcini wrote:
> > On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote:
> >> but anyway this should not be done per-driver bindings, but
> >> in more general way. Either the problem - using one DTS for different
> >> temperature grades - looks generic or is wrong at the core. In the first
> >> option, the generic bindings should be fixed. In the second case - using
> >> same DTS for different HW is not correct approach and why only thermal
> >> should be specific? I can imagine that cooling devices might have
> >> different settings, regulator voltages for DVFS could be a bit different...
> > 
> > Let me try to explain the problem I am trying to solve here.
> > 
> > Currently the imx-thermal driver harcode the critical trip threshold,
> > this trip point is read-only as it is considered a system property that
> > should not be changed and it is set to a value that is less than the
> > actual SoC maximum temperature. NO thermal_of driver used.
> > 
> > Because of that there are systems that cannot work on some valid
> > temperature range.
> > 
> > We are currently looking at a solution that would be backward compatible
> > with old device tree.
> > 
> > I proposed the following:
> > 1- just increase the threshold to the actual max value allowed according
> >    to the SoC thermal grade. 
> > 
> >    As easy as 
> > 
> > -	data->temp_critical = data->temp_max - (1000 * 5);
> > +	data->temp_critical = data->temp_max;
> >    
> >    in drivers/thermal/imx_thermal.c 
> > 
> >    https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/
> > 
> >    It was not considered good enough by Lucas since this is a overall
> >    system design question, therefore should be configurable.
> > 
> > 2- make the critical trip write-able from userspace/sysfs.
> > 
> >    Daniel is against this since critical trip point is a system
> >    property, not something the user should be allowed to change.
> > 
> > 3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@toradex.com/
> > 
> >    Initially proposed by Daniel, but Marco did not like the idea.
> > 
> > 4- New device tree property, fsl,tempmon-critical-offset, ditched also
> >    by Marco
> > 
> > 5- The current solution in this patch, with the existing trip points
> >    that are hardcoded in the code exposed in the device tree as trips.
> 
> Thanks for the explanation, I see the problem.
> 
> > 
> > Ideally one could just implement the imx6/7 thermal sensor reading and
> > just make use of the thermal_of driver, however that would break
> > compatibility with a lot of existing system ... to me this is just a
> > no-go.
> 
> This I did not understand...  What is not implemented in thermal sensor
> which would solve the issue? And why it cannot be implemented in
> backwards compatible way?

Currently the imx_thermal driver defines its own trip points. How would
you change the code to work with old device tree binaries using the
generic thermal_of driver? imx_thermal would need to be changed to be a
thermal sensor device, all the thermal trip point code removed.
The driver is using thermal_zone_device_register().

Maybe I am missing an obvious solution, just correct me if I am wrong.


> Your change is also not backwards friendly, which means existing boards
> (old DTS) will not receive the update.
The change proposed in this series is 100% backward compatible,
the code-defined trip point are optionally overwritten by the dts.


> > Adding only one set of thermal trip point in the dts (no thermal-grade
> > specific set) could work in some specific scenario, however it does not
> > work for me since I have the same dts files using different temperature
> > grade SoC. I would need to update this in the firmware before starting
> > Linux.
> 
> 2. If the devices are in general compatible but have discoverable
> differences, use one DTS, discover the differences and apply them
> dynamically via driver (e.g. read the temperature offset from some
> nvmem/OTP).

Yes, of course, I agree.
That would work and it would be a reasonable approach in general, but it
has one big drawback, it will force an update on the firmware on
well-established products. Anyway, would you accept a change on the
thermal_imx driver using a single set of trips from the dts, but not
using the thermal_of driver?

> > Lucas, are you really that against the simple working solution I
> > proposed initially [1]? I feel like I am running in circles ...
> 
> Yes, because it is not generic and skips other similar cases, like
> regulator voltages or battery properties. I can easily imagine that next
> week someone comes with duplicated opp tables, then duplicated voltages,
> then duplicated CPU nodes and finally we have one DTS for imx6 and imx7
> but everything is in multiple variants. :)

The patch I proposed in [1] was just making the hard-coded threshold
more reasonable, instead of setting the critical threshold to max-5 to
just max. Your concern here does not really applies.

Here the patch https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/

Francesco



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Francesco Dolcini]

On Mon, Jun 20, 2022 at 06:44:23PM +0200, Krzysztof Kozlowski wrote:
> On 20/06/2022 17:48, Francesco Dolcini wrote:
> > On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote:
> >> but anyway this should not be done per-driver bindings, but
> >> in more general way. Either the problem - using one DTS for different
> >> temperature grades - looks generic or is wrong at the core. In the first
> >> option, the generic bindings should be fixed. In the second case - using
> >> same DTS for different HW is not correct approach and why only thermal
> >> should be specific? I can imagine that cooling devices might have
> >> different settings, regulator voltages for DVFS could be a bit different...
> > 
> > Let me try to explain the problem I am trying to solve here.
> > 
> > Currently the imx-thermal driver harcode the critical trip threshold,
> > this trip point is read-only as it is considered a system property that
> > should not be changed and it is set to a value that is less than the
> > actual SoC maximum temperature. NO thermal_of driver used.
> > 
> > Because of that there are systems that cannot work on some valid
> > temperature range.
> > 
> > We are currently looking at a solution that would be backward compatible
> > with old device tree.
> > 
> > I proposed the following:
> > 1- just increase the threshold to the actual max value allowed according
> >    to the SoC thermal grade. 
> > 
> >    As easy as 
> > 
> > -	data->temp_critical = data->temp_max - (1000 * 5);
> > +	data->temp_critical = data->temp_max;
> >    
> >    in drivers/thermal/imx_thermal.c 
> > 
> >    https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/
> > 
> >    It was not considered good enough by Lucas since this is a overall
> >    system design question, therefore should be configurable.
> > 
> > 2- make the critical trip write-able from userspace/sysfs.
> > 
> >    Daniel is against this since critical trip point is a system
> >    property, not something the user should be allowed to change.
> > 
> > 3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@toradex.com/
> > 
> >    Initially proposed by Daniel, but Marco did not like the idea.
> > 
> > 4- New device tree property, fsl,tempmon-critical-offset, ditched also
> >    by Marco
> > 
> > 5- The current solution in this patch, with the existing trip points
> >    that are hardcoded in the code exposed in the device tree as trips.
> 
> Thanks for the explanation, I see the problem.
> 
> > 
> > Ideally one could just implement the imx6/7 thermal sensor reading and
> > just make use of the thermal_of driver, however that would break
> > compatibility with a lot of existing system ... to me this is just a
> > no-go.
> 
> This I did not understand...  What is not implemented in thermal sensor
> which would solve the issue? And why it cannot be implemented in
> backwards compatible way?

Currently the imx_thermal driver defines its own trip points. How would
you change the code to work with old device tree binaries using the
generic thermal_of driver? imx_thermal would need to be changed to be a
thermal sensor device, all the thermal trip point code removed.
The driver is using thermal_zone_device_register().

Maybe I am missing an obvious solution, just correct me if I am wrong.


> Your change is also not backwards friendly, which means existing boards
> (old DTS) will not receive the update.
The change proposed in this series is 100% backward compatible,
the code-defined trip point are optionally overwritten by the dts.


> > Adding only one set of thermal trip point in the dts (no thermal-grade
> > specific set) could work in some specific scenario, however it does not
> > work for me since I have the same dts files using different temperature
> > grade SoC. I would need to update this in the firmware before starting
> > Linux.
> 
> 2. If the devices are in general compatible but have discoverable
> differences, use one DTS, discover the differences and apply them
> dynamically via driver (e.g. read the temperature offset from some
> nvmem/OTP).

Yes, of course, I agree.
That would work and it would be a reasonable approach in general, but it
has one big drawback, it will force an update on the firmware on
well-established products. Anyway, would you accept a change on the
thermal_imx driver using a single set of trips from the dts, but not
using the thermal_of driver?

> > Lucas, are you really that against the simple working solution I
> > proposed initially [1]? I feel like I am running in circles ...
> 
> Yes, because it is not generic and skips other similar cases, like
> regulator voltages or battery properties. I can easily imagine that next
> week someone comes with duplicated opp tables, then duplicated voltages,
> then duplicated CPU nodes and finally we have one DTS for imx6 and imx7
> but everything is in multiple variants. :)

The patch I proposed in [1] was just making the hard-coded threshold
more reasonable, instead of setting the critical threshold to max-5 to
just max. Your concern here does not really applies.

Here the patch https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/

Francesco

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Krzysztof Kozlowski]

On 20/06/2022 19:43, Francesco Dolcini wrote:
> On Mon, Jun 20, 2022 at 06:44:23PM +0200, Krzysztof Kozlowski wrote:
>> On 20/06/2022 17:48, Francesco Dolcini wrote:
>>> On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote:
>>>> but anyway this should not be done per-driver bindings, but
>>>> in more general way. Either the problem - using one DTS for different
>>>> temperature grades - looks generic or is wrong at the core. In the first
>>>> option, the generic bindings should be fixed. In the second case - using
>>>> same DTS for different HW is not correct approach and why only thermal
>>>> should be specific? I can imagine that cooling devices might have
>>>> different settings, regulator voltages for DVFS could be a bit different...
>>>
>>> Let me try to explain the problem I am trying to solve here.
>>>
>>> Currently the imx-thermal driver harcode the critical trip threshold,
>>> this trip point is read-only as it is considered a system property that
>>> should not be changed and it is set to a value that is less than the
>>> actual SoC maximum temperature. NO thermal_of driver used.
>>>
>>> Because of that there are systems that cannot work on some valid
>>> temperature range.
>>>
>>> We are currently looking at a solution that would be backward compatible
>>> with old device tree.
>>>
>>> I proposed the following:
>>> 1- just increase the threshold to the actual max value allowed according
>>>    to the SoC thermal grade. 
>>>
>>>    As easy as 
>>>
>>> -	data->temp_critical = data->temp_max - (1000 * 5);
>>> +	data->temp_critical = data->temp_max;
>>>    
>>>    in drivers/thermal/imx_thermal.c 
>>>
>>>    https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/
>>>
>>>    It was not considered good enough by Lucas since this is a overall
>>>    system design question, therefore should be configurable.
>>>
>>> 2- make the critical trip write-able from userspace/sysfs.
>>>
>>>    Daniel is against this since critical trip point is a system
>>>    property, not something the user should be allowed to change.
>>>
>>> 3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@toradex.com/
>>>
>>>    Initially proposed by Daniel, but Marco did not like the idea.
>>>
>>> 4- New device tree property, fsl,tempmon-critical-offset, ditched also
>>>    by Marco
>>>
>>> 5- The current solution in this patch, with the existing trip points
>>>    that are hardcoded in the code exposed in the device tree as trips.
>>
>> Thanks for the explanation, I see the problem.
>>
>>>
>>> Ideally one could just implement the imx6/7 thermal sensor reading and
>>> just make use of the thermal_of driver, however that would break
>>> compatibility with a lot of existing system ... to me this is just a
>>> no-go.
>>
>> This I did not understand...  What is not implemented in thermal sensor
>> which would solve the issue? And why it cannot be implemented in
>> backwards compatible way?
> 
> Currently the imx_thermal driver defines its own trip points. How would
> you change the code to work with old device tree binaries using the
> generic thermal_of driver? imx_thermal would need to be changed to be a
> thermal sensor device, all the thermal trip point code removed.
> The driver is using thermal_zone_device_register().
> 
> Maybe I am missing an obvious solution, just correct me if I am wrong.

Probably you would need to support both solutions in the same driver,
based on presence of thermal-sensor-cells property. It won't be
particularly easy code, but maybe it is worth anyway... I am quite
surprised to see that IMX thermal driver does not use generic framework
and does not support generic bindings.

>> Your change is also not backwards friendly, which means existing boards
>> (old DTS) will not receive the update.
> The change proposed in this series is 100% backward compatible,
> the code-defined trip point are optionally overwritten by the dts.
> 
> 
>>> Adding only one set of thermal trip point in the dts (no thermal-grade
>>> specific set) could work in some specific scenario, however it does not
>>> work for me since I have the same dts files using different temperature
>>> grade SoC. I would need to update this in the firmware before starting
>>> Linux.
>>
>> 2. If the devices are in general compatible but have discoverable
>> differences, use one DTS, discover the differences and apply them
>> dynamically via driver (e.g. read the temperature offset from some
>> nvmem/OTP).
> 
> Yes, of course, I agree.
> That would work and it would be a reasonable approach in general, but it
> has one big drawback, it will force an update on the firmware on
> well-established products. Anyway, would you accept a change on the
> thermal_imx driver using a single set of trips from the dts, but not
> using the thermal_of driver?

More comments were not concering the Linux IMX thermal implementation
but rather bindings. From my point of view, you can use the same generic
thermal bindings even if your implementation does not use thermal_of.

> 
>>> Lucas, are you really that against the simple working solution I
>>> proposed initially [1]? I feel like I am running in circles ...
>>
>> Yes, because it is not generic and skips other similar cases, like
>> regulator voltages or battery properties. I can easily imagine that next
>> week someone comes with duplicated opp tables, then duplicated voltages,
>> then duplicated CPU nodes and finally we have one DTS for imx6 and imx7
>> but everything is in multiple variants. :)
> 
> The patch I proposed in [1] was just making the hard-coded threshold
> more reasonable, instead of setting the critical threshold to max-5 to
> just max. Your concern here does not really applies.
> 
> Here the patch https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/


Ah, that comment was not to me but to Lucas.

Best regards,
Krzysztof

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Krzysztof Kozlowski]

On 20/06/2022 19:43, Francesco Dolcini wrote:
> On Mon, Jun 20, 2022 at 06:44:23PM +0200, Krzysztof Kozlowski wrote:
>> On 20/06/2022 17:48, Francesco Dolcini wrote:
>>> On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote:
>>>> but anyway this should not be done per-driver bindings, but
>>>> in more general way. Either the problem - using one DTS for different
>>>> temperature grades - looks generic or is wrong at the core. In the first
>>>> option, the generic bindings should be fixed. In the second case - using
>>>> same DTS for different HW is not correct approach and why only thermal
>>>> should be specific? I can imagine that cooling devices might have
>>>> different settings, regulator voltages for DVFS could be a bit different...
>>>
>>> Let me try to explain the problem I am trying to solve here.
>>>
>>> Currently the imx-thermal driver harcode the critical trip threshold,
>>> this trip point is read-only as it is considered a system property that
>>> should not be changed and it is set to a value that is less than the
>>> actual SoC maximum temperature. NO thermal_of driver used.
>>>
>>> Because of that there are systems that cannot work on some valid
>>> temperature range.
>>>
>>> We are currently looking at a solution that would be backward compatible
>>> with old device tree.
>>>
>>> I proposed the following:
>>> 1- just increase the threshold to the actual max value allowed according
>>>    to the SoC thermal grade. 
>>>
>>>    As easy as 
>>>
>>> -	data->temp_critical = data->temp_max - (1000 * 5);
>>> +	data->temp_critical = data->temp_max;
>>>    
>>>    in drivers/thermal/imx_thermal.c 
>>>
>>>    https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/
>>>
>>>    It was not considered good enough by Lucas since this is a overall
>>>    system design question, therefore should be configurable.
>>>
>>> 2- make the critical trip write-able from userspace/sysfs.
>>>
>>>    Daniel is against this since critical trip point is a system
>>>    property, not something the user should be allowed to change.
>>>
>>> 3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@toradex.com/
>>>
>>>    Initially proposed by Daniel, but Marco did not like the idea.
>>>
>>> 4- New device tree property, fsl,tempmon-critical-offset, ditched also
>>>    by Marco
>>>
>>> 5- The current solution in this patch, with the existing trip points
>>>    that are hardcoded in the code exposed in the device tree as trips.
>>
>> Thanks for the explanation, I see the problem.
>>
>>>
>>> Ideally one could just implement the imx6/7 thermal sensor reading and
>>> just make use of the thermal_of driver, however that would break
>>> compatibility with a lot of existing system ... to me this is just a
>>> no-go.
>>
>> This I did not understand...  What is not implemented in thermal sensor
>> which would solve the issue? And why it cannot be implemented in
>> backwards compatible way?
> 
> Currently the imx_thermal driver defines its own trip points. How would
> you change the code to work with old device tree binaries using the
> generic thermal_of driver? imx_thermal would need to be changed to be a
> thermal sensor device, all the thermal trip point code removed.
> The driver is using thermal_zone_device_register().
> 
> Maybe I am missing an obvious solution, just correct me if I am wrong.

Probably you would need to support both solutions in the same driver,
based on presence of thermal-sensor-cells property. It won't be
particularly easy code, but maybe it is worth anyway... I am quite
surprised to see that IMX thermal driver does not use generic framework
and does not support generic bindings.

>> Your change is also not backwards friendly, which means existing boards
>> (old DTS) will not receive the update.
> The change proposed in this series is 100% backward compatible,
> the code-defined trip point are optionally overwritten by the dts.
> 
> 
>>> Adding only one set of thermal trip point in the dts (no thermal-grade
>>> specific set) could work in some specific scenario, however it does not
>>> work for me since I have the same dts files using different temperature
>>> grade SoC. I would need to update this in the firmware before starting
>>> Linux.
>>
>> 2. If the devices are in general compatible but have discoverable
>> differences, use one DTS, discover the differences and apply them
>> dynamically via driver (e.g. read the temperature offset from some
>> nvmem/OTP).
> 
> Yes, of course, I agree.
> That would work and it would be a reasonable approach in general, but it
> has one big drawback, it will force an update on the firmware on
> well-established products. Anyway, would you accept a change on the
> thermal_imx driver using a single set of trips from the dts, but not
> using the thermal_of driver?

More comments were not concering the Linux IMX thermal implementation
but rather bindings. From my point of view, you can use the same generic
thermal bindings even if your implementation does not use thermal_of.

> 
>>> Lucas, are you really that against the simple working solution I
>>> proposed initially [1]? I feel like I am running in circles ...
>>
>> Yes, because it is not generic and skips other similar cases, like
>> regulator voltages or battery properties. I can easily imagine that next
>> week someone comes with duplicated opp tables, then duplicated voltages,
>> then duplicated CPU nodes and finally we have one DTS for imx6 and imx7
>> but everything is in multiple variants. :)
> 
> The patch I proposed in [1] was just making the hard-coded threshold
> more reasonable, instead of setting the critical threshold to max-5 to
> just max. Your concern here does not really applies.
> 
> Here the patch https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/


Ah, that comment was not to me but to Lucas.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Francesco Dolcini]

Hello Krzysztof,
thanks for the discussion and the review.

On Mon, Jun 20, 2022 at 08:05:48PM +0200, Krzysztof Kozlowski wrote:
> On 20/06/2022 19:43, Francesco Dolcini wrote:
> > On Mon, Jun 20, 2022 at 06:44:23PM +0200, Krzysztof Kozlowski wrote:
> >> On 20/06/2022 17:48, Francesco Dolcini wrote:
> >>> Ideally one could just implement the imx6/7 thermal sensor reading and
> >>> just make use of the thermal_of driver, however that would break
> >>> compatibility with a lot of existing system ... to me this is just a
> >>> no-go.
> >>
> >> This I did not understand...  What is not implemented in thermal sensor
> >> which would solve the issue? And why it cannot be implemented in
> >> backwards compatible way?
> > 
> > Currently the imx_thermal driver defines its own trip points. How would
> > you change the code to work with old device tree binaries using the
> > generic thermal_of driver? imx_thermal would need to be changed to be a
> > thermal sensor device, all the thermal trip point code removed.
> > The driver is using thermal_zone_device_register().
> > 
> > Maybe I am missing an obvious solution, just correct me if I am wrong.
> 
> Probably you would need to support both solutions in the same driver,
> based on presence of thermal-sensor-cells property. It won't be
> particularly easy code, but maybe it is worth anyway... I am quite
> surprised to see that IMX thermal driver does not use generic framework
> and does not support generic bindings.

#thermal-sensor-cells is already present in all dtsi using the
imx_thermal driver without reason (see my previous comment on the
topic [0]).

From my point of view the situation is somehow a mess, not sure how to
proceed to be honest.

Francesco

[0] https://lore.kernel.org/all/acbf8ed3-0b8c-a0b2-88ef-7b13ad0908d5@linaro.org/

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Francesco Dolcini]

Hello Krzysztof,
thanks for the discussion and the review.

On Mon, Jun 20, 2022 at 08:05:48PM +0200, Krzysztof Kozlowski wrote:
> On 20/06/2022 19:43, Francesco Dolcini wrote:
> > On Mon, Jun 20, 2022 at 06:44:23PM +0200, Krzysztof Kozlowski wrote:
> >> On 20/06/2022 17:48, Francesco Dolcini wrote:
> >>> Ideally one could just implement the imx6/7 thermal sensor reading and
> >>> just make use of the thermal_of driver, however that would break
> >>> compatibility with a lot of existing system ... to me this is just a
> >>> no-go.
> >>
> >> This I did not understand...  What is not implemented in thermal sensor
> >> which would solve the issue? And why it cannot be implemented in
> >> backwards compatible way?
> > 
> > Currently the imx_thermal driver defines its own trip points. How would
> > you change the code to work with old device tree binaries using the
> > generic thermal_of driver? imx_thermal would need to be changed to be a
> > thermal sensor device, all the thermal trip point code removed.
> > The driver is using thermal_zone_device_register().
> > 
> > Maybe I am missing an obvious solution, just correct me if I am wrong.
> 
> Probably you would need to support both solutions in the same driver,
> based on presence of thermal-sensor-cells property. It won't be
> particularly easy code, but maybe it is worth anyway... I am quite
> surprised to see that IMX thermal driver does not use generic framework
> and does not support generic bindings.

#thermal-sensor-cells is already present in all dtsi using the
imx_thermal driver without reason (see my previous comment on the
topic [0]).

From my point of view the situation is somehow a mess, not sure how to
proceed to be honest.

Francesco

[0] https://lore.kernel.org/all/acbf8ed3-0b8c-a0b2-88ef-7b13ad0908d5@linaro.org/


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Krzysztof Kozlowski]

On 20/06/2022 17:48, Francesco Dolcini wrote:
> Hello Krzysztof,
> thanks for your comment, let me try to provide you some additional
> background to better understand this change.
> 
> On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote:
>> On 17/06/2022 00:08, Francesco Dolcini wrote:
>>> Move `trips` definition to `#/$defs/trips-base` and just reference it
>>> from the trips node. This allows to easily re-use this binding from
>>> another binding file.
>>>
>>> No functional changes expected.
>>
>> If you want to re-use trips, they should be rather moved to separate
>> YAML file...
> 
> Fine, this should not be a big deal to achieve. Let's agree on the rest
> first, however.
> 
>> but anyway this should not be done per-driver bindings, but
>> in more general way. Either the problem - using one DTS for different
>> temperature grades - looks generic or is wrong at the core. In the first
>> option, the generic bindings should be fixed. In the second case - using
>> same DTS for different HW is not correct approach and why only thermal
>> should be specific? I can imagine that cooling devices might have
>> different settings, regulator voltages for DVFS could be a bit different...
> 
> Let me try to explain the problem I am trying to solve here.
> 
> Currently the imx-thermal driver harcode the critical trip threshold,
> this trip point is read-only as it is considered a system property that
> should not be changed and it is set to a value that is less than the
> actual SoC maximum temperature. NO thermal_of driver used.
> 
> Because of that there are systems that cannot work on some valid
> temperature range.
> 
> We are currently looking at a solution that would be backward compatible
> with old device tree.
> 
> I proposed the following:
> 1- just increase the threshold to the actual max value allowed according
>    to the SoC thermal grade. 
> 
>    As easy as 
> 
> -	data->temp_critical = data->temp_max - (1000 * 5);
> +	data->temp_critical = data->temp_max;
>    
>    in drivers/thermal/imx_thermal.c 
> 
>    https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/
> 
>    It was not considered good enough by Lucas since this is a overall
>    system design question, therefore should be configurable.
> 
> 2- make the critical trip write-able from userspace/sysfs.
> 
>    Daniel is against this since critical trip point is a system
>    property, not something the user should be allowed to change.
> 
> 3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@toradex.com/
> 
>    Initially proposed by Daniel, but Marco did not like the idea.
> 
> 4- New device tree property, fsl,tempmon-critical-offset, ditched also
>    by Marco
> 
> 5- The current solution in this patch, with the existing trip points
>    that are hardcoded in the code exposed in the device tree as trips.
> 
> 
> Ideally one could just implement the imx6/7 thermal sensor reading and
> just make use of the thermal_of driver, however that would break
> compatibility with a lot of existing system ... to me this is just a
> no-go.
> 
> Adding only one set of thermal trip point in the dts (no thermal-grade
> specific set) could work in some specific scenario, however it does not
> work for me since I have the same dts files using different temperature
> grade SoC. I would need to update this in the firmware before starting
> Linux.
> 
> Krzysztof, what do you think? I would not mind to get back to one of
> the more simpler approach I proposed.
> 
> Lucas, are you really that against the simple working solution I
> proposed initially [1]? I feel like I am running in circles ...

BTW, the link [1] was missing in your email, so I understood that you
meant this patchset. If [1] refers to something else, then we need to
discuss that something else.

Best regards,
Krzysztof

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Krzysztof Kozlowski]

On 20/06/2022 17:48, Francesco Dolcini wrote:
> Hello Krzysztof,
> thanks for your comment, let me try to provide you some additional
> background to better understand this change.
> 
> On Fri, Jun 17, 2022 at 06:02:39PM -0700, Krzysztof Kozlowski wrote:
>> On 17/06/2022 00:08, Francesco Dolcini wrote:
>>> Move `trips` definition to `#/$defs/trips-base` and just reference it
>>> from the trips node. This allows to easily re-use this binding from
>>> another binding file.
>>>
>>> No functional changes expected.
>>
>> If you want to re-use trips, they should be rather moved to separate
>> YAML file...
> 
> Fine, this should not be a big deal to achieve. Let's agree on the rest
> first, however.
> 
>> but anyway this should not be done per-driver bindings, but
>> in more general way. Either the problem - using one DTS for different
>> temperature grades - looks generic or is wrong at the core. In the first
>> option, the generic bindings should be fixed. In the second case - using
>> same DTS for different HW is not correct approach and why only thermal
>> should be specific? I can imagine that cooling devices might have
>> different settings, regulator voltages for DVFS could be a bit different...
> 
> Let me try to explain the problem I am trying to solve here.
> 
> Currently the imx-thermal driver harcode the critical trip threshold,
> this trip point is read-only as it is considered a system property that
> should not be changed and it is set to a value that is less than the
> actual SoC maximum temperature. NO thermal_of driver used.
> 
> Because of that there are systems that cannot work on some valid
> temperature range.
> 
> We are currently looking at a solution that would be backward compatible
> with old device tree.
> 
> I proposed the following:
> 1- just increase the threshold to the actual max value allowed according
>    to the SoC thermal grade. 
> 
>    As easy as 
> 
> -	data->temp_critical = data->temp_max - (1000 * 5);
> +	data->temp_critical = data->temp_max;
>    
>    in drivers/thermal/imx_thermal.c 
> 
>    https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/
> 
>    It was not considered good enough by Lucas since this is a overall
>    system design question, therefore should be configurable.
> 
> 2- make the critical trip write-able from userspace/sysfs.
> 
>    Daniel is against this since critical trip point is a system
>    property, not something the user should be allowed to change.
> 
> 3- kernel parameter: https://lore.kernel.org/all/20220516190001.147919-1-francesco.dolcini@toradex.com/
> 
>    Initially proposed by Daniel, but Marco did not like the idea.
> 
> 4- New device tree property, fsl,tempmon-critical-offset, ditched also
>    by Marco
> 
> 5- The current solution in this patch, with the existing trip points
>    that are hardcoded in the code exposed in the device tree as trips.
> 
> 
> Ideally one could just implement the imx6/7 thermal sensor reading and
> just make use of the thermal_of driver, however that would break
> compatibility with a lot of existing system ... to me this is just a
> no-go.
> 
> Adding only one set of thermal trip point in the dts (no thermal-grade
> specific set) could work in some specific scenario, however it does not
> work for me since I have the same dts files using different temperature
> grade SoC. I would need to update this in the firmware before starting
> Linux.
> 
> Krzysztof, what do you think? I would not mind to get back to one of
> the more simpler approach I proposed.
> 
> Lucas, are you really that against the simple working solution I
> proposed initially [1]? I feel like I am running in circles ...

BTW, the link [1] was missing in your email, so I understood that you
meant this patchset. If [1] refers to something else, then we need to
discuss that something else.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Francesco Dolcini]

On Mon, Jun 20, 2022 at 06:45:41PM +0200, Krzysztof Kozlowski wrote:
> On 20/06/2022 17:48, Francesco Dolcini wrote:
> > Lucas, are you really that against the simple working solution I
> > proposed initially [1]? I feel like I am running in circles ...
> 
> BTW, the link [1] was missing in your email, so I understood that you
> meant this patchset. If [1] refers to something else, then we need to
> discuss that something else.

We misunderstood each other, this is what I meant:

> 1- just increase the threshold to the actual max value allowed according
>    to the SoC thermal grade.
>
>    As easy as
>
> -     data->temp_critical = data->temp_max - (1000 * 5);
> +     data->temp_critical = data->temp_max;
>
>    in drivers/thermal/imx_thermal.c
>
>    https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/
>
>    It was not considered good enough by Lucas since this is a overall
>    system design question, therefore should be configurable.

Francesco

Re: [PATCH v2 1/9] dt-bindings: thermal: Define trips node in $defs

[Francesco Dolcini]

On Mon, Jun 20, 2022 at 06:45:41PM +0200, Krzysztof Kozlowski wrote:
> On 20/06/2022 17:48, Francesco Dolcini wrote:
> > Lucas, are you really that against the simple working solution I
> > proposed initially [1]? I feel like I am running in circles ...
> 
> BTW, the link [1] was missing in your email, so I understood that you
> meant this patchset. If [1] refers to something else, then we need to
> discuss that something else.

We misunderstood each other, this is what I meant:

> 1- just increase the threshold to the actual max value allowed according
>    to the SoC thermal grade.
>
>    As easy as
>
> -     data->temp_critical = data->temp_max - (1000 * 5);
> +     data->temp_critical = data->temp_max;
>
>    in drivers/thermal/imx_thermal.c
>
>    https://lore.kernel.org/all/20220420091300.179753-1-francesco.dolcini@toradex.com/
>
>    It was not considered good enough by Lucas since this is a overall
>    system design question, therefore should be configurable.

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/9] thermal: thermal: Export OF trip helper function

[Francesco Dolcini]

Export function that populate thermal trip struct from a of node to be
able to re-use it in thermal drivers different from thermal_of.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: fix build error, use EOPNOTSUPP
---
 drivers/thermal/thermal_core.h | 7 +++++++
 drivers/thermal/thermal_of.c   | 5 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 726e327b4205..7d429d299d82 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -155,6 +155,8 @@ int of_thermal_get_ntrips(struct thermal_zone_device *);
 bool of_thermal_is_trip_valid(struct thermal_zone_device *, int);
 const struct thermal_trip *
 of_thermal_get_trip_points(struct thermal_zone_device *);
+int thermal_of_populate_trip(struct device_node *np,
+			     struct thermal_trip *trip);
 #else
 static inline int of_parse_thermal_zones(void) { return 0; }
 static inline int of_thermal_get_ntrips(struct thermal_zone_device *tz)
@@ -171,6 +173,11 @@ of_thermal_get_trip_points(struct thermal_zone_device *tz)
 {
 	return NULL;
 }
+static inline int thermal_of_populate_trip(struct device_node *np,
+					   struct thermal_trip *trip)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 int thermal_zone_device_is_enabled(struct thermal_zone_device *tz);
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index b65d435cb92f..dcd6571a3871 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -817,8 +817,8 @@ static int thermal_of_get_trip_type(struct device_node *np,
  *
  * Return: 0 on success, proper error code otherwise
  */
-static int thermal_of_populate_trip(struct device_node *np,
-				    struct thermal_trip *trip)
+int thermal_of_populate_trip(struct device_node *np,
+			     struct thermal_trip *trip)
 {
 	int prop;
 	int ret;
@@ -849,6 +849,7 @@ static int thermal_of_populate_trip(struct device_node *np,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(thermal_of_populate_trip);
 
 /**
  * thermal_of_build_thermal_zone - parse and fill one thermal zone data
-- 
2.25.1

[PATCH v2 2/9] thermal: thermal: Export OF trip helper function

[Francesco Dolcini]

Export function that populate thermal trip struct from a of node to be
able to re-use it in thermal drivers different from thermal_of.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: fix build error, use EOPNOTSUPP
---
 drivers/thermal/thermal_core.h | 7 +++++++
 drivers/thermal/thermal_of.c   | 5 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 726e327b4205..7d429d299d82 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -155,6 +155,8 @@ int of_thermal_get_ntrips(struct thermal_zone_device *);
 bool of_thermal_is_trip_valid(struct thermal_zone_device *, int);
 const struct thermal_trip *
 of_thermal_get_trip_points(struct thermal_zone_device *);
+int thermal_of_populate_trip(struct device_node *np,
+			     struct thermal_trip *trip);
 #else
 static inline int of_parse_thermal_zones(void) { return 0; }
 static inline int of_thermal_get_ntrips(struct thermal_zone_device *tz)
@@ -171,6 +173,11 @@ of_thermal_get_trip_points(struct thermal_zone_device *tz)
 {
 	return NULL;
 }
+static inline int thermal_of_populate_trip(struct device_node *np,
+					   struct thermal_trip *trip)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 int thermal_zone_device_is_enabled(struct thermal_zone_device *tz);
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index b65d435cb92f..dcd6571a3871 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -817,8 +817,8 @@ static int thermal_of_get_trip_type(struct device_node *np,
  *
  * Return: 0 on success, proper error code otherwise
  */
-static int thermal_of_populate_trip(struct device_node *np,
-				    struct thermal_trip *trip)
+int thermal_of_populate_trip(struct device_node *np,
+			     struct thermal_trip *trip)
 {
 	int prop;
 	int ret;
@@ -849,6 +849,7 @@ static int thermal_of_populate_trip(struct device_node *np,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(thermal_of_populate_trip);
 
 /**
  * thermal_of_build_thermal_zone - parse and fill one thermal zone data
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/9] dt-bindings: thermal: imx: Add trips point

[Francesco Dolcini]

Add trips point to i.MX Thermal bindings for each temperature grade
(automotive, commercial, extended-commercial and industrial) to enable
specifying a different trip point than the hard-coded value.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 .../bindings/thermal/imx-thermal.yaml         | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/imx-thermal.yaml b/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
index 16b57f57d103..e6349e40d6c6 100644
--- a/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
@@ -54,6 +54,18 @@ properties:
   clocks:
     maxItems: 1
 
+patternProperties:
+  "^(automotive|commercial|extended-commercial|industrial)-thermal$":
+    type: object
+    description:
+      Thermal characteristics for each available temperature grade, this allows
+      to override the passive and critical trip points.
+    properties:
+      trips:
+        $ref: "thermal-zones.yaml#/$defs/trips-base"
+
+      additionalProperties: false
+
 required:
   - compatible
   - interrupts
@@ -98,5 +110,20 @@ examples:
              nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
              nvmem-cell-names = "calib", "temp_grade";
              clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
+
+             industrial-thermal {
+                 trips {
+                     temp_trip_passive_industrial: trip-point0 {
+                         temperature = <95000>;
+                         hysteresis = <0>;
+                         type = "passive";
+                     };
+                     temp_trip_crit_industrial: trip-point1 {
+                         temperature = <100000>;
+                         hysteresis = <0>;
+                         type = "critical";
+                     };
+                 };
+             };
         };
     };
-- 
2.25.1

[PATCH v2 3/9] dt-bindings: thermal: imx: Add trips point

[Francesco Dolcini]

Add trips point to i.MX Thermal bindings for each temperature grade
(automotive, commercial, extended-commercial and industrial) to enable
specifying a different trip point than the hard-coded value.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 .../bindings/thermal/imx-thermal.yaml         | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/imx-thermal.yaml b/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
index 16b57f57d103..e6349e40d6c6 100644
--- a/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
@@ -54,6 +54,18 @@ properties:
   clocks:
     maxItems: 1
 
+patternProperties:
+  "^(automotive|commercial|extended-commercial|industrial)-thermal$":
+    type: object
+    description:
+      Thermal characteristics for each available temperature grade, this allows
+      to override the passive and critical trip points.
+    properties:
+      trips:
+        $ref: "thermal-zones.yaml#/$defs/trips-base"
+
+      additionalProperties: false
+
 required:
   - compatible
   - interrupts
@@ -98,5 +110,20 @@ examples:
              nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
              nvmem-cell-names = "calib", "temp_grade";
              clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
+
+             industrial-thermal {
+                 trips {
+                     temp_trip_passive_industrial: trip-point0 {
+                         temperature = <95000>;
+                         hysteresis = <0>;
+                         type = "passive";
+                     };
+                     temp_trip_crit_industrial: trip-point1 {
+                         temperature = <100000>;
+                         hysteresis = <0>;
+                         type = "critical";
+                     };
+                 };
+             };
         };
     };
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/9] imx: thermal: Configure trip point from DT

[Francesco Dolcini]

Allow over-writing critical and passive trip point for each
temperature grade from the device tree, by default the pre-existing
hard-coded trip points are used.

This change enables configuring the system thermal characteristics into
the system-specific device tree instead of relying on global hard-coded
temperature thresholds that does not take into account the specific
system thermal design.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2:
 - return immediately if no thermal node present in the dts
 - use dev_info instead of dev_dbg if there is an invalid trip
 - additional comment in case passive trip point is higher than critical
---
 drivers/thermal/imx_thermal.c | 58 +++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 16663373b682..a964baf802fc 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -17,6 +17,8 @@
 #include <linux/nvmem-consumer.h>
 #include <linux/pm_runtime.h>
 
+#include "thermal_core.h"
+
 #define REG_SET		0x4
 #define REG_CLR		0x8
 #define REG_TOG		0xc
@@ -479,36 +481,92 @@ static int imx_init_calib(struct platform_device *pdev, u32 ocotp_ana1)
 	return 0;
 }
 
+static void imx_init_temp_from_of(struct platform_device *pdev, const char *name)
+{
+	struct imx_thermal_data *data = platform_get_drvdata(pdev);
+	struct device_node *thermal, *trips, *trip_point;
+
+	thermal = of_get_child_by_name(pdev->dev.of_node, name);
+	if (!thermal)
+		return;
+
+	trips = of_get_child_by_name(thermal, "trips");
+
+	for_each_child_of_node(trips, trip_point) {
+		struct thermal_trip t;
+
+		if (thermal_of_populate_trip(trip_point, &t))
+			continue;
+
+		switch (t.type) {
+		case THERMAL_TRIP_PASSIVE:
+			data->temp_passive = t.temperature;
+			break;
+		case THERMAL_TRIP_CRITICAL:
+			data->temp_critical = t.temperature;
+			break;
+		default:
+			dev_info(&pdev->dev, "Ignoring trip type %d\n", t.type);
+			break;
+		}
+	};
+
+	of_node_put(trips);
+	of_node_put(thermal);
+
+	if (data->temp_passive >= data->temp_critical) {
+		dev_warn(&pdev->dev,
+			 "passive trip point must be lower than critical, fixing it up\n");
+		/*
+		 * In case of misconfiguration set passive temperature to
+		 * 5°C less than critical, this seems like a reasonable
+		 * default and the same is done when no thermal trips are
+		 * available in the device tree.
+		 */
+		data->temp_passive = data->temp_critical - (1000 * 5);
+	}
+}
+
 static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0)
 {
 	struct imx_thermal_data *data = platform_get_drvdata(pdev);
+	const char *thermal_node_name;
 
 	/* The maximum die temp is specified by the Temperature Grade */
 	switch ((ocotp_mem0 >> 6) & 0x3) {
 	case 0: /* Commercial (0 to 95 °C) */
+		thermal_node_name = "commercial-thermal";
 		data->temp_grade = "Commercial";
 		data->temp_max = 95000;
 		break;
 	case 1: /* Extended Commercial (-20 °C to 105 °C) */
+		thermal_node_name = "extended-commercial-thermal";
 		data->temp_grade = "Extended Commercial";
 		data->temp_max = 105000;
 		break;
 	case 2: /* Industrial (-40 °C to 105 °C) */
+		thermal_node_name = "industrial-thermal";
 		data->temp_grade = "Industrial";
 		data->temp_max = 105000;
 		break;
 	case 3: /* Automotive (-40 °C to 125 °C) */
+		thermal_node_name = "automotive-thermal";
 		data->temp_grade = "Automotive";
 		data->temp_max = 125000;
 		break;
 	}
 
 	/*
+	 * Set defaults trips
+	 *
 	 * Set the critical trip point at 5 °C under max
 	 * Set the passive trip point at 10 °C under max (changeable via sysfs)
 	 */
 	data->temp_critical = data->temp_max - (1000 * 5);
 	data->temp_passive = data->temp_max - (1000 * 10);
+
+	/* Override critical/passive temperature from devicetree */
+	imx_init_temp_from_of(pdev, thermal_node_name);
 }
 
 static int imx_init_from_tempmon_data(struct platform_device *pdev)
-- 
2.25.1

[PATCH v2 4/9] imx: thermal: Configure trip point from DT

[Francesco Dolcini]

Allow over-writing critical and passive trip point for each
temperature grade from the device tree, by default the pre-existing
hard-coded trip points are used.

This change enables configuring the system thermal characteristics into
the system-specific device tree instead of relying on global hard-coded
temperature thresholds that does not take into account the specific
system thermal design.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2:
 - return immediately if no thermal node present in the dts
 - use dev_info instead of dev_dbg if there is an invalid trip
 - additional comment in case passive trip point is higher than critical
---
 drivers/thermal/imx_thermal.c | 58 +++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 16663373b682..a964baf802fc 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -17,6 +17,8 @@
 #include <linux/nvmem-consumer.h>
 #include <linux/pm_runtime.h>
 
+#include "thermal_core.h"
+
 #define REG_SET		0x4
 #define REG_CLR		0x8
 #define REG_TOG		0xc
@@ -479,36 +481,92 @@ static int imx_init_calib(struct platform_device *pdev, u32 ocotp_ana1)
 	return 0;
 }
 
+static void imx_init_temp_from_of(struct platform_device *pdev, const char *name)
+{
+	struct imx_thermal_data *data = platform_get_drvdata(pdev);
+	struct device_node *thermal, *trips, *trip_point;
+
+	thermal = of_get_child_by_name(pdev->dev.of_node, name);
+	if (!thermal)
+		return;
+
+	trips = of_get_child_by_name(thermal, "trips");
+
+	for_each_child_of_node(trips, trip_point) {
+		struct thermal_trip t;
+
+		if (thermal_of_populate_trip(trip_point, &t))
+			continue;
+
+		switch (t.type) {
+		case THERMAL_TRIP_PASSIVE:
+			data->temp_passive = t.temperature;
+			break;
+		case THERMAL_TRIP_CRITICAL:
+			data->temp_critical = t.temperature;
+			break;
+		default:
+			dev_info(&pdev->dev, "Ignoring trip type %d\n", t.type);
+			break;
+		}
+	};
+
+	of_node_put(trips);
+	of_node_put(thermal);
+
+	if (data->temp_passive >= data->temp_critical) {
+		dev_warn(&pdev->dev,
+			 "passive trip point must be lower than critical, fixing it up\n");
+		/*
+		 * In case of misconfiguration set passive temperature to
+		 * 5°C less than critical, this seems like a reasonable
+		 * default and the same is done when no thermal trips are
+		 * available in the device tree.
+		 */
+		data->temp_passive = data->temp_critical - (1000 * 5);
+	}
+}
+
 static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0)
 {
 	struct imx_thermal_data *data = platform_get_drvdata(pdev);
+	const char *thermal_node_name;
 
 	/* The maximum die temp is specified by the Temperature Grade */
 	switch ((ocotp_mem0 >> 6) & 0x3) {
 	case 0: /* Commercial (0 to 95 °C) */
+		thermal_node_name = "commercial-thermal";
 		data->temp_grade = "Commercial";
 		data->temp_max = 95000;
 		break;
 	case 1: /* Extended Commercial (-20 °C to 105 °C) */
+		thermal_node_name = "extended-commercial-thermal";
 		data->temp_grade = "Extended Commercial";
 		data->temp_max = 105000;
 		break;
 	case 2: /* Industrial (-40 °C to 105 °C) */
+		thermal_node_name = "industrial-thermal";
 		data->temp_grade = "Industrial";
 		data->temp_max = 105000;
 		break;
 	case 3: /* Automotive (-40 °C to 125 °C) */
+		thermal_node_name = "automotive-thermal";
 		data->temp_grade = "Automotive";
 		data->temp_max = 125000;
 		break;
 	}
 
 	/*
+	 * Set defaults trips
+	 *
 	 * Set the critical trip point at 5 °C under max
 	 * Set the passive trip point at 10 °C under max (changeable via sysfs)
 	 */
 	data->temp_critical = data->temp_max - (1000 * 5);
 	data->temp_passive = data->temp_max - (1000 * 10);
+
+	/* Override critical/passive temperature from devicetree */
+	imx_init_temp_from_of(pdev, thermal_node_name);
 }
 
 static int imx_init_from_tempmon_data(struct platform_device *pdev)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 5/9] ARM: dts: imx[67]: Add trips points

[Francesco Dolcini]

Add thermal trip point to the i.MX[67]* dtsi for each available
temperature grade.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 arch/arm/boot/dts/imx-thermal.dtsi | 61 ++++++++++++++++++++++++++++++
 arch/arm/boot/dts/imx6qdl.dtsi     |  2 +
 arch/arm/boot/dts/imx6sl.dtsi      |  2 +
 arch/arm/boot/dts/imx6sll.dtsi     |  2 +
 arch/arm/boot/dts/imx6sx.dtsi      |  2 +
 arch/arm/boot/dts/imx6ul.dtsi      |  2 +
 arch/arm/boot/dts/imx7s.dtsi       |  2 +
 7 files changed, 73 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx-thermal.dtsi

diff --git a/arch/arm/boot/dts/imx-thermal.dtsi b/arch/arm/boot/dts/imx-thermal.dtsi
new file mode 100644
index 000000000000..2303f1a99d84
--- /dev/null
+++ b/arch/arm/boot/dts/imx-thermal.dtsi
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0+ OR MIT
+//
+// Copyright 2022 Toradex
+
+automotive-thermal {
+	trips {
+		temp_trip_passive_automotive: trip-point0 {
+			temperature = <115000>;
+			hysteresis = <0>;
+			type = "passive";
+		};
+		temp_trip_crit_automotive: trip-point1 {
+			temperature = <120000>;
+			hysteresis = <0>;
+			type = "critical";
+		};
+	};
+};
+commercial-thermal {
+	trips {
+		temp_trip_passive_commercial: trip-point0 {
+			temperature = <85000>;
+			hysteresis = <0>;
+			type = "passive";
+		};
+		temp_trip_crit_commercial: trip-point1 {
+			temperature = <90000>;
+			hysteresis = <0>;
+			type = "critical";
+		};
+	};
+};
+extended-commercial-thermal {
+	trips {
+		temp_trip_passive_ecommercial: trip-point0 {
+			temperature = <95000>;
+			hysteresis = <0>;
+			type = "passive";
+		};
+		temp_trip_crit_ecommercial: trip-point1 {
+			temperature = <100000>;
+			hysteresis = <0>;
+			type = "critical";
+		};
+	};
+};
+industrial-thermal {
+	trips {
+		temp_trip_passive_industrial: trip-point0 {
+			temperature = <95000>;
+			hysteresis = <0>;
+			type = "passive";
+		};
+		temp_trip_crit_industrial: trip-point1 {
+			temperature = <100000>;
+			hysteresis = <0>;
+			type = "critical";
+		};
+	};
+};
+
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index d27beb47f9a3..0a492d9750dd 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -800,6 +800,8 @@ tempmon: tempmon {
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
 					#thermal-sensor-cells = <0>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 06a515121dfc..3719225126d0 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -628,6 +628,8 @@ tempmon: tempmon {
 					nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX6SL_CLK_PLL3_USB_OTG>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
index d4a000c3dde7..3192dae452fd 100644
--- a/arch/arm/boot/dts/imx6sll.dtsi
+++ b/arch/arm/boot/dts/imx6sll.dtsi
@@ -530,6 +530,8 @@ tempmon: temperature-sensor {
 					nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX6SLL_CLK_PLL3_USB_OTG>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index fc6334336b3d..d88c89696554 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -718,6 +718,8 @@ tempmon: tempmon {
 					nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index afeec01f6522..70d503c74e73 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -639,6 +639,8 @@ tempmon: tempmon {
 					nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX6UL_CLK_PLL3_USB_OTG>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 008e3da460f1..887b3618d20e 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -604,6 +604,8 @@ tempmon: tempmon {
 					nvmem-cells = <&tempmon_calib>,	<&fuse_grade>;
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX7D_PLL_SYS_MAIN_CLK>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
-- 
2.25.1

[PATCH v2 5/9] ARM: dts: imx[67]: Add trips points

[Francesco Dolcini]

Add thermal trip point to the i.MX[67]* dtsi for each available
temperature grade.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 arch/arm/boot/dts/imx-thermal.dtsi | 61 ++++++++++++++++++++++++++++++
 arch/arm/boot/dts/imx6qdl.dtsi     |  2 +
 arch/arm/boot/dts/imx6sl.dtsi      |  2 +
 arch/arm/boot/dts/imx6sll.dtsi     |  2 +
 arch/arm/boot/dts/imx6sx.dtsi      |  2 +
 arch/arm/boot/dts/imx6ul.dtsi      |  2 +
 arch/arm/boot/dts/imx7s.dtsi       |  2 +
 7 files changed, 73 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx-thermal.dtsi

diff --git a/arch/arm/boot/dts/imx-thermal.dtsi b/arch/arm/boot/dts/imx-thermal.dtsi
new file mode 100644
index 000000000000..2303f1a99d84
--- /dev/null
+++ b/arch/arm/boot/dts/imx-thermal.dtsi
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0+ OR MIT
+//
+// Copyright 2022 Toradex
+
+automotive-thermal {
+	trips {
+		temp_trip_passive_automotive: trip-point0 {
+			temperature = <115000>;
+			hysteresis = <0>;
+			type = "passive";
+		};
+		temp_trip_crit_automotive: trip-point1 {
+			temperature = <120000>;
+			hysteresis = <0>;
+			type = "critical";
+		};
+	};
+};
+commercial-thermal {
+	trips {
+		temp_trip_passive_commercial: trip-point0 {
+			temperature = <85000>;
+			hysteresis = <0>;
+			type = "passive";
+		};
+		temp_trip_crit_commercial: trip-point1 {
+			temperature = <90000>;
+			hysteresis = <0>;
+			type = "critical";
+		};
+	};
+};
+extended-commercial-thermal {
+	trips {
+		temp_trip_passive_ecommercial: trip-point0 {
+			temperature = <95000>;
+			hysteresis = <0>;
+			type = "passive";
+		};
+		temp_trip_crit_ecommercial: trip-point1 {
+			temperature = <100000>;
+			hysteresis = <0>;
+			type = "critical";
+		};
+	};
+};
+industrial-thermal {
+	trips {
+		temp_trip_passive_industrial: trip-point0 {
+			temperature = <95000>;
+			hysteresis = <0>;
+			type = "passive";
+		};
+		temp_trip_crit_industrial: trip-point1 {
+			temperature = <100000>;
+			hysteresis = <0>;
+			type = "critical";
+		};
+	};
+};
+
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index d27beb47f9a3..0a492d9750dd 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -800,6 +800,8 @@ tempmon: tempmon {
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
 					#thermal-sensor-cells = <0>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 06a515121dfc..3719225126d0 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -628,6 +628,8 @@ tempmon: tempmon {
 					nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX6SL_CLK_PLL3_USB_OTG>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
index d4a000c3dde7..3192dae452fd 100644
--- a/arch/arm/boot/dts/imx6sll.dtsi
+++ b/arch/arm/boot/dts/imx6sll.dtsi
@@ -530,6 +530,8 @@ tempmon: temperature-sensor {
 					nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX6SLL_CLK_PLL3_USB_OTG>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index fc6334336b3d..d88c89696554 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -718,6 +718,8 @@ tempmon: tempmon {
 					nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index afeec01f6522..70d503c74e73 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -639,6 +639,8 @@ tempmon: tempmon {
 					nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX6UL_CLK_PLL3_USB_OTG>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 008e3da460f1..887b3618d20e 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -604,6 +604,8 @@ tempmon: tempmon {
 					nvmem-cells = <&tempmon_calib>,	<&fuse_grade>;
 					nvmem-cell-names = "calib", "temp_grade";
 					clocks = <&clks IMX7D_PLL_SYS_MAIN_CLK>;
+
+					#include "imx-thermal.dtsi"
 				};
 			};
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 6/9] ARM: dts: imx6qdl-apalis: Set CPU critical trip point

[Francesco Dolcini]

Set CPU thermal critical trip point to the system designed value,
95 degrees for commercial grade modules, 105 for industrial and extended
commercial grade.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 arch/arm/boot/dts/imx6qdl-apalis.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-apalis.dtsi b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
index bd763bae596b..21ee16f3926f 100644
--- a/arch/arm/boot/dts/imx6qdl-apalis.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
@@ -390,6 +390,18 @@ &ssi1 {
 	status = "okay";
 };
 
+&temp_trip_crit_commercial {
+	temperature = <95000>;
+};
+
+&temp_trip_crit_ecommercial {
+	temperature = <105000>;
+};
+
+&temp_trip_crit_industrial {
+	temperature = <105000>;
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1_dte &pinctrl_uart1_ctrl>;
-- 
2.25.1

[PATCH v2 6/9] ARM: dts: imx6qdl-apalis: Set CPU critical trip point

[Francesco Dolcini]

Set CPU thermal critical trip point to the system designed value,
95 degrees for commercial grade modules, 105 for industrial and extended
commercial grade.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 arch/arm/boot/dts/imx6qdl-apalis.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-apalis.dtsi b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
index bd763bae596b..21ee16f3926f 100644
--- a/arch/arm/boot/dts/imx6qdl-apalis.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
@@ -390,6 +390,18 @@ &ssi1 {
 	status = "okay";
 };
 
+&temp_trip_crit_commercial {
+	temperature = <95000>;
+};
+
+&temp_trip_crit_ecommercial {
+	temperature = <105000>;
+};
+
+&temp_trip_crit_industrial {
+	temperature = <105000>;
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1_dte &pinctrl_uart1_ctrl>;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 7/9] ARM: dts: imx7-colibri: Set CPU critical trip point

[Francesco Dolcini]

Set CPU thermal critical trip point to the system designed value,
95 degrees for commercial grade modules, 105 for industrial and extended
commercial grade.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 arch/arm/boot/dts/imx7-colibri.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
index f1c60b0cb143..ac6dfb664d53 100644
--- a/arch/arm/boot/dts/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri.dtsi
@@ -430,6 +430,18 @@ &sai1 {
 	status = "okay";
 };
 
+&temp_trip_crit_commercial {
+	temperature = <95000>;
+};
+
+&temp_trip_crit_ecommercial {
+	temperature = <105000>;
+};
+
+&temp_trip_crit_industrial {
+	temperature = <105000>;
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1 &pinctrl_uart1_ctrl1 &pinctrl_uart1_ctrl2>;
-- 
2.25.1

[PATCH v2 7/9] ARM: dts: imx7-colibri: Set CPU critical trip point

[Francesco Dolcini]

Set CPU thermal critical trip point to the system designed value,
95 degrees for commercial grade modules, 105 for industrial and extended
commercial grade.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 arch/arm/boot/dts/imx7-colibri.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
index f1c60b0cb143..ac6dfb664d53 100644
--- a/arch/arm/boot/dts/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri.dtsi
@@ -430,6 +430,18 @@ &sai1 {
 	status = "okay";
 };
 
+&temp_trip_crit_commercial {
+	temperature = <95000>;
+};
+
+&temp_trip_crit_ecommercial {
+	temperature = <105000>;
+};
+
+&temp_trip_crit_industrial {
+	temperature = <105000>;
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1 &pinctrl_uart1_ctrl1 &pinctrl_uart1_ctrl2>;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 8/9] ARM: dts: imx6ull-colibri: Set CPU critical trip point

[Francesco Dolcini]

Set CPU thermal critical trip point to the system designed value,
95 degrees for commercial grade modules, 105 for industrial and extended
commercial grade.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 arch/arm/boot/dts/imx6ull-colibri.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi b/arch/arm/boot/dts/imx6ull-colibri.dtsi
index 15621e03fa4d..6d8e1ef0315b 100644
--- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
@@ -250,6 +250,18 @@ &snvs_pwrkey {
 	status = "disabled";
 };
 
+&temp_trip_crit_commercial {
+	temperature = <95000>;
+};
+
+&temp_trip_crit_ecommercial {
+	temperature = <105000>;
+};
+
+&temp_trip_crit_industrial {
+	temperature = <105000>;
+};
+
 /* Colibri UART_A */
 &uart1 {
 	pinctrl-names = "default";
-- 
2.25.1

[PATCH v2 8/9] ARM: dts: imx6ull-colibri: Set CPU critical trip point

[Francesco Dolcini]

Set CPU thermal critical trip point to the system designed value,
95 degrees for commercial grade modules, 105 for industrial and extended
commercial grade.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 arch/arm/boot/dts/imx6ull-colibri.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi b/arch/arm/boot/dts/imx6ull-colibri.dtsi
index 15621e03fa4d..6d8e1ef0315b 100644
--- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
@@ -250,6 +250,18 @@ &snvs_pwrkey {
 	status = "disabled";
 };
 
+&temp_trip_crit_commercial {
+	temperature = <95000>;
+};
+
+&temp_trip_crit_ecommercial {
+	temperature = <105000>;
+};
+
+&temp_trip_crit_industrial {
+	temperature = <105000>;
+};
+
 /* Colibri UART_A */
 &uart1 {
 	pinctrl-names = "default";
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 9/9] ARM: dts: imx6qdl-colibri: Set CPU critical trip point

[Francesco Dolcini]

Set CPU thermal critical trip point to the system designed value,
95 degrees for commercial grade modules, 105 for industrial and extended
commercial grade.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 arch/arm/boot/dts/imx6qdl-colibri.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-colibri.dtsi b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
index c383e0e4110c..fb7bdf65a0bd 100644
--- a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
@@ -642,6 +642,18 @@ &ssi1 {
 	status = "okay";
 };
 
+&temp_trip_crit_commercial {
+	temperature = <95000>;
+};
+
+&temp_trip_crit_ecommercial {
+	temperature = <105000>;
+};
+
+&temp_trip_crit_industrial {
+	temperature = <105000>;
+};
+
 /* Colibri UART_A */
 &uart1 {
 	fsl,dte-mode;
-- 
2.25.1

[PATCH v2 9/9] ARM: dts: imx6qdl-colibri: Set CPU critical trip point

[Francesco Dolcini]

Set CPU thermal critical trip point to the system designed value,
95 degrees for commercial grade modules, 105 for industrial and extended
commercial grade.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2: no changes
---
 arch/arm/boot/dts/imx6qdl-colibri.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-colibri.dtsi b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
index c383e0e4110c..fb7bdf65a0bd 100644
--- a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
@@ -642,6 +642,18 @@ &ssi1 {
 	status = "okay";
 };
 
+&temp_trip_crit_commercial {
+	temperature = <95000>;
+};
+
+&temp_trip_crit_ecommercial {
+	temperature = <105000>;
+};
+
+&temp_trip_crit_industrial {
+	temperature = <105000>;
+};
+
 /* Colibri UART_A */
 &uart1 {
 	fsl,dte-mode;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 0/9] imx: thermal: Allow trip point configuration from DT

[Krzysztof Kozlowski]

On 17/06/2022 00:08, Francesco Dolcini wrote:
> This series allows to specify the imx thermal drivers trip point from the device tree,
> without this change the threshold are hard-coded and this might not be correct given the
> thermal design of the final system.
> 
> This change is backward compatible with the existing device tree, and even
> with this change in by default the thresholds are the same as before.
> 
> Toradex board are also updated to use a system-specific thresholds.
> 
> Discussion on the current design is here:
> https://lore.kernel.org/all/4ba1d7d2-3e8c-ba60-37fd-9598f415c076@linaro.org/
> 
> One side note, after this change the dtbs checker starts complaining with this message
> 
> ```
> linux/arch/arm/boot/dts/imx6dl-alti6p.dtb: tempmon: '#thermal-sensor-cells' does not match any of the regexes: '^(automotive|commercial|extended-commercial|industrial)-thermal$', 'pinctrl-[0-9]+'
> 	From schema: linux/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
> ```
> 
> to my understanding this is just a side effect, 

If it starts complaining, it does not look like a side effect but error
needing to be fixed/addressed.


> '#thermal-sensor-cells' is not changed in
> any way by this series. I can fix that, I wonder if I should remove the property from the
> imx dtsi files or add it to the binding yaml definition, not sure about it.
> Anybody can advise?

Depends. Is the device a thermal-sensor provider?

Best regards,
Krzysztof

Re: [PATCH v1 0/9] imx: thermal: Allow trip point configuration from DT

[Krzysztof Kozlowski]

On 17/06/2022 00:08, Francesco Dolcini wrote:
> This series allows to specify the imx thermal drivers trip point from the device tree,
> without this change the threshold are hard-coded and this might not be correct given the
> thermal design of the final system.
> 
> This change is backward compatible with the existing device tree, and even
> with this change in by default the thresholds are the same as before.
> 
> Toradex board are also updated to use a system-specific thresholds.
> 
> Discussion on the current design is here:
> https://lore.kernel.org/all/4ba1d7d2-3e8c-ba60-37fd-9598f415c076@linaro.org/
> 
> One side note, after this change the dtbs checker starts complaining with this message
> 
> ```
> linux/arch/arm/boot/dts/imx6dl-alti6p.dtb: tempmon: '#thermal-sensor-cells' does not match any of the regexes: '^(automotive|commercial|extended-commercial|industrial)-thermal$', 'pinctrl-[0-9]+'
> 	From schema: linux/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
> ```
> 
> to my understanding this is just a side effect, 

If it starts complaining, it does not look like a side effect but error
needing to be fixed/addressed.


> '#thermal-sensor-cells' is not changed in
> any way by this series. I can fix that, I wonder if I should remove the property from the
> imx dtsi files or add it to the binding yaml definition, not sure about it.
> Anybody can advise?

Depends. Is the device a thermal-sensor provider?

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 0/9] imx: thermal: Allow trip point configuration from DT

[Francesco Dolcini]

This series allows to specify the imx thermal drivers trip point from the device tree,
without this change the threshold are hard-coded and this might not be correct given the
thermal design of the final system.

This change is backward compatible with the existing device tree, and even
with this change in by default the thresholds are the same as before.

Toradex board are also updated to use a system-specific thresholds.

Discussion on the current design is here:
https://lore.kernel.org/all/4ba1d7d2-3e8c-ba60-37fd-9598f415c076@linaro.org/

One side note, after this change the dtbs checker starts complaining with this message

```
linux/arch/arm/boot/dts/imx6dl-alti6p.dtb: tempmon: '#thermal-sensor-cells' does not match any of the regexes: '^(automotive|commercial|extended-commercial|industrial)-thermal$', 'pinctrl-[0-9]+'
	From schema: linux/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
```

to my understanding this is just a side effect, '#thermal-sensor-cells' is not changed in
any way by this series. I can fix that, I wonder if I should remove the property from the
imx dtsi files or add it to the binding yaml definition, not sure about it.
Anybody can advise?


Francesco Dolcini (9):
  dt-bindings: thermal: Define trips node in $defs
  thermal: thermal: Export OF trip helper function
  dt-bindings: thermal: imx: Add trips point
  imx: thermal: Configure trip point from DT
  ARM: dts: imx[67]: Add trips points
  ARM: dts: imx6qdl-apalis: Set CPU critical trip point
  ARM: dts: imx7-colibri: Set CPU critical trip point
  ARM: dts: imx6ull-colibri: Set CPU critical trip point
  ARM: dts: imx6qdl-colibri: Set CPU critical trip point

 .../bindings/thermal/imx-thermal.yaml         |  27 ++++
 .../bindings/thermal/thermal-zones.yaml       | 130 +++++++++---------
 arch/arm/boot/dts/imx-thermal.dtsi            |  61 ++++++++
 arch/arm/boot/dts/imx6qdl-apalis.dtsi         |  12 ++
 arch/arm/boot/dts/imx6qdl-colibri.dtsi        |  12 ++
 arch/arm/boot/dts/imx6qdl.dtsi                |   2 +
 arch/arm/boot/dts/imx6sl.dtsi                 |   2 +
 arch/arm/boot/dts/imx6sll.dtsi                |   2 +
 arch/arm/boot/dts/imx6sx.dtsi                 |   2 +
 arch/arm/boot/dts/imx6ul.dtsi                 |   2 +
 arch/arm/boot/dts/imx6ull-colibri.dtsi        |  12 ++
 arch/arm/boot/dts/imx7-colibri.dtsi           |  12 ++
 arch/arm/boot/dts/imx7s.dtsi                  |   2 +
 drivers/thermal/imx_thermal.c                 |  49 +++++++
 drivers/thermal/thermal_core.h                |   7 +
 drivers/thermal/thermal_of.c                  |   5 +-
 16 files changed, 274 insertions(+), 65 deletions(-)
 create mode 100644 arch/arm/boot/dts/imx-thermal.dtsi

-- 
2.25.1

[PATCH v1 0/9] imx: thermal: Allow trip point configuration from DT

[Francesco Dolcini]

This series allows to specify the imx thermal drivers trip point from the device tree,
without this change the threshold are hard-coded and this might not be correct given the
thermal design of the final system.

This change is backward compatible with the existing device tree, and even
with this change in by default the thresholds are the same as before.

Toradex board are also updated to use a system-specific thresholds.

Discussion on the current design is here:
https://lore.kernel.org/all/4ba1d7d2-3e8c-ba60-37fd-9598f415c076@linaro.org/

One side note, after this change the dtbs checker starts complaining with this message

```
linux/arch/arm/boot/dts/imx6dl-alti6p.dtb: tempmon: '#thermal-sensor-cells' does not match any of the regexes: '^(automotive|commercial|extended-commercial|industrial)-thermal$', 'pinctrl-[0-9]+'
	From schema: linux/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
```

to my understanding this is just a side effect, '#thermal-sensor-cells' is not changed in
any way by this series. I can fix that, I wonder if I should remove the property from the
imx dtsi files or add it to the binding yaml definition, not sure about it.
Anybody can advise?


Francesco Dolcini (9):
  dt-bindings: thermal: Define trips node in $defs
  thermal: thermal: Export OF trip helper function
  dt-bindings: thermal: imx: Add trips point
  imx: thermal: Configure trip point from DT
  ARM: dts: imx[67]: Add trips points
  ARM: dts: imx6qdl-apalis: Set CPU critical trip point
  ARM: dts: imx7-colibri: Set CPU critical trip point
  ARM: dts: imx6ull-colibri: Set CPU critical trip point
  ARM: dts: imx6qdl-colibri: Set CPU critical trip point

 .../bindings/thermal/imx-thermal.yaml         |  27 ++++
 .../bindings/thermal/thermal-zones.yaml       | 130 +++++++++---------
 arch/arm/boot/dts/imx-thermal.dtsi            |  61 ++++++++
 arch/arm/boot/dts/imx6qdl-apalis.dtsi         |  12 ++
 arch/arm/boot/dts/imx6qdl-colibri.dtsi        |  12 ++
 arch/arm/boot/dts/imx6qdl.dtsi                |   2 +
 arch/arm/boot/dts/imx6sl.dtsi                 |   2 +
 arch/arm/boot/dts/imx6sll.dtsi                |   2 +
 arch/arm/boot/dts/imx6sx.dtsi                 |   2 +
 arch/arm/boot/dts/imx6ul.dtsi                 |   2 +
 arch/arm/boot/dts/imx6ull-colibri.dtsi        |  12 ++
 arch/arm/boot/dts/imx7-colibri.dtsi           |  12 ++
 arch/arm/boot/dts/imx7s.dtsi                  |   2 +
 drivers/thermal/imx_thermal.c                 |  49 +++++++
 drivers/thermal/thermal_core.h                |   7 +
 drivers/thermal/thermal_of.c                  |   5 +-
 16 files changed, 274 insertions(+), 65 deletions(-)
 create mode 100644 arch/arm/boot/dts/imx-thermal.dtsi

-- 
2.25.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 0/9] imx: thermal: Allow trip point configuration from DT

[Marco Felsch]

Hi Francesco,

nice work :)

On 22-06-15, Francesco Dolcini wrote:
> This series allows to specify the imx thermal drivers trip point from the device tree,
> without this change the threshold are hard-coded and this might not be correct given the
> thermal design of the final system.
> 
> This change is backward compatible with the existing device tree, and even
> with this change in by default the thresholds are the same as before.
> 
> Toradex board are also updated to use a system-specific thresholds.
> 
> Discussion on the current design is here:
> https://lore.kernel.org/all/4ba1d7d2-3e8c-ba60-37fd-9598f415c076@linaro.org/

Thanks for thanking our abbroaches and forming this patchset. I added
only a few comments.

Regards,
  Marco

> 
> One side note, after this change the dtbs checker starts complaining with this message
> 
> ```
> linux/arch/arm/boot/dts/imx6dl-alti6p.dtb: tempmon: '#thermal-sensor-cells' does not match any of the regexes: '^(automotive|commercial|extended-commercial|industrial)-thermal$', 'pinctrl-[0-9]+'
> 	From schema: linux/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
> ```
> 
> to my understanding this is just a side effect, '#thermal-sensor-cells' is not changed in
> any way by this series. I can fix that, I wonder if I should remove the property from the
> imx dtsi files or add it to the binding yaml definition, not sure about it.
> Anybody can advise?
> 
> 
> Francesco Dolcini (9):
>   dt-bindings: thermal: Define trips node in $defs
>   thermal: thermal: Export OF trip helper function
>   dt-bindings: thermal: imx: Add trips point
>   imx: thermal: Configure trip point from DT
>   ARM: dts: imx[67]: Add trips points
>   ARM: dts: imx6qdl-apalis: Set CPU critical trip point
>   ARM: dts: imx7-colibri: Set CPU critical trip point
>   ARM: dts: imx6ull-colibri: Set CPU critical trip point
>   ARM: dts: imx6qdl-colibri: Set CPU critical trip point
> 
>  .../bindings/thermal/imx-thermal.yaml         |  27 ++++
>  .../bindings/thermal/thermal-zones.yaml       | 130 +++++++++---------
>  arch/arm/boot/dts/imx-thermal.dtsi            |  61 ++++++++
>  arch/arm/boot/dts/imx6qdl-apalis.dtsi         |  12 ++
>  arch/arm/boot/dts/imx6qdl-colibri.dtsi        |  12 ++
>  arch/arm/boot/dts/imx6qdl.dtsi                |   2 +
>  arch/arm/boot/dts/imx6sl.dtsi                 |   2 +
>  arch/arm/boot/dts/imx6sll.dtsi                |   2 +
>  arch/arm/boot/dts/imx6sx.dtsi                 |   2 +
>  arch/arm/boot/dts/imx6ul.dtsi                 |   2 +
>  arch/arm/boot/dts/imx6ull-colibri.dtsi        |  12 ++
>  arch/arm/boot/dts/imx7-colibri.dtsi           |  12 ++
>  arch/arm/boot/dts/imx7s.dtsi                  |   2 +
>  drivers/thermal/imx_thermal.c                 |  49 +++++++
>  drivers/thermal/thermal_core.h                |   7 +
>  drivers/thermal/thermal_of.c                  |   5 +-
>  16 files changed, 274 insertions(+), 65 deletions(-)
>  create mode 100644 arch/arm/boot/dts/imx-thermal.dtsi
> 
> -- 
> 2.25.1
> 
> 
> 

Re: [PATCH v1 0/9] imx: thermal: Allow trip point configuration from DT

[Marco Felsch]

Hi Francesco,

nice work :)

On 22-06-15, Francesco Dolcini wrote:
> This series allows to specify the imx thermal drivers trip point from the device tree,
> without this change the threshold are hard-coded and this might not be correct given the
> thermal design of the final system.
> 
> This change is backward compatible with the existing device tree, and even
> with this change in by default the thresholds are the same as before.
> 
> Toradex board are also updated to use a system-specific thresholds.
> 
> Discussion on the current design is here:
> https://lore.kernel.org/all/4ba1d7d2-3e8c-ba60-37fd-9598f415c076@linaro.org/

Thanks for thanking our abbroaches and forming this patchset. I added
only a few comments.

Regards,
  Marco

> 
> One side note, after this change the dtbs checker starts complaining with this message
> 
> ```
> linux/arch/arm/boot/dts/imx6dl-alti6p.dtb: tempmon: '#thermal-sensor-cells' does not match any of the regexes: '^(automotive|commercial|extended-commercial|industrial)-thermal$', 'pinctrl-[0-9]+'
> 	From schema: linux/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
> ```
> 
> to my understanding this is just a side effect, '#thermal-sensor-cells' is not changed in
> any way by this series. I can fix that, I wonder if I should remove the property from the
> imx dtsi files or add it to the binding yaml definition, not sure about it.
> Anybody can advise?
> 
> 
> Francesco Dolcini (9):
>   dt-bindings: thermal: Define trips node in $defs
>   thermal: thermal: Export OF trip helper function
>   dt-bindings: thermal: imx: Add trips point
>   imx: thermal: Configure trip point from DT
>   ARM: dts: imx[67]: Add trips points
>   ARM: dts: imx6qdl-apalis: Set CPU critical trip point
>   ARM: dts: imx7-colibri: Set CPU critical trip point
>   ARM: dts: imx6ull-colibri: Set CPU critical trip point
>   ARM: dts: imx6qdl-colibri: Set CPU critical trip point
> 
>  .../bindings/thermal/imx-thermal.yaml         |  27 ++++
>  .../bindings/thermal/thermal-zones.yaml       | 130 +++++++++---------
>  arch/arm/boot/dts/imx-thermal.dtsi            |  61 ++++++++
>  arch/arm/boot/dts/imx6qdl-apalis.dtsi         |  12 ++
>  arch/arm/boot/dts/imx6qdl-colibri.dtsi        |  12 ++
>  arch/arm/boot/dts/imx6qdl.dtsi                |   2 +
>  arch/arm/boot/dts/imx6sl.dtsi                 |   2 +
>  arch/arm/boot/dts/imx6sll.dtsi                |   2 +
>  arch/arm/boot/dts/imx6sx.dtsi                 |   2 +
>  arch/arm/boot/dts/imx6ul.dtsi                 |   2 +
>  arch/arm/boot/dts/imx6ull-colibri.dtsi        |  12 ++
>  arch/arm/boot/dts/imx7-colibri.dtsi           |  12 ++
>  arch/arm/boot/dts/imx7s.dtsi                  |   2 +
>  drivers/thermal/imx_thermal.c                 |  49 +++++++
>  drivers/thermal/thermal_core.h                |   7 +
>  drivers/thermal/thermal_of.c                  |   5 +-
>  16 files changed, 274 insertions(+), 65 deletions(-)
>  create mode 100644 arch/arm/boot/dts/imx-thermal.dtsi
> 
> -- 
> 2.25.1
> 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel