Re: [PATCH v5 1/5] dt-bindings: thermal: sun8i: Add A523 THS0/1 controllers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The fallback `else` branch fails to explicitly restrict `nvmem-cell-names` to `maxItems: 1` for legacy devices.
- [Low] The `clocks: minItems: 2` constraint in the new `allOf` block for A523 devices is redundant.
--

commit 3b55695ed8b61c3a70dec40b0b70e9958c234bfc
Author: Mikhail Kalashnikov <iuncuim@gmail.com>

dt-bindings: thermal: sun8i: Add A523 THS0/1 controllers

This commit adds device tree binding descriptions for the thermal sensors
in the A523 processor. It updates the schema to require an additional
clock for these variants and supports split calibration data across two
nvmem cells.

> diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> index 3e61689f6dd41..6f5b2f3b94f80 100644
> --- a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> +++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml

[ ... ]

> @@ -132,6 +140,29 @@ allOf:
>          - clock-names
>          - resets
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - allwinner,sun55i-a523-ths0
> +              - allwinner,sun55i-a523-ths1
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2

[Severity: Low]
Is this constraint redundant? The first allOf block in this file checks
for single-clock devices and its else branch already applies minItems: 2
for clocks and clock-names to all other devices. Since the A523 variant
is not in that single-clock list, shouldn't it already be constrained to
2 clocks?

> +        nvmem-cells:
> +          minItems: 2
> +        nvmem-cell-names:
> +          minItems: 2
> +    else:
> +      properties:
> +        nvmem-cells:
> +          maxItems: 1
> +        nvmem-cell-names:
> +          items:
> +            - const: calibration

[Severity: Low]
Does this else block also need to specify maxItems: 1 for
nvmem-cell-names?

Redefining items as a tuple only validates the specified elements but
does not restrict the total array length. Without maxItems: 1, legacy
device trees could erroneously define two nvmem-cell-names and still
pass validation.

> +
>  required:
>    - compatible
>    - reg

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704171411.1413349-1-iuncuim@gmail.com?part=1