diff mbox series

[RFC,1/3] media: dt-bindings: media: Document RZ/G2L CRU block

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

Checks

Context Check Description
robh/checkpatch success
robh/dtbs-check success
robh/dt-meta-schema success

Commit Message

Lad Prabhakar Dec. 7, 2021, 1:23 a.m. UTC
Document the CRU block found on Renesas RZ/G2L SoC's.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../bindings/media/renesas,rzg2l-cru.yaml     | 227 ++++++++++++++++++
 1 file changed, 227 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml

Comments

Rob Herring Dec. 13, 2021, 10:58 p.m. UTC | #1
On Tue, Dec 07, 2021 at 01:23:49AM +0000, Lad Prabhakar wrote:
> Document the CRU block found on Renesas RZ/G2L SoC's.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  .../bindings/media/renesas,rzg2l-cru.yaml     | 227 ++++++++++++++++++
>  1 file changed, 227 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> new file mode 100644
> index 000000000000..7b2835810516
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> @@ -0,0 +1,227 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (C) 2021 Renesas Electronics Corp.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/renesas,rzg2l-cru.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L Camera Data Receiving Unit (CRU)
> +
> +maintainers:
> +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +
> +description:
> +  The RZ/G2L Camera Data Receiving Unit (CRU) device provides video input
> +  capabilities for the Renesas RZ/G2L family of devices.
> +
> +  Depending on the instance the Image Processing input is connected to
> +  external SoC pins or to a CSI-2 receiver.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:

Don't need oneOf with only 1 entry.

> +          - enum:
> +              - renesas,r9a07g044-cru     # RZ/G2{L,LC}
> +          - const: renesas,rzg2l-cru
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 4
> +
> +  interrupt-names:
> +    items:
> +      - const: csi2_link_int
> +      - const: image_conv_int
> +      - const: image_conv_err_int
> +      - const: axi_mst_err_int

_int is redundant.

> +
> +  clocks:
> +    items:
> +      - description: Internal clock for connecting CRU and MIPI
> +      - description: CRU Main clock
> +      - description: CPU Register access clock
> +      - description: CRU image transfer clock
> +
> +  clock-names:
> +    items:
> +      - const: sysclk
> +      - const: vclk
> +      - const: pclk
> +      - const: aclk
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: CRU_CMN_RSTB reset terminal
> +      - description: CRU_PRESETN reset terminal
> +      - description: CRU_ARESETN reset terminal
> +
> +  reset-names:
> +    items:
> +      - const: cmn-rstb
> +      - const: presetn
> +      - const: aresetn
> +
> +  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 a parallel input source.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              hsync-active: true
> +
> +              vsync-active: true
> +
> +              bus-width: true
> +
> +              data-shift: true
> +
> +      port@1:
> +        $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:
> +              clock-lanes:
> +                maxItems: 1
> +
> +              data-lanes:
> +                maxItems: 1
> +
> +            required:
> +              - clock-lanes
> +              - data-lanes
> +
> +      port@2:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Output port node, describing the RZ/G2L Image Processing module
> +          connected the CSI-2 receiver
> +
> +        properties:
> +          endpoint@0:

Unless you have mutiple endpoints to define or endpoint properties to 
add, you don't need to specify anything more than the port.

> +            $ref: /schemas/graph.yaml#/properties/endpoint
> +            description: Endpoint connected to CSI2.
> +
> +        anyOf:
> +          - required:
> +              - endpoint@0
> +
> +      port@3:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Input port node, describing the RZ/G2L CSI-2 module connected the
> +          Image Processing block.
> +
> +        properties:
> +          endpoint@0:
> +            $ref: /schemas/graph.yaml#/properties/endpoint
> +            description: Endpoint connected to CSI2.
> +
> +        anyOf:
> +          - required:
> +              - endpoint@0
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - power-domains
> +
> +additionalProperties: false
> +
> +examples:
> +  # Device node example with CSI-2
> +  - |
> +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    cru: video@10830000 {
> +            compatible = "renesas,r9a07g044-cru", "renesas,rzg2l-cru";
> +            reg = <0x10830000 0x10000>;
> +            interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "csi2_link_int", "image_conv_int",
> +                              "image_conv_err_int", "axi_mst_err_int";
> +            clocks = <&cpg CPG_MOD R9A07G044_CRU_SYSCLK>,
> +                     <&cpg CPG_MOD R9A07G044_CRU_VCLK>,
> +                     <&cpg CPG_MOD R9A07G044_CRU_PCLK>,
> +                     <&cpg CPG_MOD R9A07G044_CRU_ACLK>;
> +            clock-names = "sysclk", "vclk", "pclk", "aclk";
> +            power-domains = <&cpg>;
> +            resets = <&cpg R9A07G044_CRU_CMN_RSTB>,
> +                     <&cpg R9A07G044_CRU_PRESETN>,
> +                     <&cpg R9A07G044_CRU_ARESETN>;
> +            reset-names = "cmn-rstb", "presetn", "aresetn";
> +
> +            ports {
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +
> +                    port@1 {
> +                            #address-cells = <1>;
> +                            #size-cells = <0>;
> +
> +                            reg = <1>;
> +
> +                            csi2_in: endpoint@0 {
> +                                    reg = <0>;
> +                                    clock-lanes = <0>;
> +                                    data-lanes = <1 2>;
> +                                    remote-endpoint = <&ov5645_ep>;
> +                            };
> +                    };
> +
> +                    port@2 {
> +                            #address-cells = <1>;
> +                            #size-cells = <0>;
> +
> +                            reg = <2>;
> +
> +                            csi2cru: endpoint@0 {
> +                                    reg = <0>;
> +                                    remote-endpoint= <&crucsi2>;
> +                            };
> +                    };
> +
> +                    port@3 {
> +                            #address-cells = <1>;
> +                            #size-cells = <0>;
> +
> +                            reg = <3>;
> +
> +                            crucsi2: endpoint@0 {
> +                                    reg = <0>;
> +                                    remote-endpoint= <&csi2cru>;
> +                            };
> +                    };
> +            };
> +    };
> -- 
> 2.17.1
> 
>
Lad, Prabhakar Dec. 14, 2021, 12:07 p.m. UTC | #2
Hi Rob,

Thank you for the review.

On Mon, Dec 13, 2021 at 10:58 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Dec 07, 2021 at 01:23:49AM +0000, Lad Prabhakar wrote:
> > Document the CRU block found on Renesas RZ/G2L SoC's.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  .../bindings/media/renesas,rzg2l-cru.yaml     | 227 ++++++++++++++++++
> >  1 file changed, 227 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > new file mode 100644
> > index 000000000000..7b2835810516
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > @@ -0,0 +1,227 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright (C) 2021 Renesas Electronics Corp.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/renesas,rzg2l-cru.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/G2L Camera Data Receiving Unit (CRU)
> > +
> > +maintainers:
> > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +
> > +description:
> > +  The RZ/G2L Camera Data Receiving Unit (CRU) device provides video input
> > +  capabilities for the Renesas RZ/G2L family of devices.
> > +
> > +  Depending on the instance the Image Processing input is connected to
> > +  external SoC pins or to a CSI-2 receiver.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
>
> Don't need oneOf with only 1 entry.
>
I added this as there will be couple of more SoC's using this driver.

> > +          - enum:
> > +              - renesas,r9a07g044-cru     # RZ/G2{L,LC}
> > +          - const: renesas,rzg2l-cru
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 4
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: csi2_link_int
> > +      - const: image_conv_int
> > +      - const: image_conv_err_int
> > +      - const: axi_mst_err_int
>
> _int is redundant.
>
Agreed will drop "_int".

> > +
> > +  clocks:
> > +    items:
> > +      - description: Internal clock for connecting CRU and MIPI
> > +      - description: CRU Main clock
> > +      - description: CPU Register access clock
> > +      - description: CRU image transfer clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: sysclk
> > +      - const: vclk
> > +      - const: pclk
> > +      - const: aclk
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    items:
> > +      - description: CRU_CMN_RSTB reset terminal
> > +      - description: CRU_PRESETN reset terminal
> > +      - description: CRU_ARESETN reset terminal
> > +
> > +  reset-names:
> > +    items:
> > +      - const: cmn-rstb
> > +      - const: presetn
> > +      - const: aresetn
> > +
> > +  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 a parallel input source.
> > +
> > +        properties:
> > +          endpoint:
> > +            $ref: video-interfaces.yaml#
> > +            unevaluatedProperties: false
> > +
> > +            properties:
> > +              hsync-active: true
> > +
> > +              vsync-active: true
> > +
> > +              bus-width: true
> > +
> > +              data-shift: true
> > +
> > +      port@1:
> > +        $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:
> > +              clock-lanes:
> > +                maxItems: 1
> > +
> > +              data-lanes:
> > +                maxItems: 1
> > +
> > +            required:
> > +              - clock-lanes
> > +              - data-lanes
> > +
> > +      port@2:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          Output port node, describing the RZ/G2L Image Processing module
> > +          connected the CSI-2 receiver
> > +
> > +        properties:
> > +          endpoint@0:
>
> Unless you have mutiple endpoints to define or endpoint properties to
> add, you don't need to specify anything more than the port.
>
Agreed will drop it.

Cheers,
Prabhakar

> > +            $ref: /schemas/graph.yaml#/properties/endpoint
> > +            description: Endpoint connected to CSI2.
> > +
> > +        anyOf:
> > +          - required:
> > +              - endpoint@0
> > +
> > +      port@3:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          Input port node, describing the RZ/G2L CSI-2 module connected the
> > +          Image Processing block.
> > +
> > +        properties:
> > +          endpoint@0:
> > +            $ref: /schemas/graph.yaml#/properties/endpoint
> > +            description: Endpoint connected to CSI2.
> > +
> > +        anyOf:
> > +          - required:
> > +              - endpoint@0
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - interrupt-names
> > +  - clocks
> > +  - clock-names
> > +  - resets
> > +  - reset-names
> > +  - power-domains
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  # Device node example with CSI-2
> > +  - |
> > +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    cru: video@10830000 {
> > +            compatible = "renesas,r9a07g044-cru", "renesas,rzg2l-cru";
> > +            reg = <0x10830000 0x10000>;
> > +            interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> > +            interrupt-names = "csi2_link_int", "image_conv_int",
> > +                              "image_conv_err_int", "axi_mst_err_int";
> > +            clocks = <&cpg CPG_MOD R9A07G044_CRU_SYSCLK>,
> > +                     <&cpg CPG_MOD R9A07G044_CRU_VCLK>,
> > +                     <&cpg CPG_MOD R9A07G044_CRU_PCLK>,
> > +                     <&cpg CPG_MOD R9A07G044_CRU_ACLK>;
> > +            clock-names = "sysclk", "vclk", "pclk", "aclk";
> > +            power-domains = <&cpg>;
> > +            resets = <&cpg R9A07G044_CRU_CMN_RSTB>,
> > +                     <&cpg R9A07G044_CRU_PRESETN>,
> > +                     <&cpg R9A07G044_CRU_ARESETN>;
> > +            reset-names = "cmn-rstb", "presetn", "aresetn";
> > +
> > +            ports {
> > +                    #address-cells = <1>;
> > +                    #size-cells = <0>;
> > +
> > +                    port@1 {
> > +                            #address-cells = <1>;
> > +                            #size-cells = <0>;
> > +
> > +                            reg = <1>;
> > +
> > +                            csi2_in: endpoint@0 {
> > +                                    reg = <0>;
> > +                                    clock-lanes = <0>;
> > +                                    data-lanes = <1 2>;
> > +                                    remote-endpoint = <&ov5645_ep>;
> > +                            };
> > +                    };
> > +
> > +                    port@2 {
> > +                            #address-cells = <1>;
> > +                            #size-cells = <0>;
> > +
> > +                            reg = <2>;
> > +
> > +                            csi2cru: endpoint@0 {
> > +                                    reg = <0>;
> > +                                    remote-endpoint= <&crucsi2>;
> > +                            };
> > +                    };
> > +
> > +                    port@3 {
> > +                            #address-cells = <1>;
> > +                            #size-cells = <0>;
> > +
> > +                            reg = <3>;
> > +
> > +                            crucsi2: endpoint@0 {
> > +                                    reg = <0>;
> > +                                    remote-endpoint= <&csi2cru>;
> > +                            };
> > +                    };
> > +            };
> > +    };
> > --
> > 2.17.1
> >
> >
Laurent Pinchart Dec. 21, 2021, 8:47 a.m. UTC | #3
Hi Prabhakar,

Thank you for the patch.

On Tue, Dec 14, 2021 at 12:07:14PM +0000, Lad, Prabhakar wrote:
> On Mon, Dec 13, 2021 at 10:58 PM Rob Herring wrote:
> > On Tue, Dec 07, 2021 at 01:23:49AM +0000, Lad Prabhakar wrote:
> > > Document the CRU block found on Renesas RZ/G2L SoC's.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  .../bindings/media/renesas,rzg2l-cru.yaml     | 227 ++++++++++++++++++
> > >  1 file changed, 227 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > > new file mode 100644
> > > index 000000000000..7b2835810516
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > > @@ -0,0 +1,227 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +# Copyright (C) 2021 Renesas Electronics Corp.
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/renesas,rzg2l-cru.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Renesas RZ/G2L Camera Data Receiving Unit (CRU)
> > > +
> > > +maintainers:
> > > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > +
> > > +description:
> > > +  The RZ/G2L Camera Data Receiving Unit (CRU) device provides video input
> > > +  capabilities for the Renesas RZ/G2L family of devices.
> > > +
> > > +  Depending on the instance the Image Processing input is connected to
> > > +  external SoC pins or to a CSI-2 receiver.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> >
> > Don't need oneOf with only 1 entry.
>
> I added this as there will be couple of more SoC's using this driver.

Will that be SoCs not compatible tieh the rzg2l-cru compatible string ?
If so oneOf may be needed, but you can also add it later.

> > > +          - enum:
> > > +              - renesas,r9a07g044-cru     # RZ/G2{L,LC}
> > > +          - const: renesas,rzg2l-cru
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 4
> > > +
> > > +  interrupt-names:
> > > +    items:
> > > +      - const: csi2_link_int

I'm not thrilled by this. It looks like the CSI-2 receiver and the image
processing block are separate IP cores. Can we model them as separate DT
nodes ? I expect the CSI-2 receiver to possibly be reused in other SoCs
with a different integration.

> > > +      - const: image_conv_int
> > > +      - const: image_conv_err_int
> > > +      - const: axi_mst_err_int
> >
> > _int is redundant.
>
> Agreed will drop "_int".
> 
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: Internal clock for connecting CRU and MIPI
> > > +      - description: CRU Main clock
> > > +      - description: CPU Register access clock
> > > +      - description: CRU image transfer clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: sysclk
> > > +      - const: vclk
> > > +      - const: pclk
> > > +      - const: aclk
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +
> > > +  resets:
> > > +    items:
> > > +      - description: CRU_CMN_RSTB reset terminal
> > > +      - description: CRU_PRESETN reset terminal
> > > +      - description: CRU_ARESETN reset terminal
> > > +
> > > +  reset-names:
> > > +    items:
> > > +      - const: cmn-rstb
> > > +      - const: presetn
> > > +      - const: aresetn
> > > +
> > > +  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 a parallel input source.
> > > +
> > > +        properties:
> > > +          endpoint:
> > > +            $ref: video-interfaces.yaml#
> > > +            unevaluatedProperties: false
> > > +
> > > +            properties:
> > > +              hsync-active: true
> > > +
> > > +              vsync-active: true
> > > +
> > > +              bus-width: true
> > > +
> > > +              data-shift: true

No need for a blank line between all properties.

> > > +
> > > +      port@1:
> > > +        $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:
> > > +              clock-lanes:
> > > +                maxItems: 1

If lane reordering isn't supported, you could omit this.

> > > +
> > > +              data-lanes:
> > > +                maxItems: 1

Doesn't the CSI-2 receiver support more than one lane ?

> > > +
> > > +            required:
> > > +              - clock-lanes
> > > +              - data-lanes
> > > +
> > > +      port@2:
> > > +        $ref: /schemas/graph.yaml#/properties/port
> > > +        description:
> > > +          Output port node, describing the RZ/G2L Image Processing module
> > > +          connected the CSI-2 receiver
> > > +
> > > +        properties:
> > > +          endpoint@0:
> >
> > Unless you have mutiple endpoints to define or endpoint properties to
> > add, you don't need to specify anything more than the port.
>
> Agreed will drop it.
> 
> > > +            $ref: /schemas/graph.yaml#/properties/endpoint
> > > +            description: Endpoint connected to CSI2.
> > > +
> > > +        anyOf:
> > > +          - required:
> > > +              - endpoint@0
> > > +
> > > +      port@3:
> > > +        $ref: /schemas/graph.yaml#/properties/port
> > > +        description:
> > > +          Input port node, describing the RZ/G2L CSI-2 module connected the
> > > +          Image Processing block.
> > > +
> > > +        properties:
> > > +          endpoint@0:
> > > +            $ref: /schemas/graph.yaml#/properties/endpoint
> > > +            description: Endpoint connected to CSI2.
> > > +
> > > +        anyOf:
> > > +          - required:
> > > +              - endpoint@0
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - interrupt-names
> > > +  - clocks
> > > +  - clock-names
> > > +  - resets
> > > +  - reset-names
> > > +  - power-domains
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  # Device node example with CSI-2
> > > +  - |
> > > +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +
> > > +    cru: video@10830000 {
> > > +            compatible = "renesas,r9a07g044-cru", "renesas,rzg2l-cru";
> > > +            reg = <0x10830000 0x10000>;
> > > +            interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
> > > +                         <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
> > > +                         <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
> > > +                         <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> > > +            interrupt-names = "csi2_link_int", "image_conv_int",
> > > +                              "image_conv_err_int", "axi_mst_err_int";
> > > +            clocks = <&cpg CPG_MOD R9A07G044_CRU_SYSCLK>,
> > > +                     <&cpg CPG_MOD R9A07G044_CRU_VCLK>,
> > > +                     <&cpg CPG_MOD R9A07G044_CRU_PCLK>,
> > > +                     <&cpg CPG_MOD R9A07G044_CRU_ACLK>;
> > > +            clock-names = "sysclk", "vclk", "pclk", "aclk";
> > > +            power-domains = <&cpg>;
> > > +            resets = <&cpg R9A07G044_CRU_CMN_RSTB>,
> > > +                     <&cpg R9A07G044_CRU_PRESETN>,
> > > +                     <&cpg R9A07G044_CRU_ARESETN>;
> > > +            reset-names = "cmn-rstb", "presetn", "aresetn";
> > > +
> > > +            ports {
> > > +                    #address-cells = <1>;
> > > +                    #size-cells = <0>;
> > > +
> > > +                    port@1 {
> > > +                            #address-cells = <1>;
> > > +                            #size-cells = <0>;
> > > +
> > > +                            reg = <1>;
> > > +
> > > +                            csi2_in: endpoint@0 {
> > > +                                    reg = <0>;
> > > +                                    clock-lanes = <0>;
> > > +                                    data-lanes = <1 2>;
> > > +                                    remote-endpoint = <&ov5645_ep>;
> > > +                            };
> > > +                    };
> > > +
> > > +                    port@2 {
> > > +                            #address-cells = <1>;
> > > +                            #size-cells = <0>;
> > > +
> > > +                            reg = <2>;
> > > +
> > > +                            csi2cru: endpoint@0 {
> > > +                                    reg = <0>;
> > > +                                    remote-endpoint= <&crucsi2>;
> > > +                            };
> > > +                    };
> > > +
> > > +                    port@3 {
> > > +                            #address-cells = <1>;
> > > +                            #size-cells = <0>;
> > > +
> > > +                            reg = <3>;
> > > +
> > > +                            crucsi2: endpoint@0 {
> > > +                                    reg = <0>;
> > > +                                    remote-endpoint= <&csi2cru>;
> > > +                            };
> > > +                    };
> > > +            };
> > > +    };
Lad, Prabhakar Dec. 22, 2021, 9:16 a.m. UTC | #4
Hi Laurent,

Thank you for the review.

On Tue, Dec 21, 2021 at 8:47 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Tue, Dec 14, 2021 at 12:07:14PM +0000, Lad, Prabhakar wrote:
> > On Mon, Dec 13, 2021 at 10:58 PM Rob Herring wrote:
> > > On Tue, Dec 07, 2021 at 01:23:49AM +0000, Lad Prabhakar wrote:
> > > > Document the CRU block found on Renesas RZ/G2L SoC's.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > ---
> > > >  .../bindings/media/renesas,rzg2l-cru.yaml     | 227 ++++++++++++++++++
> > > >  1 file changed, 227 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > > > new file mode 100644
> > > > index 000000000000..7b2835810516
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > > > @@ -0,0 +1,227 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +# Copyright (C) 2021 Renesas Electronics Corp.
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/renesas,rzg2l-cru.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Renesas RZ/G2L Camera Data Receiving Unit (CRU)
> > > > +
> > > > +maintainers:
> > > > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > +
> > > > +description:
> > > > +  The RZ/G2L Camera Data Receiving Unit (CRU) device provides video input
> > > > +  capabilities for the Renesas RZ/G2L family of devices.
> > > > +
> > > > +  Depending on the instance the Image Processing input is connected to
> > > > +  external SoC pins or to a CSI-2 receiver.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    oneOf:
> > > > +      - items:
> > >
> > > Don't need oneOf with only 1 entry.
> >
> > I added this as there will be couple of more SoC's using this driver.
>
> Will that be SoCs not compatible tieh the rzg2l-cru compatible string ?
>
Soc will be compatible with  rzg2l-cru but we will be adding SoC
specific string and then fallback to "renesas,rzg2l-cru" likewise done
for "renesas,r9a07g044-cru"

> If so oneOf may be needed, but you can also add it later.
>
Ive already added oneOf.

> > > > +          - enum:
> > > > +              - renesas,r9a07g044-cru     # RZ/G2{L,LC}
> > > > +          - const: renesas,rzg2l-cru
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +    maxItems: 4
> > > > +
> > > > +  interrupt-names:
> > > > +    items:
> > > > +      - const: csi2_link_int
>
> I'm not thrilled by this. It looks like the CSI-2 receiver and the image
> processing block are separate IP cores. Can we model them as separate DT
> nodes ? I expect the CSI-2 receiver to possibly be reused in other SoCs
> with a different integration.
>
The main reason for doing so is the registers for CSI are in same
block as CRU and the clock for register access is single. I'll have to
make the clock as shared and then can split this into a separate node.

> > > > +      - const: image_conv_int
> > > > +      - const: image_conv_err_int
> > > > +      - const: axi_mst_err_int
> > >
> > > _int is redundant.
> >
> > Agreed will drop "_int".
> >
> > > > +
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: Internal clock for connecting CRU and MIPI
> > > > +      - description: CRU Main clock
> > > > +      - description: CPU Register access clock
> > > > +      - description: CRU image transfer clock
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: sysclk
> > > > +      - const: vclk
> > > > +      - const: pclk
> > > > +      - const: aclk
> > > > +
> > > > +  power-domains:
> > > > +    maxItems: 1
> > > > +
> > > > +  resets:
> > > > +    items:
> > > > +      - description: CRU_CMN_RSTB reset terminal
> > > > +      - description: CRU_PRESETN reset terminal
> > > > +      - description: CRU_ARESETN reset terminal
> > > > +
> > > > +  reset-names:
> > > > +    items:
> > > > +      - const: cmn-rstb
> > > > +      - const: presetn
> > > > +      - const: aresetn
> > > > +
> > > > +  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 a parallel input source.
> > > > +
> > > > +        properties:
> > > > +          endpoint:
> > > > +            $ref: video-interfaces.yaml#
> > > > +            unevaluatedProperties: false
> > > > +
> > > > +            properties:
> > > > +              hsync-active: true
> > > > +
> > > > +              vsync-active: true
> > > > +
> > > > +              bus-width: true
> > > > +
> > > > +              data-shift: true
>
> No need for a blank line between all properties.
>
OK.

> > > > +
> > > > +      port@1:
> > > > +        $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:
> > > > +              clock-lanes:
> > > > +                maxItems: 1
>
> If lane reordering isn't supported, you could omit this.
>
OK will drop it.

> > > > +
> > > > +              data-lanes:
> > > > +                maxItems: 1
>
> Doesn't the CSI-2 receiver support more than one lane ?
>
my bad it should be enum with [1, 2, 4].

Cheers,
Prabhakar
> > > > +
> > > > +            required:
> > > > +              - clock-lanes
> > > > +              - data-lanes
> > > > +
> > > > +      port@2:
> > > > +        $ref: /schemas/graph.yaml#/properties/port
> > > > +        description:
> > > > +          Output port node, describing the RZ/G2L Image Processing module
> > > > +          connected the CSI-2 receiver
> > > > +
> > > > +        properties:
> > > > +          endpoint@0:
> > >
> > > Unless you have mutiple endpoints to define or endpoint properties to
> > > add, you don't need to specify anything more than the port.
> >
> > Agreed will drop it.
> >
> > > > +            $ref: /schemas/graph.yaml#/properties/endpoint
> > > > +            description: Endpoint connected to CSI2.
> > > > +
> > > > +        anyOf:
> > > > +          - required:
> > > > +              - endpoint@0
> > > > +
> > > > +      port@3:
> > > > +        $ref: /schemas/graph.yaml#/properties/port
> > > > +        description:
> > > > +          Input port node, describing the RZ/G2L CSI-2 module connected the
> > > > +          Image Processing block.
> > > > +
> > > > +        properties:
> > > > +          endpoint@0:
> > > > +            $ref: /schemas/graph.yaml#/properties/endpoint
> > > > +            description: Endpoint connected to CSI2.
> > > > +
> > > > +        anyOf:
> > > > +          - required:
> > > > +              - endpoint@0
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - interrupts
> > > > +  - interrupt-names
> > > > +  - clocks
> > > > +  - clock-names
> > > > +  - resets
> > > > +  - reset-names
> > > > +  - power-domains
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  # Device node example with CSI-2
> > > > +  - |
> > > > +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> > > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > +
> > > > +    cru: video@10830000 {
> > > > +            compatible = "renesas,r9a07g044-cru", "renesas,rzg2l-cru";
> > > > +            reg = <0x10830000 0x10000>;
> > > > +            interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                         <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                         <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                         <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> > > > +            interrupt-names = "csi2_link_int", "image_conv_int",
> > > > +                              "image_conv_err_int", "axi_mst_err_int";
> > > > +            clocks = <&cpg CPG_MOD R9A07G044_CRU_SYSCLK>,
> > > > +                     <&cpg CPG_MOD R9A07G044_CRU_VCLK>,
> > > > +                     <&cpg CPG_MOD R9A07G044_CRU_PCLK>,
> > > > +                     <&cpg CPG_MOD R9A07G044_CRU_ACLK>;
> > > > +            clock-names = "sysclk", "vclk", "pclk", "aclk";
> > > > +            power-domains = <&cpg>;
> > > > +            resets = <&cpg R9A07G044_CRU_CMN_RSTB>,
> > > > +                     <&cpg R9A07G044_CRU_PRESETN>,
> > > > +                     <&cpg R9A07G044_CRU_ARESETN>;
> > > > +            reset-names = "cmn-rstb", "presetn", "aresetn";
> > > > +
> > > > +            ports {
> > > > +                    #address-cells = <1>;
> > > > +                    #size-cells = <0>;
> > > > +
> > > > +                    port@1 {
> > > > +                            #address-cells = <1>;
> > > > +                            #size-cells = <0>;
> > > > +
> > > > +                            reg = <1>;
> > > > +
> > > > +                            csi2_in: endpoint@0 {
> > > > +                                    reg = <0>;
> > > > +                                    clock-lanes = <0>;
> > > > +                                    data-lanes = <1 2>;
> > > > +                                    remote-endpoint = <&ov5645_ep>;
> > > > +                            };
> > > > +                    };
> > > > +
> > > > +                    port@2 {
> > > > +                            #address-cells = <1>;
> > > > +                            #size-cells = <0>;
> > > > +
> > > > +                            reg = <2>;
> > > > +
> > > > +                            csi2cru: endpoint@0 {
> > > > +                                    reg = <0>;
> > > > +                                    remote-endpoint= <&crucsi2>;
> > > > +                            };
> > > > +                    };
> > > > +
> > > > +                    port@3 {
> > > > +                            #address-cells = <1>;
> > > > +                            #size-cells = <0>;
> > > > +
> > > > +                            reg = <3>;
> > > > +
> > > > +                            crucsi2: endpoint@0 {
> > > > +                                    reg = <0>;
> > > > +                                    remote-endpoint= <&csi2cru>;
> > > > +                            };
> > > > +                    };
> > > > +            };
> > > > +    };
>
> --
> Regards,
>
> Laurent Pinchart
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
new file mode 100644
index 000000000000..7b2835810516
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
@@ -0,0 +1,227 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (C) 2021 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/renesas,rzg2l-cru.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L Camera Data Receiving Unit (CRU)
+
+maintainers:
+  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+
+description:
+  The RZ/G2L Camera Data Receiving Unit (CRU) device provides video input
+  capabilities for the Renesas RZ/G2L family of devices.
+
+  Depending on the instance the Image Processing input is connected to
+  external SoC pins or to a CSI-2 receiver.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - renesas,r9a07g044-cru     # RZ/G2{L,LC}
+          - const: renesas,rzg2l-cru
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 4
+
+  interrupt-names:
+    items:
+      - const: csi2_link_int
+      - const: image_conv_int
+      - const: image_conv_err_int
+      - const: axi_mst_err_int
+
+  clocks:
+    items:
+      - description: Internal clock for connecting CRU and MIPI
+      - description: CRU Main clock
+      - description: CPU Register access clock
+      - description: CRU image transfer clock
+
+  clock-names:
+    items:
+      - const: sysclk
+      - const: vclk
+      - const: pclk
+      - const: aclk
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    items:
+      - description: CRU_CMN_RSTB reset terminal
+      - description: CRU_PRESETN reset terminal
+      - description: CRU_ARESETN reset terminal
+
+  reset-names:
+    items:
+      - const: cmn-rstb
+      - const: presetn
+      - const: aresetn
+
+  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 a parallel input source.
+
+        properties:
+          endpoint:
+            $ref: video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              hsync-active: true
+
+              vsync-active: true
+
+              bus-width: true
+
+              data-shift: true
+
+      port@1:
+        $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:
+              clock-lanes:
+                maxItems: 1
+
+              data-lanes:
+                maxItems: 1
+
+            required:
+              - clock-lanes
+              - data-lanes
+
+      port@2:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          Output port node, describing the RZ/G2L Image Processing module
+          connected the CSI-2 receiver
+
+        properties:
+          endpoint@0:
+            $ref: /schemas/graph.yaml#/properties/endpoint
+            description: Endpoint connected to CSI2.
+
+        anyOf:
+          - required:
+              - endpoint@0
+
+      port@3:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          Input port node, describing the RZ/G2L CSI-2 module connected the
+          Image Processing block.
+
+        properties:
+          endpoint@0:
+            $ref: /schemas/graph.yaml#/properties/endpoint
+            description: Endpoint connected to CSI2.
+
+        anyOf:
+          - required:
+              - endpoint@0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - power-domains
+
+additionalProperties: false
+
+examples:
+  # Device node example with CSI-2
+  - |
+    #include <dt-bindings/clock/r9a07g044-cpg.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    cru: video@10830000 {
+            compatible = "renesas,r9a07g044-cru", "renesas,rzg2l-cru";
+            reg = <0x10830000 0x10000>;
+            interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "csi2_link_int", "image_conv_int",
+                              "image_conv_err_int", "axi_mst_err_int";
+            clocks = <&cpg CPG_MOD R9A07G044_CRU_SYSCLK>,
+                     <&cpg CPG_MOD R9A07G044_CRU_VCLK>,
+                     <&cpg CPG_MOD R9A07G044_CRU_PCLK>,
+                     <&cpg CPG_MOD R9A07G044_CRU_ACLK>;
+            clock-names = "sysclk", "vclk", "pclk", "aclk";
+            power-domains = <&cpg>;
+            resets = <&cpg R9A07G044_CRU_CMN_RSTB>,
+                     <&cpg R9A07G044_CRU_PRESETN>,
+                     <&cpg R9A07G044_CRU_ARESETN>;
+            reset-names = "cmn-rstb", "presetn", "aresetn";
+
+            ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@1 {
+                            #address-cells = <1>;
+                            #size-cells = <0>;
+
+                            reg = <1>;
+
+                            csi2_in: endpoint@0 {
+                                    reg = <0>;
+                                    clock-lanes = <0>;
+                                    data-lanes = <1 2>;
+                                    remote-endpoint = <&ov5645_ep>;
+                            };
+                    };
+
+                    port@2 {
+                            #address-cells = <1>;
+                            #size-cells = <0>;
+
+                            reg = <2>;
+
+                            csi2cru: endpoint@0 {
+                                    reg = <0>;
+                                    remote-endpoint= <&crucsi2>;
+                            };
+                    };
+
+                    port@3 {
+                            #address-cells = <1>;
+                            #size-cells = <0>;
+
+                            reg = <3>;
+
+                            crucsi2: endpoint@0 {
+                                    reg = <0>;
+                                    remote-endpoint= <&csi2cru>;
+                            };
+                    };
+            };
+    };