[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2f8b8305.80ab.1943abad3ba.Coremail.andyshrk@163.com>
Date: Mon, 6 Jan 2025 16:28:30 +0800 (CST)
From: "Andy Yan" <andyshrk@....com>
To: "Rob Herring" <robh@...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,
"Andy Yan" <andy.yan@...k-chips.com>
Subject: Re:Re: [PATCH v8 8/9] dt-bindings: display: vop2: Add rk3576
support
Hi Rob,
At 2025-01-04 00:34:49, "Rob Herring" <robh@...nel.org> wrote:
>On Tue, Dec 31, 2024 at 05:07:51PM +0800, Andy Yan wrote:
>> From: Andy Yan <andy.yan@...k-chips.com>
>>
>> Add vop found on rk3576, the main difference between rk3576 and the
>> previous vop is that each VP has its own interrupt line.
>>
>> Signed-off-by: Andy Yan <andy.yan@...k-chips.com>
>>
>> ---
>>
>> Changes in v8:
>> - Fix dt_binding_check errors
>> - ordered by soc name
>> - Link to the previous version:
>> https://lore.kernel.org/linux-rockchip/6pn3qjxotdtpzucpul24yro7ppddezwuizneovqvmgdwyv2j7p@ztg4mqyiqmjf/T/#u
>>
>> Changes in v4:
>> - describe constraint SOC by SOC, as interrupts of rk3576 is very
>> different from others
>> - Drop Krzysztof's Reviewed-by, as this version changed a lot.
>>
>> Changes in v3:
>> - ordered by soc name
>> - Add description for newly added interrupt
>>
>> Changes in v2:
>> - Add dt bindings
>>
>> .../display/rockchip/rockchip-vop2.yaml | 81 ++++++++++++++-----
>> 1 file changed, 63 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
>> index 2531726af306..4cdd9cced10c 100644
>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
>> @@ -20,6 +20,7 @@ properties:
>> enum:
>> - rockchip,rk3566-vop
>> - rockchip,rk3568-vop
>> + - rockchip,rk3576-vop
>> - rockchip,rk3588-vop
>>
>> reg:
>> @@ -37,10 +38,21 @@ properties:
>> - const: gamma-lut
>>
>> interrupts:
>> - maxItems: 1
>> + minItems: 1
>> + maxItems: 4
>> description:
>> - The VOP interrupt is shared by several interrupt sources, such as
>> - frame start (VSYNC), line flag and other status interrupts.
>> + For VOP version under rk3576, the interrupt is shared by several interrupt
>> + sources, such as frame start (VSYNC), line flag and other interrupt status.
>> + For VOP version from rk3576 there is a system interrupt for bus error, and
>> + every video port has it's independent interrupts for vsync and other video
>> + port related error interrupts.
>> +
>> + interrupt-names:
>> + items:
>> + - const: vop-sys
>> + - const: vop-vp0
>> + - const: vop-vp1
>> + - const: vop-vp2
>
>Drop 'vop-' as it is redundant.
Okay, 'vop-' will be dropped in next version.
>
>>
>> # See compatible-specific constraints below.
>> clocks:
>> @@ -120,43 +132,76 @@ allOf:
>> properties:
>> compatible:
>> contains:
>> - const: rockchip,rk3588-vop
>> + enum:
>> + - rockchip,rk3566-vop
>> + - rockchip,rk3568-vop
>> then:
>> properties:
>> clocks:
>> - minItems: 7
>> + minItems: 5
>> clock-names:
>> - minItems: 7
>> -
>
>Keep the blank line between DT properties
Will do in the next version.
>
>> + minItems: 5
>> ports:
>> required:
>> - port@0
>> - port@1
>> - port@2
>> - - port@3
>>
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - rockchip,rk3576-vop
>> + then:
>> + properties:
>> + clocks:
>> + minItems: 5
>
>blank line
Will do.
>
>> + clock-names:
>> + minItems: 5
>
>blank line, and so on.
Will do.
>
>> + ports:
>> + required:
>> + - port@0
>> + - port@1
>> + - port@2
>> + interrupts:
>> + items:
>> + - description: vop system interrupt, such as axi bus error
>> + - description: interrupts for video port0, such as vsync, dsp_hold.
>> + - description: interrupts for video port1, such as vsync, dsp_hold.
>> + - description: interrupts for video port2, such as vsync, dsp_hold.
>
>These descriptions belong at the top level.
I saw samsung,ufs-phy.yaml descripte clocks like this, If this style is not preferred,
I will change it.
>
>> + interrupt-names:
>> + items:
>> + - const: vop-sys
>> + - const: vop-vp0
>> + - const: vop-vp1
>> + - const: vop-vp2
>
>Why are these names defined twice?
Maybe I should remove the interrupt-names at the top.
>
>You just need 'minItems: 4' in both of these.
>
>> required:
>> - rockchip,grf
>> - - rockchip,vo1-grf
>> - - rockchip,vop-grf
>> - rockchip,pmu
>>
>> - else:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: rockchip,rk3588-vop
>> + then:
>> properties:
>> - rockchip,vo1-grf: false
>> - rockchip,vop-grf: false
>> - rockchip,pmu: false
>
>How is there no long a case without these properties?
I'm sorry, I'm not quite sure what you mean here,
properties vo1-grf, vop-grf are needed by rk3588 , and rk356x don't need it.
'rockchip,pmu' and 'rockchip,grf' are needed by rk3576 and rk3588, rk356x don't need them.
So I changed to descritpe them SoC by SoC.
Thanks for your review.
>
>> -
>> clocks:
>> - maxItems: 5
>> + minItems: 7
>> clock-names:
>> - maxItems: 5
>> -
>> + minItems: 7
>> ports:
>> required:
>> - port@0
>> - port@1
>> - port@2
>> + - port@3
>> + required:
>> + - rockchip,grf
>> + - rockchip,vo1-grf
>> + - rockchip,vop-grf
>> + - rockchip,pmu
>>
>> additionalProperties: false
>>
>> --
>> 2.34.1
>>
Powered by blists - more mailing lists