Re: [PATCH v2 3/4] dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `reg` property loses its strict size constraints for different variants because the explicit `minItems` and `maxItems` bounds were replaced by `items` descriptions inside the `if/then` blocks without restricting the array length.
- [Medium] The compatible string added to the syscon schema contains a typo (`cphipid` instead of `chipid`).
--

--- Patch [3]: [PATCH v2 3/4] dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300 ---
commit 685d641d1fef4db6e1df5ead93f5d8354f9d319d
Author: Binbin Zhou <zhoubinbin@loongson.cn>

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

This commit adds a new compatible string loongson,ls2k0300-thermal for the
thermal sensor on the Loongson-2K0300 SoC. It updates the binding to make
loongson,chipid required for this new compatible and restructures the register
region constraints for each variant.

> 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.
> +

[Severity: Medium]
It appears the compatible string added to the syscon schema for this device
contains a typo: loongson,ls2k0300-cphipid-syscon instead of chipid.

Documentation/devicetree/bindings/mfd/syscon.yaml:
          - loongson,ls1b-syscon
          - loongson,ls1c-syscon
          - loongson,ls2k0300-cphipid-syscon

Given the loongson,chipid property name here and the commit message referencing 
the chip id, was the cphipid spelling in syscon.yaml unintended?

>  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

[Severity: Medium]
Does removing the explicit maxItems: 1 constraint here cause a regression where
the reg property loses its strict size limits?

In JSON Schema and dt-schema, specifying items inside an if/then/properties
block does not restrict the maximum array length unless maxItems is explicitly
defined. Since the root level allows reg: maxItems: 2, will this configuration
mistakenly accept up to two register regions for loongson,ls2k0300-thermal?

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

[Severity: Medium]
Similarly, does this block also need maxItems: 1 to prevent
loongson,ls2k1000-thermal from accepting two register regions?

>  
> -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: Medium]
The previous schema explicitly enforced lengths for loongson,ls2k2000-thermal
by stating minItems: 2 and maxItems: 2.

Without minItems: 2 here, will dt-schema now accept an incomplete single
register region for loongson,ls2k2000-thermal?

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