linux-kernel - Re: [PATCH v11 10/11] dt-bindings: display: vop2: Add rk3576 support

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: <i5mp7wrogfc7w3fbmvqgqxsbgcx4xnfnjik4rpd35amdwgppan@fistrhikrrmd>
Date: Sun, 12 Jan 2025 10:27:18 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Andy Yan <andyshrk@....com>
Cc: heiko@...ech.de, hjc@...k-chips.com, krzk+dt@...nel.org, 
	devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org, 
	derek.foreman@...labora.com, detlev.casanova@...labora.com, daniel@...ishbar.org, 
	robh@...nel.org, sebastian.reichel@...labora.com, 
	Andy Yan <andy.yan@...k-chips.com>
Subject: Re: [PATCH v11 10/11] dt-bindings: display: vop2: Add rk3576 support

On Sat, Jan 11, 2025 at 07:26:08PM +0800, Andy Yan wrote:
>    # See compatible-specific constraints below.
>    clocks:
> @@ -120,43 +133,98 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: rockchip,rk3588-vop
> +            enum:
> +              - rockchip,rk3566-vop
> +              - rockchip,rk3568-vop
>      then:
>        properties:
>          clocks:
> -          minItems: 7
> +          minItems: 5

That's wrong, why maxItems became minItems? How is this related to rk3576?

> +
>          clock-names:
> -          minItems: 7
> +          minItems: 5

You are doing here way too much. You need to split reorganizing, so we
will see what comes new.

And of course you need to explain why you are changing EXISTING binding
(I am not talking about shuffling around - you change the binding).


> +
> +        interrupts:
> +          maxItems: 1
> +
> +        interrupt-names: false
>  
>          ports:
>            required:
>              - port@0
>              - port@1
>              - port@2
> -            - port@3
> +
> +        rockchip,vo1-grf: false
> +        rockchip,vop-grf: false
> +        rockchip,pmu: false
>  
>        required:
>          - rockchip,grf
> -        - rockchip,vo1-grf
> -        - rockchip,vop-grf
> -        - rockchip,pmu
>  
> -    else:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - rockchip,rk3576-vop
> +    then:
>        properties:
> +        clocks:
> +          minItems: 5


Why minItems? Nothing in this patch makes sense for me. Neither changing
existing binding nor new binding for rk3576.

> +
> +        clock-names:
> +          minItems: 5
> +
> +        interrupts:
> +          minItems: 4
> +
> +        interrupt-names:
> +          minItems: 4
> +
> +        ports:
> +          required:
> +            - port@0
> +            - port@1
> +            - port@2
> +
>          rockchip,vo1-grf: false
>          rockchip,vop-grf: false
> -        rockchip,pmu: false
>  
> +      required:
> +        - rockchip,grf
> +        - rockchip,pmu
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: rockchip,rk3588-vop
> +    then:
> +      properties:
>          clocks:
> -          maxItems: 5
> +          minItems: 7

And again weird change to the binding.

Best regards,
Krzysztof