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

["Andy Yan" <andyshrk@163.com>]

Hi Krzysztof,

At 2025-01-12 17:27:18, "Krzysztof Kozlowski" <krzk@kernel.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@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-rockchip