Re: [PATCH v2 04/16] dt-bindings: mfd: mediatek: mt6397: add mt6323 PMIC thermal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `#thermal-sensor-cells` property is constrained to 1, but the MT6323 hardware only possesses a single thermal sensor, violating DT ABI design principles.
- [Low] The device tree example is missing the `adc` node and its corresponding `mt6323_adc` label, which is referenced by the new `thermal` node.
--

commit 5d14cb32b8298f2c8dd1f6c1a6921b9e3942bf24
Author: Roman Vivchar <rva333@protonmail.com>

dt-bindings: mfd: mediatek: mt6397: add mt6323 PMIC thermal

This commit adds devicetree binding documentation for the MediaTek mt6323
thermal monitoring hardware, which reads temperature via an AUXADC channel.
It configures #thermal-sensor-cells to 1 and includes a thermal node in
the example block.

> While mt6323 exposes only a single thermal sensor, newer PMICs like
> mt6358 provide more than one sensor. Therefore define #thermal-sensor-cells
> as 1 to avoid breaking devicetree ABI in the future.

Since the mt6323 hardware only possesses a single thermal sensor, shouldn't
#thermal-sensor-cells be constrained to 0?

Future hardware variants like the mt6358 will use different compatible
strings, which can then define their own #thermal-sensor-cells: const: 1
constraint without affecting the mt6323 ABI.

Setting it to 1 here forces a dummy cell argument on single-sensor mt6323
devices, misrepresenting the actual hardware.

> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> index beaa67bf0df2c..2c826737189e7 100644
> --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
[ ... ]
> @@ -511,6 +547,14 @@ examples:
>          rtc {
>              compatible = "mediatek,mt6323-rtc";
>          };
> +
> +        thermal {
> +          compatible = "mediatek,mt6323-thermal";
> +          nvmem-cells = <&mt6323_thermal_calibration_data>;
> +
> +          io-channels = <&mt6323_adc MT6323_AUXADC_CHIP_TEMP>;

Does the mt6323_adc label exist in this example?

Looking through the example block, the adc sub-node and its corresponding
mt6323_adc label do not seem to be defined. Will this cause dtc to fail with
a reference to non-existent node or label error during dt_binding_check?

> +          #thermal-sensor-cells = <1>;
> +        };
>      };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-mt6323-v2-0-3efcba579e88@protonmail.com?part=4