Hi Krzysztof, At 2025-01-12 18:46:28, "Andy Yan" <andys...@163.com> wrote: > >Hi Krzysztof, > >At 2025-01-12 17:27:18, "Krzysztof Kozlowski" <k...@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 ?
Can you confirm whether this approach is acceptable? I want to clarify with you before sending the next version. > > >> >> >>> + >>> + 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-rockc...@lists.infradead.org >>http://lists.infradead.org/mailman/listinfo/linux-rockchip