Re: [PATCH v2 01/16] dt-bindings: media: fsl-pxp: convert to yaml

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 13/01/2023 16:09, Michael Tretter wrote:
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - const: fsl,imx6ul-pxp
>>> +      - const: fsl,imx6ull-pxp
>>> +      - const: fsl,imx7d-pxp
>>
>> These three are an enum.
> 
> These are alternatives to the 'items:' entry below.
> 
> Are you suggesting to use the following statement?
> 
> oneOf:
>   - enum:
>       - fsl,imx6ul-pxp
>       - fsl,imx6ull-pxp
>       - fsl,imx7d-pxp
>   - items:
>       - enum:
>           - fsl,imx6sll-pxp
>           - fsl,imx6sx-pxp

Yes.

> 
> Why is this better than the one that I used?

Because that's the convention - use enum for enumeration which nicely
groups all of them and is the easiest to read.

> 
>>
>>> +      - items:
>>> +          - enum:
>>> +              - fsl,imx6sll-pxp
>>> +              - fsl,imx6sx-pxp
>>> +          - const: fsl,imx6ull-pxp
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    minItems: 1
>>> +    maxItems: 2
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  clock-names:
>>> +    const: axi
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupts
>>> +  - clocks
>>> +  - clock-names
>>> +
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - fsl,imx6sx-pxp
>>> +    then:
>>> +      properties:
>>> +        interrupts:
>>> +          numItems: 1
>>
>> That's not correct syntax... I am surprised that it works. Did you test
>> the bindings?
> 
> I copied this syntax from renesas,wdt.yaml and ran
> 
> 	make ARCH=arm dtbs_check DT_SCHEMA_FILES=fsl,imx6ull-pxp.yaml
> 
> with SOC_IMX7D=y, SOC_IMX6UL=y, SOC_IMX6SLL=y, and SOC_IMX6SX=y. The latter
> two were not enabled in the v1, which is why it didn't catch the missing
> compatibles.
> 
> On a closer look, I just saw that the checker ignored the schema due to the
> incorrect syntax and didn't produce any further errors. With the syntax fixed,
> the checker now produces also a few more errors about power-domains, which I
> will fix in v3.
> 
> Is this syntax correct?
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - fsl,imx6sx-pxp
>     then:
>       properties:
>         interrupts:
>           minItems: 1

Drop this one


>           maxItems: 1
>     else:
>       properties:
>         interrupts:
>           minItems: 2
>           maxItems: 2
> 
> Michael

Rest is ok

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel