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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cjf4n2tg5qcqpik3j6ufesl4lf2fftyccrqmurb2xvib6iyt2d@fbhkwoiwfjuy>
Date: Fri, 13 Dec 2024 10:18:12 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Dave Stevenson <dave.stevenson@...pberrypi.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Florian Fainelli <florian.fainelli@...adcom.com>, 
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, Eric Anholt <eric@...olt.net>, 
	Maíra Canal <mcanal@...lia.com>, Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>, 
	Ray Jui <rjui@...adcom.com>, Scott Branden <sbranden@...adcom.com>, 
	Doug Berger <opendmb@...il.com>, Linus Walleij <linus.walleij@...aro.org>, 
	Bartosz Golaszewski <brgl@...ev.pl>, Thomas Gleixner <tglx@...utronix.de>, 
	Stefan Wahren <wahrenst@....net>, dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org, 
	linux-rpi-kernel@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, Florian Fainelli <f.fainelli@...il.com>, 
	linux-gpio@...r.kernel.org
Subject: Re: [PATCH v3 1/7] dt-bindings: display: bcm2711-hdmi: Add interrupt
 details for BCM2712

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@...pberrypi.com>
> ---
>  .../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