diff mbox series

[RFC,v2,1/4] media: dt-bindings: media: Document RZ/G2L CSI-2 block

Message ID 20220121010543.31385-2-prabhakar.mahadev-lad.rj@bp.renesas.com
State Superseded
Headers show
Series Add driver for Renesas RZ/G2L CRU module | expand

Commit Message

Prabhakar Mahadev Lad Jan. 21, 2022, 1:05 a.m. UTC
Document the CSI-2 block which is part of CRU found in Renesas
RZ/G2L SoC.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
Hi Geert/All,

vclk and pclk clocks are shared with CRU both CSI and CRU driver are using
pm_runtime. pclk clock is necessary for register access where as vclk clock
is only used for calculations. So would you suggest passing vclk as part of
clocks (as currently implemented) or pass the vclk clock rate as a dt property.

Cheers,
Prabhakar

v1->v2
* New patch
---
 .../bindings/media/renesas,rzg2l-csi2.yaml    | 151 ++++++++++++++++++
 1 file changed, 151 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml

Comments

Jacopo Mondi Feb. 15, 2022, 1:01 p.m. UTC | #1
Hi Prabhakar

On Fri, Jan 21, 2022 at 01:05:40AM +0000, Lad Prabhakar wrote:
> Document the CSI-2 block which is part of CRU found in Renesas
> RZ/G2L SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> Hi Geert/All,
>
> vclk and pclk clocks are shared with CRU both CSI and CRU driver are using
> pm_runtime. pclk clock is necessary for register access where as vclk clock
> is only used for calculations. So would you suggest passing vclk as part of
> clocks (as currently implemented) or pass the vclk clock rate as a dt property.
>
> Cheers,
> Prabhakar
>
> v1->v2
> * New patch
> ---
>  .../bindings/media/renesas,rzg2l-csi2.yaml    | 151 ++++++++++++++++++
>  1 file changed, 151 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> new file mode 100644
> index 000000000000..bf907768a157
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> @@ -0,0 +1,151 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (C) 2022 Renesas Electronics Corp.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/renesas,rzg2l-csi2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L MIPI CSI-2 receiver
> +
> +maintainers:
> +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +
> +description:
> +  The RZ/G2L CSI-2 receiver device provides MIPI CSI-2 capabilities for the
> +  Renesas RZ/G2L family of devices. MIPI CSI-2 is part of the CRU block which
> +  is used in conjunction with the Image Processing module, which provides the
> +  video capture capabilities.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - renesas,r9a07g044-csi2     # RZ/G2{L,LC}
> +          - const: renesas,rzg2l-csi2

As per Rob's comment on the CRU bindings, you can remove oneOf:

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-names:
> +    items:
> +      - const: csi2_link

Can this be just

  interrupt-names:
    const: csi2_link
?

(I've run dt_binding_check and it does not complain)

> +
> +  clocks:
> +    items:
> +      - description: Internal clock for connecting CRU and MIPI
> +      - description: CRU Main clock
> +      - description: CPU Register access clock
> +
> +  clock-names:
> +    items:
> +      - const: sysclk
> +      - const: vclk
> +      - const: pclk
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: CRU_CMN_RSTB reset terminal
> +
> +  reset-names:
> +    items:
> +      - const: cmn-rstb

Here and above, is items: needed for a single entry ?
(again, dt_binding_check does not complain if I remove it)

> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port node, single endpoint describing the CSI-2 transmitter.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +                items:
> +                  maximum: 4
> +
> +            required:
> +              - clock-lanes
> +              - data-lanes
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Output port node, Image Processing block connected to the CSI-2 receiver.

Isn't the next processing block the CRU ? IOW, isn't this driver the
CSI-2 receiver ?

> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - resets
> +  - reset-names
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    csi20: csi2@10830400 {
> +            compatible = "renesas,r9a07g044-csi2", "renesas,rzg2l-csi2";
> +            reg = <0x10830400 0xfc00>;
> +            interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&cpg CPG_MOD R9A07G044_CRU_SYSCLK>,
> +                     <&cpg CPG_MOD R9A07G044_CRU_VCLK>,
> +                     <&cpg CPG_MOD R9A07G044_CRU_PCLK>;
> +            clock-names = "sysclk", "vclk", "pclk";
> +            power-domains = <&cpg>;
> +            resets = <&cpg R9A07G044_CRU_CMN_RSTB>;
> +            reset-names = "cmn-rstb";
> +
> +            ports {
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +
> +                    port@0 {
> +                            reg = <0>;
> +
> +                            csi2_in: endpoint {
> +                                    clock-lanes = <0>;
> +                                    data-lanes = <1 2>;
> +                                    remote-endpoint = <&ov5645_ep>;
> +                            };
> +                    };
> +
> +                    port@1 {
> +                            #address-cells = <1>;
> +                            #size-cells = <0>;
> +
> +                            reg = <1>;
> +
> +                            csi2cru: endpoint@0 {
> +                                    reg = <0>;
> +                                    remote-endpoint = <&crucsi2>;
> +                            };
> +                    };
> +            };
> +    };
> --
> 2.17.1
>
Lad, Prabhakar March 18, 2022, 4:58 p.m. UTC | #2
Hi Jacopo,

Thank you for the review.

On Tue, Feb 15, 2022 at 1:00 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Prabhakar
>
> On Fri, Jan 21, 2022 at 01:05:40AM +0000, Lad Prabhakar wrote:
> > Document the CSI-2 block which is part of CRU found in Renesas
> > RZ/G2L SoC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > Hi Geert/All,
> >
> > vclk and pclk clocks are shared with CRU both CSI and CRU driver are using
> > pm_runtime. pclk clock is necessary for register access where as vclk clock
> > is only used for calculations. So would you suggest passing vclk as part of
> > clocks (as currently implemented) or pass the vclk clock rate as a dt property.
> >
> > Cheers,
> > Prabhakar
> >
> > v1->v2
> > * New patch
> > ---
> >  .../bindings/media/renesas,rzg2l-csi2.yaml    | 151 ++++++++++++++++++
> >  1 file changed, 151 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > new file mode 100644
> > index 000000000000..bf907768a157
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > @@ -0,0 +1,151 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright (C) 2022 Renesas Electronics Corp.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/renesas,rzg2l-csi2.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/G2L MIPI CSI-2 receiver
> > +
> > +maintainers:
> > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +
> > +description:
> > +  The RZ/G2L CSI-2 receiver device provides MIPI CSI-2 capabilities for the
> > +  Renesas RZ/G2L family of devices. MIPI CSI-2 is part of the CRU block which
> > +  is used in conjunction with the Image Processing module, which provides the
> > +  video capture capabilities.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - enum:
> > +              - renesas,r9a07g044-csi2     # RZ/G2{L,LC}
> > +          - const: renesas,rzg2l-csi2
>
> As per Rob's comment on the CRU bindings, you can remove oneOf:
>
I would like to keep it, as there will be RZ/V2L and RZ/G2UL entries
following as soon once as this gets merged in. So just want to avoid
the change hunk later.

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: csi2_link
>
> Can this be just
>
>   interrupt-names:
>     const: csi2_link
> ?
>
Agreed.

> (I've run dt_binding_check and it does not complain)
>
> > +
> > +  clocks:
> > +    items:
> > +      - description: Internal clock for connecting CRU and MIPI
> > +      - description: CRU Main clock
> > +      - description: CPU Register access clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: sysclk
> > +      - const: vclk
> > +      - const: pclk
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    items:
> > +      - description: CRU_CMN_RSTB reset terminal
> > +
> > +  reset-names:
> > +    items:
> > +      - const: cmn-rstb
>
> Here and above, is items: needed for a single entry ?
> (again, dt_binding_check does not complain if I remove it)
>
Agreed.

> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port@0:
> > +        $ref: /schemas/graph.yaml#/$defs/port-base
> > +        unevaluatedProperties: false
> > +        description:
> > +          Input port node, single endpoint describing the CSI-2 transmitter.
> > +
> > +        properties:
> > +          endpoint:
> > +            $ref: video-interfaces.yaml#
> > +            unevaluatedProperties: false
> > +
> > +            properties:
> > +              data-lanes:
> > +                minItems: 1
> > +                maxItems: 4
> > +                items:
> > +                  maximum: 4
> > +
> > +            required:
> > +              - clock-lanes
> > +              - data-lanes
> > +
> > +      port@1:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          Output port node, Image Processing block connected to the CSI-2 receiver.
>
> Isn't the next processing block the CRU ? IOW, isn't this driver the
> CSI-2 receiver ?
>
On RZ/G2L CRU consists of CSI + image processing block (as seen in
[0]). As requested by Laurent in previous version I split up the
driver for CSI. So instead of mentioning CRU I have mentioned it as
"Image Processing block".

[0] https://renesas.info/wiki/File:CRU.png

Cheers,
Prabhakar

> > +
> > +    required:
> > +      - port@0
> > +      - port@1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - power-domains
> > +  - resets
> > +  - reset-names
> > +  - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    csi20: csi2@10830400 {
> > +            compatible = "renesas,r9a07g044-csi2", "renesas,rzg2l-csi2";
> > +            reg = <0x10830400 0xfc00>;
> > +            interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>;
> > +            clocks = <&cpg CPG_MOD R9A07G044_CRU_SYSCLK>,
> > +                     <&cpg CPG_MOD R9A07G044_CRU_VCLK>,
> > +                     <&cpg CPG_MOD R9A07G044_CRU_PCLK>;
> > +            clock-names = "sysclk", "vclk", "pclk";
> > +            power-domains = <&cpg>;
> > +            resets = <&cpg R9A07G044_CRU_CMN_RSTB>;
> > +            reset-names = "cmn-rstb";
> > +
> > +            ports {
> > +                    #address-cells = <1>;
> > +                    #size-cells = <0>;
> > +
> > +                    port@0 {
> > +                            reg = <0>;
> > +
> > +                            csi2_in: endpoint {
> > +                                    clock-lanes = <0>;
> > +                                    data-lanes = <1 2>;
> > +                                    remote-endpoint = <&ov5645_ep>;
> > +                            };
> > +                    };
> > +
> > +                    port@1 {
> > +                            #address-cells = <1>;
> > +                            #size-cells = <0>;
> > +
> > +                            reg = <1>;
> > +
> > +                            csi2cru: endpoint@0 {
> > +                                    reg = <0>;
> > +                                    remote-endpoint = <&crucsi2>;
> > +                            };
> > +                    };
> > +            };
> > +    };
> > --
> > 2.17.1
> >
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
new file mode 100644
index 000000000000..bf907768a157
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
@@ -0,0 +1,151 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (C) 2022 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/renesas,rzg2l-csi2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L MIPI CSI-2 receiver
+
+maintainers:
+  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+
+description:
+  The RZ/G2L CSI-2 receiver device provides MIPI CSI-2 capabilities for the
+  Renesas RZ/G2L family of devices. MIPI CSI-2 is part of the CRU block which
+  is used in conjunction with the Image Processing module, which provides the
+  video capture capabilities.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - renesas,r9a07g044-csi2     # RZ/G2{L,LC}
+          - const: renesas,rzg2l-csi2
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    items:
+      - const: csi2_link
+
+  clocks:
+    items:
+      - description: Internal clock for connecting CRU and MIPI
+      - description: CRU Main clock
+      - description: CPU Register access clock
+
+  clock-names:
+    items:
+      - const: sysclk
+      - const: vclk
+      - const: pclk
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    items:
+      - description: CRU_CMN_RSTB reset terminal
+
+  reset-names:
+    items:
+      - const: cmn-rstb
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description:
+          Input port node, single endpoint describing the CSI-2 transmitter.
+
+        properties:
+          endpoint:
+            $ref: video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                minItems: 1
+                maxItems: 4
+                items:
+                  maximum: 4
+
+            required:
+              - clock-lanes
+              - data-lanes
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          Output port node, Image Processing block connected to the CSI-2 receiver.
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+  - reset-names
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a07g044-cpg.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    csi20: csi2@10830400 {
+            compatible = "renesas,r9a07g044-csi2", "renesas,rzg2l-csi2";
+            reg = <0x10830400 0xfc00>;
+            interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cpg CPG_MOD R9A07G044_CRU_SYSCLK>,
+                     <&cpg CPG_MOD R9A07G044_CRU_VCLK>,
+                     <&cpg CPG_MOD R9A07G044_CRU_PCLK>;
+            clock-names = "sysclk", "vclk", "pclk";
+            power-domains = <&cpg>;
+            resets = <&cpg R9A07G044_CRU_CMN_RSTB>;
+            reset-names = "cmn-rstb";
+
+            ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@0 {
+                            reg = <0>;
+
+                            csi2_in: endpoint {
+                                    clock-lanes = <0>;
+                                    data-lanes = <1 2>;
+                                    remote-endpoint = <&ov5645_ep>;
+                            };
+                    };
+
+                    port@1 {
+                            #address-cells = <1>;
+                            #size-cells = <0>;
+
+                            reg = <1>;
+
+                            csi2cru: endpoint@0 {
+                                    reg = <0>;
+                                    remote-endpoint = <&crucsi2>;
+                            };
+                    };
+            };
+    };