Re: [PATCH v9 6/7] dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] dt-bindings: thermal: loongson,ls2k-thermal: unwieldy allOf
  conditionals
--

commit f12fbd90282902d144235a2ecc3e0dfbcabab2a2
Author: Binbin Zhou <zhoubinbin@loongson.cn>

dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300

This commit adds the loongson,ls2k0300-thermal compatible string along
with the loongson,chipid phandle property and per-variant allOf
conditionals for reg and loongson,chipid.

> diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> index 79e691b08341c..d312e27737a20 100644
> --- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
[ ... ]
> @@ -34,28 +32,56 @@ properties:
>    '#thermal-sensor-cells':
>      const: 1
>  
> +  loongson,chipid:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to the syscon with the Loongson-2K0300 CHIP ID.
> +
>  required:
>    - compatible
>    - reg
>    - interrupts
>  
> -if:
> -  properties:
> -    compatible:
> -      contains:
> -        enum:
> -          - loongson,ls2k2000-thermal
> +allOf:
> +  - $ref: /schemas/thermal/thermal-sensor.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: loongson,ls2k0300-thermal
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - description: Thermal base register region
> +      required:
> +        - loongson,chipid
>  
> -then:
> -  properties:
> -    reg:
> -      minItems: 2
> -      maxItems: 2
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: loongson,ls2k1000-thermal
> +    then:
> +      properties:
> +        loongson,chipid: false
> +        reg:
> +          items:
> +            - description: Thermal base register region
>  
> -else:
> -  properties:
> -    reg:
> -      maxItems: 1
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: loongson,ls2k2000-thermal
> +    then:
> +      properties:
> +        loongson,chipid: false
> +        reg:
> +          items:
> +            - description: Thermal base register region
> +            - description: Thermal data output register region

[Severity: Low]
This isn't a bug, but with loongson,ls2k0300-thermal added, every
standalone variant in this binding now has its own if-then block in
allOf duplicating the reg item descriptions and differing on register
regions or loongson,chipid requirements.  Would it be cleaner to split
the diverging variants into separate schema documents, or at least
define the reg items descriptions once under top-level properties and
only narrow the constraints in the conditionals?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794211535.git.zhoubinbin@loongson.cn?part=6