[PATCH 0/3] drm/rockchip: vop2: Add VP clock resets support

[PATCH 0/3] drm/rockchip: vop2: Add VP clock resets support

[Detlev Casanova]

The clock reset must be used when the VOP is configured. Skipping it can
put the VOP in an unknown state where the HDMI signal is either lost or
not matching the selected mode.

This adds support for rk3588(s) based SoCs.

Detlev Casanova (3):
  drm/rockchip: vop2: Add clock resets support
  arm64: dts: rockchip: Add VOP clock resets for rk3588s
  dt-bindings: display: vop2: Add VP clock resets

 .../display/rockchip/rockchip-vop2.yaml       | 27 +++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi     |  8 +++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c  | 30 +++++++++++++++++++
 3 files changed, 65 insertions(+)

-- 
2.43.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 1/3] drm/rockchip: vop2: Add clock resets support

[Detlev Casanova]

At the end of initialization, each VP clock needs to be reset before
they can be used.

Failing to do so can put the VOP in an undefined state where the
generated HDMI signal is either lost or not matching the selected mode.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 30 ++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index fdd768bbd487c..e81a67161d29a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -17,6 +17,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/swab.h>
 
 #include <drm/drm.h>
@@ -157,6 +158,7 @@ struct vop2_win {
 struct vop2_video_port {
 	struct drm_crtc crtc;
 	struct vop2 *vop2;
+	struct reset_control *dclk_rst;
 	struct clk *dclk;
 	unsigned int id;
 	const struct vop2_video_port_data *data;
@@ -1915,6 +1917,26 @@ static int us_to_vertical_line(struct drm_display_mode *mode, int us)
 	return us * mode->clock / mode->htotal / 1000;
 }
 
+static int vop2_clk_reset(struct vop2_video_port *vp)
+{
+	struct reset_control *rstc = vp->dclk_rst;
+	struct vop2 *vop2 = vp->vop2;
+	int ret;
+
+	if (!rstc)
+		return 0;
+
+	ret = reset_control_assert(rstc);
+	if (ret < 0)
+		drm_warn(vop2->drm, "failed to assert reset\n");
+	udelay(10);
+	ret = reset_control_deassert(rstc);
+	if (ret < 0)
+		drm_warn(vop2->drm, "failed to deassert reset\n");
+
+	return ret;
+}
+
 static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
 				    struct drm_atomic_state *state)
 {
@@ -2055,6 +2077,8 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);
 
+	vop2_clk_reset(vp);
+
 	drm_crtc_vblank_on(crtc);
 
 	vop2_unlock(vop2);
@@ -2706,6 +2730,12 @@ static int vop2_create_crtcs(struct vop2 *vop2)
 		vp->data = vp_data;
 
 		snprintf(dclk_name, sizeof(dclk_name), "dclk_vp%d", vp->id);
+		vp->dclk_rst = devm_reset_control_get_optional(vop2->dev, dclk_name);
+		if (IS_ERR(vp->dclk_rst)) {
+		        drm_err(vop2->drm, "failed to get %s reset\n", dclk_name);
+		        return PTR_ERR(vp->dclk_rst);
+		}
+
 		vp->dclk = devm_clk_get(vop2->dev, dclk_name);
 		if (IS_ERR(vp->dclk)) {
 			drm_err(vop2->drm, "failed to get %s\n", dclk_name);
-- 
2.43.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 2/3] arm64: dts: rockchip: Add VOP clock resets for rk3588s

[Detlev Casanova]

This adds the needed clock resets for all rk3588(s) based SOCs.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 6ac5ac8b48abb..8560c92cd406c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -1193,6 +1193,14 @@ vop: vop@fdd90000 {
 			      "pclk_vop";
 		iommus = <&vop_mmu>;
 		power-domains = <&power RK3588_PD_VOP>;
+		resets = <&cru SRST_D_VOP0>,
+			 <&cru SRST_D_VOP1>,
+			 <&cru SRST_D_VOP2>,
+			 <&cru SRST_D_VOP3>;
+		reset-names = "dclk_vp0",
+			      "dclk_vp1",
+			      "dclk_vp2",
+			      "dclk_vp3";
 		rockchip,grf = <&sys_grf>;
 		rockchip,vop-grf = <&vop_grf>;
 		rockchip,vo1-grf = <&vo1_grf>;
-- 
2.43.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 3/3] dt-bindings: display: vop2: Add VP clock resets

[Detlev Casanova]

Add the documentation for VOP2 video ports reset clocks.
One reset can be set per video port.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 .../display/rockchip/rockchip-vop2.yaml       | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
index 2531726af306b..941fd059498d4 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
@@ -65,6 +65,22 @@ properties:
       - const: dclk_vp3
       - const: pclk_vop
 
+  resets:
+    minItems: 3
+    items:
+      - description: Pixel clock reset for video port 0.
+      - description: Pixel clock reset for video port 1.
+      - description: Pixel clock reset for video port 2.
+      - description: Pixel clock reset for video port 3.
+
+  reset-names:
+    minItems: 3
+    items:
+      - const: dclk_vp0
+      - const: dclk_vp1
+      - const: dclk_vp2
+      - const: dclk_vp3
+
   rockchip,grf:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
@@ -128,6 +144,11 @@ allOf:
         clock-names:
           minItems: 7
 
+        resets:
+          minItems: 4
+        reset-names:
+          minItems: 4
+
         ports:
           required:
             - port@0
@@ -183,6 +204,12 @@ examples:
                               "dclk_vp0",
                               "dclk_vp1",
                               "dclk_vp2";
+                resets = <&cru SRST_VOP0>,
+                         <&cru SRST_VOP1>,
+                         <&cru SRST_VOP2>;
+                reset-names = "dclk_vp0",
+                              "dclk_vp1",
+                              "dclk_vp2";
                 power-domains = <&power RK3568_PD_VO>;
                 iommus = <&vop_mmu>;
                 vop_out: ports {
-- 
2.43.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 3/3] dt-bindings: display: vop2: Add VP clock resets

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2312 bytes --]

On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote:
> Add the documentation for VOP2 video ports reset clocks.
> One reset can be set per video port.
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>

Are these resets valid for all VOPs or just the one on 3588?

> ---
>  .../display/rockchip/rockchip-vop2.yaml       | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> index 2531726af306b..941fd059498d4 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> @@ -65,6 +65,22 @@ properties:
>        - const: dclk_vp3
>        - const: pclk_vop
>  
> +  resets:
> +    minItems: 3
> +    items:
> +      - description: Pixel clock reset for video port 0.
> +      - description: Pixel clock reset for video port 1.
> +      - description: Pixel clock reset for video port 2.
> +      - description: Pixel clock reset for video port 3.
> +
> +  reset-names:
> +    minItems: 3
> +    items:
> +      - const: dclk_vp0
> +      - const: dclk_vp1
> +      - const: dclk_vp2
> +      - const: dclk_vp3
> +
>    rockchip,grf:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description:
> @@ -128,6 +144,11 @@ allOf:
>          clock-names:
>            minItems: 7
>  
> +        resets:
> +          minItems: 4
> +        reset-names:
> +          minItems: 4
> +
>          ports:
>            required:
>              - port@0
> @@ -183,6 +204,12 @@ examples:
>                                "dclk_vp0",
>                                "dclk_vp1",
>                                "dclk_vp2";
> +                resets = <&cru SRST_VOP0>,
> +                         <&cru SRST_VOP1>,
> +                         <&cru SRST_VOP2>;
> +                reset-names = "dclk_vp0",
> +                              "dclk_vp1",
> +                              "dclk_vp2";
>                  power-domains = <&power RK3568_PD_VO>;
>                  iommus = <&vop_mmu>;
>                  vop_out: ports {
> -- 
> 2.43.2
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 3/3] dt-bindings: display: vop2: Add VP clock resets

[Heiko Stübner]

Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley:
> On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote:
> > Add the documentation for VOP2 video ports reset clocks.
> > One reset can be set per video port.
> > 
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> 
> Are these resets valid for all VOPs or just the one on 3588?

Not in that form.
I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2).

So the binding should take into account that rk3568 also has the
SRST_VOP0 ... SRST_VOP2.


Also, I guess we might not want to limit ourself to stuff we use?
I.e. the new VOP-design is one block with multiple video-ports

So for rk3568 I see
#define SRST_A_VOP
#define SRST_H_VOP
#define SRST_VOP0
#define SRST_VOP1
#define SRST_VOP2

similarly rk3588 has

#define SRST_H_VOP
#define SRST_A_VOP
#define SRST_D_VOP0
#define SRST_D_VOP1
#define SRST_D_VOP2
#define SRST_D_VOP3

as generalized reset lines.





> 
> > ---
> >  .../display/rockchip/rockchip-vop2.yaml       | 27 +++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> > index 2531726af306b..941fd059498d4 100644
> > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> > @@ -65,6 +65,22 @@ properties:
> >        - const: dclk_vp3
> >        - const: pclk_vop
> >  
> > +  resets:
> > +    minItems: 3
> > +    items:
> > +      - description: Pixel clock reset for video port 0.
> > +      - description: Pixel clock reset for video port 1.
> > +      - description: Pixel clock reset for video port 2.
> > +      - description: Pixel clock reset for video port 3.
> > +
> > +  reset-names:
> > +    minItems: 3
> > +    items:
> > +      - const: dclk_vp0
> > +      - const: dclk_vp1
> > +      - const: dclk_vp2
> > +      - const: dclk_vp3
> > +
> >    rockchip,grf:
> >      $ref: /schemas/types.yaml#/definitions/phandle
> >      description:
> > @@ -128,6 +144,11 @@ allOf:
> >          clock-names:
> >            minItems: 7
> >  
> > +        resets:
> > +          minItems: 4
> > +        reset-names:
> > +          minItems: 4
> > +
> >          ports:
> >            required:
> >              - port@0
> > @@ -183,6 +204,12 @@ examples:
> >                                "dclk_vp0",
> >                                "dclk_vp1",
> >                                "dclk_vp2";
> > +                resets = <&cru SRST_VOP0>,
> > +                         <&cru SRST_VOP1>,
> > +                         <&cru SRST_VOP2>;
> > +                reset-names = "dclk_vp0",
> > +                              "dclk_vp1",
> > +                              "dclk_vp2";
> >                  power-domains = <&power RK3568_PD_VO>;
> >                  iommus = <&vop_mmu>;
> >                  vop_out: ports {
> 





_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 3/3] dt-bindings: display: vop2: Add VP clock resets

[Detlev Casanova]

[-- Attachment #1.1: Type: text/plain, Size: 3713 bytes --]

On Wednesday, May 15, 2024 12:33:22 P.M. EDT Heiko Stübner wrote:
> Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley:
> > On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote:
> > > Add the documentation for VOP2 video ports reset clocks.
> > > One reset can be set per video port.
> > > 
> > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > 
> > Are these resets valid for all VOPs or just the one on 3588?
> 
> Not in that form.
> I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2).
> 
> So the binding should take into account that rk3568 also has the
> SRST_VOP0 ... SRST_VOP2.

That is what is set in the example and the reason why I set minItems to 3 in 
the main bindings.
Then, the rk3588 specific part sets it to 4.

Isn't that enough ?

> Also, I guess we might not want to limit ourself to stuff we use?
> I.e. the new VOP-design is one block with multiple video-ports
> 
> So for rk3568 I see
> #define SRST_A_VOP
> #define SRST_H_VOP
> #define SRST_VOP0
> #define SRST_VOP1
> #define SRST_VOP2
> 
> similarly rk3588 has
> 
> #define SRST_H_VOP
> #define SRST_A_VOP
> #define SRST_D_VOP0
> #define SRST_D_VOP1
> #define SRST_D_VOP2
> #define SRST_D_VOP3
> 
> as generalized reset lines.

Argh, I added them at first then removed them as they are not used. Will add 
them again then.

Detlev.

> > > ---
> > > 
> > >  .../display/rockchip/rockchip-vop2.yaml       | 27 +++++++++++++++++++
> > >  1 file changed, 27 insertions(+)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> > > b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> > > index 2531726af306b..941fd059498d4 100644
> > > ---
> > > a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> > > +++
> > > b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> > > 
> > > @@ -65,6 +65,22 @@ properties:
> > >        - const: dclk_vp3
> > >        - const: pclk_vop
> > > 
> > > +  resets:
> > > +    minItems: 3
> > > +    items:
> > > +      - description: Pixel clock reset for video port 0.
> > > +      - description: Pixel clock reset for video port 1.
> > > +      - description: Pixel clock reset for video port 2.
> > > +      - description: Pixel clock reset for video port 3.
> > > +
> > > +  reset-names:
> > > +    minItems: 3
> > > +    items:
> > > +      - const: dclk_vp0
> > > +      - const: dclk_vp1
> > > +      - const: dclk_vp2
> > > +      - const: dclk_vp3
> > > +
> > > 
> > >    rockchip,grf:
> > >      $ref: /schemas/types.yaml#/definitions/phandle
> > > 
> > >      description:
> > > @@ -128,6 +144,11 @@ allOf:
> > >          clock-names:
> > >            minItems: 7
> > > 
> > > +        resets:
> > > +          minItems: 4
> > > +        reset-names:
> > > +          minItems: 4
> > > +
> > > 
> > >          ports:
> > >            required:
> > >              - port@0
> > > 
> > > @@ -183,6 +204,12 @@ examples:
> > >                                "dclk_vp0",
> > >                                "dclk_vp1",
> > >                                "dclk_vp2";
> > > 
> > > +                resets = <&cru SRST_VOP0>,
> > > +                         <&cru SRST_VOP1>,
> > > +                         <&cru SRST_VOP2>;
> > > +                reset-names = "dclk_vp0",
> > > +                              "dclk_vp1",
> > > +                              "dclk_vp2";
> > > 
> > >                  power-domains = <&power RK3568_PD_VO>;
> > >                  iommus = <&vop_mmu>;
> > >                  vop_out: ports {


[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 3/3] dt-bindings: display: vop2: Add VP clock resets

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1127 bytes --]

On Tue, May 21, 2024 at 01:15:46PM -0400, Detlev Casanova wrote:
> On Wednesday, May 15, 2024 12:33:22 P.M. EDT Heiko Stübner wrote:
> > Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley:
> > > On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote:
> > > > Add the documentation for VOP2 video ports reset clocks.
> > > > One reset can be set per video port.
> > > > 
> > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > > 
> > > Are these resets valid for all VOPs or just the one on 3588?
> > 
> > Not in that form.
> > I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2).
> > 
> > So the binding should take into account that rk3568 also has the
> > SRST_VOP0 ... SRST_VOP2.
> 
> That is what is set in the example and the reason why I set minItems to 3 in 
> the main bindings.
> Then, the rk3588 specific part sets it to 4.
> 
> Isn't that enough ?

Not quite - you need to restrict maxItems to 3 for the other devices if
the clocks are not valid. What you've got says that 4 clocks are
possible but not needed on !rk3588.

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 3/3] dt-bindings: display: vop2: Add VP clock resets

[Detlev Casanova]

[-- Attachment #1.1: Type: text/plain, Size: 1754 bytes --]

On Tuesday, May 21, 2024 2:31:51 P.M. EDT Conor Dooley wrote:
> On Tue, May 21, 2024 at 01:15:46PM -0400, Detlev Casanova wrote:
> > On Wednesday, May 15, 2024 12:33:22 P.M. EDT Heiko Stübner wrote:
> > > Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley:
> > > > On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote:
> > > > > Add the documentation for VOP2 video ports reset clocks.
> > > > > One reset can be set per video port.
> > > > > 
> > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > > > 
> > > > Are these resets valid for all VOPs or just the one on 3588?
> > > 
> > > Not in that form.
> > > I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2).
> > > 
> > > So the binding should take into account that rk3568 also has the
> > > SRST_VOP0 ... SRST_VOP2.
> > 
> > That is what is set in the example and the reason why I set minItems to 3
> > in the main bindings.
> > Then, the rk3588 specific part sets it to 4.
> > 
> > Isn't that enough ?
> 
> Not quite - you need to restrict maxItems to 3 for the other devices if
> the clocks are not valid. What you've got says that 4 clocks are
> possible but not needed on !rk3588.
> 
> Cheers,
> Conor.

I don't understand what "properties: resets: minItems: 3" means then. I 
thought it means that all devices should have at least 3 resets. Then the 
allOf below specifies the special case of rk3588 which has a minimum of 4 
resets.

Do I need to add 
        resets:
          minItems: 3
        reset-names:
          minItems: 3
in the "else:" ?

So in that case, I can remove "properties: resets: minItems: 3" above ?

Also, what do you mean "If the clocks are not valid" ?

Detlev.


[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 3/3] dt-bindings: display: vop2: Add VP clock resets

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2104 bytes --]

On Wed, May 22, 2024 at 11:31:36AM -0400, Detlev Casanova wrote:
> On Tuesday, May 21, 2024 2:31:51 P.M. EDT Conor Dooley wrote:
> > On Tue, May 21, 2024 at 01:15:46PM -0400, Detlev Casanova wrote:
> > > On Wednesday, May 15, 2024 12:33:22 P.M. EDT Heiko Stübner wrote:
> > > > Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley:
> > > > > On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote:
> > > > > > Add the documentation for VOP2 video ports reset clocks.
> > > > > > One reset can be set per video port.
> > > > > > 
> > > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > > > > 
> > > > > Are these resets valid for all VOPs or just the one on 3588?
> > > > 
> > > > Not in that form.
> > > > I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2).
> > > > 
> > > > So the binding should take into account that rk3568 also has the
> > > > SRST_VOP0 ... SRST_VOP2.
> > > 
> > > That is what is set in the example and the reason why I set minItems to 3
> > > in the main bindings.
> > > Then, the rk3588 specific part sets it to 4.
> > > 
> > > Isn't that enough ?
> > 
> > Not quite - you need to restrict maxItems to 3 for the other devices if
> > the clocks are not valid. What you've got says that 4 clocks are
> > possible but not needed on !rk3588.
> > 
> I don't understand what "properties: resets: minItems: 3" means then. I 
> thought it means that all devices should have at least 3 resets. Then the 
> allOf below specifies the special case of rk3588 which has a minimum of 4 
> resets.

The change you made to the bindings allows someone to define either 3
(because of minItems 3) or 4 (because there are 4 items in the list) resets
for the rk3568.

> Do I need to add 
>         resets:
>           minItems: 3
>         reset-names:
>           minItems: 3
> in the "else:" ?

No, you need to add maxItems: 3 to the else.

> So in that case, I can remove "properties: resets: minItems: 3" above ?
> 
> Also, what do you mean "If the clocks are not valid" ?

s/clocks/resets/ ;)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip