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

[Rob Herring <robh@kernel.org>]

On Wed, May 13, 2026 at 05:00:07AM +0000, sashiko-bot@kernel.org wrote:
> 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.

No, that's fine.

> 
> > 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?

Annoyingly, sashiko seems to consistently get this comment wrong.

Rob