[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