linux-kernel - Re: [PATCH v3 1/2] dt-bindings: thermal: mediatek: Add MT8189 LVTS thermal controller bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251205-nostalgic-just-mussel-5cbf89@quoll>
Date: Fri, 5 Dec 2025 09:39:03 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Hanchien Lin <hanchien.lin@...iatek.com>
Cc: "Rafael J . Wysocki" <rafael@...nel.org>, 
	Daniel Lezcano <daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>, 
	Lukasz Luba <lukasz.luba@....com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Philipp Zabel <p.zabel@...gutronix.de>, Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Balsam CHIHI <bchihi@...libre.com>, Irving-CH.lin@...iatek.com, 
	Jh Hsu <Jh.Hsu@...iatek.com>, WH Wu <vincent.wu@...iatek.com>, 
	Raymond Sun <Raymond.Sun@...iatek.com>, Sirius Wang <Sirius.Wang@...iatek.com>, linux-pm@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org, 
	Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v3 1/2] dt-bindings: thermal: mediatek: Add MT8189 LVTS
 thermal controller bindings

On Tue, Dec 02, 2025 at 05:10:55PM +0800, Hanchien Lin wrote:
> Add support for the MediaTek MT8189 LVTS thermal controller to the device tree bindings.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> This includes new compatible strings and required properties for MT8189.

Drop, redundant. Don't explain the obvious. Can you document new device
wihout "new compatible strings"?

Your commit msg should explain non obvious things, e.g. why this is not
compatible with existing devices.

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18


> 
> Signed-off-by: Hanchien Lin <hanchien.lin@...iatek.com>
> ---
>  .../thermal/mediatek,lvts-thermal.yaml        | 27 +++++++++++++++++--
>  .../thermal/mediatek,lvts-thermal.h           | 20 ++++++++++++++
>  2 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> index 0259cd3ce9c5..0f7fd69f5fdf 100644
> --- a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> @@ -22,6 +22,8 @@ properties:
>        - mediatek,mt8186-lvts
>        - mediatek,mt8188-lvts-ap
>        - mediatek,mt8188-lvts-mcu
> +      - mediatek,mt8189-lvts-ap
> +      - mediatek,mt8189-lvts-mcu
>        - mediatek,mt8192-lvts-ap
>        - mediatek,mt8192-lvts-mcu
>        - mediatek,mt8195-lvts-ap
> @@ -58,6 +60,21 @@ properties:
>  allOf:
>    - $ref: thermal-sensor.yaml#
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - mediatek,mt8189-lvts-ap
> +              - mediatek,mt8189-lvts-mcu

Why new if:then? Why cannot it be part of existing block? I don't see
differences.

> +    then:
> +      properties:
> +        nvmem-cells:
> +          minItems: 2
> +
> +        nvmem-cell-names:
> +          minItems: 2
> +
>    - if:
>        properties:
>          compatible:
> @@ -75,6 +92,10 @@ allOf:
>          nvmem-cell-names:
>            maxItems: 1
>  
> +      required:
> +        - clocks
> +        - resets
> +
>    - if:
>        properties:
>          compatible:
> @@ -91,12 +112,14 @@ allOf:
>          nvmem-cell-names:
>            minItems: 2
>  
> +      required:
> +        - clocks
> +        - resets
> +
>  required:
>    - compatible
>    - reg
>    - interrupts
> -  - clocks
> -  - resets


Maybe that's the difference, but commit msg explained nothing. Your
commit is just redundant - tells zero, but all the unexpected things are
totally not explained.

Device cannot work without resets and clocks. If you think otherwise,
prove your point in commit msg (in terms of datasheet, hardware, not
drivers!).

>    - nvmem-cells
>    - nvmem-cell-names

Best regards,
Krzysztof