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

Open Source and information security mailing list archives

Message-ID: <3a93b0e3.1b0c.1945a1f4dbd.Coremail.andyshrk@163.com>
Date: Sun, 12 Jan 2025 18:46:28 +0800 (CST)
From: "Andy Yan" <andyshrk@....com>
To: "Krzysztof Kozlowski" <krzk@...nel.org>
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:Re: [PATCH v11 10/11] dt-bindings: display: vop2: Add rk3576
 support

Hi Krzysztof,

At 2025-01-12 17:27:18, "Krzysztof Kozlowski" <krzk@...nel.org> wrote:
>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).

How about split this patch to two: One rework the existing binding,  make it
more suitable for expanding to include new SoCs.
Then add rk3576 in the second patch ?

>
>
>> +
>> +        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
>
>
>_______________________________________________
>Linux-rockchip mailing list
>Linux-rockchip@...ts.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-rockchip

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