RE: [PATCH 1/3] dt-bindings: thermal: samsung: Add tmu-name and sensor-index-ranges properties

[손신 <shin.son@samsung.com>]

Hello Krzysztof Kozlowski,

> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzk@kernel.org]
> Sent: Saturday, August 30, 2025 6:07 PM
> To: Shin Son <shin.son@samsung.com>; Bartlomiej Zolnierkiewicz
> <bzolnier@gmail.com>; Rafael J . Wysocki <rafael@kernel.org>; Daniel
> Lezcano <daniel.lezcano@linaro.org>; Zhang Rui <rui.zhang@intel.com>;
> Lukasz Luba <lukasz.luba@arm.com>; Rob Herring <robh@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Alim Akhtar <alim.akhtar@samsung.com>
> Cc: linux-pm@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 1/3] dt-bindings: thermal: samsung: Add tmu-name and
> sensor-index-ranges properties
> 
> On 25/08/2025 08:49, Shin Son wrote:
> > The exynosautov920 TMU requires per-sensor interrupt enablement for
> > its critical trip points.
> > Add two new DT properties to the Samsung thermal bindings to support
> > this requirement:
> >
> > - **tmu-name**: an explicit identifier for each TMU,
> > 		used to skip specific sensors
> > (e.g., sensor 5 is temporarily disabled on the TMU_SUB1 block).
> >
> > - **sensor-index-ranges**: defines valid sensor index ranges
> > 			   for the driver’s bitmap in private data,
> > 			   enabling per-sensor interrupt setup and data access.
> >
> > Signed-off-by: Shin Son <shin.son@samsung.com>
> > ---
> >  .../thermal/samsung,exynos-thermal.yaml       | 23 ++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yam
> > l
> > b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yam
> > l index 29a08b0729ee..420fb7a944e3 100644
> > ---
> > a/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal.yam
> > l
> > +++ b/Documentation/devicetree/bindings/thermal/samsung,exynos-thermal
> > +++ .yaml
> > @@ -8,6 +8,7 @@ title: Samsung Exynos SoC Thermal Management Unit
> > (TMU)
> >
> >  maintainers:
> >    - Krzysztof Kozlowski <krzk@kernel.org>
> > +  - Shin Son <shin.son@samsung.com>
> 
> This needs also explanation in commit msg.

Ok, I'll add an explanation for this

> 
> >
> >  description: |
> >    For multi-instance tmu each instance should have an alias correctly
> > numbered @@ -27,6 +28,7 @@ properties:
> >        - samsung,exynos5420-tmu-ext-triminfo
> >        - samsung,exynos5433-tmu
> >        - samsung,exynos7-tmu
> > +      - samsung,exynosautov920-tmu
> >
> >    clocks:
> >      minItems: 1
> > @@ -62,11 +64,29 @@ properties:
> >      minItems: 1
> >
> >    '#thermal-sensor-cells':
> > -    const: 0
> > +    enum:
> > +      - 0
> > +      - 1
> >
> >    vtmu-supply:
> >      description: The regulator node supplying voltage to TMU.
> >
> > +  tmu-name:
> 
> Generic property? Where is it defined.

Ok, I'll remove this.

> 
> > +    description: The TMU hardware name.
> 
> Anyway, you do not get instance IDs. I talked about this at OSSE25.

I've read your feedback and also reviewed your presentation at OSSE25. 
(https://osseu2025.sched.com/event/25Vsl/dts-101-from-roots-to-trees-aka-devicetree-for-beginners-krzysztof-kozlowski-linaro)
I will remove this and I utilized another way.

> 
> 
> > +    $ref: /schemas/types.yaml#/definitions/string-array
> > +    minItems: 1
> > +    maxItems: 1
> > +
> > +  sensor-index-ranges:
> 
> Where is the property defined? You keep adding generic properties.

I'll remove the generic property and change it to "samsung,hw-sensor-indexes".

> > +    description: |
> > +      Valid Sensor index ranges for the TMU hardware.
> 
> I don't understand what is this for.

I'll add more explanation for this.

> 
> > +
> > +      Note:: On the ExynosautoV920 variant, the fifth sensor in the TMU
> SUB1 is disabled,
> > +      so the driver skips it when matching by tmu-name.
> 
> That's not name, so why are you referring to tmu-name? And driver has
> nothing to do here. Describe hardware.
> 
> None of this is really correct. :/
> 
> 
> Best regards,
> Krzysztof

I'll rework the binding as you suggested.
Instead of using ranges, I'll list the sensor indices explicitly,
Which should address the issues you pointed out.

I'll include this change in the next revision,
so I would appreciate your review again.
Thank you.

Best regards,
Shin Son