Re: [PATCH v3 1/7] dt-bindings: display: bcm2711-hdmi: Add interrupt details for BCM2712

[Krzysztof Kozlowski <krzk@xxxxxxxxxx>]

On Thu, Dec 12, 2024 at 06:36:28PM +0000, Dave Stevenson wrote:
> Commit 62948c62abca ("dt-bindings: display: Add BCM2712 HDMI bindings")
> added the compatible strings for BCM2712, but missed out that the
> number of interrupts changed.
> 
> Update the schema to include the interrupt requirements.
> 
> Fixes: 62948c62abca ("dt-bindings: display: Add BCM2712 HDMI bindings")
> Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> ---
>  .../bindings/display/brcm,bcm2711-hdmi.yaml        | 107 ++++++++++++++++++---
>  1 file changed, 93 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> index 6d11f5955b51..dd7dea60183b 100644
> --- a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> @@ -56,22 +56,38 @@ properties:
>        - const: cec
>  
>    interrupts:

> -    items:
> -      - description: CEC TX interrupt
> -      - description: CEC RX interrupt
> -      - description: CEC stuck at low interrupt
> -      - description: Wake-up interrupt
> -      - description: Hotplug connected interrupt
> -      - description: Hotplug removed interrupt
> +    oneOf:
> +      - items:
> +          - description: CEC TX interrupt
> +          - description: CEC RX interrupt
> +          - description: CEC stuck at low interrupt
> +          - description: Wake-up interrupt
> +          - description: Hotplug connected interrupt
> +          - description: Hotplug removed interrupt
> +
> +      - items:
> +          - description: CEC TX interrupt
> +          - description: CEC RX interrupt
> +          - description: CEC stuck at low interrupt
> +          - description: Hotplug connected interrupt
> +          - description: Hotplug removed interrupt

You have chosen unusual syntax. There are no bindings doing this way, so
I really do not get which file you used as template.

Expected here are minItems and maxItems. These are the widest
constraints. Otherwise you are repeating the same in allOf:if:then. And
then the allOf:if:then: defines the items.

You can do the opposite - define the items here then just choose
constraints in if:then:. Less popular if you have list without common
part (so no minItems here), but sure, if you insist... yet you chosen
some third way of duplicating it in both places.

Look:
https://elixir.bootlin.com/linux/v6.11-rc6/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L127
https://elixir.bootlin.com/linux/v6.11-rc6/source/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml#L39
https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L91

There is nowhere syntax like here, with duplicating everything twice.

BTW, drop the full stop from your subjects in some other patches.
Subject never ends with full stop.


Best regards,
Krzysztof