[PATCH 0/4] Add driver for CSI2 and CRU modules found on Renesas RZ/G2L SoC

[PATCH 0/4] Add driver for CSI2 and CRU modules found on Renesas RZ/G2L SoC

[Lad Prabhakar]

Hi All,

This patch series aims to add driver support to CRU module found
on Renesas RZ/G2L SoC.

The Camera Data Receiving Unit (CRU) consists of a MIPI CSI-2
block and an Image Processing block. The Image Processing block
can receive video data received from the external Digital Parallel
Interface or MIPI CSI-2 block, and perform appropriate image
processing for each.

More details:
* https://renesas.info/wiki/File:CRU.png
* https://www.renesas.com/document/mah/rzg2l-group-rzg2lc-group-users-manual-hardware-0?language=en&r=1467981

Currently the driver has been tested using yavta and Gstreamer
on RZ/G2L SMARC EVK using the ov5645 sensor on CSI2 interface
only.

RFC v2: https://patchwork.kernel.org/project/linux-renesas-soc/cover/20220121010543.31385-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

RFC v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/20211207012351.15754-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar

Lad Prabhakar (4):
  media: dt-bindings: media: Document RZ/G2L CSI-2 block
  media: dt-bindings: media: Document RZ/G2L CRU
  media: platform: Add Renesas RZ/G2L MIPI CSI-2 receiver driver
  media: platform: Add Renesas RZ/G2L CRU driver

 .../bindings/media/renesas,rzg2l-cru.yaml     | 142 ++++
 .../bindings/media/renesas,rzg2l-csi2.yaml    | 149 ++++
 drivers/media/platform/renesas/Kconfig        |   1 +
 drivers/media/platform/renesas/Makefile       |   1 +
 .../media/platform/renesas/rzg2l-cru/Kconfig  |  34 +
 .../media/platform/renesas/rzg2l-cru/Makefile |   6 +
 .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 371 +++++++++
 .../platform/renesas/rzg2l-cru/rzg2l-cru.h    | 144 ++++
 .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 702 +++++++++++++++++
 .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  49 ++
 .../platform/renesas/rzg2l-cru/rzg2l-dma.c    | 728 ++++++++++++++++++
 .../platform/renesas/rzg2l-cru/rzg2l-v4l2.c   | 354 +++++++++
 12 files changed, 2681 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
 create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-dma.c
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-v4l2.c

-- 
2.25.1

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

[Lad Prabhakar]

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>
---
RFC v2 -> v1
* Fixed review comments pointed by Rob and Jacopo.

RFC v1 -> RFC v2
* New patch
---
 .../bindings/media/renesas,rzg2l-csi2.yaml    | 149 ++++++++++++++++++
 1 file changed, 149 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..f82f88c096df
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
@@ -0,0 +1,149 @@
+# 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 (and alike SoC's) MIPI CSI-2 receiver
+
+maintainers:
+  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+
+description:
+  The CSI-2 receiver device provides MIPI CSI-2 capabilities for the Renesas RZ/G2L
+  (and alike SoCs). 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}
+              - renesas,r9a07g054-csi2       # RZ/V2L
+          - const: renesas,rzg2l-csi2
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    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:
+    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>;
+                            };
+                    };
+            };
+    };
-- 
2.25.1

Re: [PATCH 1/4] media: dt-bindings: media: Document RZ/G2L CSI-2 block

[Krzysztof Kozlowski]

On 01/08/2022 23:47, 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>
> ---
> RFC v2 -> v1

Too many "media" in the subject prefixes.

> * Fixed review comments pointed by Rob and Jacopo.
> 
> RFC v1 -> RFC v2
> * New patch
> ---
>  .../bindings/media/renesas,rzg2l-csi2.yaml    | 149 ++++++++++++++++++
>  1 file changed, 149 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..f82f88c096df
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> @@ -0,0 +1,149 @@
> +# 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 (and alike SoC's) MIPI CSI-2 receiver
> +
> +maintainers:
> +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +
> +description:
> +  The CSI-2 receiver device provides MIPI CSI-2 capabilities for the Renesas RZ/G2L
> +  (and alike SoCs). 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}
> +              - renesas,r9a07g054-csi2       # RZ/V2L
> +          - const: renesas,rzg2l-csi2
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-names:
> +    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:
> +    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 {

Node name just "csi"

https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +            compatible = "renesas,r9a07g044-csi2", "renesas,rzg2l-csi2";
> +            reg = <0x10830400 0xfc00>;

Please use 4-space indentation for example DTS.

> +            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>;
> +                            };
> +                    };
> +            };
> +    };


Best regards,
Krzysztof

Re: [PATCH 1/4] media: dt-bindings: media: Document RZ/G2L CSI-2 block

[Lad, Prabhakar]

Hi Krzysztof,

Thank you for the review.

On Tue, Aug 2, 2022 at 8:36 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 01/08/2022 23:47, 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>
> > ---
> > RFC v2 -> v1
>
> Too many "media" in the subject prefixes.
>
I followed the pattern seen by "git log --oneline
Documentation/devicetree/bindings/media/", but I do agree I can drop
the extra "media" after "dt-bindings:"

> > * Fixed review comments pointed by Rob and Jacopo.
> >
> > RFC v1 -> RFC v2
> > * New patch
> > ---
> >  .../bindings/media/renesas,rzg2l-csi2.yaml    | 149 ++++++++++++++++++
> >  1 file changed, 149 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..f82f88c096df
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > @@ -0,0 +1,149 @@
> > +# 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 (and alike SoC's) MIPI CSI-2 receiver
> > +
> > +maintainers:
> > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +
> > +description:
> > +  The CSI-2 receiver device provides MIPI CSI-2 capabilities for the Renesas RZ/G2L
> > +  (and alike SoCs). 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}
> > +              - renesas,r9a07g054-csi2       # RZ/V2L
> > +          - const: renesas,rzg2l-csi2
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-names:
> > +    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:
> > +    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 {
>
> Node name just "csi"
>
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
OK will replace it with csi.

>
> > +            compatible = "renesas,r9a07g044-csi2", "renesas,rzg2l-csi2";
> > +            reg = <0x10830400 0xfc00>;
>
> Please use 4-space indentation for example DTS.
>
Sure will do.

Cheers,
Prabhakar

> > +            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>;
> > +                            };
> > +                    };
> > +            };
> > +    };
>
>
> Best regards,
> Krzysztof

Re: [PATCH 1/4] media: dt-bindings: media: Document RZ/G2L CSI-2 block

[Geert Uytterhoeven]

Hi Prabhakar,

On Mon, Aug 1, 2022 at 11:47 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> 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>
> ---
> RFC v2 -> v1
> * Fixed review comments pointed by Rob and Jacopo.

Thanks for the update!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> @@ -0,0 +1,149 @@

> +  resets:
> +    items:
> +      - description: CRU_CMN_RSTB reset terminal
> +
> +  reset-names:
> +    const: cmn-rstb

While it doesn't hurt to always have reset-names, you may want to drop
it, as there is only a single reset.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/4] media: dt-bindings: media: Document RZ/G2L CSI-2 block

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Thu, Aug 18, 2022 at 9:56 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Aug 1, 2022 at 11:47 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> 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>
> > ---
> > RFC v2 -> v1
> > * Fixed review comments pointed by Rob and Jacopo.
>
> Thanks for the update!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > @@ -0,0 +1,149 @@
>
> > +  resets:
> > +    items:
> > +      - description: CRU_CMN_RSTB reset terminal
> > +
> > +  reset-names:
> > +    const: cmn-rstb
>
> While it doesn't hurt to always have reset-names, you may want to drop
> it, as there is only a single reset.
>
Agreed, I will drop it.

Cheers,
Prabhakar

Re: [PATCH 1/4] media: dt-bindings: media: Document RZ/G2L CSI-2 block

[Laurent Pinchart]

Hi Prabhakar,

Thank you for the patch.

On Mon, Aug 01, 2022 at 10:47:15PM +0100, 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>
> ---
> RFC v2 -> v1
> * Fixed review comments pointed by Rob and Jacopo.
> 
> RFC v1 -> RFC v2
> * New patch
> ---
>  .../bindings/media/renesas,rzg2l-csi2.yaml    | 149 ++++++++++++++++++
>  1 file changed, 149 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..f82f88c096df
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> @@ -0,0 +1,149 @@
> +# 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 (and alike SoC's) MIPI CSI-2 receiver
> +
> +maintainers:
> +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +
> +description:
> +  The CSI-2 receiver device provides MIPI CSI-2 capabilities for the Renesas RZ/G2L
> +  (and alike SoCs). 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:

You can drop the oneOf and write

    items:
      - enum:
          - renesas,r9a07g044-csi2       # RZ/G2{L,LC}
          - renesas,r9a07g054-csi2       # RZ/V2L
      - const: renesas,rzg2l-csi2

> +          - enum:
> +              - renesas,r9a07g044-csi2       # RZ/G2{L,LC}
> +              - renesas,r9a07g054-csi2       # RZ/V2L
> +          - const: renesas,rzg2l-csi2
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-names:
> +    const: csi2_link

If there's a single interrupt you can drop the name.

> +
> +  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:
> +    const: cmn-rstb

Same here.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +
> +  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>;
> +                            };
> +                    };
> +            };
> +    };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] media: dt-bindings: media: Document RZ/G2L CSI-2 block

[Lad, Prabhakar]

Hi Laurent,

Thank you for the patch.

On Tue, Aug 30, 2022 at 12:32 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Mon, Aug 01, 2022 at 10:47:15PM +0100, 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>
> > ---
> > RFC v2 -> v1
> > * Fixed review comments pointed by Rob and Jacopo.
> >
> > RFC v1 -> RFC v2
> > * New patch
> > ---
> >  .../bindings/media/renesas,rzg2l-csi2.yaml    | 149 ++++++++++++++++++
> >  1 file changed, 149 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..f82f88c096df
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > @@ -0,0 +1,149 @@
> > +# 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 (and alike SoC's) MIPI CSI-2 receiver
> > +
> > +maintainers:
> > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +
> > +description:
> > +  The CSI-2 receiver device provides MIPI CSI-2 capabilities for the Renesas RZ/G2L
> > +  (and alike SoCs). 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:
>
> You can drop the oneOf and write
>
Agreed.

>     items:
>       - enum:
>           - renesas,r9a07g044-csi2       # RZ/G2{L,LC}
>           - renesas,r9a07g054-csi2       # RZ/V2L
>       - const: renesas,rzg2l-csi2
>
> > +          - enum:
> > +              - renesas,r9a07g044-csi2       # RZ/G2{L,LC}
> > +              - renesas,r9a07g054-csi2       # RZ/V2L
> > +          - const: renesas,rzg2l-csi2
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-names:
> > +    const: csi2_link
>
> If there's a single interrupt you can drop the name.
>
OK, I will drop it.

Cheers,
Prabhakar

> > +
> > +  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:
> > +    const: cmn-rstb
>
> Same here.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> > +
> > +  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>;
> > +                            };
> > +                    };
> > +            };
> > +    };
>
> --
> Regards,
>
> Laurent Pinchart

[PATCH 2/4] media: dt-bindings: media: Document RZ/G2L CRU

[Lad Prabhakar]

Document the CRU block found on Renesas RZ/G2L SoC's.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC v2 -> v1
* Dropped endpoint stuff from port1 as suggested by Rob
* Updated description for endpoint

RFC v1 -> RFC v2
* Dropped CSI
---
 .../bindings/media/renesas,rzg2l-cru.yaml     | 142 ++++++++++++++++++
 1 file changed, 142 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..d7389693dae9
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
@@ -0,0 +1,142 @@
+# 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-cru.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L (and alike SoC's) Camera Data Receiving Unit (CRU) Image processing
+
+maintainers:
+  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+
+description:
+  The CRU image processing module is a data conversion module equipped with pixel
+  color space conversion, LUT, pixel format conversion, etc. An MIPI CSI-2 input and
+  parallel (including ITU-R BT.656) input are provided as the image sensor interface.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - renesas,r9a07g044-cru     # RZ/G2{L,LC}
+              - renesas,r9a07g054-cru     # RZ/V2L
+          - const: renesas,rzg2l-cru
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 3
+
+  interrupt-names:
+    items:
+      - const: image_conv
+      - const: image_conv_err
+      - const: axi_mst_err
+
+  clocks:
+    items:
+      - description: CRU Main clock
+      - description: CPU Register access clock
+      - description: CRU image transfer clock
+
+  clock-names:
+    items:
+      - const: vclk
+      - const: pclk
+      - const: aclk
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    items:
+      - description: CRU_PRESETN reset terminal
+      - description: CRU_ARESETN reset terminal
+
+  reset-names:
+    items:
+      - 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#/properties/port
+        description:
+          Input port node, describing the Image Processing module connected to the
+          CSI-2 receiver.
+
+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 0x400>;
+            interrupts = <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "image_conv", "image_conv_err", "axi_mst_err";
+            clocks = <&cpg CPG_MOD R9A07G044_CRU_VCLK>,
+                     <&cpg CPG_MOD R9A07G044_CRU_PCLK>,
+                     <&cpg CPG_MOD R9A07G044_CRU_ACLK>;
+            clock-names = "vclk", "pclk", "aclk";
+            power-domains = <&cpg>;
+            resets = <&cpg R9A07G044_CRU_PRESETN>,
+                     <&cpg R9A07G044_CRU_ARESETN>;
+            reset-names = "presetn", "aresetn";
+
+            ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@1 {
+                            #address-cells = <1>;
+                            #size-cells = <0>;
+
+                            reg = <1>;
+
+                            crucsi2: endpoint@0 {
+                                    reg = <0>;
+                                    remote-endpoint= <&csi2cru>;
+                            };
+                    };
+            };
+    };
-- 
2.25.1

Re: [PATCH 2/4] media: dt-bindings: media: Document RZ/G2L CRU

[Krzysztof Kozlowski]

On 01/08/2022 23:47, 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>
> ---
> RFC v2 -> v1
> * Dropped endpoint stuff from port1 as suggested by Rob
> * Updated description for endpoint
> 
> RFC v1 -> RFC v2
> * Dropped CSI
> ---
>  .../bindings/media/renesas,rzg2l-cru.yaml     | 142 ++++++++++++++++++
>  1 file changed, 142 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..d7389693dae9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> @@ -0,0 +1,142 @@
> +# 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-cru.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L (and alike SoC's) Camera Data Receiving Unit (CRU) Image processing
> +
> +maintainers:
> +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +
> +description:
> +  The CRU image processing module is a data conversion module equipped with pixel
> +  color space conversion, LUT, pixel format conversion, etc. An MIPI CSI-2 input and
> +  parallel (including ITU-R BT.656) input are provided as the image sensor interface.
> +
> +properties:
> +  compatible:
> +    oneOf:

No need for oneOf, unless you already have a patch adding second case to
oneOf.

> +      - items:
> +          - enum:
> +              - renesas,r9a07g044-cru     # RZ/G2{L,LC}
> +              - renesas,r9a07g054-cru     # RZ/V2L
> +          - const: renesas,rzg2l-cru
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 3
> +
> +  interrupt-names:
> +    items:
> +      - const: image_conv
> +      - const: image_conv_err
> +      - const: axi_mst_err
> +
> +  clocks:
> +    items:
> +      - description: CRU Main clock
> +      - description: CPU Register access clock
> +      - description: CRU image transfer clock
> +
> +  clock-names:
> +    items:
> +      - const: vclk
> +      - const: pclk
> +      - const: aclk
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: CRU_PRESETN reset terminal
> +      - description: CRU_ARESETN reset terminal
> +
> +  reset-names:
> +    items:
> +      - 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#/properties/port
> +        description:
> +          Input port node, describing the Image Processing module connected to the
> +          CSI-2 receiver.
> +
> +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";

Also 4-space for DTS example, please.



Best regards,
Krzysztof

Re: [PATCH 2/4] media: dt-bindings: media: Document RZ/G2L CRU

[Lad, Prabhakar]

Hi Krzysztof,

Thank you for the review.

On Tue, Aug 2, 2022 at 8:37 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 01/08/2022 23:47, 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>
> > ---
> > RFC v2 -> v1
> > * Dropped endpoint stuff from port1 as suggested by Rob
> > * Updated description for endpoint
> >
> > RFC v1 -> RFC v2
> > * Dropped CSI
> > ---
> >  .../bindings/media/renesas,rzg2l-cru.yaml     | 142 ++++++++++++++++++
> >  1 file changed, 142 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..d7389693dae9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > @@ -0,0 +1,142 @@
> > +# 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-cru.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/G2L (and alike SoC's) Camera Data Receiving Unit (CRU) Image processing
> > +
> > +maintainers:
> > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +
> > +description:
> > +  The CRU image processing module is a data conversion module equipped with pixel
> > +  color space conversion, LUT, pixel format conversion, etc. An MIPI CSI-2 input and
> > +  parallel (including ITU-R BT.656) input are provided as the image sensor interface.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
>
> No need for oneOf, unless you already have a patch adding second case to
> oneOf.
>
Agreed, I will drop that.

> > +      - items:
> > +          - enum:
> > +              - renesas,r9a07g044-cru     # RZ/G2{L,LC}
> > +              - renesas,r9a07g054-cru     # RZ/V2L
> > +          - const: renesas,rzg2l-cru
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 3
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: image_conv
> > +      - const: image_conv_err
> > +      - const: axi_mst_err
> > +
> > +  clocks:
> > +    items:
> > +      - description: CRU Main clock
> > +      - description: CPU Register access clock
> > +      - description: CRU image transfer clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: vclk
> > +      - const: pclk
> > +      - const: aclk
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    items:
> > +      - description: CRU_PRESETN reset terminal
> > +      - description: CRU_ARESETN reset terminal
> > +
> > +  reset-names:
> > +    items:
> > +      - 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#/properties/port
> > +        description:
> > +          Input port node, describing the Image Processing module connected to the
> > +          CSI-2 receiver.
> > +
> > +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";
>
> Also 4-space for DTS example, please.
>
Agreed.

Cheers,
Prabhakar

Re: [PATCH 2/4] media: dt-bindings: media: Document RZ/G2L CRU

[Laurent Pinchart]

Hi Prabhakar,

Thank you for the patch.

On Mon, Aug 01, 2022 at 10:47:16PM +0100, 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>
> ---
> RFC v2 -> v1
> * Dropped endpoint stuff from port1 as suggested by Rob
> * Updated description for endpoint
> 
> RFC v1 -> RFC v2
> * Dropped CSI
> ---
>  .../bindings/media/renesas,rzg2l-cru.yaml     | 142 ++++++++++++++++++
>  1 file changed, 142 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..d7389693dae9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> @@ -0,0 +1,142 @@
> +# 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-cru.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L (and alike SoC's) Camera Data Receiving Unit (CRU) Image processing
> +
> +maintainers:
> +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +
> +description:
> +  The CRU image processing module is a data conversion module equipped with pixel
> +  color space conversion, LUT, pixel format conversion, etc. An MIPI CSI-2 input and
> +  parallel (including ITU-R BT.656) input are provided as the image sensor interface.
> +
> +properties:
> +  compatible:
> +    oneOf:

No oneOf here either.

> +      - items:
> +          - enum:
> +              - renesas,r9a07g044-cru     # RZ/G2{L,LC}
> +              - renesas,r9a07g054-cru     # RZ/V2L
> +          - const: renesas,rzg2l-cru
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 3
> +
> +  interrupt-names:
> +    items:
> +      - const: image_conv
> +      - const: image_conv_err
> +      - const: axi_mst_err
> +
> +  clocks:
> +    items:
> +      - description: CRU Main clock
> +      - description: CPU Register access clock
> +      - description: CRU image transfer clock
> +
> +  clock-names:
> +    items:
> +      - const: vclk
> +      - const: pclk
> +      - const: aclk
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: CRU_PRESETN reset terminal
> +      - description: CRU_ARESETN reset terminal
> +
> +  reset-names:
> +    items:
> +      - 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#/properties/port
> +        description:
> +          Input port node, describing the Image Processing module connected to the
> +          CSI-2 receiver.

Both ports should be mandatory as they exist at the hardware level.
Connections (through endpoints) are optional.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +
> +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 0x400>;
> +            interrupts = <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "image_conv", "image_conv_err", "axi_mst_err";
> +            clocks = <&cpg CPG_MOD R9A07G044_CRU_VCLK>,
> +                     <&cpg CPG_MOD R9A07G044_CRU_PCLK>,
> +                     <&cpg CPG_MOD R9A07G044_CRU_ACLK>;
> +            clock-names = "vclk", "pclk", "aclk";
> +            power-domains = <&cpg>;
> +            resets = <&cpg R9A07G044_CRU_PRESETN>,
> +                     <&cpg R9A07G044_CRU_ARESETN>;
> +            reset-names = "presetn", "aresetn";
> +
> +            ports {
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +
> +                    port@1 {
> +                            #address-cells = <1>;
> +                            #size-cells = <0>;
> +
> +                            reg = <1>;
> +
> +                            crucsi2: endpoint@0 {
> +                                    reg = <0>;
> +                                    remote-endpoint= <&csi2cru>;
> +                            };
> +                    };
> +            };
> +    };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] media: dt-bindings: media: Document RZ/G2L CRU

[Lad, Prabhakar]

Hi Laurent,

Thank you for the review.

On Tue, Aug 30, 2022 at 12:42 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Mon, Aug 01, 2022 at 10:47:16PM +0100, 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>
> > ---
> > RFC v2 -> v1
> > * Dropped endpoint stuff from port1 as suggested by Rob
> > * Updated description for endpoint
> >
> > RFC v1 -> RFC v2
> > * Dropped CSI
> > ---
> >  .../bindings/media/renesas,rzg2l-cru.yaml     | 142 ++++++++++++++++++
> >  1 file changed, 142 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..d7389693dae9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > @@ -0,0 +1,142 @@
> > +# 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-cru.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/G2L (and alike SoC's) Camera Data Receiving Unit (CRU) Image processing
> > +
> > +maintainers:
> > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +
> > +description:
> > +  The CRU image processing module is a data conversion module equipped with pixel
> > +  color space conversion, LUT, pixel format conversion, etc. An MIPI CSI-2 input and
> > +  parallel (including ITU-R BT.656) input are provided as the image sensor interface.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
>
> No oneOf here either.
>
Agreed, I will drop it.

> > +      - items:
> > +          - enum:
> > +              - renesas,r9a07g044-cru     # RZ/G2{L,LC}
> > +              - renesas,r9a07g054-cru     # RZ/V2L
> > +          - const: renesas,rzg2l-cru
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 3
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: image_conv
> > +      - const: image_conv_err
> > +      - const: axi_mst_err
> > +
> > +  clocks:
> > +    items:
> > +      - description: CRU Main clock
> > +      - description: CPU Register access clock
> > +      - description: CRU image transfer clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: vclk
> > +      - const: pclk
> > +      - const: aclk
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    items:
> > +      - description: CRU_PRESETN reset terminal
> > +      - description: CRU_ARESETN reset terminal
> > +
> > +  reset-names:
> > +    items:
> > +      - 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#/properties/port
> > +        description:
> > +          Input port node, describing the Image Processing module connected to the
> > +          CSI-2 receiver.
>
> Both ports should be mandatory as they exist at the hardware level.
> Connections (through endpoints) are optional.
>
Agreed, I will make the ports as required.

Cheers,
Prabhakar

[PATCH 3/4] media: platform: Add Renesas RZ/G2L MIPI CSI-2 receiver driver

[Lad Prabhakar]

Add MIPI CSI-2 receiver driver for Renesas RZ/G2L. The MIPI
CSI-2 is part of the CRU module found on RZ/G2L family.

Based on a patch in the BSP by Hien Huynh
<hien.huynh.px@renesas.com>

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC v2 -> v1
* Fixed initialization sequence of DPHY and link
* Exported DPHY and link initialization functions so that the
  CRU core driver can initialize the CRU and CSI2 as per the HW manual.

RFC v1 -> RFC v2
* new patch (split up as new driver compared to v1)
---
 drivers/media/platform/renesas/Kconfig        |   1 +
 drivers/media/platform/renesas/Makefile       |   1 +
 .../media/platform/renesas/rzg2l-cru/Kconfig  |  17 +
 .../media/platform/renesas/rzg2l-cru/Makefile |   3 +
 .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 702 ++++++++++++++++++
 .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  49 ++
 6 files changed, 773 insertions(+)
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h

diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
index 9fd90672ea2d..ed788e991f74 100644
--- a/drivers/media/platform/renesas/Kconfig
+++ b/drivers/media/platform/renesas/Kconfig
@@ -41,6 +41,7 @@ config VIDEO_SH_VOU
 	  Support for the Video Output Unit (VOU) on SuperH SoCs.
 
 source "drivers/media/platform/renesas/rcar-vin/Kconfig"
+source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
 
 # Mem2mem drivers
 
diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
index 3ec226ef5fd2..55854e868887 100644
--- a/drivers/media/platform/renesas/Makefile
+++ b/drivers/media/platform/renesas/Makefile
@@ -4,6 +4,7 @@
 #
 
 obj-y += rcar-vin/
+obj-y += rzg2l-cru/
 obj-y += vsp1/
 
 obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
diff --git a/drivers/media/platform/renesas/rzg2l-cru/Kconfig b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
new file mode 100644
index 000000000000..150bf16f167c
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config VIDEO_RZG2L_CSI2
+	tristate "RZ/G2L MIPI CSI-2 Receiver"
+	depends on V4L_PLATFORM_DRIVERS
+	depends on VIDEO_DEV && OF
+	depends on ARCH_RENESAS || COMPILE_TEST
+	select MEDIA_CONTROLLER
+	select VIDEO_V4L2_SUBDEV_API
+	select RESET_CONTROLLER
+	select V4L2_FWNODE
+	help
+	  Support for Renesas RZ/G2L (and alike SoC's) MIPI CSI-2
+	  Receiver driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rzg2l-csi2.
diff --git a/drivers/media/platform/renesas/rzg2l-cru/Makefile b/drivers/media/platform/renesas/rzg2l-cru/Makefile
new file mode 100644
index 000000000000..91ea97a944e6
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_VIDEO_RZG2L_CSI2) += rzg2l-csi2.o
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
new file mode 100644
index 000000000000..252e33ddb6e8
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -0,0 +1,702 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Renesas RZ/G2L MIPI CSI-2 Receiver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/sys_soc.h>
+#include <linux/units.h>
+
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-subdev.h>
+
+#include "rzg2l-csi2.h"
+
+/* LINK registers */
+/* Module Configuration Register */
+#define CSI2nMCG			0x0
+#define CSI2nMCG_SDLN			GENMASK(11, 8)
+
+/* Module Control Register 0 */
+#define CSI2nMCT0			0x10
+#define CSI2nMCT0_VDLN(x)		((x) << 0)
+
+/* Module Control Register 2 */
+#define CSI2nMCT2			0x18
+#define CSI2nMCT2_FRRSKW(x)		((x) << 16)
+#define CSI2nMCT2_FRRCLK(x)		((x) << 0)
+
+/* Module Control Register 3 */
+#define CSI2nMCT3			0x1c
+#define CSI2nMCT3_RXEN			BIT(0)
+
+/* Reset Control Register */
+#define CSI2nRTCT			0x28
+#define CSI2nRTCT_VSRST			BIT(0)
+
+/* Reset Status Register */
+#define CSI2nRTST			0x2c
+#define CSI2nRTST_VSRSTS		BIT(0)
+
+/* Receive Data Type Enable Low Register */
+#define CSI2nDTEL			0x60
+
+/* Receive Data Type Enable High Register */
+#define CSI2nDTEH			0x64
+
+/* Power Management Status Register */
+#define CSI2nPMST			0x200
+
+/* Power Management Status Clear Register */
+#define CSI2nPMSC			0x204
+
+/* DPHY registers */
+/* D-PHY Control Register 0 */
+#define CSIDPHYCTRL0			0x400
+#define CSIDPHYCTRL0_EN_LDO1200		BIT(1)
+#define CSIDPHYCTRL0_EN_BGR		BIT(0)
+
+/* D-PHY Timing Register 0 */
+#define CSIDPHYTIM0			0x404
+#define CSIDPHYTIM0_TCLK_MISS(x)	((x) << 24)
+#define CSIDPHYTIM0_T_INIT(x)		((x) << 0)
+
+/* D-PHY Timing Register 1 */
+#define CSIDPHYTIM1			0x408
+#define CSIDPHYTIM1_THS_PREPARE(x)	((x) << 24)
+#define CSIDPHYTIM1_TCLK_PREPARE(x)	((x) << 16)
+#define CSIDPHYTIM1_THS_SETTLE(x)	((x) << 8)
+#define CSIDPHYTIM1_TCLK_SETTLE(x)	((x) << 0)
+
+/* D-PHY Skew Adjustment Function */
+#define CSIDPHYSKW0			0x460
+#define CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(x)	((x) & 0x3)
+#define CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(x)	(((x) & 0x3) << 4)
+#define CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(x)	(((x) & 0x3) << 8)
+#define CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(x)	(((x) & 0x3) << 12)
+#define CSIDPHYSKW0_DEFAULT_SKW		CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(1) | \
+					CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(1) | \
+					CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
+					CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1)
+
+#define VSRSTS_RETRIES			20
+
+struct timings {
+	u32 t_init;
+	u32 tclk_miss;
+	u32 tclk_settle;
+	u32 ths_settle;
+	u32 tclk_prepare;
+	u32 ths_prepare;
+};
+
+enum dphy_timings {
+	TRANSMISSION_RATE_80_MBPS = 0,
+	TRANSMISSION_RATE_125_MBPS,
+	TRANSMISSION_RATE_250_MBPS,
+	TRANSMISSION_RATE_360_MBPS,
+	TRANSMISSION_RATE_360_PLUS_MBPS,
+};
+
+static const struct timings global_timings[] = {
+	[TRANSMISSION_RATE_80_MBPS] = {
+		.t_init = 79801,
+		.tclk_miss = 4,
+		.tclk_settle = 23,
+		.ths_settle = 31,
+		.tclk_prepare = 10,
+		.ths_prepare = 19,
+	},
+	[TRANSMISSION_RATE_125_MBPS] = {
+		.t_init = 79801,
+		.tclk_miss = 4,
+		.tclk_settle = 23,
+		.ths_settle = 28,
+		.tclk_prepare = 10,
+		.ths_prepare = 19,
+	},
+	[TRANSMISSION_RATE_250_MBPS] = {
+		.t_init = 79801,
+		.tclk_miss = 4,
+		.tclk_settle = 23,
+		.ths_settle = 22,
+		.tclk_prepare = 10,
+		.ths_prepare = 16,
+	},
+	[TRANSMISSION_RATE_360_MBPS] = {
+		.t_init = 79801,
+		.tclk_miss = 4,
+		.tclk_settle = 18,
+		.ths_settle = 19,
+		.tclk_prepare = 10,
+		.ths_prepare = 10,
+	},
+	[TRANSMISSION_RATE_360_PLUS_MBPS] = {
+		.t_init = 79801,
+		.tclk_miss = 4,
+		.tclk_settle = 18,
+		.ths_settle = 18,
+		.tclk_prepare = 10,
+		.ths_prepare = 10,
+	},
+};
+
+struct rzg2l_csi2_format {
+	u32 code;
+	unsigned int datatype;
+	unsigned int bpp;
+};
+
+static const struct rzg2l_csi2_format rzg2l_csi2_formats[] = {
+	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
+};
+
+static const struct rzg2l_csi2_format *rzg2l_csi2_code_to_fmt(unsigned int code)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(rzg2l_csi2_formats); i++)
+		if (rzg2l_csi2_formats[i].code == code)
+			return &rzg2l_csi2_formats[i];
+
+	return NULL;
+}
+
+static inline struct rzg2l_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct rzg2l_csi2, notifier);
+}
+
+static u32 rzg2l_csi2_read(struct rzg2l_csi2 *csi2, unsigned int reg)
+{
+	return ioread32(csi2->base + reg);
+}
+
+static void rzg2l_csi2_write(struct rzg2l_csi2 *csi2, unsigned int reg,
+			     u32 data)
+{
+	iowrite32(data, csi2->base + reg);
+}
+
+static void rzg2l_csi2_set(struct rzg2l_csi2 *csi2, unsigned int reg, u32 set)
+{
+	rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) | set);
+}
+
+static void rzg2l_csi2_clr(struct rzg2l_csi2 *csi2, unsigned int reg, u32 clr)
+{
+	rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) & ~clr);
+}
+
+static int rzg2l_csi2_calc_mbps(struct rzg2l_csi2 *csi2)
+{
+	const struct rzg2l_csi2_format *format;
+	struct v4l2_subdev *source;
+	struct v4l2_ctrl *ctrl;
+	u64 mbps;
+
+	if (!csi2->remote)
+		return -ENODEV;
+
+	source = csi2->remote;
+
+	/* Read the pixel rate control from remote. */
+	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
+	if (!ctrl) {
+		dev_err(csi2->dev, "no pixel rate control in subdev %s\n",
+			source->name);
+		return -EINVAL;
+	}
+
+	format = rzg2l_csi2_code_to_fmt(csi2->mf.code);
+
+	/*
+	 * Calculate hsfreq in Mbps
+	 * hsfreq = (pixel_rate * bits_per_sample) / number_of_lanes
+	 */
+	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
+	do_div(mbps, csi2->lanes * 1000000);
+
+	return mbps;
+}
+
+int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2)
+{
+	return reset_control_deassert(csi2->rstc);
+}
+EXPORT_SYMBOL_GPL(rzg2l_csi2_cmn_rstb_deassert);
+
+/* -----------------------------------------------------------------------------
+ * DPHY setting
+ */
+
+int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on)
+{
+	int lanes, mbps;
+
+	dev_dbg(csi2->dev, "Input size (%ux%u%c)\n",
+		csi2->mf.width, csi2->mf.height,
+		csi2->mf.field == V4L2_FIELD_NONE ? 'p' : 'i');
+
+	/* Checking the maximum lanes support for CSI-2 module */
+	lanes = (rzg2l_csi2_read(csi2, CSI2nMCG) & CSI2nMCG_SDLN) >> 8;
+	if (lanes < csi2->lanes) {
+		dev_err(csi2->dev,
+			"Failed to support %d data lanes\n", csi2->lanes);
+		return -EINVAL;
+	}
+
+	mbps = rzg2l_csi2_calc_mbps(csi2);
+	if (mbps < 0)
+		return mbps;
+
+	csi2->hsfreq = mbps;
+
+	if (on) {
+		struct timings dphy_timing;
+		u32 dphytim0, dphytim1;
+
+		/* Set DPHY timing parameters */
+		if (csi2->hsfreq <= 80)
+			dphy_timing = global_timings[TRANSMISSION_RATE_80_MBPS];
+		else if (csi2->hsfreq <= 125)
+			dphy_timing = global_timings[TRANSMISSION_RATE_125_MBPS];
+		else if (csi2->hsfreq <= 250)
+			dphy_timing = global_timings[TRANSMISSION_RATE_250_MBPS];
+		else if (csi2->hsfreq <= 360)
+			dphy_timing = global_timings[TRANSMISSION_RATE_360_MBPS];
+		else if (csi2->hsfreq <= 1500)
+			dphy_timing = global_timings[TRANSMISSION_RATE_360_PLUS_MBPS];
+		else
+			return -EINVAL;
+
+		/* Set D-PHY timing parameters */
+		dphytim0 = CSIDPHYTIM0_TCLK_MISS(dphy_timing.tclk_miss) |
+			   CSIDPHYTIM0_T_INIT(dphy_timing.t_init);
+		dphytim1 = CSIDPHYTIM1_THS_PREPARE(dphy_timing.ths_prepare) |
+			   CSIDPHYTIM1_TCLK_PREPARE(dphy_timing.tclk_prepare) |
+			   CSIDPHYTIM1_THS_SETTLE(dphy_timing.ths_settle) |
+			   CSIDPHYTIM1_TCLK_SETTLE(dphy_timing.tclk_settle);
+		rzg2l_csi2_write(csi2, CSIDPHYTIM0, dphytim0);
+		rzg2l_csi2_write(csi2, CSIDPHYTIM1, dphytim1);
+
+		/* Enable D-PHY power control 0 */
+		rzg2l_csi2_write(csi2, CSIDPHYSKW0, CSIDPHYSKW0_DEFAULT_SKW);
+
+		/* Set the EN_BGR bit */
+		rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
+
+		/* Delay 20us to be stable */
+		usleep_range(20, 40);
+
+		/* Enable D-PHY power control 1 */
+		rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
+
+		/* Delay 10us to be stable */
+		usleep_range(10, 20);
+
+		/* Start supplying the internal clock for the D-PHY block */
+		pm_runtime_get_sync(csi2->dev);
+	} else {
+		/* Reset the CRU (D-PHY) */
+		reset_control_assert(csi2->rstc);
+
+		/* Stop the D-PHY clock */
+		pm_runtime_put(csi2->dev);
+
+		/* Cancel the EN_LDO1200 register setting */
+		rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
+
+		/* Cancel the EN_BGR register setting */
+		rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rzg2l_csi2_dphy_setting);
+
+void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on)
+{
+	if (on) {
+		unsigned long vclk_rate = csi2->vclk_rate / HZ_PER_MHZ;
+		u32 frrskw, frrclk, frrskw_coeff, frrclk_coeff;
+
+		/* Select data lanes */
+		rzg2l_csi2_write(csi2, CSI2nMCT0, CSI2nMCT0_VDLN(csi2->lanes));
+
+		frrskw_coeff = 3 * vclk_rate * 8;
+		frrclk_coeff = frrskw_coeff / 2;
+		frrskw = DIV_ROUND_UP(frrskw_coeff, csi2->hsfreq);
+		frrclk = DIV_ROUND_UP(frrclk_coeff, csi2->hsfreq);
+		rzg2l_csi2_write(csi2, CSI2nMCT2,
+				 CSI2nMCT2_FRRSKW(frrskw) | CSI2nMCT2_FRRCLK(frrclk));
+
+		/*
+		 * Select data type.
+		 * FS, FE, LS, LE, Generic Short Packet Codes 1 to 8,
+		 * Generic Long Packet Data Types 1 to 4 YUV422 8-bit,
+		 * RGB565, RGB888, RAW8 to RAW20, User-defined 8-bit
+		 * data types 1 to 8
+		 */
+		rzg2l_csi2_write(csi2, CSI2nDTEL, 0xf778ff0f);
+		rzg2l_csi2_write(csi2, CSI2nDTEH, 0x00ffff1f);
+
+		/* Enable LINK reception */
+		rzg2l_csi2_set(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
+	} else {
+		unsigned int timeout = VSRSTS_RETRIES;
+
+		/* Stop LINK reception */
+		rzg2l_csi2_clr(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
+
+		/* Request a software reset of the LINK Video Pixel Interface */
+		rzg2l_csi2_write(csi2, CSI2nRTCT, CSI2nRTCT_VSRST);
+
+		/* Make sure CSI2nRTST.VSRSTS bit is cleared */
+		while (timeout--) {
+			if (!(rzg2l_csi2_read(csi2, CSI2nRTST) & CSI2nRTST_VSRSTS))
+				break;
+			usleep_range(100, 200);
+		};
+	}
+}
+EXPORT_SYMBOL_GPL(rzg2l_csi2_mipi_link_setting);
+
+static int rzg2l_csi2_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
+	int ret = 0;
+
+	mutex_lock(&csi2->lock);
+
+	if (!csi2->remote) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	if (enable && !csi2->stream_count) {
+		ret = v4l2_subdev_call(csi2->remote, video, s_stream, 1);
+		if (ret)
+			goto out;
+	} else if (!enable && csi2->stream_count == 1) {
+		v4l2_subdev_call(csi2->remote, video, s_stream, 0);
+	}
+
+	csi2->stream_count += enable ? 1 : -1;
+out:
+	mutex_unlock(&csi2->lock);
+
+	return ret;
+}
+
+static int rzg2l_csi2_set_format(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *state,
+				 struct v4l2_subdev_format *format)
+{
+	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
+	struct v4l2_mbus_framefmt *framefmt;
+
+	mutex_lock(&csi2->lock);
+
+	if (!rzg2l_csi2_code_to_fmt(format->format.code))
+		format->format.code = rzg2l_csi2_formats[0].code;
+
+	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+		csi2->mf = format->format;
+	} else {
+		framefmt = v4l2_subdev_get_try_format(sd, state, 0);
+		*framefmt = format->format;
+	}
+
+	mutex_unlock(&csi2->lock);
+
+	return 0;
+}
+
+static int rzg2l_csi2_get_pad_format(struct v4l2_subdev *sd,
+				     struct v4l2_subdev_state *sd_state,
+				     struct v4l2_subdev_format *format)
+{
+	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
+
+	mutex_lock(&csi2->lock);
+	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+		format->format = csi2->mf;
+	else
+		format->format = *v4l2_subdev_get_try_format(sd, sd_state, 0);
+	mutex_unlock(&csi2->lock);
+
+	return 0;
+}
+
+static const struct v4l2_subdev_video_ops rzg2l_csi2_video_ops = {
+	.s_stream = rzg2l_csi2_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops rzg2l_csi2_pad_ops = {
+	.set_fmt = rzg2l_csi2_set_format,
+	.get_fmt = rzg2l_csi2_get_pad_format,
+};
+
+static const struct v4l2_subdev_ops rzg2l_csi2_subdev_ops = {
+	.video	= &rzg2l_csi2_video_ops,
+	.pad	= &rzg2l_csi2_pad_ops,
+};
+
+/* -----------------------------------------------------------------------------
+ * Async handling and registration of subdevices and links.
+ */
+
+static int rzg2l_csi2_notify_bound(struct v4l2_async_notifier *notifier,
+				   struct v4l2_subdev *subdev,
+				   struct v4l2_async_subdev *asd)
+{
+	struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
+	int pad;
+
+	pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode,
+					  MEDIA_PAD_FL_SOURCE);
+	if (pad < 0) {
+		dev_err(csi2->dev, "Failed to find pad for %s\n", subdev->name);
+		return pad;
+	}
+
+	csi2->remote = subdev;
+
+	dev_dbg(csi2->dev, "Bound %s pad: %d\n", subdev->name, pad);
+
+	return media_create_pad_link(&subdev->entity, pad,
+				     &csi2->subdev.entity, 0,
+				     MEDIA_LNK_FL_ENABLED |
+				     MEDIA_LNK_FL_IMMUTABLE);
+}
+
+static void rzg2l_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
+				     struct v4l2_subdev *subdev,
+				     struct v4l2_async_subdev *asd)
+{
+	struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
+
+	csi2->remote = NULL;
+
+	dev_dbg(csi2->dev, "Unbind %s\n", subdev->name);
+}
+
+static const struct v4l2_async_notifier_operations rzg2l_csi2_notify_ops = {
+	.bound = rzg2l_csi2_notify_bound,
+	.unbind = rzg2l_csi2_notify_unbind,
+};
+
+static int rzg2l_csi2_parse_v4l2(struct rzg2l_csi2 *csi2,
+				 struct v4l2_fwnode_endpoint *vep)
+{
+	/* Only port 0 endpoint 0 is valid. */
+	if (vep->base.port || vep->base.id)
+		return -ENOTCONN;
+
+	if (vep->bus_type != V4L2_MBUS_CSI2_DPHY) {
+		dev_err(csi2->dev, "Unsupported bus: %u\n", vep->bus_type);
+		return -EINVAL;
+	}
+
+	csi2->lanes = vep->bus.mipi_csi2.num_data_lanes;
+	if (csi2->lanes != 1 && csi2->lanes != 2 && csi2->lanes != 4) {
+		dev_err(csi2->dev, "Unsupported number of data-lanes: %u\n",
+			csi2->lanes);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int rzg2l_csi2_parse_dt(struct rzg2l_csi2 *csi2)
+{
+	struct v4l2_fwnode_endpoint v4l2_ep = {
+		.bus_type = V4L2_MBUS_CSI2_DPHY
+	};
+	struct v4l2_async_subdev *asd;
+	struct fwnode_handle *fwnode;
+	struct fwnode_handle *ep;
+	int ret;
+
+	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(csi2->dev), 0, 0, 0);
+	if (!ep) {
+		dev_err(csi2->dev, "Not connected to subdevice\n");
+		return -EINVAL;
+	}
+
+	ret = v4l2_fwnode_endpoint_parse(ep, &v4l2_ep);
+	if (ret) {
+		dev_err(csi2->dev, "Could not parse v4l2 endpoint\n");
+		fwnode_handle_put(ep);
+		return -EINVAL;
+	}
+
+	ret = rzg2l_csi2_parse_v4l2(csi2, &v4l2_ep);
+	if (ret) {
+		fwnode_handle_put(ep);
+		return ret;
+	}
+
+	fwnode = fwnode_graph_get_remote_endpoint(ep);
+	fwnode_handle_put(ep);
+
+	dev_dbg(csi2->dev, "Found '%pOF'\n", to_of_node(fwnode));
+
+	v4l2_async_nf_init(&csi2->notifier);
+	csi2->notifier.ops = &rzg2l_csi2_notify_ops;
+
+	asd = v4l2_async_nf_add_fwnode(&csi2->notifier, fwnode,
+				       struct v4l2_async_subdev);
+	fwnode_handle_put(fwnode);
+	if (IS_ERR(asd))
+		return PTR_ERR(asd);
+
+	ret = v4l2_async_subdev_nf_register(&csi2->subdev, &csi2->notifier);
+	if (ret)
+		v4l2_async_nf_cleanup(&csi2->notifier);
+
+	return ret;
+}
+
+/* -----------------------------------------------------------------------------
+ * Platform Device Driver.
+ */
+
+static const struct media_entity_operations rzg2l_csi2_entity_ops = {
+	.link_validate = v4l2_subdev_link_validate,
+};
+
+static int rzg2l_csi2_probe(struct platform_device *pdev)
+{
+	struct rzg2l_csi2 *csi2;
+	struct clk *vclk;
+	unsigned int i;
+	int ret;
+
+	csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
+	if (!csi2)
+		return -ENOMEM;
+
+	csi2->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(csi2->base))
+		return PTR_ERR(csi2->base);
+
+	csi2->rstc = devm_reset_control_get(&pdev->dev, "cmn-rstb");
+	if (IS_ERR(csi2->rstc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(csi2->rstc),
+				     "failed to get cpg cmn-rstb\n");
+
+	vclk = devm_clk_get(&pdev->dev, "vclk");
+	if (IS_ERR(vclk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(vclk),
+				     "failed to get vclk clock\n");
+
+	csi2->vclk_rate = clk_get_rate(vclk);
+	devm_clk_put(&pdev->dev, vclk);
+
+	csi2->dev = &pdev->dev;
+	mutex_init(&csi2->lock);
+	csi2->stream_count = 0;
+
+	platform_set_drvdata(pdev, csi2);
+
+	ret = rzg2l_csi2_parse_dt(csi2);
+	if (ret)
+		goto error_mutex;
+
+	csi2->subdev.owner = THIS_MODULE;
+	csi2->subdev.dev = &pdev->dev;
+	v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
+	v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
+	snprintf(csi2->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
+		 KBUILD_MODNAME, dev_name(&pdev->dev));
+	csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+	csi2->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
+	csi2->subdev.entity.ops = &rzg2l_csi2_entity_ops;
+
+	csi2->pads[RZG2L_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
+	for (i = RZG2L_CSI2_SOURCE_VC0; i < NR_OF_RZG2L_CSI2_PAD; i++)
+		csi2->pads[i].flags = MEDIA_PAD_FL_SOURCE;
+
+	ret = media_entity_pads_init(&csi2->subdev.entity, NR_OF_RZG2L_CSI2_PAD, csi2->pads);
+	if (ret)
+		goto error_async;
+
+	pm_runtime_enable(&pdev->dev);
+
+	ret = v4l2_subdev_init_finalize(&csi2->subdev);
+	if (ret < 0)
+		goto error_pm;
+
+	ret = v4l2_async_register_subdev(&csi2->subdev);
+	if (ret < 0)
+		goto error_subdev;
+
+	dev_info(csi2->dev, "%d lanes found\n", csi2->lanes);
+
+	return 0;
+
+error_subdev:
+	v4l2_subdev_cleanup(&csi2->subdev);
+error_pm:
+	pm_runtime_disable(&pdev->dev);
+error_async:
+	v4l2_async_nf_unregister(&csi2->notifier);
+	v4l2_async_nf_cleanup(&csi2->notifier);
+error_mutex:
+	mutex_destroy(&csi2->lock);
+
+	return ret;
+}
+
+static const struct of_device_id rzg2l_csi2_of_table[] = {
+	{ .compatible = "renesas,rzg2l-csi2", },
+	{ /* sentinel */ }
+};
+
+static int rzg2l_csi2_remove(struct platform_device *pdev)
+{
+	struct rzg2l_csi2 *csi2 = platform_get_drvdata(pdev);
+
+	v4l2_async_nf_unregister(&csi2->notifier);
+	v4l2_async_nf_cleanup(&csi2->notifier);
+	v4l2_async_unregister_subdev(&csi2->subdev);
+	v4l2_subdev_cleanup(&csi2->subdev);
+
+	pm_runtime_disable(&pdev->dev);
+	mutex_destroy(&csi2->lock);
+
+	return 0;
+}
+
+static struct platform_driver rzg2l_csi2_pdrv = {
+	.remove	= rzg2l_csi2_remove,
+	.probe	= rzg2l_csi2_probe,
+	.driver	= {
+		.name = "rzg2l-csi2",
+		.of_match_table = rzg2l_csi2_of_table,
+	},
+};
+
+module_platform_driver(rzg2l_csi2_pdrv);
+
+MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L MIPI CSI2 receiver driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
new file mode 100644
index 000000000000..7192923517f6
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ */
+
+#ifndef __RZG2L_CSI2__
+#define __RZG2L_CSI2__
+
+enum rzg2l_csi2_pads {
+	RZG2L_CSI2_SINK = 0,
+	RZG2L_CSI2_SOURCE_VC0,
+	RZG2L_CSI2_SOURCE_VC1,
+	RZG2L_CSI2_SOURCE_VC2,
+	RZG2L_CSI2_SOURCE_VC3,
+	NR_OF_RZG2L_CSI2_PAD,
+};
+
+struct rzg2l_csi2 {
+	struct device *dev;
+	void __iomem *base;
+	struct reset_control *rstc;
+	unsigned long vclk_rate;
+
+	struct v4l2_subdev subdev;
+	struct media_pad pads[NR_OF_RZG2L_CSI2_PAD];
+
+	struct v4l2_async_notifier notifier;
+	struct v4l2_subdev *remote;
+
+	struct v4l2_mbus_framefmt mf;
+
+	struct mutex lock;
+	int stream_count;
+
+	unsigned short lanes;
+
+	unsigned long hsfreq;
+};
+
+static inline struct rzg2l_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct rzg2l_csi2, subdev);
+}
+
+int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2);
+int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on);
+void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on);
+
+#endif
-- 
2.25.1

Re: [PATCH 3/4] media: platform: Add Renesas RZ/G2L MIPI CSI-2 receiver driver

[Laurent Pinchart]

Hi Prabhakar,

Thank you for the patch.

On Mon, Aug 01, 2022 at 10:47:17PM +0100, Lad Prabhakar wrote:
> Add MIPI CSI-2 receiver driver for Renesas RZ/G2L. The MIPI
> CSI-2 is part of the CRU module found on RZ/G2L family.
> 
> Based on a patch in the BSP by Hien Huynh
> <hien.huynh.px@renesas.com>
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC v2 -> v1
> * Fixed initialization sequence of DPHY and link
> * Exported DPHY and link initialization functions so that the
>   CRU core driver can initialize the CRU and CSI2 as per the HW manual.
> 
> RFC v1 -> RFC v2
> * new patch (split up as new driver compared to v1)
> ---
>  drivers/media/platform/renesas/Kconfig        |   1 +
>  drivers/media/platform/renesas/Makefile       |   1 +
>  .../media/platform/renesas/rzg2l-cru/Kconfig  |  17 +
>  .../media/platform/renesas/rzg2l-cru/Makefile |   3 +
>  .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 702 ++++++++++++++++++
>  .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  49 ++
>  6 files changed, 773 insertions(+)
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> 
> diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
> index 9fd90672ea2d..ed788e991f74 100644
> --- a/drivers/media/platform/renesas/Kconfig
> +++ b/drivers/media/platform/renesas/Kconfig
> @@ -41,6 +41,7 @@ config VIDEO_SH_VOU
>  	  Support for the Video Output Unit (VOU) on SuperH SoCs.
>  
>  source "drivers/media/platform/renesas/rcar-vin/Kconfig"
> +source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
>  
>  # Mem2mem drivers
>  
> diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
> index 3ec226ef5fd2..55854e868887 100644
> --- a/drivers/media/platform/renesas/Makefile
> +++ b/drivers/media/platform/renesas/Makefile
> @@ -4,6 +4,7 @@
>  #
>  
>  obj-y += rcar-vin/
> +obj-y += rzg2l-cru/
>  obj-y += vsp1/
>  
>  obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/Kconfig b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> new file mode 100644
> index 000000000000..150bf16f167c
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +config VIDEO_RZG2L_CSI2
> +	tristate "RZ/G2L MIPI CSI-2 Receiver"
> +	depends on V4L_PLATFORM_DRIVERS
> +	depends on VIDEO_DEV && OF
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	select MEDIA_CONTROLLER
> +	select VIDEO_V4L2_SUBDEV_API
> +	select RESET_CONTROLLER
> +	select V4L2_FWNODE

Please order these alphabetically.

> +	help
> +	  Support for Renesas RZ/G2L (and alike SoC's) MIPI CSI-2
> +	  Receiver driver.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called rzg2l-csi2.
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/Makefile b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> new file mode 100644
> index 000000000000..91ea97a944e6
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_VIDEO_RZG2L_CSI2) += rzg2l-csi2.o
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> new file mode 100644
> index 000000000000..252e33ddb6e8
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> @@ -0,0 +1,702 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Renesas RZ/G2L MIPI CSI-2 Receiver
> + *
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +#include <linux/sys_soc.h>
> +#include <linux/units.h>
> +
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mc.h>
> +#include <media/v4l2-subdev.h>
> +
> +#include "rzg2l-csi2.h"
> +
> +/* LINK registers */
> +/* Module Configuration Register */
> +#define CSI2nMCG			0x0
> +#define CSI2nMCG_SDLN			GENMASK(11, 8)
> +
> +/* Module Control Register 0 */
> +#define CSI2nMCT0			0x10
> +#define CSI2nMCT0_VDLN(x)		((x) << 0)
> +
> +/* Module Control Register 2 */
> +#define CSI2nMCT2			0x18
> +#define CSI2nMCT2_FRRSKW(x)		((x) << 16)
> +#define CSI2nMCT2_FRRCLK(x)		((x) << 0)
> +
> +/* Module Control Register 3 */
> +#define CSI2nMCT3			0x1c
> +#define CSI2nMCT3_RXEN			BIT(0)
> +
> +/* Reset Control Register */
> +#define CSI2nRTCT			0x28
> +#define CSI2nRTCT_VSRST			BIT(0)
> +
> +/* Reset Status Register */
> +#define CSI2nRTST			0x2c
> +#define CSI2nRTST_VSRSTS		BIT(0)
> +
> +/* Receive Data Type Enable Low Register */
> +#define CSI2nDTEL			0x60
> +
> +/* Receive Data Type Enable High Register */
> +#define CSI2nDTEH			0x64
> +
> +/* Power Management Status Register */
> +#define CSI2nPMST			0x200
> +
> +/* Power Management Status Clear Register */
> +#define CSI2nPMSC			0x204
> +
> +/* DPHY registers */
> +/* D-PHY Control Register 0 */
> +#define CSIDPHYCTRL0			0x400
> +#define CSIDPHYCTRL0_EN_LDO1200		BIT(1)
> +#define CSIDPHYCTRL0_EN_BGR		BIT(0)
> +
> +/* D-PHY Timing Register 0 */
> +#define CSIDPHYTIM0			0x404
> +#define CSIDPHYTIM0_TCLK_MISS(x)	((x) << 24)
> +#define CSIDPHYTIM0_T_INIT(x)		((x) << 0)
> +
> +/* D-PHY Timing Register 1 */
> +#define CSIDPHYTIM1			0x408
> +#define CSIDPHYTIM1_THS_PREPARE(x)	((x) << 24)
> +#define CSIDPHYTIM1_TCLK_PREPARE(x)	((x) << 16)
> +#define CSIDPHYTIM1_THS_SETTLE(x)	((x) << 8)
> +#define CSIDPHYTIM1_TCLK_SETTLE(x)	((x) << 0)
> +
> +/* D-PHY Skew Adjustment Function */
> +#define CSIDPHYSKW0			0x460
> +#define CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(x)	((x) & 0x3)
> +#define CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(x)	(((x) & 0x3) << 4)
> +#define CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(x)	(((x) & 0x3) << 8)
> +#define CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(x)	(((x) & 0x3) << 12)
> +#define CSIDPHYSKW0_DEFAULT_SKW		CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(1) | \
> +					CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(1) | \
> +					CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
> +					CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1)
> +
> +#define VSRSTS_RETRIES			20
> +
> +struct timings {

Such a generic name risks creating conflicts later. Could you name the
structure rzg2l_csi2_timings ? Same for the dphy_timings enum.

> +	u32 t_init;
> +	u32 tclk_miss;
> +	u32 tclk_settle;
> +	u32 ths_settle;
> +	u32 tclk_prepare;
> +	u32 ths_prepare;
> +};
> +
> +enum dphy_timings {
> +	TRANSMISSION_RATE_80_MBPS = 0,
> +	TRANSMISSION_RATE_125_MBPS,
> +	TRANSMISSION_RATE_250_MBPS,
> +	TRANSMISSION_RATE_360_MBPS,
> +	TRANSMISSION_RATE_360_PLUS_MBPS,
> +};
> +
> +static const struct timings global_timings[] = {

And there, rzg2l_csi2_global_timings.

> +	[TRANSMISSION_RATE_80_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 23,
> +		.ths_settle = 31,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 19,
> +	},
> +	[TRANSMISSION_RATE_125_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 23,
> +		.ths_settle = 28,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 19,
> +	},
> +	[TRANSMISSION_RATE_250_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 23,
> +		.ths_settle = 22,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 16,
> +	},
> +	[TRANSMISSION_RATE_360_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 18,
> +		.ths_settle = 19,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 10,
> +	},
> +	[TRANSMISSION_RATE_360_PLUS_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 18,
> +		.ths_settle = 18,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 10,
> +	},
> +};
> +
> +struct rzg2l_csi2_format {
> +	u32 code;
> +	unsigned int datatype;
> +	unsigned int bpp;
> +};
> +
> +static const struct rzg2l_csi2_format rzg2l_csi2_formats[] = {
> +	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
> +};
> +
> +static const struct rzg2l_csi2_format *rzg2l_csi2_code_to_fmt(unsigned int code)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(rzg2l_csi2_formats); i++)
> +		if (rzg2l_csi2_formats[i].code == code)
> +			return &rzg2l_csi2_formats[i];
> +
> +	return NULL;
> +}
> +
> +static inline struct rzg2l_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
> +{
> +	return container_of(n, struct rzg2l_csi2, notifier);
> +}
> +
> +static u32 rzg2l_csi2_read(struct rzg2l_csi2 *csi2, unsigned int reg)
> +{
> +	return ioread32(csi2->base + reg);
> +}
> +
> +static void rzg2l_csi2_write(struct rzg2l_csi2 *csi2, unsigned int reg,
> +			     u32 data)
> +{
> +	iowrite32(data, csi2->base + reg);
> +}
> +
> +static void rzg2l_csi2_set(struct rzg2l_csi2 *csi2, unsigned int reg, u32 set)
> +{
> +	rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) | set);
> +}
> +
> +static void rzg2l_csi2_clr(struct rzg2l_csi2 *csi2, unsigned int reg, u32 clr)
> +{
> +	rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) & ~clr);
> +}
> +
> +static int rzg2l_csi2_calc_mbps(struct rzg2l_csi2 *csi2)
> +{
> +	const struct rzg2l_csi2_format *format;
> +	struct v4l2_subdev *source;
> +	struct v4l2_ctrl *ctrl;
> +	u64 mbps;
> +
> +	if (!csi2->remote)
> +		return -ENODEV;

Can this happen, given that the remote field is set in the bound()
handler ?

> +
> +	source = csi2->remote;
> +
> +	/* Read the pixel rate control from remote. */
> +	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> +	if (!ctrl) {
> +		dev_err(csi2->dev, "no pixel rate control in subdev %s\n",
> +			source->name);
> +		return -EINVAL;
> +	}
> +
> +	format = rzg2l_csi2_code_to_fmt(csi2->mf.code);
> +
> +	/*
> +	 * Calculate hsfreq in Mbps
> +	 * hsfreq = (pixel_rate * bits_per_sample) / number_of_lanes
> +	 */
> +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
> +	do_div(mbps, csi2->lanes * 1000000);
> +
> +	return mbps;
> +}
> +
> +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2)
> +{
> +	return reset_control_deassert(csi2->rstc);
> +}
> +EXPORT_SYMBOL_GPL(rzg2l_csi2_cmn_rstb_deassert);

I don't think you need to export this, or the
rzg2l_csi2_mipi_link_setting() function. They're called by the CRU
driver just before or after calling the .s_stream() operation, so you
can move the calls there.

> +
> +/* -----------------------------------------------------------------------------
> + * DPHY setting
> + */
> +
> +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on)
> +{
> +	int lanes, mbps;
> +
> +	dev_dbg(csi2->dev, "Input size (%ux%u%c)\n",
> +		csi2->mf.width, csi2->mf.height,
> +		csi2->mf.field == V4L2_FIELD_NONE ? 'p' : 'i');
> +
> +	/* Checking the maximum lanes support for CSI-2 module */
> +	lanes = (rzg2l_csi2_read(csi2, CSI2nMCG) & CSI2nMCG_SDLN) >> 8;
> +	if (lanes < csi2->lanes) {
> +		dev_err(csi2->dev,
> +			"Failed to support %d data lanes\n", csi2->lanes);
> +		return -EINVAL;
> +	}

You should read and validate the number of lanes at probe time.

> +
> +	mbps = rzg2l_csi2_calc_mbps(csi2);
> +	if (mbps < 0)
> +		return mbps;
> +
> +	csi2->hsfreq = mbps;

This is only needed in the (on) branch.

> +
> +	if (on) {

The two branches share no code. Split them in two separate functions.

> +		struct timings dphy_timing;

Make this a const pointer.

> +		u32 dphytim0, dphytim1;
> +
> +		/* Set DPHY timing parameters */
> +		if (csi2->hsfreq <= 80)
> +			dphy_timing = global_timings[TRANSMISSION_RATE_80_MBPS];
> +		else if (csi2->hsfreq <= 125)
> +			dphy_timing = global_timings[TRANSMISSION_RATE_125_MBPS];
> +		else if (csi2->hsfreq <= 250)
> +			dphy_timing = global_timings[TRANSMISSION_RATE_250_MBPS];
> +		else if (csi2->hsfreq <= 360)
> +			dphy_timing = global_timings[TRANSMISSION_RATE_360_MBPS];
> +		else if (csi2->hsfreq <= 1500)
> +			dphy_timing = global_timings[TRANSMISSION_RATE_360_PLUS_MBPS];
> +		else
> +			return -EINVAL;
> +
> +		/* Set D-PHY timing parameters */
> +		dphytim0 = CSIDPHYTIM0_TCLK_MISS(dphy_timing.tclk_miss) |
> +			   CSIDPHYTIM0_T_INIT(dphy_timing.t_init);
> +		dphytim1 = CSIDPHYTIM1_THS_PREPARE(dphy_timing.ths_prepare) |
> +			   CSIDPHYTIM1_TCLK_PREPARE(dphy_timing.tclk_prepare) |
> +			   CSIDPHYTIM1_THS_SETTLE(dphy_timing.ths_settle) |
> +			   CSIDPHYTIM1_TCLK_SETTLE(dphy_timing.tclk_settle);
> +		rzg2l_csi2_write(csi2, CSIDPHYTIM0, dphytim0);
> +		rzg2l_csi2_write(csi2, CSIDPHYTIM1, dphytim1);
> +
> +		/* Enable D-PHY power control 0 */
> +		rzg2l_csi2_write(csi2, CSIDPHYSKW0, CSIDPHYSKW0_DEFAULT_SKW);
> +
> +		/* Set the EN_BGR bit */
> +		rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> +
> +		/* Delay 20us to be stable */
> +		usleep_range(20, 40);
> +
> +		/* Enable D-PHY power control 1 */
> +		rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> +
> +		/* Delay 10us to be stable */
> +		usleep_range(10, 20);
> +
> +		/* Start supplying the internal clock for the D-PHY block */
> +		pm_runtime_get_sync(csi2->dev);

Missing error handling (and use pm_runtime_resume_and_get() to make that
easier). But it doesn't seem quite right, you're not supposed to resume
the device after writing register, but before.

> +	} else {
> +		/* Reset the CRU (D-PHY) */
> +		reset_control_assert(csi2->rstc);
> +
> +		/* Stop the D-PHY clock */
> +		pm_runtime_put(csi2->dev);

Same here, that should to after the register writes.

> +
> +		/* Cancel the EN_LDO1200 register setting */
> +		rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> +
> +		/* Cancel the EN_BGR register setting */
> +		rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rzg2l_csi2_dphy_setting);

Would it be a problem to also move this call to .s_stream() to avoid
direct calls from the CRU driver ?

> +
> +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on)
> +{
> +	if (on) {

Here too the two branches share no code, split them in two separate
functions.

> +		unsigned long vclk_rate = csi2->vclk_rate / HZ_PER_MHZ;
> +		u32 frrskw, frrclk, frrskw_coeff, frrclk_coeff;
> +
> +		/* Select data lanes */
> +		rzg2l_csi2_write(csi2, CSI2nMCT0, CSI2nMCT0_VDLN(csi2->lanes));
> +
> +		frrskw_coeff = 3 * vclk_rate * 8;
> +		frrclk_coeff = frrskw_coeff / 2;
> +		frrskw = DIV_ROUND_UP(frrskw_coeff, csi2->hsfreq);
> +		frrclk = DIV_ROUND_UP(frrclk_coeff, csi2->hsfreq);
> +		rzg2l_csi2_write(csi2, CSI2nMCT2,
> +				 CSI2nMCT2_FRRSKW(frrskw) | CSI2nMCT2_FRRCLK(frrclk));
> +
> +		/*
> +		 * Select data type.
> +		 * FS, FE, LS, LE, Generic Short Packet Codes 1 to 8,
> +		 * Generic Long Packet Data Types 1 to 4 YUV422 8-bit,
> +		 * RGB565, RGB888, RAW8 to RAW20, User-defined 8-bit
> +		 * data types 1 to 8
> +		 */
> +		rzg2l_csi2_write(csi2, CSI2nDTEL, 0xf778ff0f);
> +		rzg2l_csi2_write(csi2, CSI2nDTEH, 0x00ffff1f);
> +
> +		/* Enable LINK reception */
> +		rzg2l_csi2_set(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);

Would it be an issue to use rzg2l_csi2_write() here ?

> +	} else {
> +		unsigned int timeout = VSRSTS_RETRIES;
> +
> +		/* Stop LINK reception */
> +		rzg2l_csi2_clr(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);

Same here, rzg2l_csi2_write(csi2, CSI2nMCT3, 0);

Same for the other usage of rzg2l_csi2_set() and rzg2l_csi2_clr() above,
to avoid having to read back the value first.

> +
> +		/* Request a software reset of the LINK Video Pixel Interface */
> +		rzg2l_csi2_write(csi2, CSI2nRTCT, CSI2nRTCT_VSRST);
> +
> +		/* Make sure CSI2nRTST.VSRSTS bit is cleared */
> +		while (timeout--) {
> +			if (!(rzg2l_csi2_read(csi2, CSI2nRTST) & CSI2nRTST_VSRSTS))
> +				break;
> +			usleep_range(100, 200);
> +		};
> +	}
> +}
> +EXPORT_SYMBOL_GPL(rzg2l_csi2_mipi_link_setting);
> +
> +static int rzg2l_csi2_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> +	int ret = 0;
> +
> +	mutex_lock(&csi2->lock);
> +
> +	if (!csi2->remote) {
> +		ret = -ENODEV;
> +		goto out;
> +	}

I don't think this can happen either.

> +
> +	if (enable && !csi2->stream_count) {
> +		ret = v4l2_subdev_call(csi2->remote, video, s_stream, 1);
> +		if (ret)
> +			goto out;
> +	} else if (!enable && csi2->stream_count == 1) {
> +		v4l2_subdev_call(csi2->remote, video, s_stream, 0);
> +	}
> +
> +	csi2->stream_count += enable ? 1 : -1;

Drop stream_count, .s_stream() is not reference-counted.

> +out:
> +	mutex_unlock(&csi2->lock);
> +
> +	return ret;
> +}
> +
> +static int rzg2l_csi2_set_format(struct v4l2_subdev *sd,
> +				 struct v4l2_subdev_state *state,
> +				 struct v4l2_subdev_format *format)
> +{
> +	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> +	struct v4l2_mbus_framefmt *framefmt;
> +
> +	mutex_lock(&csi2->lock);
> +
> +	if (!rzg2l_csi2_code_to_fmt(format->format.code))
> +		format->format.code = rzg2l_csi2_formats[0].code;
> +
> +	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> +		csi2->mf = format->format;

Please use the subdev active state storage to avoid manually storing the
format. See https://lore.kernel.org/linux-media/20220823005822.13805-3-laurent.pinchart@ideasonboard.com/
and in particular

- How the .get_fmt() and .set_fmt() handlers use
  v4l2_subdev_get_pad_format() unconditionally to get the format,
  regardless of whether it's the active or try format.
- The v4l2_subdev_lock_and_get_active_state() call in .s_stream() to get
  the active state, and access the format from it.
- You're already calling v4l2_subdev_init_finalize() in probe, good :-)

You'll need to implement .init_cfg(), and you'll be able to drop the
lock field in rzg2l_csi2.

> +	} else {
> +		framefmt = v4l2_subdev_get_try_format(sd, state, 0);
> +		*framefmt = format->format;
> +	}
> +
> +	mutex_unlock(&csi2->lock);
> +
> +	return 0;
> +}
> +
> +static int rzg2l_csi2_get_pad_format(struct v4l2_subdev *sd,
> +				     struct v4l2_subdev_state *sd_state,
> +				     struct v4l2_subdev_format *format)
> +{
> +	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> +
> +	mutex_lock(&csi2->lock);
> +	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> +		format->format = csi2->mf;
> +	else
> +		format->format = *v4l2_subdev_get_try_format(sd, sd_state, 0);
> +	mutex_unlock(&csi2->lock);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops rzg2l_csi2_video_ops = {
> +	.s_stream = rzg2l_csi2_s_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops rzg2l_csi2_pad_ops = {
> +	.set_fmt = rzg2l_csi2_set_format,
> +	.get_fmt = rzg2l_csi2_get_pad_format,

Missing enum functions for format and sizes, and missing .init_cfg().

> +};
> +
> +static const struct v4l2_subdev_ops rzg2l_csi2_subdev_ops = {
> +	.video	= &rzg2l_csi2_video_ops,
> +	.pad	= &rzg2l_csi2_pad_ops,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Async handling and registration of subdevices and links.
> + */
> +
> +static int rzg2l_csi2_notify_bound(struct v4l2_async_notifier *notifier,
> +				   struct v4l2_subdev *subdev,
> +				   struct v4l2_async_subdev *asd)
> +{
> +	struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> +	int pad;
> +
> +	pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode,
> +					  MEDIA_PAD_FL_SOURCE);
> +	if (pad < 0) {
> +		dev_err(csi2->dev, "Failed to find pad for %s\n", subdev->name);
> +		return pad;
> +	}


I think you can skip this and just hardcode pad to RZG2L_CSI2_SINK in
the media_create_pad_link(), there's no other input.

> +
> +	csi2->remote = subdev;
> +
> +	dev_dbg(csi2->dev, "Bound %s pad: %d\n", subdev->name, pad);
> +
> +	return media_create_pad_link(&subdev->entity, pad,
> +				     &csi2->subdev.entity, 0,
> +				     MEDIA_LNK_FL_ENABLED |
> +				     MEDIA_LNK_FL_IMMUTABLE);
> +}
> +
> +static void rzg2l_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
> +				     struct v4l2_subdev *subdev,
> +				     struct v4l2_async_subdev *asd)
> +{
> +	struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> +
> +	csi2->remote = NULL;
> +
> +	dev_dbg(csi2->dev, "Unbind %s\n", subdev->name);
> +}
> +
> +static const struct v4l2_async_notifier_operations rzg2l_csi2_notify_ops = {
> +	.bound = rzg2l_csi2_notify_bound,
> +	.unbind = rzg2l_csi2_notify_unbind,
> +};
> +
> +static int rzg2l_csi2_parse_v4l2(struct rzg2l_csi2 *csi2,
> +				 struct v4l2_fwnode_endpoint *vep)
> +{
> +	/* Only port 0 endpoint 0 is valid. */
> +	if (vep->base.port || vep->base.id)
> +		return -ENOTCONN;
> +
> +	if (vep->bus_type != V4L2_MBUS_CSI2_DPHY) {
> +		dev_err(csi2->dev, "Unsupported bus: %u\n", vep->bus_type);
> +		return -EINVAL;
> +	}

I'm not sure you need this check as the DT bindings should make it clear
that only CSI-2 sources are valid. Maybe require the bus-type property
in DT ? Up to you.

> +
> +	csi2->lanes = vep->bus.mipi_csi2.num_data_lanes;
> +	if (csi2->lanes != 1 && csi2->lanes != 2 && csi2->lanes != 4) {
> +		dev_err(csi2->dev, "Unsupported number of data-lanes: %u\n",
> +			csi2->lanes);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rzg2l_csi2_parse_dt(struct rzg2l_csi2 *csi2)
> +{
> +	struct v4l2_fwnode_endpoint v4l2_ep = {
> +		.bus_type = V4L2_MBUS_CSI2_DPHY
> +	};
> +	struct v4l2_async_subdev *asd;
> +	struct fwnode_handle *fwnode;
> +	struct fwnode_handle *ep;
> +	int ret;
> +
> +	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(csi2->dev), 0, 0, 0);
> +	if (!ep) {
> +		dev_err(csi2->dev, "Not connected to subdevice\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = v4l2_fwnode_endpoint_parse(ep, &v4l2_ep);
> +	if (ret) {
> +		dev_err(csi2->dev, "Could not parse v4l2 endpoint\n");
> +		fwnode_handle_put(ep);
> +		return -EINVAL;
> +	}
> +
> +	ret = rzg2l_csi2_parse_v4l2(csi2, &v4l2_ep);
> +	if (ret) {
> +		fwnode_handle_put(ep);
> +		return ret;
> +	}
> +
> +	fwnode = fwnode_graph_get_remote_endpoint(ep);
> +	fwnode_handle_put(ep);
> +
> +	dev_dbg(csi2->dev, "Found '%pOF'\n", to_of_node(fwnode));
> +
> +	v4l2_async_nf_init(&csi2->notifier);
> +	csi2->notifier.ops = &rzg2l_csi2_notify_ops;
> +
> +	asd = v4l2_async_nf_add_fwnode(&csi2->notifier, fwnode,
> +				       struct v4l2_async_subdev);
> +	fwnode_handle_put(fwnode);
> +	if (IS_ERR(asd))
> +		return PTR_ERR(asd);
> +
> +	ret = v4l2_async_subdev_nf_register(&csi2->subdev, &csi2->notifier);
> +	if (ret)
> +		v4l2_async_nf_cleanup(&csi2->notifier);
> +
> +	return ret;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Platform Device Driver.
> + */
> +
> +static const struct media_entity_operations rzg2l_csi2_entity_ops = {
> +	.link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static int rzg2l_csi2_probe(struct platform_device *pdev)
> +{
> +	struct rzg2l_csi2 *csi2;
> +	struct clk *vclk;
> +	unsigned int i;
> +	int ret;
> +
> +	csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
> +	if (!csi2)
> +		return -ENOMEM;
> +
> +	csi2->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(csi2->base))
> +		return PTR_ERR(csi2->base);
> +
> +	csi2->rstc = devm_reset_control_get(&pdev->dev, "cmn-rstb");
> +	if (IS_ERR(csi2->rstc))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(csi2->rstc),
> +				     "failed to get cpg cmn-rstb\n");
> +
> +	vclk = devm_clk_get(&pdev->dev, "vclk");
> +	if (IS_ERR(vclk))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(vclk),
> +				     "failed to get vclk clock\n");
> +
> +	csi2->vclk_rate = clk_get_rate(vclk);
> +	devm_clk_put(&pdev->dev, vclk);

Is there no need to ensure at runtime at the clock is on ?

> +
> +	csi2->dev = &pdev->dev;
> +	mutex_init(&csi2->lock);
> +	csi2->stream_count = 0;
> +
> +	platform_set_drvdata(pdev, csi2);
> +
> +	ret = rzg2l_csi2_parse_dt(csi2);
> +	if (ret)
> +		goto error_mutex;
> +
> +	csi2->subdev.owner = THIS_MODULE;

You don't need this, v4l2_subdev_init() handles it.

> +	csi2->subdev.dev = &pdev->dev;
> +	v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
> +	v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
> +	snprintf(csi2->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",

sizeof(csi2->subdev.name) instead of V4L2_SUBDEV_NAME_SIZE, it's less
error-prone.

> +		 KBUILD_MODNAME, dev_name(&pdev->dev));

Don't use KBUILD_MODNAME but set a device name here, for instance use
"csi-%s" as the format string. The subdev name shouldn't change if the
module is renamed.

> +	csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> +	csi2->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;

I think MEDIA_ENT_F_VID_IF_BRIDGE would be more appropriate. The pixel
formatter reads or writes pixels from/to memory.

> +	csi2->subdev.entity.ops = &rzg2l_csi2_entity_ops;
> +
> +	csi2->pads[RZG2L_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> +	for (i = RZG2L_CSI2_SOURCE_VC0; i < NR_OF_RZG2L_CSI2_PAD; i++)
> +		csi2->pads[i].flags = MEDIA_PAD_FL_SOURCE;

One source pad only. Support for multiple virtual channels should be
implemented using the stream API that is under development (see
https://lore.kernel.org/linux-media/20220810121122.3149086-1-tomi.valkeinen@ideasonboard.com/T/#t).

> +
> +	ret = media_entity_pads_init(&csi2->subdev.entity, NR_OF_RZG2L_CSI2_PAD, csi2->pads);
> +	if (ret)
> +		goto error_async;
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	ret = v4l2_subdev_init_finalize(&csi2->subdev);
> +	if (ret < 0)
> +		goto error_pm;
> +
> +	ret = v4l2_async_register_subdev(&csi2->subdev);
> +	if (ret < 0)
> +		goto error_subdev;
> +
> +	dev_info(csi2->dev, "%d lanes found\n", csi2->lanes);
> +
> +	return 0;
> +
> +error_subdev:
> +	v4l2_subdev_cleanup(&csi2->subdev);
> +error_pm:
> +	pm_runtime_disable(&pdev->dev);
> +error_async:
> +	v4l2_async_nf_unregister(&csi2->notifier);
> +	v4l2_async_nf_cleanup(&csi2->notifier);
> +error_mutex:
> +	mutex_destroy(&csi2->lock);
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id rzg2l_csi2_of_table[] = {
> +	{ .compatible = "renesas,rzg2l-csi2", },
> +	{ /* sentinel */ }
> +};
> +
> +static int rzg2l_csi2_remove(struct platform_device *pdev)
> +{
> +	struct rzg2l_csi2 *csi2 = platform_get_drvdata(pdev);
> +
> +	v4l2_async_nf_unregister(&csi2->notifier);
> +	v4l2_async_nf_cleanup(&csi2->notifier);
> +	v4l2_async_unregister_subdev(&csi2->subdev);
> +	v4l2_subdev_cleanup(&csi2->subdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +	mutex_destroy(&csi2->lock);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver rzg2l_csi2_pdrv = {
> +	.remove	= rzg2l_csi2_remove,
> +	.probe	= rzg2l_csi2_probe,
> +	.driver	= {
> +		.name = "rzg2l-csi2",
> +		.of_match_table = rzg2l_csi2_of_table,
> +	},
> +};
> +
> +module_platform_driver(rzg2l_csi2_pdrv);
> +
> +MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
> +MODULE_DESCRIPTION("Renesas RZ/G2L MIPI CSI2 receiver driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> new file mode 100644
> index 000000000000..7192923517f6
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + */
> +
> +#ifndef __RZG2L_CSI2__
> +#define __RZG2L_CSI2__
> +
> +enum rzg2l_csi2_pads {
> +	RZG2L_CSI2_SINK = 0,
> +	RZG2L_CSI2_SOURCE_VC0,
> +	RZG2L_CSI2_SOURCE_VC1,
> +	RZG2L_CSI2_SOURCE_VC2,
> +	RZG2L_CSI2_SOURCE_VC3,
> +	NR_OF_RZG2L_CSI2_PAD,
> +};
> +
> +struct rzg2l_csi2 {
> +	struct device *dev;
> +	void __iomem *base;
> +	struct reset_control *rstc;
> +	unsigned long vclk_rate;
> +
> +	struct v4l2_subdev subdev;
> +	struct media_pad pads[NR_OF_RZG2L_CSI2_PAD];
> +
> +	struct v4l2_async_notifier notifier;
> +	struct v4l2_subdev *remote;

You can name this field source (or remote_source, up to you), it will be
more explicit.

> +
> +	struct v4l2_mbus_framefmt mf;
> +
> +	struct mutex lock;
> +	int stream_count;
> +
> +	unsigned short lanes;
> +
> +	unsigned long hsfreq;
> +};
> +
> +static inline struct rzg2l_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
> +{
> +	return container_of(sd, struct rzg2l_csi2, subdev);
> +}
> +
> +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2);
> +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on);
> +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on);
> +
> +#endif

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] media: platform: Add Renesas RZ/G2L MIPI CSI-2 receiver driver

[Lad, Prabhakar]

Hi Laurent,

Thank you for the review.

On Tue, Aug 30, 2022 at 1:28 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Mon, Aug 01, 2022 at 10:47:17PM +0100, Lad Prabhakar wrote:
> > Add MIPI CSI-2 receiver driver for Renesas RZ/G2L. The MIPI
> > CSI-2 is part of the CRU module found on RZ/G2L family.
> >
> > Based on a patch in the BSP by Hien Huynh
> > <hien.huynh.px@renesas.com>
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC v2 -> v1
> > * Fixed initialization sequence of DPHY and link
> > * Exported DPHY and link initialization functions so that the
> >   CRU core driver can initialize the CRU and CSI2 as per the HW manual.
> >
> > RFC v1 -> RFC v2
> > * new patch (split up as new driver compared to v1)
> > ---
> >  drivers/media/platform/renesas/Kconfig        |   1 +
> >  drivers/media/platform/renesas/Makefile       |   1 +
> >  .../media/platform/renesas/rzg2l-cru/Kconfig  |  17 +
> >  .../media/platform/renesas/rzg2l-cru/Makefile |   3 +
> >  .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 702 ++++++++++++++++++
> >  .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  49 ++
> >  6 files changed, 773 insertions(+)
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> >
> > diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
> > index 9fd90672ea2d..ed788e991f74 100644
> > --- a/drivers/media/platform/renesas/Kconfig
> > +++ b/drivers/media/platform/renesas/Kconfig
> > @@ -41,6 +41,7 @@ config VIDEO_SH_VOU
> >         Support for the Video Output Unit (VOU) on SuperH SoCs.
> >
> >  source "drivers/media/platform/renesas/rcar-vin/Kconfig"
> > +source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
> >
> >  # Mem2mem drivers
> >
> > diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
> > index 3ec226ef5fd2..55854e868887 100644
> > --- a/drivers/media/platform/renesas/Makefile
> > +++ b/drivers/media/platform/renesas/Makefile
> > @@ -4,6 +4,7 @@
> >  #
> >
> >  obj-y += rcar-vin/
> > +obj-y += rzg2l-cru/
> >  obj-y += vsp1/
> >
> >  obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/Kconfig b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> > new file mode 100644
> > index 000000000000..150bf16f167c
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> > @@ -0,0 +1,17 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +config VIDEO_RZG2L_CSI2
> > +     tristate "RZ/G2L MIPI CSI-2 Receiver"
> > +     depends on V4L_PLATFORM_DRIVERS
> > +     depends on VIDEO_DEV && OF
> > +     depends on ARCH_RENESAS || COMPILE_TEST
> > +     select MEDIA_CONTROLLER
> > +     select VIDEO_V4L2_SUBDEV_API
> > +     select RESET_CONTROLLER
> > +     select V4L2_FWNODE
>
> Please order these alphabetically.
>
Ok, I will sort them.

> > +     help
> > +       Support for Renesas RZ/G2L (and alike SoC's) MIPI CSI-2
> > +       Receiver driver.
> > +
> > +       To compile this driver as a module, choose M here: the
> > +       module will be called rzg2l-csi2.
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/Makefile b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> > new file mode 100644
> > index 000000000000..91ea97a944e6
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_VIDEO_RZG2L_CSI2) += rzg2l-csi2.o
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > new file mode 100644
> > index 000000000000..252e33ddb6e8
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > @@ -0,0 +1,702 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Renesas RZ/G2L MIPI CSI-2 Receiver
> > + *
> > + * Copyright (C) 2022 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +#include <linux/sys_soc.h>
> > +#include <linux/units.h>
> > +
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-mc.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#include "rzg2l-csi2.h"
> > +
> > +/* LINK registers */
> > +/* Module Configuration Register */
> > +#define CSI2nMCG                     0x0
> > +#define CSI2nMCG_SDLN                        GENMASK(11, 8)
> > +
> > +/* Module Control Register 0 */
> > +#define CSI2nMCT0                    0x10
> > +#define CSI2nMCT0_VDLN(x)            ((x) << 0)
> > +
> > +/* Module Control Register 2 */
> > +#define CSI2nMCT2                    0x18
> > +#define CSI2nMCT2_FRRSKW(x)          ((x) << 16)
> > +#define CSI2nMCT2_FRRCLK(x)          ((x) << 0)
> > +
> > +/* Module Control Register 3 */
> > +#define CSI2nMCT3                    0x1c
> > +#define CSI2nMCT3_RXEN                       BIT(0)
> > +
> > +/* Reset Control Register */
> > +#define CSI2nRTCT                    0x28
> > +#define CSI2nRTCT_VSRST                      BIT(0)
> > +
> > +/* Reset Status Register */
> > +#define CSI2nRTST                    0x2c
> > +#define CSI2nRTST_VSRSTS             BIT(0)
> > +
> > +/* Receive Data Type Enable Low Register */
> > +#define CSI2nDTEL                    0x60
> > +
> > +/* Receive Data Type Enable High Register */
> > +#define CSI2nDTEH                    0x64
> > +
> > +/* Power Management Status Register */
> > +#define CSI2nPMST                    0x200
> > +
> > +/* Power Management Status Clear Register */
> > +#define CSI2nPMSC                    0x204
> > +
> > +/* DPHY registers */
> > +/* D-PHY Control Register 0 */
> > +#define CSIDPHYCTRL0                 0x400
> > +#define CSIDPHYCTRL0_EN_LDO1200              BIT(1)
> > +#define CSIDPHYCTRL0_EN_BGR          BIT(0)
> > +
> > +/* D-PHY Timing Register 0 */
> > +#define CSIDPHYTIM0                  0x404
> > +#define CSIDPHYTIM0_TCLK_MISS(x)     ((x) << 24)
> > +#define CSIDPHYTIM0_T_INIT(x)                ((x) << 0)
> > +
> > +/* D-PHY Timing Register 1 */
> > +#define CSIDPHYTIM1                  0x408
> > +#define CSIDPHYTIM1_THS_PREPARE(x)   ((x) << 24)
> > +#define CSIDPHYTIM1_TCLK_PREPARE(x)  ((x) << 16)
> > +#define CSIDPHYTIM1_THS_SETTLE(x)    ((x) << 8)
> > +#define CSIDPHYTIM1_TCLK_SETTLE(x)   ((x) << 0)
> > +
> > +/* D-PHY Skew Adjustment Function */
> > +#define CSIDPHYSKW0                  0x460
> > +#define CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(x)      ((x) & 0x3)
> > +#define CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(x)      (((x) & 0x3) << 4)
> > +#define CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(x)      (((x) & 0x3) << 8)
> > +#define CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(x)      (((x) & 0x3) << 12)
> > +#define CSIDPHYSKW0_DEFAULT_SKW              CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(1) | \
> > +                                     CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(1) | \
> > +                                     CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
> > +                                     CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1)
> > +
> > +#define VSRSTS_RETRIES                       20
> > +
> > +struct timings {
>
> Such a generic name risks creating conflicts later. Could you name the
> structure rzg2l_csi2_timings ? Same for the dphy_timings enum.
>
Agreed, I will rename it.

> > +     u32 t_init;
> > +     u32 tclk_miss;
> > +     u32 tclk_settle;
> > +     u32 ths_settle;
> > +     u32 tclk_prepare;
> > +     u32 ths_prepare;
> > +};
> > +
> > +enum dphy_timings {
> > +     TRANSMISSION_RATE_80_MBPS = 0,
> > +     TRANSMISSION_RATE_125_MBPS,
> > +     TRANSMISSION_RATE_250_MBPS,
> > +     TRANSMISSION_RATE_360_MBPS,
> > +     TRANSMISSION_RATE_360_PLUS_MBPS,
> > +};
> > +
> > +static const struct timings global_timings[] = {
>
> And there, rzg2l_csi2_global_timings.
>
OK.

> > +     [TRANSMISSION_RATE_80_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 23,
> > +             .ths_settle = 31,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 19,
> > +     },
> > +     [TRANSMISSION_RATE_125_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 23,
> > +             .ths_settle = 28,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 19,
> > +     },
> > +     [TRANSMISSION_RATE_250_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 23,
> > +             .ths_settle = 22,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 16,
> > +     },
> > +     [TRANSMISSION_RATE_360_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 18,
> > +             .ths_settle = 19,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 10,
> > +     },
> > +     [TRANSMISSION_RATE_360_PLUS_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 18,
> > +             .ths_settle = 18,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 10,
> > +     },
> > +};
> > +
> > +struct rzg2l_csi2_format {
> > +     u32 code;
> > +     unsigned int datatype;
> > +     unsigned int bpp;
> > +};
> > +
> > +static const struct rzg2l_csi2_format rzg2l_csi2_formats[] = {
> > +     { .code = MEDIA_BUS_FMT_UYVY8_1X16,     .datatype = 0x1e, .bpp = 16 },
> > +};
> > +
> > +static const struct rzg2l_csi2_format *rzg2l_csi2_code_to_fmt(unsigned int code)
> > +{
> > +     unsigned int i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(rzg2l_csi2_formats); i++)
> > +             if (rzg2l_csi2_formats[i].code == code)
> > +                     return &rzg2l_csi2_formats[i];
> > +
> > +     return NULL;
> > +}
> > +
> > +static inline struct rzg2l_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
> > +{
> > +     return container_of(n, struct rzg2l_csi2, notifier);
> > +}
> > +
> > +static u32 rzg2l_csi2_read(struct rzg2l_csi2 *csi2, unsigned int reg)
> > +{
> > +     return ioread32(csi2->base + reg);
> > +}
> > +
> > +static void rzg2l_csi2_write(struct rzg2l_csi2 *csi2, unsigned int reg,
> > +                          u32 data)
> > +{
> > +     iowrite32(data, csi2->base + reg);
> > +}
> > +
> > +static void rzg2l_csi2_set(struct rzg2l_csi2 *csi2, unsigned int reg, u32 set)
> > +{
> > +     rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) | set);
> > +}
> > +
> > +static void rzg2l_csi2_clr(struct rzg2l_csi2 *csi2, unsigned int reg, u32 clr)
> > +{
> > +     rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) & ~clr);
> > +}
> > +
> > +static int rzg2l_csi2_calc_mbps(struct rzg2l_csi2 *csi2)
> > +{
> > +     const struct rzg2l_csi2_format *format;
> > +     struct v4l2_subdev *source;
> > +     struct v4l2_ctrl *ctrl;
> > +     u64 mbps;
> > +
> > +     if (!csi2->remote)
> > +             return -ENODEV;
>
> Can this happen, given that the remote field is set in the bound()
> handler ?
>
Agreed, the check can be dropped.

> > +
> > +     source = csi2->remote;
> > +
> > +     /* Read the pixel rate control from remote. */
> > +     ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> > +     if (!ctrl) {
> > +             dev_err(csi2->dev, "no pixel rate control in subdev %s\n",
> > +                     source->name);
> > +             return -EINVAL;
> > +     }
> > +
> > +     format = rzg2l_csi2_code_to_fmt(csi2->mf.code);
> > +
> > +     /*
> > +      * Calculate hsfreq in Mbps
> > +      * hsfreq = (pixel_rate * bits_per_sample) / number_of_lanes
> > +      */
> > +     mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
> > +     do_div(mbps, csi2->lanes * 1000000);
> > +
> > +     return mbps;
> > +}
> > +
> > +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2)
> > +{
> > +     return reset_control_deassert(csi2->rstc);
> > +}
> > +EXPORT_SYMBOL_GPL(rzg2l_csi2_cmn_rstb_deassert);
>
> I don't think you need to export this, or the
> rzg2l_csi2_mipi_link_setting() function. They're called by the CRU
> driver just before or after calling the .s_stream() operation, so you
> can move the calls there.
>
OK, I will drop the export symbols.

> > +
> > +/* -----------------------------------------------------------------------------
> > + * DPHY setting
> > + */
> > +
> > +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on)
> > +{
> > +     int lanes, mbps;
> > +
> > +     dev_dbg(csi2->dev, "Input size (%ux%u%c)\n",
> > +             csi2->mf.width, csi2->mf.height,
> > +             csi2->mf.field == V4L2_FIELD_NONE ? 'p' : 'i');
> > +
> > +     /* Checking the maximum lanes support for CSI-2 module */
> > +     lanes = (rzg2l_csi2_read(csi2, CSI2nMCG) & CSI2nMCG_SDLN) >> 8;
> > +     if (lanes < csi2->lanes) {
> > +             dev_err(csi2->dev,
> > +                     "Failed to support %d data lanes\n", csi2->lanes);
> > +             return -EINVAL;
> > +     }
>
> You should read and validate the number of lanes at probe time.
>
Agreed, I will move this check in probe.

> > +
> > +     mbps = rzg2l_csi2_calc_mbps(csi2);
> > +     if (mbps < 0)
> > +             return mbps;
> > +
> > +     csi2->hsfreq = mbps;
>
> This is only needed in the (on) branch.
>
Agreed.

> > +
> > +     if (on) {
>
> The two branches share no code. Split them in two separate functions.
>
OK, Ill split it into two separate functions.

> > +             struct timings dphy_timing;
>
> Make this a const pointer.
>
Agreed.

> > +             u32 dphytim0, dphytim1;
> > +
> > +             /* Set DPHY timing parameters */
> > +             if (csi2->hsfreq <= 80)
> > +                     dphy_timing = global_timings[TRANSMISSION_RATE_80_MBPS];
> > +             else if (csi2->hsfreq <= 125)
> > +                     dphy_timing = global_timings[TRANSMISSION_RATE_125_MBPS];
> > +             else if (csi2->hsfreq <= 250)
> > +                     dphy_timing = global_timings[TRANSMISSION_RATE_250_MBPS];
> > +             else if (csi2->hsfreq <= 360)
> > +                     dphy_timing = global_timings[TRANSMISSION_RATE_360_MBPS];
> > +             else if (csi2->hsfreq <= 1500)
> > +                     dphy_timing = global_timings[TRANSMISSION_RATE_360_PLUS_MBPS];
> > +             else
> > +                     return -EINVAL;
> > +
> > +             /* Set D-PHY timing parameters */
> > +             dphytim0 = CSIDPHYTIM0_TCLK_MISS(dphy_timing.tclk_miss) |
> > +                        CSIDPHYTIM0_T_INIT(dphy_timing.t_init);
> > +             dphytim1 = CSIDPHYTIM1_THS_PREPARE(dphy_timing.ths_prepare) |
> > +                        CSIDPHYTIM1_TCLK_PREPARE(dphy_timing.tclk_prepare) |
> > +                        CSIDPHYTIM1_THS_SETTLE(dphy_timing.ths_settle) |
> > +                        CSIDPHYTIM1_TCLK_SETTLE(dphy_timing.tclk_settle);
> > +             rzg2l_csi2_write(csi2, CSIDPHYTIM0, dphytim0);
> > +             rzg2l_csi2_write(csi2, CSIDPHYTIM1, dphytim1);
> > +
> > +             /* Enable D-PHY power control 0 */
> > +             rzg2l_csi2_write(csi2, CSIDPHYSKW0, CSIDPHYSKW0_DEFAULT_SKW);
> > +
> > +             /* Set the EN_BGR bit */
> > +             rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> > +
> > +             /* Delay 20us to be stable */
> > +             usleep_range(20, 40);
> > +
> > +             /* Enable D-PHY power control 1 */
> > +             rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> > +
> > +             /* Delay 10us to be stable */
> > +             usleep_range(10, 20);
> > +
> > +             /* Start supplying the internal clock for the D-PHY block */
> > +             pm_runtime_get_sync(csi2->dev);
>
> Missing error handling (and use pm_runtime_resume_and_get() to make that
> easier). But it doesn't seem quite right, you're not supposed to resume
> the device after writing register, but before.
>
> > +     } else {
> > +             /* Reset the CRU (D-PHY) */
> > +             reset_control_assert(csi2->rstc);
> > +
> > +             /* Stop the D-PHY clock */
> > +             pm_runtime_put(csi2->dev);
>
> Same here, that should to after the register writes.
>
> > +
> > +             /* Cancel the EN_LDO1200 register setting */
> > +             rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> > +
> > +             /* Cancel the EN_BGR register setting */
> > +             rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> > +     }
> > +
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(rzg2l_csi2_dphy_setting);
>
> Would it be a problem to also move this call to .s_stream() to avoid
> direct calls from the CRU driver ?
>
Yes it causes a problem, the CSI and CRU setup is as per the sequence
mentioned in the HW manual. If I move this to .stream() callback I
sometimes see no buffers being captured on the CSI part.

> > +
> > +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on)
> > +{
> > +     if (on) {
>
> Here too the two branches share no code, split them in two separate
> functions.
>
Ok will do.

> > +             unsigned long vclk_rate = csi2->vclk_rate / HZ_PER_MHZ;
> > +             u32 frrskw, frrclk, frrskw_coeff, frrclk_coeff;
> > +
> > +             /* Select data lanes */
> > +             rzg2l_csi2_write(csi2, CSI2nMCT0, CSI2nMCT0_VDLN(csi2->lanes));
> > +
> > +             frrskw_coeff = 3 * vclk_rate * 8;
> > +             frrclk_coeff = frrskw_coeff / 2;
> > +             frrskw = DIV_ROUND_UP(frrskw_coeff, csi2->hsfreq);
> > +             frrclk = DIV_ROUND_UP(frrclk_coeff, csi2->hsfreq);
> > +             rzg2l_csi2_write(csi2, CSI2nMCT2,
> > +                              CSI2nMCT2_FRRSKW(frrskw) | CSI2nMCT2_FRRCLK(frrclk));
> > +
> > +             /*
> > +              * Select data type.
> > +              * FS, FE, LS, LE, Generic Short Packet Codes 1 to 8,
> > +              * Generic Long Packet Data Types 1 to 4 YUV422 8-bit,
> > +              * RGB565, RGB888, RAW8 to RAW20, User-defined 8-bit
> > +              * data types 1 to 8
> > +              */
> > +             rzg2l_csi2_write(csi2, CSI2nDTEL, 0xf778ff0f);
> > +             rzg2l_csi2_write(csi2, CSI2nDTEH, 0x00ffff1f);
> > +
> > +             /* Enable LINK reception */
> > +             rzg2l_csi2_set(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
>
> Would it be an issue to use rzg2l_csi2_write() here ?
>
> > +     } else {
> > +             unsigned int timeout = VSRSTS_RETRIES;
> > +
> > +             /* Stop LINK reception */
> > +             rzg2l_csi2_clr(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
>
> Same here, rzg2l_csi2_write(csi2, CSI2nMCT3, 0);
>
> Same for the other usage of rzg2l_csi2_set() and rzg2l_csi2_clr() above,
> to avoid having to read back the value first.
>
> > +
> > +             /* Request a software reset of the LINK Video Pixel Interface */
> > +             rzg2l_csi2_write(csi2, CSI2nRTCT, CSI2nRTCT_VSRST);
> > +
> > +             /* Make sure CSI2nRTST.VSRSTS bit is cleared */
> > +             while (timeout--) {
> > +                     if (!(rzg2l_csi2_read(csi2, CSI2nRTST) & CSI2nRTST_VSRSTS))
> > +                             break;
> > +                     usleep_range(100, 200);
> > +             };
> > +     }
> > +}
> > +EXPORT_SYMBOL_GPL(rzg2l_csi2_mipi_link_setting);
> > +
> > +static int rzg2l_csi2_s_stream(struct v4l2_subdev *sd, int enable)
> > +{
> > +     struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> > +     int ret = 0;
> > +
> > +     mutex_lock(&csi2->lock);
> > +
> > +     if (!csi2->remote) {
> > +             ret = -ENODEV;
> > +             goto out;
> > +     }
>
> I don't think this can happen either.
>
Agreed, I will drop this check.

> > +
> > +     if (enable && !csi2->stream_count) {
> > +             ret = v4l2_subdev_call(csi2->remote, video, s_stream, 1);
> > +             if (ret)
> > +                     goto out;
> > +     } else if (!enable && csi2->stream_count == 1) {
> > +             v4l2_subdev_call(csi2->remote, video, s_stream, 0);
> > +     }
> > +
> > +     csi2->stream_count += enable ? 1 : -1;
>
> Drop stream_count, .s_stream() is not reference-counted.
>
Ok, I'll drop it.

> > +out:
> > +     mutex_unlock(&csi2->lock);
> > +
> > +     return ret;
> > +}
> > +
> > +static int rzg2l_csi2_set_format(struct v4l2_subdev *sd,
> > +                              struct v4l2_subdev_state *state,
> > +                              struct v4l2_subdev_format *format)
> > +{
> > +     struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> > +     struct v4l2_mbus_framefmt *framefmt;
> > +
> > +     mutex_lock(&csi2->lock);
> > +
> > +     if (!rzg2l_csi2_code_to_fmt(format->format.code))
> > +             format->format.code = rzg2l_csi2_formats[0].code;
> > +
> > +     if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> > +             csi2->mf = format->format;
>
> Please use the subdev active state storage to avoid manually storing the
> format. See https://lore.kernel.org/linux-media/20220823005822.13805-3-laurent.pinchart@ideasonboard.com/
> and in particular
>
> - How the .get_fmt() and .set_fmt() handlers use
>   v4l2_subdev_get_pad_format() unconditionally to get the format,
>   regardless of whether it's the active or try format.
> - The v4l2_subdev_lock_and_get_active_state() call in .s_stream() to get
>   the active state, and access the format from it.
> - You're already calling v4l2_subdev_init_finalize() in probe, good :-)
>
> You'll need to implement .init_cfg(), and you'll be able to drop the
> lock field in rzg2l_csi2.
>
OK, I'll switch to active state storage.

> > +     } else {
> > +             framefmt = v4l2_subdev_get_try_format(sd, state, 0);
> > +             *framefmt = format->format;
> > +     }
> > +
> > +     mutex_unlock(&csi2->lock);
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_csi2_get_pad_format(struct v4l2_subdev *sd,
> > +                                  struct v4l2_subdev_state *sd_state,
> > +                                  struct v4l2_subdev_format *format)
> > +{
> > +     struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> > +
> > +     mutex_lock(&csi2->lock);
> > +     if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> > +             format->format = csi2->mf;
> > +     else
> > +             format->format = *v4l2_subdev_get_try_format(sd, sd_state, 0);
> > +     mutex_unlock(&csi2->lock);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops rzg2l_csi2_video_ops = {
> > +     .s_stream = rzg2l_csi2_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops rzg2l_csi2_pad_ops = {
> > +     .set_fmt = rzg2l_csi2_set_format,
> > +     .get_fmt = rzg2l_csi2_get_pad_format,
>
> Missing enum functions for format and sizes, and missing .init_cfg().
>
I'll implement the callbacks.

> > +};
> > +
> > +static const struct v4l2_subdev_ops rzg2l_csi2_subdev_ops = {
> > +     .video  = &rzg2l_csi2_video_ops,
> > +     .pad    = &rzg2l_csi2_pad_ops,
> > +};
> > +
> > +/* -----------------------------------------------------------------------------
> > + * Async handling and registration of subdevices and links.
> > + */
.> > +
> > +static int rzg2l_csi2_notify_bound(struct v4l2_async_notifier *notifier,
> > +                                struct v4l2_subdev *subdev,
> > +                                struct v4l2_async_subdev *asd)
> > +{
> > +     struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> > +     int pad;
> > +
> > +     pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode,
> > +                                       MEDIA_PAD_FL_SOURCE);
> > +     if (pad < 0) {
> > +             dev_err(csi2->dev, "Failed to find pad for %s\n", subdev->name);
> > +             return pad;
> > +     }
>
>
> I think you can skip this and just hardcode pad to RZG2L_CSI2_SINK in
> the media_create_pad_link(), there's no other input.
>
OK, I'll drop it and hardcode RZG2L_CSI2_SINK.

> > +
> > +     csi2->remote = subdev;
> > +
> > +     dev_dbg(csi2->dev, "Bound %s pad: %d\n", subdev->name, pad);
> > +
> > +     return media_create_pad_link(&subdev->entity, pad,
> > +                                  &csi2->subdev.entity, 0,
> > +                                  MEDIA_LNK_FL_ENABLED |
> > +                                  MEDIA_LNK_FL_IMMUTABLE);
> > +}
> > +
> > +static void rzg2l_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
> > +                                  struct v4l2_subdev *subdev,
> > +                                  struct v4l2_async_subdev *asd)
> > +{
> > +     struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> > +
> > +     csi2->remote = NULL;
> > +
> > +     dev_dbg(csi2->dev, "Unbind %s\n", subdev->name);
> > +}
> > +
> > +static const struct v4l2_async_notifier_operations rzg2l_csi2_notify_ops = {
> > +     .bound = rzg2l_csi2_notify_bound,
> > +     .unbind = rzg2l_csi2_notify_unbind,
> > +};
> > +
> > +static int rzg2l_csi2_parse_v4l2(struct rzg2l_csi2 *csi2,
> > +                              struct v4l2_fwnode_endpoint *vep)
> > +{
> > +     /* Only port 0 endpoint 0 is valid. */
> > +     if (vep->base.port || vep->base.id)
> > +             return -ENOTCONN;
> > +
> > +     if (vep->bus_type != V4L2_MBUS_CSI2_DPHY) {
> > +             dev_err(csi2->dev, "Unsupported bus: %u\n", vep->bus_type);
> > +             return -EINVAL;
> > +     }
>
> I'm not sure you need this check as the DT bindings should make it clear
> that only CSI-2 sources are valid. Maybe require the bus-type property
> in DT ? Up to you.
>
OK, I'll drop this check and make it clear in the DT binding doc.

> > +
> > +     csi2->lanes = vep->bus.mipi_csi2.num_data_lanes;
> > +     if (csi2->lanes != 1 && csi2->lanes != 2 && csi2->lanes != 4) {
> > +             dev_err(csi2->dev, "Unsupported number of data-lanes: %u\n",
> > +                     csi2->lanes);
> > +             return -EINVAL;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_csi2_parse_dt(struct rzg2l_csi2 *csi2)
> > +{
> > +     struct v4l2_fwnode_endpoint v4l2_ep = {
> > +             .bus_type = V4L2_MBUS_CSI2_DPHY
> > +     };
> > +     struct v4l2_async_subdev *asd;
> > +     struct fwnode_handle *fwnode;
> > +     struct fwnode_handle *ep;
> > +     int ret;
> > +
> > +     ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(csi2->dev), 0, 0, 0);
> > +     if (!ep) {
> > +             dev_err(csi2->dev, "Not connected to subdevice\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = v4l2_fwnode_endpoint_parse(ep, &v4l2_ep);
> > +     if (ret) {
> > +             dev_err(csi2->dev, "Could not parse v4l2 endpoint\n");
> > +             fwnode_handle_put(ep);
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = rzg2l_csi2_parse_v4l2(csi2, &v4l2_ep);
> > +     if (ret) {
> > +             fwnode_handle_put(ep);
> > +             return ret;
> > +     }
> > +
> > +     fwnode = fwnode_graph_get_remote_endpoint(ep);
> > +     fwnode_handle_put(ep);
> > +
> > +     dev_dbg(csi2->dev, "Found '%pOF'\n", to_of_node(fwnode));
> > +
> > +     v4l2_async_nf_init(&csi2->notifier);
> > +     csi2->notifier.ops = &rzg2l_csi2_notify_ops;
> > +
> > +     asd = v4l2_async_nf_add_fwnode(&csi2->notifier, fwnode,
> > +                                    struct v4l2_async_subdev);
> > +     fwnode_handle_put(fwnode);
> > +     if (IS_ERR(asd))
> > +             return PTR_ERR(asd);
> > +
> > +     ret = v4l2_async_subdev_nf_register(&csi2->subdev, &csi2->notifier);
> > +     if (ret)
> > +             v4l2_async_nf_cleanup(&csi2->notifier);
> > +
> > +     return ret;
> > +}
> > +
> > +/* -----------------------------------------------------------------------------
> > + * Platform Device Driver.
> > + */
> > +
> > +static const struct media_entity_operations rzg2l_csi2_entity_ops = {
> > +     .link_validate = v4l2_subdev_link_validate,
> > +};
> > +
> > +static int rzg2l_csi2_probe(struct platform_device *pdev)
> > +{
> > +     struct rzg2l_csi2 *csi2;
> > +     struct clk *vclk;
> > +     unsigned int i;
> > +     int ret;
> > +
> > +     csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
> > +     if (!csi2)
> > +             return -ENOMEM;
> > +
> > +     csi2->base = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(csi2->base))
> > +             return PTR_ERR(csi2->base);
> > +
> > +     csi2->rstc = devm_reset_control_get(&pdev->dev, "cmn-rstb");
> > +     if (IS_ERR(csi2->rstc))
> > +             return dev_err_probe(&pdev->dev, PTR_ERR(csi2->rstc),
> > +                                  "failed to get cpg cmn-rstb\n");
> > +
> > +     vclk = devm_clk_get(&pdev->dev, "vclk");
> > +     if (IS_ERR(vclk))
> > +             return dev_err_probe(&pdev->dev, PTR_ERR(vclk),
> > +                                  "failed to get vclk clock\n");
> > +
> > +     csi2->vclk_rate = clk_get_rate(vclk);
> > +     devm_clk_put(&pdev->dev, vclk);
>
> Is there no need to ensure at runtime at the clock is on ?
>
pm_runtime is used to turn on/off the clocks. Do you think I need to
add a check here?

> > +
> > +     csi2->dev = &pdev->dev;
> > +     mutex_init(&csi2->lock);
> > +     csi2->stream_count = 0;
> > +
> > +     platform_set_drvdata(pdev, csi2);
> > +
> > +     ret = rzg2l_csi2_parse_dt(csi2);
> > +     if (ret)
> > +             goto error_mutex;
> > +
> > +     csi2->subdev.owner = THIS_MODULE;
>
> You don't need this, v4l2_subdev_init() handles it.
>
OK, I'll drop it.

> > +     csi2->subdev.dev = &pdev->dev;
> > +     v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
> > +     v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
> > +     snprintf(csi2->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
>
> sizeof(csi2->subdev.name) instead of V4L2_SUBDEV_NAME_SIZE, it's less
> error-prone.
>
Agreed.

> > +              KBUILD_MODNAME, dev_name(&pdev->dev));
>
> Don't use KBUILD_MODNAME but set a device name here, for instance use
> "csi-%s" as the format string. The subdev name shouldn't change if the
> module is renamed.
>
Agreed, I'll update it.

> > +     csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> > +
> > +     csi2->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
>
> I think MEDIA_ENT_F_VID_IF_BRIDGE would be more appropriate. The pixel
> formatter reads or writes pixels from/to memory.
>
OK, I'll update it.

> > +     csi2->subdev.entity.ops = &rzg2l_csi2_entity_ops;
> > +
> > +     csi2->pads[RZG2L_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> > +     for (i = RZG2L_CSI2_SOURCE_VC0; i < NR_OF_RZG2L_CSI2_PAD; i++)
> > +             csi2->pads[i].flags = MEDIA_PAD_FL_SOURCE;
>
> One source pad only. Support for multiple virtual channels should be
> implemented using the stream API that is under development (see
> https://lore.kernel.org/linux-media/20220810121122.3149086-1-tomi.valkeinen@ideasonboard.com/T/#t).
>
My RFCv2 had this, but I dropped it for this version as adding support
for the multiplex stream was/is moving at a slower pace. So instead I
thought we can patch this up later.

> > +
> > +     ret = media_entity_pads_init(&csi2->subdev.entity, NR_OF_RZG2L_CSI2_PAD, csi2->pads);
> > +     if (ret)
> > +             goto error_async;
> > +
> > +     pm_runtime_enable(&pdev->dev);
> > +
> > +     ret = v4l2_subdev_init_finalize(&csi2->subdev);
> > +     if (ret < 0)
> > +             goto error_pm;
> > +
> > +     ret = v4l2_async_register_subdev(&csi2->subdev);
> > +     if (ret < 0)
> > +             goto error_subdev;
> > +
> > +     dev_info(csi2->dev, "%d lanes found\n", csi2->lanes);
> > +
> > +     return 0;
> > +
> > +error_subdev:
> > +     v4l2_subdev_cleanup(&csi2->subdev);
> > +error_pm:
> > +     pm_runtime_disable(&pdev->dev);
> > +error_async:
> > +     v4l2_async_nf_unregister(&csi2->notifier);
> > +     v4l2_async_nf_cleanup(&csi2->notifier);
> > +error_mutex:
> > +     mutex_destroy(&csi2->lock);
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct of_device_id rzg2l_csi2_of_table[] = {
> > +     { .compatible = "renesas,rzg2l-csi2", },
> > +     { /* sentinel */ }
> > +};
> > +
> > +static int rzg2l_csi2_remove(struct platform_device *pdev)
> > +{
> > +     struct rzg2l_csi2 *csi2 = platform_get_drvdata(pdev);
> > +
> > +     v4l2_async_nf_unregister(&csi2->notifier);
> > +     v4l2_async_nf_cleanup(&csi2->notifier);
> > +     v4l2_async_unregister_subdev(&csi2->subdev);
> > +     v4l2_subdev_cleanup(&csi2->subdev);
> > +
> > +     pm_runtime_disable(&pdev->dev);
> > +     mutex_destroy(&csi2->lock);
> > +
> > +     return 0;
> > +}
> > +
> > +static struct platform_driver rzg2l_csi2_pdrv = {
> > +     .remove = rzg2l_csi2_remove,
> > +     .probe  = rzg2l_csi2_probe,
> > +     .driver = {
> > +             .name = "rzg2l-csi2",
> > +             .of_match_table = rzg2l_csi2_of_table,
> > +     },
> > +};
> > +
> > +module_platform_driver(rzg2l_csi2_pdrv);
> > +
> > +MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
> > +MODULE_DESCRIPTION("Renesas RZ/G2L MIPI CSI2 receiver driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> > new file mode 100644
> > index 000000000000..7192923517f6
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> > @@ -0,0 +1,49 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (C) 2022 Renesas Electronics Corp.
> > + */
> > +
> > +#ifndef __RZG2L_CSI2__
> > +#define __RZG2L_CSI2__
> > +
> > +enum rzg2l_csi2_pads {
> > +     RZG2L_CSI2_SINK = 0,
> > +     RZG2L_CSI2_SOURCE_VC0,
> > +     RZG2L_CSI2_SOURCE_VC1,
> > +     RZG2L_CSI2_SOURCE_VC2,
> > +     RZG2L_CSI2_SOURCE_VC3,
> > +     NR_OF_RZG2L_CSI2_PAD,
> > +};
> > +
> > +struct rzg2l_csi2 {
> > +     struct device *dev;
> > +     void __iomem *base;
> > +     struct reset_control *rstc;
> > +     unsigned long vclk_rate;
> > +
> > +     struct v4l2_subdev subdev;
> > +     struct media_pad pads[NR_OF_RZG2L_CSI2_PAD];
> > +
> > +     struct v4l2_async_notifier notifier;
> > +     struct v4l2_subdev *remote;
>
> You can name this field source (or remote_source, up to you), it will be
> more explicit.
>
OK, I will rename it.

Cheers,
Prabhakar

Re: [PATCH 3/4] media: platform: Add Renesas RZ/G2L MIPI CSI-2 receiver driver

[Laurent Pinchart]

Hi Prabhakar,

On Tue, Aug 30, 2022 at 01:42:33PM +0100, Lad, Prabhakar wrote:
> On Tue, Aug 30, 2022 at 1:28 AM Laurent Pinchart wrote:
> > On Mon, Aug 01, 2022 at 10:47:17PM +0100, Lad Prabhakar wrote:
> > > Add MIPI CSI-2 receiver driver for Renesas RZ/G2L. The MIPI
> > > CSI-2 is part of the CRU module found on RZ/G2L family.
> > >
> > > Based on a patch in the BSP by Hien Huynh
> > > <hien.huynh.px@renesas.com>
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > > RFC v2 -> v1
> > > * Fixed initialization sequence of DPHY and link
> > > * Exported DPHY and link initialization functions so that the
> > >   CRU core driver can initialize the CRU and CSI2 as per the HW manual.
> > >
> > > RFC v1 -> RFC v2
> > > * new patch (split up as new driver compared to v1)
> > > ---
> > >  drivers/media/platform/renesas/Kconfig        |   1 +
> > >  drivers/media/platform/renesas/Makefile       |   1 +
> > >  .../media/platform/renesas/rzg2l-cru/Kconfig  |  17 +
> > >  .../media/platform/renesas/rzg2l-cru/Makefile |   3 +
> > >  .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 702 ++++++++++++++++++
> > >  .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  49 ++
> > >  6 files changed, 773 insertions(+)
> > >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
> > >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
> > >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h

[snip]

> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > > new file mode 100644
> > > index 000000000000..252e33ddb6e8
> > > --- /dev/null
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > > @@ -0,0 +1,702 @@

[snip]

> > > +/* -----------------------------------------------------------------------------
> > > + * DPHY setting
> > > + */
> > > +
> > > +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on)
> > > +{
> > > +     int lanes, mbps;
> > > +
> > > +     dev_dbg(csi2->dev, "Input size (%ux%u%c)\n",
> > > +             csi2->mf.width, csi2->mf.height,
> > > +             csi2->mf.field == V4L2_FIELD_NONE ? 'p' : 'i');
> > > +
> > > +     /* Checking the maximum lanes support for CSI-2 module */
> > > +     lanes = (rzg2l_csi2_read(csi2, CSI2nMCG) & CSI2nMCG_SDLN) >> 8;
> > > +     if (lanes < csi2->lanes) {
> > > +             dev_err(csi2->dev,
> > > +                     "Failed to support %d data lanes\n", csi2->lanes);
> > > +             return -EINVAL;
> > > +     }
> >
> > You should read and validate the number of lanes at probe time.
> >
> Agreed, I will move this check in probe.
> 
> > > +
> > > +     mbps = rzg2l_csi2_calc_mbps(csi2);
> > > +     if (mbps < 0)
> > > +             return mbps;
> > > +
> > > +     csi2->hsfreq = mbps;
> >
> > This is only needed in the (on) branch.
> >
> Agreed.
> 
> > > +
> > > +     if (on) {
> >
> > The two branches share no code. Split them in two separate functions.
> >
> OK, Ill split it into two separate functions.
> 
> > > +             struct timings dphy_timing;
> >
> > Make this a const pointer.
> >
> Agreed.
> 
> > > +             u32 dphytim0, dphytim1;
> > > +
> > > +             /* Set DPHY timing parameters */
> > > +             if (csi2->hsfreq <= 80)
> > > +                     dphy_timing = global_timings[TRANSMISSION_RATE_80_MBPS];
> > > +             else if (csi2->hsfreq <= 125)
> > > +                     dphy_timing = global_timings[TRANSMISSION_RATE_125_MBPS];
> > > +             else if (csi2->hsfreq <= 250)
> > > +                     dphy_timing = global_timings[TRANSMISSION_RATE_250_MBPS];
> > > +             else if (csi2->hsfreq <= 360)
> > > +                     dphy_timing = global_timings[TRANSMISSION_RATE_360_MBPS];
> > > +             else if (csi2->hsfreq <= 1500)
> > > +                     dphy_timing = global_timings[TRANSMISSION_RATE_360_PLUS_MBPS];
> > > +             else
> > > +                     return -EINVAL;
> > > +
> > > +             /* Set D-PHY timing parameters */
> > > +             dphytim0 = CSIDPHYTIM0_TCLK_MISS(dphy_timing.tclk_miss) |
> > > +                        CSIDPHYTIM0_T_INIT(dphy_timing.t_init);
> > > +             dphytim1 = CSIDPHYTIM1_THS_PREPARE(dphy_timing.ths_prepare) |
> > > +                        CSIDPHYTIM1_TCLK_PREPARE(dphy_timing.tclk_prepare) |
> > > +                        CSIDPHYTIM1_THS_SETTLE(dphy_timing.ths_settle) |
> > > +                        CSIDPHYTIM1_TCLK_SETTLE(dphy_timing.tclk_settle);
> > > +             rzg2l_csi2_write(csi2, CSIDPHYTIM0, dphytim0);
> > > +             rzg2l_csi2_write(csi2, CSIDPHYTIM1, dphytim1);
> > > +
> > > +             /* Enable D-PHY power control 0 */
> > > +             rzg2l_csi2_write(csi2, CSIDPHYSKW0, CSIDPHYSKW0_DEFAULT_SKW);
> > > +
> > > +             /* Set the EN_BGR bit */
> > > +             rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> > > +
> > > +             /* Delay 20us to be stable */
> > > +             usleep_range(20, 40);
> > > +
> > > +             /* Enable D-PHY power control 1 */
> > > +             rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> > > +
> > > +             /* Delay 10us to be stable */
> > > +             usleep_range(10, 20);
> > > +
> > > +             /* Start supplying the internal clock for the D-PHY block */
> > > +             pm_runtime_get_sync(csi2->dev);
> >
> > Missing error handling (and use pm_runtime_resume_and_get() to make that
> > easier). But it doesn't seem quite right, you're not supposed to resume
> > the device after writing register, but before.
> >
> > > +     } else {
> > > +             /* Reset the CRU (D-PHY) */
> > > +             reset_control_assert(csi2->rstc);
> > > +
> > > +             /* Stop the D-PHY clock */
> > > +             pm_runtime_put(csi2->dev);
> >
> > Same here, that should to after the register writes.
> >
> > > +
> > > +             /* Cancel the EN_LDO1200 register setting */
> > > +             rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> > > +
> > > +             /* Cancel the EN_BGR register setting */
> > > +             rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(rzg2l_csi2_dphy_setting);
> >
> > Would it be a problem to also move this call to .s_stream() to avoid
> > direct calls from the CRU driver ?
> >
> Yes it causes a problem, the CSI and CRU setup is as per the sequence
> mentioned in the HW manual. If I move this to .stream() callback I
> sometimes see no buffers being captured on the CSI part.

OK, let's keep it for now and see how it will look like in the next
version.

[snip]

> > > +/* -----------------------------------------------------------------------------
> > > + * Platform Device Driver.
> > > + */
> > > +
> > > +static const struct media_entity_operations rzg2l_csi2_entity_ops = {
> > > +     .link_validate = v4l2_subdev_link_validate,
> > > +};
> > > +
> > > +static int rzg2l_csi2_probe(struct platform_device *pdev)
> > > +{
> > > +     struct rzg2l_csi2 *csi2;
> > > +     struct clk *vclk;
> > > +     unsigned int i;
> > > +     int ret;
> > > +
> > > +     csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
> > > +     if (!csi2)
> > > +             return -ENOMEM;
> > > +
> > > +     csi2->base = devm_platform_ioremap_resource(pdev, 0);
> > > +     if (IS_ERR(csi2->base))
> > > +             return PTR_ERR(csi2->base);
> > > +
> > > +     csi2->rstc = devm_reset_control_get(&pdev->dev, "cmn-rstb");
> > > +     if (IS_ERR(csi2->rstc))
> > > +             return dev_err_probe(&pdev->dev, PTR_ERR(csi2->rstc),
> > > +                                  "failed to get cpg cmn-rstb\n");
> > > +
> > > +     vclk = devm_clk_get(&pdev->dev, "vclk");
> > > +     if (IS_ERR(vclk))
> > > +             return dev_err_probe(&pdev->dev, PTR_ERR(vclk),
> > > +                                  "failed to get vclk clock\n");
> > > +
> > > +     csi2->vclk_rate = clk_get_rate(vclk);
> > > +     devm_clk_put(&pdev->dev, vclk);
> >
> > Is there no need to ensure at runtime at the clock is on ?
> >
> pm_runtime is used to turn on/off the clocks. Do you think I need to
> add a check here?

If the clock is fully handled by RPM without intervention from the
driver, then this is fine. I would however use clk_get() and clk_put()
instead of the devm_* versions in that case, that would have a lower
overhead.

> > > +
> > > +     csi2->dev = &pdev->dev;
> > > +     mutex_init(&csi2->lock);
> > > +     csi2->stream_count = 0;
> > > +
> > > +     platform_set_drvdata(pdev, csi2);
> > > +
> > > +     ret = rzg2l_csi2_parse_dt(csi2);
> > > +     if (ret)
> > > +             goto error_mutex;
> > > +
> > > +     csi2->subdev.owner = THIS_MODULE;
> >
> > You don't need this, v4l2_subdev_init() handles it.
> 
> OK, I'll drop it.
> 
> > > +     csi2->subdev.dev = &pdev->dev;
> > > +     v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
> > > +     v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
> > > +     snprintf(csi2->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
> >
> > sizeof(csi2->subdev.name) instead of V4L2_SUBDEV_NAME_SIZE, it's less
> > error-prone.
> 
> Agreed.
> 
> > > +              KBUILD_MODNAME, dev_name(&pdev->dev));
> >
> > Don't use KBUILD_MODNAME but set a device name here, for instance use
> > "csi-%s" as the format string. The subdev name shouldn't change if the
> > module is renamed.
> 
> Agreed, I'll update it.
> 
> > > +     csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > +
> > > +     csi2->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> >
> > I think MEDIA_ENT_F_VID_IF_BRIDGE would be more appropriate. The pixel
> > formatter reads or writes pixels from/to memory.
> 
> OK, I'll update it.
> 
> > > +     csi2->subdev.entity.ops = &rzg2l_csi2_entity_ops;
> > > +
> > > +     csi2->pads[RZG2L_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> > > +     for (i = RZG2L_CSI2_SOURCE_VC0; i < NR_OF_RZG2L_CSI2_PAD; i++)
> > > +             csi2->pads[i].flags = MEDIA_PAD_FL_SOURCE;
> >
> > One source pad only. Support for multiple virtual channels should be
> > implemented using the stream API that is under development (see
> > https://lore.kernel.org/linux-media/20220810121122.3149086-1-tomi.valkeinen@ideasonboard.com/T/#t).
> 
> My RFCv2 had this, but I dropped it for this version as adding support
> for the multiplex stream was/is moving at a slower pace. So instead I
> thought we can patch this up later.

There's no need to use the streams API for now as it's not ready yet,
but multiple pads isn't an option either. You can hardcode usage of VC 0
for now.

> > > +
> > > +     ret = media_entity_pads_init(&csi2->subdev.entity, NR_OF_RZG2L_CSI2_PAD, csi2->pads);
> > > +     if (ret)
> > > +             goto error_async;
> > > +
> > > +     pm_runtime_enable(&pdev->dev);
> > > +
> > > +     ret = v4l2_subdev_init_finalize(&csi2->subdev);
> > > +     if (ret < 0)
> > > +             goto error_pm;
> > > +
> > > +     ret = v4l2_async_register_subdev(&csi2->subdev);
> > > +     if (ret < 0)
> > > +             goto error_subdev;
> > > +
> > > +     dev_info(csi2->dev, "%d lanes found\n", csi2->lanes);
> > > +
> > > +     return 0;
> > > +
> > > +error_subdev:
> > > +     v4l2_subdev_cleanup(&csi2->subdev);
> > > +error_pm:
> > > +     pm_runtime_disable(&pdev->dev);
> > > +error_async:
> > > +     v4l2_async_nf_unregister(&csi2->notifier);
> > > +     v4l2_async_nf_cleanup(&csi2->notifier);
> > > +error_mutex:
> > > +     mutex_destroy(&csi2->lock);
> > > +
> > > +     return ret;
> > > +}

-- 
Regards,

Laurent Pinchart

[PATCH 4/4] media: platform: Add Renesas RZ/G2L CRU driver

[Lad Prabhakar]

Add v4l driver for Renesas RZ/G2L Camera data Receiving Unit.

Based on a patch in the BSP by Hien Huynh
<hien.huynh.px@renesas.com>

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC v2 -> v1
* Moved the driver to renesas folder 
* Fixed review comments pointed by Jacopo

RFC v1 -> RFC v2
* Dropped group
* Dropped CSI subdev and implemented as new driver
* Dropped "mc_" from function names
* Moved the driver to renesas folder
---
 .../media/platform/renesas/rzg2l-cru/Kconfig  |  17 +
 .../media/platform/renesas/rzg2l-cru/Makefile |   3 +
 .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 371 +++++++++
 .../platform/renesas/rzg2l-cru/rzg2l-cru.h    | 144 ++++
 .../platform/renesas/rzg2l-cru/rzg2l-dma.c    | 728 ++++++++++++++++++
 .../platform/renesas/rzg2l-cru/rzg2l-v4l2.c   | 354 +++++++++
 6 files changed, 1617 insertions(+)
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-dma.c
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-v4l2.c

diff --git a/drivers/media/platform/renesas/rzg2l-cru/Kconfig b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
index 150bf16f167c..fff41dfbe41f 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/Kconfig
+++ b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
@@ -15,3 +15,20 @@ config VIDEO_RZG2L_CSI2
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called rzg2l-csi2.
+
+config VIDEO_RZG2L_CRU
+	tristate "RZ/G2L Camera Receiving Unit (CRU) Driver"
+	depends on V4L_PLATFORM_DRIVERS
+	depends on VIDEO_DEV && OF
+	depends on ARCH_RENESAS || COMPILE_TEST
+	select VIDEO_RZG2L_CSI2
+	select MEDIA_CONTROLLER
+	select VIDEO_V4L2_SUBDEV_API
+	select VIDEOBUF2_DMA_CONTIG
+	select V4L2_FWNODE
+	help
+	  Support for Renesas RZ/G2L (and alike SoC's) Camera Receiving
+	  Unit (CRU) driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rzg2l-cru.
diff --git a/drivers/media/platform/renesas/rzg2l-cru/Makefile b/drivers/media/platform/renesas/rzg2l-cru/Makefile
index 91ea97a944e6..7628809e953f 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/Makefile
+++ b/drivers/media/platform/renesas/rzg2l-cru/Makefile
@@ -1,3 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_VIDEO_RZG2L_CSI2) += rzg2l-csi2.o
+
+rzg2l-cru-objs = rzg2l-core.o rzg2l-dma.o rzg2l-v4l2.o
+obj-$(CONFIG_VIDEO_RZG2L_CRU) += rzg2l-cru.o
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
new file mode 100644
index 000000000000..085e29fb2cce
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Renesas RZ/G2L CRU
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ *
+ * Based on Renesas R-Car VIN
+ * Copyright (C) 2011-2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
+ * Copyright (C) 2008 Magnus Damm
+ */
+
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+
+#include "rzg2l-cru.h"
+
+#define v4l2_dev_to_cru(d)	container_of(d, struct rzg2l_cru_dev, v4l2_dev)
+
+static int rzg2l_cru_csi2_link_notify(struct media_link *link, u32 flags,
+				      unsigned int notification)
+{
+	struct media_entity *entity;
+	struct rzg2l_cru_dev *cru;
+	struct media_pad *csi_pad;
+	struct v4l2_subdev *sd;
+	int ret;
+
+	ret = v4l2_pipeline_link_notify(link, flags, notification);
+	if (ret)
+		return ret;
+
+	/* Only care about link enablement for CRU nodes. */
+	if (!(flags & MEDIA_LNK_FL_ENABLED))
+		return 0;
+
+	cru = container_of(link->graph_obj.mdev, struct rzg2l_cru_dev, mdev);
+	/*
+	 * Don't allow link changes if any entity in the graph is
+	 * streaming, modifying the CHSEL register fields can disrupt
+	 * running streams.
+	 */
+	media_device_for_each_entity(entity, &cru->mdev)
+		if (media_entity_is_streaming(entity))
+			return -EBUSY;
+
+	mutex_lock(&cru->mdev_lock);
+
+	csi_pad = media_pad_remote_pad_first(&cru->vdev.entity.pads[0]);
+	if (csi_pad) {
+		ret = -EMLINK;
+		goto out;
+	}
+
+	sd = media_entity_to_v4l2_subdev(link->source->entity);
+	if (cru->csi.subdev == sd) {
+		cru->csi.channel = link->source->index - 1;
+		cru->is_csi = true;
+	} else {
+		ret = -ENODEV;
+	}
+
+out:
+	mutex_unlock(&cru->mdev_lock);
+
+	return ret;
+}
+
+static const struct media_device_ops rzg2l_cru_media_ops = {
+	.link_notify = rzg2l_cru_csi2_link_notify,
+};
+
+/* -----------------------------------------------------------------------------
+ * Group async notifier
+ */
+
+static int rzg2l_cru_group_notify_complete(struct v4l2_async_notifier *notifier)
+{
+	struct rzg2l_cru_dev *cru = v4l2_dev_to_cru(notifier->v4l2_dev);
+	unsigned int i;
+	int ret;
+
+	ret = media_device_register(&cru->mdev);
+	if (ret)
+		return ret;
+
+	ret = v4l2_device_register_subdev_nodes(&cru->v4l2_dev);
+	if (ret) {
+		dev_err(cru->dev, "Failed to register subdev nodes\n");
+		return ret;
+	}
+
+	if (!video_is_registered(&cru->vdev)) {
+		ret = rzg2l_cru_v4l2_register(cru);
+		if (ret)
+			return ret;
+	}
+
+	/* Create all media device links between CRU and CSI-2's. */
+	for (i = 1; i <= CSI2_VCHANNEL; i++) {
+		struct media_entity *source, *sink;
+
+		source = &cru->csi.subdev->entity;
+		sink = &cru->vdev.entity;
+
+		ret = media_create_pad_link(source, i, sink, 0, 0);
+		if (ret) {
+			dev_err(cru->dev, "Error adding link from %s to %s\n",
+				source->name, sink->name);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static void rzg2l_cru_group_notify_unbind(struct v4l2_async_notifier *notifier,
+					  struct v4l2_subdev *subdev,
+					  struct v4l2_async_subdev *asd)
+{
+	struct rzg2l_cru_dev *cru = v4l2_dev_to_cru(notifier->v4l2_dev);
+
+	rzg2l_cru_v4l2_unregister(cru);
+
+	mutex_lock(&cru->mdev_lock);
+
+	if (cru->csi.asd == asd) {
+		cru->csi.subdev = NULL;
+		dev_dbg(cru->dev, "Unbind CSI-2 %s\n", subdev->name);
+	}
+
+	mutex_unlock(&cru->mdev_lock);
+
+	media_device_unregister(&cru->mdev);
+}
+
+static int rzg2l_cru_group_notify_bound(struct v4l2_async_notifier *notifier,
+					struct v4l2_subdev *subdev,
+					struct v4l2_async_subdev *asd)
+{
+	struct rzg2l_cru_dev *cru = v4l2_dev_to_cru(notifier->v4l2_dev);
+
+	mutex_lock(&cru->mdev_lock);
+
+	if (cru->csi.asd == asd) {
+		cru->csi.subdev = subdev;
+		dev_dbg(cru->dev, "Bound CSI-2 %s\n", subdev->name);
+	}
+
+	mutex_unlock(&cru->mdev_lock);
+
+	return 0;
+}
+
+static const struct v4l2_async_notifier_operations rzg2l_cru_async_ops = {
+	.bound = rzg2l_cru_group_notify_bound,
+	.unbind = rzg2l_cru_group_notify_unbind,
+	.complete = rzg2l_cru_group_notify_complete,
+};
+
+static int rvin_mc_parse_of(struct rzg2l_cru_dev *cru, unsigned int id)
+{
+	struct v4l2_fwnode_endpoint vep = {
+		.bus_type = V4L2_MBUS_CSI2_DPHY,
+	};
+	struct fwnode_handle *ep, *fwnode;
+	struct v4l2_async_subdev *asd;
+	int ret;
+
+	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(cru->dev), 1, id, 0);
+	if (!ep)
+		return 0;
+
+	fwnode = fwnode_graph_get_remote_endpoint(ep);
+	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
+	fwnode_handle_put(ep);
+	if (ret) {
+		dev_err(cru->dev, "Failed to parse %pOF\n", to_of_node(fwnode));
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!of_device_is_available(to_of_node(fwnode))) {
+		dev_dbg(cru->dev, "OF device %pOF disabled, ignoring\n",
+			to_of_node(fwnode));
+		ret = -ENOTCONN;
+		goto out;
+	}
+
+	asd = v4l2_async_nf_add_fwnode(&cru->notifier, fwnode,
+				       struct v4l2_async_subdev);
+	if (IS_ERR(asd)) {
+		ret = PTR_ERR(asd);
+		goto out;
+	}
+
+	cru->csi.asd = asd;
+
+	dev_dbg(cru->dev, "Added OF device %pOF to slot %u\n",
+		to_of_node(fwnode), vep.base.id);
+out:
+	fwnode_handle_put(fwnode);
+
+	return ret;
+}
+
+static int rzg2l_cru_mc_parse_of_graph(struct rzg2l_cru_dev *cru)
+{
+	int ret;
+
+	v4l2_async_nf_init(&cru->notifier);
+
+	ret = rvin_mc_parse_of(cru, 0);
+	if (ret)
+		return ret;
+
+	cru->notifier.ops = &rzg2l_cru_async_ops;
+
+	if (list_empty(&cru->notifier.asd_list))
+		return 0;
+
+	ret = v4l2_async_nf_register(&cru->v4l2_dev, &cru->notifier);
+	if (ret < 0) {
+		dev_err(cru->dev, "Notifier registration failed\n");
+		v4l2_async_nf_cleanup(&cru->notifier);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int rzg2l_cru_csi2_init(struct rzg2l_cru_dev *cru)
+{
+	struct media_device *mdev = NULL;
+	const struct of_device_id *match;
+	int ret;
+
+	cru->pad.flags = MEDIA_PAD_FL_SINK;
+	ret = media_entity_pads_init(&cru->vdev.entity, 1, &cru->pad);
+	if (ret)
+		return ret;
+
+	mutex_init(&cru->mdev_lock);
+	mdev = &cru->mdev;
+	mdev->dev = cru->dev;
+	mdev->ops = &rzg2l_cru_media_ops;
+
+	match = of_match_node(cru->dev->driver->of_match_table,
+			      cru->dev->of_node);
+
+	strscpy(mdev->driver_name, KBUILD_MODNAME, sizeof(mdev->driver_name));
+	strscpy(mdev->model, match->compatible, sizeof(mdev->model));
+	snprintf(mdev->bus_info, sizeof(mdev->bus_info), "platform:%s",
+		 dev_name(mdev->dev));
+
+	cru->v4l2_dev.mdev = &cru->mdev;
+
+	media_device_init(mdev);
+
+	ret = rzg2l_cru_mc_parse_of_graph(cru);
+	if (ret) {
+		mutex_lock(&cru->mdev_lock);
+		cru->v4l2_dev.mdev = NULL;
+		mutex_unlock(&cru->mdev_lock);
+	}
+
+	return 0;
+}
+
+static int rzg2l_cru_probe(struct platform_device *pdev)
+{
+	struct rzg2l_cru_dev *cru;
+	int irq, ret;
+
+	cru = devm_kzalloc(&pdev->dev, sizeof(*cru), GFP_KERNEL);
+	if (!cru)
+		return -ENOMEM;
+
+	cru->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(cru->base))
+		return PTR_ERR(cru->base);
+
+	cru->presetn = devm_reset_control_get(&pdev->dev, "presetn");
+	if (IS_ERR(cru->presetn))
+		return dev_err_probe(&pdev->dev, PTR_ERR(cru->presetn),
+				     "failed to get cpg presetn\n");
+
+	cru->aresetn = devm_reset_control_get(&pdev->dev, "aresetn");
+	if (IS_ERR(cru->aresetn))
+		return dev_err_probe(&pdev->dev, PTR_ERR(cru->aresetn),
+				     "failed to get cpg aresetn\n");
+
+	cru->dev = &pdev->dev;
+	cru->info = of_device_get_match_data(&pdev->dev);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	ret = rzg2l_cru_dma_register(cru, irq);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, cru);
+
+	ret = rzg2l_cru_csi2_init(cru);
+	if (ret)
+		goto error_dma_unregister;
+
+	cru->num_buf = HW_BUFFER_DEFAULT;
+	pm_suspend_ignore_children(&pdev->dev, true);
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+
+error_dma_unregister:
+	rzg2l_cru_dma_unregister(cru);
+
+	return ret;
+}
+
+static const struct of_device_id rzg2l_cru_of_id_table[] = {
+	{
+		.compatible = "renesas,rzg2l-cru",
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rzg2l_cru_of_id_table);
+
+static int rzg2l_cru_remove(struct platform_device *pdev)
+{
+	struct rzg2l_cru_dev *cru = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(&pdev->dev);
+
+	rzg2l_cru_v4l2_unregister(cru);
+
+	v4l2_async_nf_unregister(&cru->notifier);
+	v4l2_async_nf_cleanup(&cru->notifier);
+
+	media_device_cleanup(&cru->mdev);
+	mutex_destroy(&cru->mdev_lock);
+	cru->v4l2_dev.mdev = NULL;
+
+	rzg2l_cru_dma_unregister(cru);
+
+	return 0;
+}
+
+static struct platform_driver rzg2l_cru_driver = {
+	.driver = {
+		.name = "rzg2l-cru",
+		.of_match_table = rzg2l_cru_of_id_table,
+	},
+	.probe = rzg2l_cru_probe,
+	.remove = rzg2l_cru_remove,
+};
+
+module_platform_driver(rzg2l_cru_driver);
+
+MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L CRU driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
new file mode 100644
index 000000000000..ec8c2b1cecfb
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -0,0 +1,144 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Driver for Renesas RZ/G2L CRU
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ *
+ */
+
+#ifndef __RZG2L_CRU__
+#define __RZG2L_CRU__
+
+#include <linux/reset.h>
+
+#include <media/v4l2-async.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
+#include <media/videobuf2-v4l2.h>
+
+/* Number of HW buffers */
+#define HW_BUFFER_MAX		8
+#define HW_BUFFER_DEFAULT	3
+
+/* Address alignment mask for HW buffers */
+#define HW_BUFFER_MASK		0x1ff
+
+/* Maximum number of CSI2 virtual channels */
+#define CSI2_VCHANNEL		4
+
+#define CRU_MAX_INPUT_WIDTH	2800
+#define CRU_MAX_INPUT_HEIGHT	4095
+
+/**
+ * enum rzg2l_cru_dma_state - DMA states
+ * @RZG2L_CRU_DMA_STOPPED:   No operation in progress
+ * @RZG2L_CRU_DMA_STARTING:  Capture starting up
+ * @RZG2L_CRU_DMA_RUNNING:   Operation in progress have buffers
+ * @RZG2L_CRU_DMA_STOPPING:  Stopping operation
+ */
+enum rzg2l_cru_dma_state {
+	RZG2L_CRU_DMA_STOPPED = 0,
+	RZG2L_CRU_DMA_STARTING,
+	RZG2L_CRU_DMA_RUNNING,
+	RZG2L_CRU_DMA_STOPPING,
+};
+
+struct rzg2l_cru_csi {
+	struct v4l2_async_subdev *asd;
+	struct v4l2_subdev *subdev;
+	u32 channel;
+};
+
+/**
+ * struct rzg2l_cru_dev - Renesas CRU device structure
+ * @dev:		(OF) device
+ * @base:		device I/O register space remapped to virtual memory
+ * @info:		info about CRU instance
+ *
+ * @presetn:		CRU_PRESETN reset line
+ * @aresetn:		CRU_ARESETN reset line
+ *
+ * @vdev:		V4L2 video device associated with CRU
+ * @v4l2_dev:		V4L2 device
+ * @ctrl_handler:	V4L2 control handler
+ * @num_buf:		Holds the current number of buffers enabled
+ * @notifier:		V4L2 asynchronous subdevs notifier
+ *
+ * @csi:		CSI info
+ * @mdev:		media device
+ * @mdev_lock:		protects the count, notifier and csi members
+ * @pad:		media pad for the video device entity
+ *
+ * @lock:		protects @queue
+ * @queue:		vb2 buffers queue
+ * @scratch:		cpu address for scratch buffer
+ * @scratch_phys:	physical address of the scratch buffer
+ *
+ * @qlock:		protects @queue_buf, @buf_list, @sequence
+ *			@state
+ * @queue_buf:		Keeps track of buffers given to HW slot
+ * @buf_list:		list of queued buffers
+ * @sequence:		V4L2 buffers sequence number
+ * @state:		keeps track of operation state
+ *
+ * @is_csi:		flag to mark the CRU as using a CSI-2 subdevice
+ *
+ * @input_is_yuv:	flag to mark the input format of CRU
+ * @output_is_yuv:	flag to mark the output format of CRU
+ *
+ * @mbus_code:		media bus format code
+ * @format:		active V4L2 pixel format
+ *
+ * @compose:		active composing
+ */
+struct rzg2l_cru_dev {
+	struct device *dev;
+	void __iomem *base;
+	const struct rzg2l_cru_info *info;
+
+	struct reset_control *presetn;
+	struct reset_control *aresetn;
+
+	struct video_device vdev;
+	struct v4l2_device v4l2_dev;
+	u8 num_buf;
+
+	struct v4l2_async_notifier notifier;
+
+	struct rzg2l_cru_csi csi;
+	struct media_device mdev;
+	struct mutex mdev_lock;
+	struct media_pad pad;
+
+	struct mutex lock;
+	struct vb2_queue queue;
+	void *scratch;
+	dma_addr_t scratch_phys;
+
+	spinlock_t qlock;
+	struct vb2_v4l2_buffer *queue_buf[HW_BUFFER_MAX];
+	struct list_head buf_list;
+	unsigned int sequence;
+	enum rzg2l_cru_dma_state state;
+
+	bool is_csi;
+
+	bool input_is_yuv;
+	bool output_is_yuv;
+
+	u32 mbus_code;
+	struct v4l2_pix_format format;
+
+	struct v4l2_rect compose;
+};
+
+int rzg2l_cru_dma_register(struct rzg2l_cru_dev *cru, int irq);
+void rzg2l_cru_dma_unregister(struct rzg2l_cru_dev *cru);
+
+int rzg2l_cru_v4l2_register(struct rzg2l_cru_dev *cru);
+void rzg2l_cru_v4l2_unregister(struct rzg2l_cru_dev *cru);
+
+const struct v4l2_format_info *rzg2l_cru_format_from_pixel(u32 format);
+
+#endif
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-dma.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-dma.c
new file mode 100644
index 000000000000..2931d5087993
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-dma.c
@@ -0,0 +1,728 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Renesas RZ/G2L CRU
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ *
+ * Based on Renesas R-Car VIN
+ * Copyright (C) 2011-2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
+ * Copyright (C) 2008 Magnus Damm
+ */
+
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
+
+#include <media/videobuf2-dma-contig.h>
+
+#include "rzg2l-cru.h"
+#include "rzg2l-csi2.h"
+
+/* HW CRU Registers Definition */
+/* CRU Control Register */
+#define CRUnCTRL			0x0
+#define CRUnCTRL_VINSEL(x)		((x) << 0)
+
+/* CRU Interrupt Enable Register */
+#define CRUnIE				0x4
+#define CRUnIE_SFE			BIT(16)
+#define CRUnIE_EFE			BIT(17)
+
+/* CRU Interrupt Status Register */
+#define CRUnINTS			0x8
+#define CRUnINTS_SFS			BIT(16)
+
+/* CRU Reset Register */
+#define CRUnRST				0xc
+#define CRUnRST_VRESETN			BIT(0)
+
+/* Memory Bank Base Address (Lower) Register for CRU Image Data */
+#define AMnMBxADDRL(x)			(0x100 + ((x) * 8))
+
+/* Memory Bank Base Address (Higher) Register for CRU Image Data */
+#define AMnMBxADDRH(x)			(0x104 + ((x) * 8))
+
+/* Memory Bank Enable Register for CRU Image Data */
+#define AMnMBVALID			0x148
+#define AMnMBVALID_MBVALID(x)		GENMASK(x, 0)
+
+/* Memory Bank Status Register for CRU Image Data */
+#define AMnMBS				0x14c
+#define AMnMBS_MBSTS			0x7
+
+/* AXI Master FIFO Pointer Register for CRU Image Data */
+#define AMnFIFOPNTR			0x168
+#define AMnFIFOPNTR_FIFOWPNTR		GENMASK(7, 0)
+#define AMnFIFOPNTR_FIFORPNTR_Y		GENMASK(23, 16)
+
+/* AXI Master Transfer Stop Register for CRU Image Data */
+#define AMnAXISTP			0x174
+#define AMnAXISTP_AXI_STOP		BIT(0)
+
+/* AXI Master Transfer Stop Status Register for CRU Image Data */
+#define AMnAXISTPACK			0x178
+#define AMnAXISTPACK_AXI_STOP_ACK	BIT(0)
+
+/* CRU Image Processing Enable Register */
+#define ICnEN				0x200
+#define ICnEN_ICEN			BIT(0)
+
+/* CRU Image Processing Main Control Register */
+#define ICnMC				0x208
+#define ICnMC_CSCTHR			BIT(5)
+#define ICnMC_INF_YUV8_422		(0x1e << 16)
+#define ICnMC_INF_USER			(0x30 << 16)
+#define ICnMC_VCSEL(x)			((x) << 22)
+#define ICnMC_INF_MASK			GENMASK(21, 16)
+
+/* CRU Module Status Register */
+#define ICnMS				0x254
+#define ICnMS_IA			BIT(2)
+
+/* CRU Data Output Mode Register */
+#define ICnDMR				0x26c
+#define ICnDMR_YCMODE_UYVY		(1 << 4)
+
+#define RZG2L_TIMEOUT_MS		100
+#define RZG2L_RETRIES			10
+
+struct rzg2l_cru_buffer {
+	struct vb2_v4l2_buffer vb;
+	struct list_head list;
+};
+
+#define to_buf_list(vb2_buffer) (&container_of(vb2_buffer, \
+						struct rzg2l_cru_buffer, \
+						vb)->list)
+
+static void rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
+{
+	iowrite32(value, cru->base + offset);
+}
+
+static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
+{
+	return ioread32(cru->base + offset);
+}
+
+/* Need to hold qlock before calling */
+static void return_unused_buffers(struct rzg2l_cru_dev *cru,
+				  enum vb2_buffer_state state)
+{
+	struct rzg2l_cru_buffer *buf, *node;
+	unsigned long flags;
+	unsigned int i;
+
+	spin_lock_irqsave(&cru->qlock, flags);
+	for (i = 0; i < cru->num_buf; i++) {
+		if (cru->queue_buf[i]) {
+			vb2_buffer_done(&cru->queue_buf[i]->vb2_buf,
+					state);
+			cru->queue_buf[i] = NULL;
+		}
+	}
+
+	list_for_each_entry_safe(buf, node, &cru->buf_list, list) {
+		vb2_buffer_done(&buf->vb.vb2_buf, state);
+		list_del(&buf->list);
+	}
+	spin_unlock_irqrestore(&cru->qlock, flags);
+}
+
+static int rzg2l_cru_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
+				 unsigned int *nplanes, unsigned int sizes[],
+				 struct device *alloc_devs[])
+{
+	struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vq);
+
+	/* Make sure the image size is large enough. */
+	if (*nplanes)
+		return sizes[0] < cru->format.sizeimage ? -EINVAL : 0;
+
+	*nplanes = 1;
+	sizes[0] = cru->format.sizeimage;
+
+	return 0;
+};
+
+static int rzg2l_cru_buffer_prepare(struct vb2_buffer *vb)
+{
+	struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vb->vb2_queue);
+	unsigned long size = cru->format.sizeimage;
+
+	if (vb2_plane_size(vb, 0) < size) {
+		dev_err(cru->dev, "buffer too small (%lu < %lu)\n",
+			vb2_plane_size(vb, 0), size);
+		return -EINVAL;
+	}
+
+	vb2_set_plane_payload(vb, 0, size);
+
+	return 0;
+}
+
+static void rzg2l_cru_buffer_queue(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vb->vb2_queue);
+	unsigned long flags;
+
+	spin_lock_irqsave(&cru->qlock, flags);
+
+	list_add_tail(to_buf_list(vbuf), &cru->buf_list);
+
+	spin_unlock_irqrestore(&cru->qlock, flags);
+}
+
+static int rzg2l_cru_mc_validate_format(struct rzg2l_cru_dev *cru,
+					struct v4l2_subdev *sd,
+					struct media_pad *pad)
+{
+	struct v4l2_subdev_format fmt = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+
+	fmt.pad = pad->index;
+	if (v4l2_subdev_call_state_active(sd, pad, get_fmt, &fmt))
+		return -EPIPE;
+
+	if (cru->is_csi) {
+		switch (fmt.format.code) {
+		case MEDIA_BUS_FMT_UYVY8_1X16:
+			break;
+		default:
+			return -EPIPE;
+		}
+	}
+	cru->mbus_code = fmt.format.code;
+
+	switch (fmt.format.field) {
+	case V4L2_FIELD_TOP:
+	case V4L2_FIELD_BOTTOM:
+	case V4L2_FIELD_NONE:
+	case V4L2_FIELD_INTERLACED_TB:
+	case V4L2_FIELD_INTERLACED_BT:
+	case V4L2_FIELD_INTERLACED:
+	case V4L2_FIELD_SEQ_TB:
+	case V4L2_FIELD_SEQ_BT:
+		break;
+	default:
+		return -EPIPE;
+	}
+
+	if (fmt.format.width != cru->format.width ||
+	    fmt.format.height != cru->format.height ||
+	    fmt.format.code != cru->mbus_code)
+		return -EPIPE;
+
+	return 0;
+}
+
+static void rzg2l_cru_set_slot_addr(struct rzg2l_cru_dev *cru,
+				    int slot, dma_addr_t addr)
+{
+	const struct v4l2_format_info *fmt;
+	int offsetx, offsety;
+	dma_addr_t offset;
+
+	fmt = rzg2l_cru_format_from_pixel(cru->format.pixelformat);
+
+	/*
+	 * There is no HW support for composition do the best we can
+	 * by modifying the buffer offset
+	 */
+	offsetx = cru->compose.left * fmt->bpp[0];
+	offsety = cru->compose.top * cru->format.bytesperline;
+	offset = addr + offsetx + offsety;
+
+	/*
+	 * The address needs to be 512 bytes aligned. Driver should never accept
+	 * settings that do not satisfy this in the first place...
+	 */
+	if (WARN_ON((offsetx | offsety | offset) & HW_BUFFER_MASK))
+		return;
+
+	/* Currently, we just use the buffer in 32 bits address */
+	rzg2l_cru_write(cru, AMnMBxADDRL(slot), offset);
+	rzg2l_cru_write(cru, AMnMBxADDRH(slot), 0);
+}
+
+/*
+ * Moves a buffer from the queue to the HW slot. If no buffer is
+ * available use the scratch buffer. The scratch buffer is never
+ * returned to userspace, its only function is to enable the capture
+ * loop to keep running.
+ */
+static void rzg2l_cru_fill_hw_slot(struct rzg2l_cru_dev *cru, int slot)
+{
+	struct vb2_v4l2_buffer *vbuf;
+	struct rzg2l_cru_buffer *buf;
+	dma_addr_t phys_addr;
+
+	/* A already populated slot shall never be overwritten. */
+	if (WARN_ON(cru->queue_buf[slot]))
+		return;
+
+	dev_dbg(cru->dev, "Filling HW slot: %d\n", slot);
+
+	if (list_empty(&cru->buf_list)) {
+		cru->queue_buf[slot] = NULL;
+		phys_addr = cru->scratch_phys;
+	} else {
+		/* Keep track of buffer we give to HW */
+		buf = list_entry(cru->buf_list.next,
+				 struct rzg2l_cru_buffer, list);
+		vbuf = &buf->vb;
+		list_del_init(to_buf_list(vbuf));
+		cru->queue_buf[slot] = vbuf;
+
+		/* Setup DMA */
+		phys_addr = vb2_dma_contig_plane_dma_addr(&vbuf->vb2_buf, 0);
+	}
+
+	rzg2l_cru_set_slot_addr(cru, slot, phys_addr);
+}
+
+static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
+{
+	unsigned int slot;
+
+	/*
+	 * Set image data memory banks.
+	 * Currently, we will use maximum address.
+	 */
+	rzg2l_cru_write(cru, AMnMBVALID, AMnMBVALID_MBVALID(cru->num_buf - 1));
+
+	for (slot = 0; slot < cru->num_buf; slot++)
+		rzg2l_cru_fill_hw_slot(cru, slot);
+}
+
+static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru)
+{
+	u32 icnmc;
+
+	switch (cru->mbus_code) {
+	case MEDIA_BUS_FMT_UYVY8_1X16:
+		icnmc = ICnMC_INF_YUV8_422;
+		cru->input_is_yuv = true;
+		break;
+	default:
+		cru->input_is_yuv = false;
+		icnmc = ICnMC_INF_USER;
+		break;
+	}
+
+	icnmc |= (rzg2l_cru_read(cru, ICnMC) & ~ICnMC_INF_MASK);
+
+	/* Set virtual channel CSI2 */
+	icnmc |= ICnMC_VCSEL(cru->csi.channel);
+
+	rzg2l_cru_write(cru, ICnMC, icnmc);
+}
+
+static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru)
+{
+	u32 icndmr;
+
+	if (cru->is_csi)
+		rzg2l_cru_csi2_setup(cru);
+
+	/* Output format */
+	switch (cru->format.pixelformat) {
+	case V4L2_PIX_FMT_UYVY:
+		icndmr = ICnDMR_YCMODE_UYVY;
+		cru->output_is_yuv = true;
+		break;
+	default:
+		dev_err(cru->dev, "Invalid pixelformat (0x%x)\n",
+			cru->format.pixelformat);
+		return -EINVAL;
+	}
+
+	/* If input and output use same colorspace, do bypass mode */
+	if (cru->output_is_yuv == cru->input_is_yuv)
+		rzg2l_cru_write(cru, ICnMC,
+				rzg2l_cru_read(cru, ICnMC) | ICnMC_CSCTHR);
+	else
+		rzg2l_cru_write(cru, ICnMC,
+				rzg2l_cru_read(cru, ICnMC) & (~ICnMC_CSCTHR));
+
+	/* Set output data format */
+	rzg2l_cru_write(cru, ICnDMR, icndmr);
+
+	return 0;
+}
+
+static int rzg2l_cru_set_stream(struct rzg2l_cru_dev *cru, int on)
+{
+	struct rzg2l_csi2 *csi2 = sd_to_csi2(cru->csi.subdev);
+	struct media_pipeline *pipe;
+	struct v4l2_subdev *sd;
+	struct media_pad *pad;
+	unsigned long flags;
+	int ret;
+
+	pad = media_pad_remote_pad_first(&cru->pad);
+	if (!pad)
+		return -EPIPE;
+
+	sd = media_entity_to_v4l2_subdev(pad->entity);
+
+	if (!on) {
+		media_pipeline_stop(&cru->vdev.entity);
+		return v4l2_subdev_call(sd, video, s_stream, 0);
+	}
+
+	ret = rzg2l_cru_mc_validate_format(cru, sd, pad);
+	if (ret)
+		return ret;
+
+	rzg2l_csi2_dphy_setting(csi2, 1);
+
+	spin_lock_irqsave(&cru->qlock, flags);
+
+	/* Select a video input */
+	if (cru->is_csi)
+		rzg2l_cru_write(cru, CRUnCTRL, CRUnCTRL_VINSEL(0));
+
+	/* Cancel the software reset for image processing block */
+	rzg2l_cru_write(cru, CRUnRST, CRUnRST_VRESETN);
+
+	/* Disable and clear the interrupt before using */
+	rzg2l_cru_write(cru, CRUnIE, 0);
+	rzg2l_cru_write(cru, CRUnINTS, 0x001f000f);
+
+	/* Initialize the AXI master */
+	rzg2l_cru_initialize_axi(cru);
+
+	/* Initialize image convert */
+	ret = rzg2l_cru_initialize_image_conv(cru);
+	if (ret) {
+		spin_unlock_irqrestore(&cru->qlock, flags);
+		return ret;
+	}
+
+	/* Enable interrupt */
+	rzg2l_cru_write(cru, CRUnIE, CRUnIE_EFE);
+
+	/* Enable image processing reception */
+	rzg2l_cru_write(cru, ICnEN, ICnEN_ICEN);
+
+	spin_unlock_irqrestore(&cru->qlock, flags);
+
+	pipe = sd->entity.pipe ? sd->entity.pipe : &cru->vdev.pipe;
+	ret = media_pipeline_start(&cru->vdev.entity, pipe);
+	if (ret)
+		return ret;
+
+	rzg2l_csi2_mipi_link_setting(csi2, 1);
+
+	rzg2l_csi2_cmn_rstb_deassert(csi2);
+
+	ret = v4l2_subdev_call(sd, video, s_stream, 1);
+	if (ret == -ENOIOCTLCMD)
+		ret = 0;
+	if (ret)
+		media_pipeline_stop(&cru->vdev.entity);
+
+	return ret;
+}
+
+static void rzg2l_cru_stop_streaming(struct rzg2l_cru_dev *cru)
+{
+	struct rzg2l_csi2 *csi2 = sd_to_csi2(cru->csi.subdev);
+	u32 amnfifopntr, amnfifopntr_w, amnfifopntr_r_y;
+	unsigned int retries = 0;
+	unsigned long flags;
+	u32 icnms;
+
+	cru->state = RZG2L_CRU_DMA_STOPPING;
+
+	rzg2l_cru_set_stream(cru, 0);
+
+	rzg2l_csi2_dphy_setting(csi2, 0);
+
+	rzg2l_csi2_mipi_link_setting(csi2, 0);
+
+	spin_lock_irqsave(&cru->qlock, flags);
+
+	/* Disable and clear the interrupt */
+	rzg2l_cru_write(cru, CRUnIE, 0);
+	rzg2l_cru_write(cru, CRUnINTS, 0x001F0F0F);
+
+	/* Stop the operation of image conversion */
+	rzg2l_cru_write(cru, ICnEN, 0);
+
+	/* Wait for streaming to stop */
+	while ((rzg2l_cru_read(cru, ICnMS) & ICnMS_IA) && retries++ < RZG2L_RETRIES) {
+		spin_unlock_irqrestore(&cru->qlock, flags);
+		msleep(RZG2L_TIMEOUT_MS);
+		spin_lock_irqsave(&cru->qlock, flags);
+	}
+
+	icnms = rzg2l_cru_read(cru, ICnMS) & ICnMS_IA;
+	if (icnms)
+		dev_err(cru->dev, "Failed stop HW, something is seriously broken\n");
+
+	cru->state = RZG2L_CRU_DMA_STOPPED;
+
+	/* Wait until the FIFO becomes empty */
+	for (retries = 5; retries > 0; retries--) {
+		amnfifopntr = rzg2l_cru_read(cru, AMnFIFOPNTR);
+
+		amnfifopntr_w = amnfifopntr & AMnFIFOPNTR_FIFOWPNTR;
+		amnfifopntr_r_y =
+			(amnfifopntr & AMnFIFOPNTR_FIFORPNTR_Y) >> 16;
+		if (amnfifopntr_w == amnfifopntr_r_y)
+			break;
+
+		usleep_range(10, 20);
+	}
+
+	/* Notify that FIFO is not empty here */
+	if (!retries)
+		dev_err(cru->dev, "Failed to empty FIFO\n");
+
+	/* Stop AXI bus */
+	rzg2l_cru_write(cru, AMnAXISTP, AMnAXISTP_AXI_STOP);
+
+	/* Wait until the AXI bus stop */
+	for (retries = 5; retries > 0; retries--) {
+		if (rzg2l_cru_read(cru, AMnAXISTPACK) &
+			AMnAXISTPACK_AXI_STOP_ACK)
+			break;
+
+		usleep_range(10, 20);
+	};
+
+	/* Notify that AXI bus can not stop here */
+	if (!retries)
+		dev_err(cru->dev, "Failed to stop AXI bus\n");
+
+	/* Cancel the AXI bus stop request */
+	rzg2l_cru_write(cru, AMnAXISTP, 0);
+
+	/* Resets the image processing module */
+	rzg2l_cru_write(cru, CRUnRST, 0);
+
+	spin_unlock_irqrestore(&cru->qlock, flags);
+
+	/* Set reset state */
+	reset_control_assert(cru->aresetn);
+}
+
+static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count)
+{
+	struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vq);
+	int ret;
+
+	/* Release reset state */
+	ret = reset_control_deassert(cru->aresetn);
+	if (ret) {
+		dev_err(cru->dev, "failed to deassert aresetn\n");
+		return ret;
+	}
+
+	/* Allocate scratch buffer. */
+	cru->scratch = dma_alloc_coherent(cru->dev, cru->format.sizeimage,
+					  &cru->scratch_phys, GFP_KERNEL);
+	if (!cru->scratch) {
+		return_unused_buffers(cru, VB2_BUF_STATE_QUEUED);
+		dev_err(cru->dev, "Failed to allocate scratch buffer\n");
+		return -ENOMEM;
+	}
+
+	cru->sequence = 0;
+
+	ret = rzg2l_cru_set_stream(cru, 1);
+	if (ret) {
+		return_unused_buffers(cru, VB2_BUF_STATE_QUEUED);
+		goto out;
+	}
+
+	cru->state = RZG2L_CRU_DMA_STARTING;
+
+	dev_dbg(cru->dev, "Starting to capture\n");
+
+out:
+	if (ret)
+		dma_free_coherent(cru->dev, cru->format.sizeimage, cru->scratch,
+				  cru->scratch_phys);
+
+	return ret;
+}
+
+static void rzg2l_cru_stop_streaming_vq(struct vb2_queue *vq)
+{
+	struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vq);
+
+	rzg2l_cru_stop_streaming(cru);
+
+	/* Free scratch buffer */
+	dma_free_coherent(cru->dev, cru->format.sizeimage, cru->scratch,
+			  cru->scratch_phys);
+
+	return_unused_buffers(cru, VB2_BUF_STATE_ERROR);
+}
+
+static const struct vb2_ops rzg2l_cru_qops = {
+	.queue_setup		= rzg2l_cru_queue_setup,
+	.buf_prepare		= rzg2l_cru_buffer_prepare,
+	.buf_queue		= rzg2l_cru_buffer_queue,
+	.start_streaming	= rzg2l_cru_start_streaming_vq,
+	.stop_streaming		= rzg2l_cru_stop_streaming_vq,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
+};
+
+static irqreturn_t rzg2l_cru_irq(int irq, void *data)
+{
+	struct rzg2l_cru_dev *cru = data;
+	unsigned int handled = 0;
+	unsigned long flags;
+	u32 irq_status;
+	u32 amnmbs;
+	int slot;
+
+	spin_lock_irqsave(&cru->qlock, flags);
+
+	irq_status = rzg2l_cru_read(cru, CRUnINTS);
+	if (!irq_status)
+		goto done;
+
+	handled = 1;
+
+	rzg2l_cru_write(cru, CRUnINTS, rzg2l_cru_read(cru, CRUnINTS));
+
+	/* Nothing to do if capture status is 'RZG2L_CRU_DMA_STOPPED' */
+	if (cru->state == RZG2L_CRU_DMA_STOPPED) {
+		dev_dbg(cru->dev, "IRQ while state stopped\n");
+		goto done;
+	}
+
+	/* Increase stop retries if capture status is 'RZG2L_CRU_DMA_STOPPING' */
+	if (cru->state == RZG2L_CRU_DMA_STOPPING) {
+		if (irq_status & CRUnINTS_SFS)
+			dev_dbg(cru->dev, "IRQ while state stopping\n");
+		goto done;
+	}
+
+	/* Prepare for capture and update state */
+	amnmbs = rzg2l_cru_read(cru, AMnMBS);
+	slot = amnmbs & AMnMBS_MBSTS;
+
+	/*
+	 * AMnMBS.MBSTS indicates the destination of Memory Bank (MB).
+	 * Recalculate to get the current transfer complete MB.
+	 */
+	if (slot == 0)
+		slot = cru->num_buf - 1;
+	else
+		slot--;
+
+	/*
+	 * To hand buffers back in a known order to userspace start
+	 * to capture first from slot 0.
+	 */
+	if (cru->state == RZG2L_CRU_DMA_STARTING) {
+		if (slot != 0) {
+			dev_dbg(cru->dev, "Starting sync slot: %d\n", slot);
+			goto done;
+		}
+
+		dev_dbg(cru->dev, "Capture start synced!\n");
+		cru->state = RZG2L_CRU_DMA_RUNNING;
+	}
+
+	/* Capture frame */
+	if (cru->queue_buf[slot]) {
+		cru->queue_buf[slot]->field = cru->format.field;
+		cru->queue_buf[slot]->sequence = cru->sequence;
+		cru->queue_buf[slot]->vb2_buf.timestamp = ktime_get_ns();
+		vb2_buffer_done(&cru->queue_buf[slot]->vb2_buf,
+				VB2_BUF_STATE_DONE);
+		cru->queue_buf[slot] = NULL;
+	} else {
+		/* Scratch buffer was used, dropping frame. */
+		dev_dbg(cru->dev, "Dropping frame %u\n", cru->sequence);
+	}
+
+	cru->sequence++;
+
+	/* Prepare for next frame */
+	rzg2l_cru_fill_hw_slot(cru, slot);
+
+done:
+	spin_unlock_irqrestore(&cru->qlock, flags);
+
+	return IRQ_RETVAL(handled);
+}
+
+void rzg2l_cru_dma_unregister(struct rzg2l_cru_dev *cru)
+{
+	mutex_destroy(&cru->lock);
+
+	v4l2_device_unregister(&cru->v4l2_dev);
+	reset_control_assert(cru->presetn);
+}
+
+int rzg2l_cru_dma_register(struct rzg2l_cru_dev *cru, int irq)
+{
+	struct vb2_queue *q = &cru->queue;
+	unsigned int i;
+	int ret;
+
+	ret = reset_control_deassert(cru->presetn);
+	if (ret) {
+		dev_err(cru->dev, "failed to deassert presetn\n");
+		return ret;
+	}
+
+	/* Initialize the top-level structure */
+	ret = v4l2_device_register(cru->dev, &cru->v4l2_dev);
+	if (ret)
+		return ret;
+
+	mutex_init(&cru->lock);
+	INIT_LIST_HEAD(&cru->buf_list);
+
+	spin_lock_init(&cru->qlock);
+
+	cru->state = RZG2L_CRU_DMA_STOPPED;
+
+	for (i = 0; i < HW_BUFFER_MAX; i++)
+		cru->queue_buf[i] = NULL;
+
+	/* buffer queue */
+	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	q->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
+	q->lock = &cru->lock;
+	q->drv_priv = cru;
+	q->buf_struct_size = sizeof(struct rzg2l_cru_buffer);
+	q->ops = &rzg2l_cru_qops;
+	q->mem_ops = &vb2_dma_contig_memops;
+	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->min_buffers_needed = 4;
+	q->dev = cru->dev;
+
+	ret = vb2_queue_init(q);
+	if (ret < 0) {
+		dev_err(cru->dev, "failed to initialize VB2 queue\n");
+		goto error;
+	}
+
+	/* IRQ */
+	ret = devm_request_irq(cru->dev, irq, rzg2l_cru_irq, IRQF_SHARED,
+			       KBUILD_MODNAME, cru);
+	if (ret) {
+		dev_err(cru->dev, "failed to request irq\n");
+		goto error;
+	}
+
+	return 0;
+
+error:
+	rzg2l_cru_dma_unregister(cru);
+	return ret;
+}
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-v4l2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-v4l2.c
new file mode 100644
index 000000000000..428d15dbe194
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-v4l2.c
@@ -0,0 +1,354 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Renesas RZ/G2L CRU
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ *
+ * Based on Renesas R-Car VIN
+ * Copyright (C) 2016 Renesas Electronics Corp.
+ * Copyright (C) 2011-2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
+ * Copyright (C) 2008 Magnus Damm
+ */
+
+#include <linux/pm_runtime.h>
+
+#include <media/v4l2-event.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-rect.h>
+
+#include "rzg2l-cru.h"
+
+#define RZG2L_CRU_DEFAULT_FORMAT	V4L2_PIX_FMT_UYVY
+#define RZG2L_CRU_DEFAULT_WIDTH		800
+#define RZG2L_CRU_DEFAULT_HEIGHT	600
+#define RZG2L_CRU_DEFAULT_FIELD		V4L2_FIELD_NONE
+#define RZG2L_CRU_DEFAULT_COLORSPACE	V4L2_COLORSPACE_SRGB
+
+/* -----------------------------------------------------------------------------
+ * Format Conversions
+ */
+
+static const struct v4l2_format_info rzg2l_cru_formats[] = {
+	{
+		.format = V4L2_PIX_FMT_UYVY,
+		.bpp[0] = 2,
+	},
+};
+
+const struct v4l2_format_info *rzg2l_cru_format_from_pixel(u32 format)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(rzg2l_cru_formats); i++)
+		if (rzg2l_cru_formats[i].format == format)
+			return rzg2l_cru_formats + i;
+
+	return NULL;
+}
+
+static u32 rzg2l_cru_format_bytesperline(struct v4l2_pix_format *pix)
+{
+	const struct v4l2_format_info *fmt;
+
+	fmt = rzg2l_cru_format_from_pixel(pix->pixelformat);
+
+	if (WARN_ON(!fmt))
+		return -EINVAL;
+
+	return pix->width * fmt->bpp[0];
+}
+
+static u32 rzg2l_cru_format_sizeimage(struct v4l2_pix_format *pix)
+{
+	return pix->bytesperline * pix->height;
+}
+
+static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
+				   struct v4l2_pix_format *pix)
+{
+	if (!rzg2l_cru_format_from_pixel(pix->pixelformat))
+		pix->pixelformat = RZG2L_CRU_DEFAULT_FORMAT;
+
+	switch (pix->field) {
+	case V4L2_FIELD_TOP:
+	case V4L2_FIELD_BOTTOM:
+	case V4L2_FIELD_NONE:
+	case V4L2_FIELD_INTERLACED_TB:
+	case V4L2_FIELD_INTERLACED_BT:
+	case V4L2_FIELD_INTERLACED:
+		break;
+	default:
+		pix->field = RZG2L_CRU_DEFAULT_FIELD;
+		break;
+	}
+
+	/* Limit to CRU capabilities */
+	v4l_bound_align_image(&pix->width, 320, CRU_MAX_INPUT_WIDTH, 1,
+			      &pix->height, 240, CRU_MAX_INPUT_HEIGHT, 2, 0);
+
+	pix->bytesperline = rzg2l_cru_format_bytesperline(pix);
+	pix->sizeimage = rzg2l_cru_format_sizeimage(pix);
+
+	dev_dbg(cru->dev, "Format %ux%u bpl: %u size: %u\n",
+		pix->width, pix->height, pix->bytesperline, pix->sizeimage);
+}
+
+static void rzg2l_cru_try_format(struct rzg2l_cru_dev *cru,
+				 struct v4l2_pix_format *pix)
+{
+	/*
+	 * The V4L2 specification clearly documents the colorspace fields
+	 * as being set by drivers for capture devices. Using the values
+	 * supplied by userspace thus wouldn't comply with the API. Until
+	 * the API is updated force fixed values.
+	 */
+	pix->colorspace = RZG2L_CRU_DEFAULT_COLORSPACE;
+	pix->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(pix->colorspace);
+	pix->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(pix->colorspace);
+	pix->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, pix->colorspace,
+							  pix->ycbcr_enc);
+
+	rzg2l_cru_format_align(cru, pix);
+}
+
+static int rzg2l_cru_querycap(struct file *file, void *priv,
+			      struct v4l2_capability *cap)
+{
+	struct rzg2l_cru_dev *cru = video_drvdata(file);
+
+	strscpy(cap->driver, KBUILD_MODNAME, sizeof(cap->driver));
+	strscpy(cap->card, "RZG2L_CRU", sizeof(cap->card));
+	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
+		 dev_name(cru->dev));
+	return 0;
+}
+
+static int rzg2l_cru_try_fmt_vid_cap(struct file *file, void *priv,
+				     struct v4l2_format *f)
+{
+	struct rzg2l_cru_dev *cru = video_drvdata(file);
+
+	rzg2l_cru_try_format(cru, &f->fmt.pix);
+
+	return 0;
+}
+
+static int rzg2l_cru_s_fmt_vid_cap(struct file *file, void *priv,
+				   struct v4l2_format *f)
+{
+	struct rzg2l_cru_dev *cru = video_drvdata(file);
+
+	if (vb2_is_busy(&cru->queue))
+		return -EBUSY;
+
+	rzg2l_cru_try_format(cru, &f->fmt.pix);
+
+	cru->format = f->fmt.pix;
+
+	cru->compose.top = 0;
+	cru->compose.left = 0;
+	cru->compose.width = cru->format.width;
+	cru->compose.height = cru->format.height;
+
+	return 0;
+}
+
+static int rzg2l_cru_g_fmt_vid_cap(struct file *file, void *priv,
+				   struct v4l2_format *f)
+{
+	struct rzg2l_cru_dev *cru = video_drvdata(file);
+
+	f->fmt.pix = cru->format;
+
+	return 0;
+}
+
+static int rzg2l_cru_enum_fmt_vid_cap(struct file *file, void *priv,
+				      struct v4l2_fmtdesc *f)
+{
+	if (f->index >= ARRAY_SIZE(rzg2l_cru_formats))
+		return -EINVAL;
+
+	f->pixelformat = rzg2l_cru_formats[f->index].format;
+
+	return 0;
+}
+
+static int rzg2l_cru_subscribe_event(struct v4l2_fh *fh,
+				     const struct v4l2_event_subscription *sub)
+{
+	switch (sub->type) {
+	case V4L2_EVENT_SOURCE_CHANGE:
+		return v4l2_event_subscribe(fh, sub, 4, NULL);
+	}
+	return v4l2_ctrl_subscribe_event(fh, sub);
+}
+
+static const struct v4l2_ioctl_ops rzg2l_cru_ioctl_ops = {
+	.vidioc_querycap		= rzg2l_cru_querycap,
+	.vidioc_try_fmt_vid_cap		= rzg2l_cru_try_fmt_vid_cap,
+	.vidioc_g_fmt_vid_cap		= rzg2l_cru_g_fmt_vid_cap,
+	.vidioc_s_fmt_vid_cap		= rzg2l_cru_s_fmt_vid_cap,
+	.vidioc_enum_fmt_vid_cap	= rzg2l_cru_enum_fmt_vid_cap,
+
+	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
+	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
+	.vidioc_querybuf		= vb2_ioctl_querybuf,
+	.vidioc_qbuf			= vb2_ioctl_qbuf,
+	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
+	.vidioc_expbuf			= vb2_ioctl_expbuf,
+	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
+	.vidioc_streamon		= vb2_ioctl_streamon,
+	.vidioc_streamoff		= vb2_ioctl_streamoff,
+
+	.vidioc_log_status		= v4l2_ctrl_log_status,
+	.vidioc_subscribe_event		= rzg2l_cru_subscribe_event,
+	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
+};
+
+/* -----------------------------------------------------------------------------
+ * Media controller file operations
+ */
+
+static int rzg2l_cru_open(struct file *file)
+{
+	struct rzg2l_cru_dev *cru = video_drvdata(file);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(cru->dev);
+	if (ret < 0)
+		return ret;
+
+	ret = mutex_lock_interruptible(&cru->lock);
+	if (ret)
+		goto err_pm;
+
+	file->private_data = cru;
+	ret = v4l2_fh_open(file);
+	if (ret)
+		goto err_unlock;
+
+	ret = v4l2_pipeline_pm_get(&cru->vdev.entity);
+	if (ret < 0)
+		goto err_open;
+
+	mutex_unlock(&cru->lock);
+
+	return 0;
+err_open:
+	v4l2_fh_release(file);
+err_unlock:
+	mutex_unlock(&cru->lock);
+err_pm:
+	pm_runtime_put(cru->dev);
+
+	return ret;
+}
+
+static int rzg2l_cru_release(struct file *file)
+{
+	struct rzg2l_cru_dev *cru = video_drvdata(file);
+	int ret;
+
+	mutex_lock(&cru->lock);
+
+	/* the release helper will cleanup any on-going streaming. */
+	ret = _vb2_fop_release(file, NULL);
+
+	v4l2_pipeline_pm_put(&cru->vdev.entity);
+	pm_runtime_put(cru->dev);
+
+	mutex_unlock(&cru->lock);
+
+	return ret;
+}
+
+static const struct v4l2_file_operations rzg2l_cru_fops = {
+	.owner		= THIS_MODULE,
+	.unlocked_ioctl	= video_ioctl2,
+	.open		= rzg2l_cru_open,
+	.release	= rzg2l_cru_release,
+	.poll		= vb2_fop_poll,
+	.mmap		= vb2_fop_mmap,
+	.read		= vb2_fop_read,
+};
+
+void rzg2l_cru_v4l2_unregister(struct rzg2l_cru_dev *cru)
+{
+	if (!video_is_registered(&cru->vdev))
+		return;
+
+	v4l2_info(&cru->v4l2_dev, "Removed %s\n",
+		  video_device_node_name(&cru->vdev));
+
+	/* Checks internally if vdev have been init or not */
+	video_unregister_device(&cru->vdev);
+}
+
+static void rzg2l_cru_notify(struct v4l2_subdev *sd,
+			     unsigned int notification, void *arg)
+{
+	struct rzg2l_cru_dev *cru =
+		container_of(sd->v4l2_dev, struct rzg2l_cru_dev, v4l2_dev);
+	struct v4l2_subdev *remote;
+	struct media_pad *pad;
+
+	pad = media_pad_remote_pad_first(&cru->pad);
+	if (!pad)
+		return;
+
+	remote = media_entity_to_v4l2_subdev(pad->entity);
+	if (remote != sd)
+		return;
+
+	switch (notification) {
+	case V4L2_DEVICE_NOTIFY_EVENT:
+		v4l2_event_queue(&cru->vdev, arg);
+		break;
+	}
+}
+
+int rzg2l_cru_v4l2_register(struct rzg2l_cru_dev *cru)
+{
+	struct video_device *vdev = &cru->vdev;
+	int ret;
+
+	cru->v4l2_dev.notify = rzg2l_cru_notify;
+
+	/* video node */
+	vdev->v4l2_dev = &cru->v4l2_dev;
+	vdev->queue = &cru->queue;
+	snprintf(vdev->name, sizeof(vdev->name), "CRU output");
+	vdev->release = video_device_release_empty;
+	vdev->lock = &cru->lock;
+	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
+			    V4L2_CAP_READWRITE;
+
+	/* Set a default format */
+	cru->format.pixelformat	= RZG2L_CRU_DEFAULT_FORMAT;
+	cru->format.width = RZG2L_CRU_DEFAULT_WIDTH;
+	cru->format.height = RZG2L_CRU_DEFAULT_HEIGHT;
+	cru->format.field = RZG2L_CRU_DEFAULT_FIELD;
+	cru->format.colorspace = RZG2L_CRU_DEFAULT_COLORSPACE;
+
+	vdev->device_caps |= V4L2_CAP_IO_MC;
+	vdev->fops = &rzg2l_cru_fops;
+	vdev->ioctl_ops = &rzg2l_cru_ioctl_ops;
+
+	rzg2l_cru_format_align(cru, &cru->format);
+
+	ret = video_register_device(&cru->vdev, VFL_TYPE_VIDEO, -1);
+	if (ret) {
+		dev_err(cru->dev, "Failed to register video device\n");
+		return ret;
+	}
+
+	video_set_drvdata(&cru->vdev, cru);
+
+	v4l2_info(&cru->v4l2_dev, "Device registered as %s\n",
+		  video_device_node_name(&cru->vdev));
+
+	return ret;
+}
-- 
2.25.1

Re: [PATCH 0/4] Add driver for CSI2 and CRU modules found on Renesas RZ/G2L SoC

[Lad, Prabhakar]

On Mon, Aug 1, 2022 at 10:47 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> Hi All,
>
> This patch series aims to add driver support to CRU module found
> on Renesas RZ/G2L SoC.
>
> The Camera Data Receiving Unit (CRU) consists of a MIPI CSI-2
> block and an Image Processing block. The Image Processing block
> can receive video data received from the external Digital Parallel
> Interface or MIPI CSI-2 block, and perform appropriate image
> processing for each.
>
> More details:
> * https://renesas.info/wiki/File:CRU.png
> * https://www.renesas.com/document/mah/rzg2l-group-rzg2lc-group-users-manual-hardware-0?language=en&r=1467981
>
> Currently the driver has been tested using yavta and Gstreamer
> on RZ/G2L SMARC EVK using the ov5645 sensor on CSI2 interface
> only.
>
> RFC v2: https://patchwork.kernel.org/project/linux-renesas-soc/cover/20220121010543.31385-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
>
> RFC v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/20211207012351.15754-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
>
> Cheers,
> Prabhakar
>
> Lad Prabhakar (4):
>   media: dt-bindings: media: Document RZ/G2L CSI-2 block
>   media: dt-bindings: media: Document RZ/G2L CRU
>   media: platform: Add Renesas RZ/G2L MIPI CSI-2 receiver driver
>   media: platform: Add Renesas RZ/G2L CRU driver
>
>  .../bindings/media/renesas,rzg2l-cru.yaml     | 142 ++++
>  .../bindings/media/renesas,rzg2l-csi2.yaml    | 149 ++++
>  drivers/media/platform/renesas/Kconfig        |   1 +
>  drivers/media/platform/renesas/Makefile       |   1 +
>  .../media/platform/renesas/rzg2l-cru/Kconfig  |  34 +
>  .../media/platform/renesas/rzg2l-cru/Makefile |   6 +
>  .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 371 +++++++++
>  .../platform/renesas/rzg2l-cru/rzg2l-cru.h    | 144 ++++
>  .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 702 +++++++++++++++++
>  .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  49 ++
>  .../platform/renesas/rzg2l-cru/rzg2l-dma.c    | 728 ++++++++++++++++++
>  .../platform/renesas/rzg2l-cru/rzg2l-v4l2.c   | 354 +++++++++
>  12 files changed, 2681 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-dma.c
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-v4l2.c
>
Gentle ping, request to review the driver code.

Cheers,
Prabhakar