[PATCH v4 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU)

[PATCH v4 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU)

[Tommaso Merciai]

Dear All,

In preparation of supporting the CRU/CSI2 IPs found into the Renesas RZ/G3E
SoC, this series adds driver/dt-bindings support.
This adds also some minor fixes into rzg2l-csi2 and rzg2l-core drivers.

The series was tested in an out of tree branch with the following hw pipeline:

ov5645 image sensor (Coral Camera) -> rzg3e CSI2 -> rzg3e CRU
imx219 image sensor (Pi PiNoir Camera Module V2.1) -> rzg3e CSI2 -> rzg3e CRU

base commit: c0eb65494e59 (tag: next-20250228, linux-next/master)

------
Some logs:

root@smarc-rzg3e:~# media-ctl -p
Media controller API version 6.14.0

Media device information
------------------------
driver          rzg2l_cru
model           renesas,r9a09g047-cru
serial
bus info        platform:16000000.video
hw revision     0x0
driver version  6.14.0

Device topology
- entity 1: csi-16000400.csi2 (2 pads, 2 links, 0 routes)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev0
        pad0: Sink
                [stream:0 fmt:UYVY8_1X16/320x240 field:none colorspace:srgb]
                <- "ov5645 0-003c":0 [ENABLED,IMMUTABLE]
        pad1: Source
                [stream:0 fmt:UYVY8_1X16/320x240 field:none colorspace:srgb]
                -> "cru-ip-16000000.video":0 [ENABLED,IMMUTABLE]

- entity 4: ov5645 0-003c (1 pad, 1 link, 0 routes)
            type V4L2 subdev subtype Sensor flags 0
            device node name /dev/v4l-subdev1
        pad0: Source
                [stream:0 fmt:UYVY8_1X16/1920x1080 field:none colorspace:srgb
                 crop:(0,0)/1920x1080]
                -> "csi-16000400.csi2":0 [ENABLED,IMMUTABLE]

- entity 8: cru-ip-16000000.video (2 pads, 2 links, 0 routes)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev2
        pad0: Sink
                [stream:0 fmt:UYVY8_1X16/320x240 field:none colorspace:srgb]
                <- "csi-16000400.csi2":1 [ENABLED,IMMUTABLE]
        pad1: Source
                [stream:0 fmt:UYVY8_1X16/320x240 field:none colorspace:srgb]
                -> "CRU output":0 [ENABLED,IMMUTABLE]

- entity 17: CRU output (1 pad, 1 link)
             type Node subtype V4L flags 0
             device node name /dev/video0
        pad0: Sink
                <- "cru-ip-16000000.video":1 [ENABLED,IMMUTABLE]

root@smarc-rzg3e:~# v4l2-compliance -d /dev/v4l-subdev0
v4l2-compliance 1.26.1-5142, 64 bits, 64-bit time_t
v4l2-compliance SHA: 4aee01a02792 2023-12-12 21:40:38

Compliance test for device /dev/v4l-subdev0:

Driver Info:
        Driver version   : 6.14.0
        Capabil[  101.574758] csi-16000400.csi2: =================  START STATUS  =================
ities     : 0x00[  101.583166] csi-16000400.csi2: ==================  END STATUS  ==================
000000

Required ioctls:
        test VIDIOC_SUDBEV_QUERYCAP: OK
        test invalid ioctls: OK

Allow for multiple opens:
        test second /dev/v4l-subdev0 open: OK
        test VIDIOC_SUBDEV_QUERYCAP: OK
        test for unlimited opens: OK

Debug ioctls:
        test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
        test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
        test VIDIOC_ENUMAUDIO: OK (Not Supported)
        test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDIO: OK (Not Supported)
        Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
        test VIDIOC_G/S_MODULATOR: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_ENUMAUDOUT: OK (Not Supported)
        test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDOUT: OK (Not Supported)
        Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
        test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
        test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
        test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
        test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
        test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
        test VIDIOC_QUERYCTRL: OK (Not Supported)
        test VIDIOC_G/S_CTRL: OK (Not Supported)
        test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
        test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
        Standard Controls: 0 Private Controls: 0

Format ioctls:
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
        test VIDIOC_G/S_PARM: OK (Not Supported)
        test VIDIOC_G_FBUF: OK (Not Supported)
        test VIDIOC_G_FMT: OK (Not Supported)
        test VIDIOC_TRY_FMT: OK (Not Supported)
        test VIDIOC_S_FMT: OK (Not Supported)
        test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
        test Cropping: OK (Not Supported)
        test Composing: OK (Not Supported)
        test Scaling: OK (Not Supported)

Codec ioctls:
        test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
        test VIDIOC_G_ENC_INDEX: OK (Not Supported)
        test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
        test CREATE_BUFS maximum buffers: OK
        test VIDIOC_EXPBUF: OK (Not Supported)
        test Requests: OK (Not Supported)

Total for device /dev/v4l-subdev0: 44, Succeeded: 44, Failed: 0, Warnings: 0

root@smarc-rzg3e:~# v4l2-compliance -d /dev/v4l-subdev1
v4l2-compliance 1.26.1-5142, 64 [  125.542264] ov5645 0-003c: =================  START STATUS  =================
bits, 64-bit tim[  125.550585] ov5645 0-003c: ==================  END STATUS  ==================
e_t
v4l2-compliance SHA: 4aee01a02792 2023-12-12 21:40:38

Compliance test for device /dev/v4l-subdev1:

Driver Info:
        Driver version   : 6.14.0
        Capabilities     : 0x00000000

Required ioctls:
        test VIDIOC_SUDBEV_QUERYCAP: OK
        test invalid ioctls: OK

Allow for multiple opens:
        test second /dev/v4l-subdev1 open: OK
        test VIDIOC_SUBDEV_QUERYCAP: OK
        test for unlimited opens: OK

Debug ioctls:
        test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
        test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
        test VIDIOC_ENUMAUDIO: OK (Not Supported)
        test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDIO: OK (Not Supported)
        Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
        test VIDIOC_G/S_MODULATOR: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_ENUMAUDOUT: OK (Not Supported)
        test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDOUT: OK (Not Supported)
        Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
        test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
        test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
        test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
        test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
        test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
        test VIDIOC_QUERYCTRL: OK
        test VIDIOC_G/S_CTRL: OK
        test VIDIOC_G/S/TRY_EXT_CTRLS: OK
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
        test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
        Standard Controls: 12 Private Controls: 0

Format ioctls:
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
        test VIDIOC_G/S_PARM: OK (Not Supported)
        test VIDIOC_G_FBUF: OK (Not Supported)
        test VIDIOC_G_FMT: OK (Not Supported)
        test VIDIOC_TRY_FMT: OK (Not Supported)
        test VIDIOC_S_FMT: OK (Not Supported)
        test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
        test Cropping: OK (Not Supported)
        test Composing: OK (Not Supported)
        test Scaling: OK (Not Supported)

Codec ioctls:
        test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
        test VIDIOC_G_ENC_INDEX: OK (Not Supported)
        test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
        test CREATE_BUFS maximum buffers: OK
        test VIDIOC_EXPBUF: OK (Not Supported)
        test Requests: OK (Not Supported)

Total for device /dev/v4l-subdev1: 44, Succeeded: 44, Failed: 0, Warnings: 0

root@smarc-rzg3e:~# v4l2-compliance -d /dev/v4l-subdev2
v4l2-compliance 1.26.1-5142, 64 [  139.054132] cru-ip-16000000.video: =================  START STATUS  =================
bits, 64-bit tim[  139.062922] cru-ip-16000000.video: ==================  END STATUS  ==================
e_t
v4l2-compliance SHA: 4aee01a02792 2023-12-12 21:40:38

Compliance test for rzg2l_cru device /dev/v4l-subdev2:

Driver Info:
        Driver version   : 6.14.0
        Capabilities     : 0x00000000
Media Driver Info:
        Driver name      : rzg2l_cru
        Model            : renesas,r9a09g047-cru
        Serial           :
        Bus info         : platform:16000000.video
        Media version    : 6.14.0
        Hardware revision: 0x00000000 (0)
        Driver version   : 6.14.0
Interface Info:
        ID               : 0x0300000f
        Type             : V4L Sub-Device
Entity Info:
        ID               : 0x00000008 (8)
        Name             : cru-ip-16000000.video
        Function         : Video Pixel Formatter
        Pad 0x01000009   : 0: Sink, Must Connect
          Link 0x02000015: from remote pad 0x1000003 of entity 'csi-16000400.csi2' (Video Interface Bridge): Data, Enabled, Immutable
        Pad 0x0100000a   : 1: Source, Must Connect
          Link 0x02000017: to remote pad 0x1000012 of entity 'CRU output' (V4L2 I/O): Data, Enabled, Immutable

Required ioctls:
        test MC information (see 'Media Driver Info' above): OK
        test VIDIOC_SUDBEV_QUERYCAP: OK
        test invalid ioctls: OK

Allow for multiple opens:
        test second /dev/v4l-subdev2 open: OK
        test VIDIOC_SUBDEV_QUERYCAP: OK
        test for unlimited opens: OK

Debug ioctls:
        test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
        test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
        test VIDIOC_ENUMAUDIO: OK (Not Supported)
        test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDIO: OK (Not Supported)
        Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
        test VIDIOC_G/S_MODULATOR: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_ENUMAUDOUT: OK (Not Supported)
        test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDOUT: OK (Not Supported)
        Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
        test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
        test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
        test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
        test VIDIOC_G/S_EDID: OK (Not Supported)

Sub-Device ioctls (Sink Pad 0):
        Try Stream 0
        test Try VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
        test Try VIDIOC_SUBDEV_G/S_FMT: OK
        test Try VIDIOC_SUBDEV_G/S_SELECTION/CROP: OK (Not Supported)
        Active Stream 0
        test Active VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
        test Active VIDIOC_SUBDEV_G/S_FMT: OK
        test Active VIDIOC_SUBDEV_G/S_SELECTION/CROP: OK (Not Supported)
        test VIDIOC_SUBDEV_G/S_FRAME_INTERVAL: OK (Not Supported)

Sub-Device ioctls (Source Pad 1):
        Try Stream 0
        test Try VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
        test Try VIDIOC_SUBDEV_G/S_FMT: OK
        test Try VIDIOC_SUBDEV_G/S_SELECTION/CROP: OK (Not Supported)
        Active Stream 0
        test Active VIDIOC_SUBDEV_ENUM_MBUS_CODE/FRAME_SIZE/FRAME_INTERVAL: OK
        test Active VIDIOC_SUBDEV_G/S_FMT: OK
        test Active VIDIOC_SUBDEV_G/S_SELECTION/CROP: OK (Not Supported)
        test VIDIOC_SUBDEV_G/S_FRAME_INTERVAL: OK (Not Supported)

Control ioctls:
        test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
        test VIDIOC_QUERYCTRL: OK (Not Supported)
        test VIDIOC_G/S_CTRL: OK (Not Supported)
        test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
        test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
        Standard Controls: 0 Private Controls: 0

Format ioctls:
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
        test VIDIOC_G/S_PARM: OK (Not Supported)
        test VIDIOC_G_FBUF: OK (Not Supported)
        test VIDIOC_G_FMT: OK (Not Supported)
        test VIDIOC_TRY_FMT: OK (Not Supported)
        test VIDIOC_S_FMT: OK (Not Supported)
        test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
        test Cropping: OK (Not Supported)
        test Composing: OK (Not Supported)
        test Scaling: OK (Not Supported)

Codec ioctls:
        test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
        test VIDIOC_G_ENC_INDEX: OK (Not Supported)
        test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
        test CREATE_BUFS maximum buffers: OK
        test VIDIOC_EXPBUF: OK (Not Supported)
        test Requests: OK (Not Supported)

Total for rzg2l_cru device /dev/v4l-subdev2: 59, Succeeded: 59, Failed: 0, Warnings: 0

Thanks & Regards,
Tommaso

Lad Prabhakar (12):
  media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
  media: rzg2l-cru: csi2: Use local variable for struct device in
    rzg2l_csi2_probe()
  media: rzg2l-cru: rzg2l-core: Use local variable for struct device in
    rzg2l_cru_probe()
  media: rzg2l-cru: csi2: Introduce SoC-specific D-PHY handling
  media: rzg2l-cru: csi2: Add support for RZ/V2H(P) SoC
  media: rzg2l-cru: Add register mapping support
  media: rzg2l-cru: Pass resolution limits via OF data
  media: rzg2l-cru: Add image_conv offset to OF data
  media: rzg2l-cru: Add IRQ handler to OF data
  media: rzg2l-cru: Add function pointer to check if FIFO is empty
  media: rzg2l-cru: Add function pointer to configure CSI
  media: rzg2l-cru: Add support for RZ/G3E SoC

Tommaso Merciai (5):
  media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2
    block
  media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC
  media: rzg2l-cru: csi2: Use devm_pm_runtime_enable()
  media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable()
  media: rzg2l-cru: csi2: Skip system clock for RZ/V2H(P) SoC

 .../bindings/media/renesas,rzg2l-cru.yaml     |  65 +++-
 .../bindings/media/renesas,rzg2l-csi2.yaml    |  62 +++-
 .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 148 ++++++++-
 .../renesas/rzg2l-cru/rzg2l-cru-regs.h        |  91 ++++--
 .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  39 ++-
 .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 165 ++++++++--
 .../platform/renesas/rzg2l-cru/rzg2l-ip.c     |  13 +-
 .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 298 ++++++++++++++++--
 8 files changed, 749 insertions(+), 132 deletions(-)

-- 
2.43.0

[PATCH v4 01/17] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The MIPI CSI-2 block on the Renesas RZ/V2H(P) SoC is similar to the one
found on the Renesas RZ/G2L SoC, with the following differences:
- A different D-PHY
- Additional registers for the MIPI CSI-2 link
- Only two clocks

Add a new compatible string, `renesas,r9a09g057-csi2`, for the RZ/V2H(P)
SoC.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
 - Dropped empty line as suggested by LPinchart
 - Fixed minItems into else conditional block as suggested by RHerring

Changes since v2:
 - Collected tags
 - Fixed CRU_CMN_RSTB description as suggested by LPinchart

Changes since v3:
 - Fixed CRU_CMN_RSTB description as suggested by GUytterhoeven

 .../bindings/media/renesas,rzg2l-csi2.yaml    | 59 ++++++++++++++-----
 1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
index 7faa12fecd5b..1f9ee37584b3 100644
--- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
@@ -17,12 +17,14 @@ description:
 
 properties:
   compatible:
-    items:
-      - enum:
-          - renesas,r9a07g043-csi2       # RZ/G2UL
-          - renesas,r9a07g044-csi2       # RZ/G2{L,LC}
-          - renesas,r9a07g054-csi2       # RZ/V2L
-      - const: renesas,rzg2l-csi2
+    oneOf:
+      - items:
+          - enum:
+              - renesas,r9a07g043-csi2 # RZ/G2UL
+              - renesas,r9a07g044-csi2 # RZ/G2{L,LC}
+              - renesas,r9a07g054-csi2 # RZ/V2L
+          - const: renesas,rzg2l-csi2
+      - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
 
   reg:
     maxItems: 1
@@ -31,16 +33,24 @@ properties:
     maxItems: 1
 
   clocks:
-    items:
-      - description: Internal clock for connecting CRU and MIPI
-      - description: CRU Main clock
-      - description: CRU Register access clock
+    oneOf:
+      - items:
+          - description: Internal clock for connecting CRU and MIPI
+          - description: CRU Main clock
+          - description: CRU Register access clock
+      - items:
+          - description: CRU Main clock
+          - description: CRU Register access clock
 
   clock-names:
-    items:
-      - const: system
-      - const: video
-      - const: apb
+    oneOf:
+      - items:
+          - const: system
+          - const: video
+          - const: apb
+      - items:
+          - const: video
+          - const: apb
 
   power-domains:
     maxItems: 1
@@ -48,7 +58,7 @@ properties:
   resets:
     items:
       - description: CRU_PRESETN reset terminal
-      - description: CRU_CMN_RSTB reset terminal
+      - description: D-PHY reset (CRU_CMN_RSTB or CRU_n_S_RESETN)
 
   reset-names:
     items:
@@ -101,6 +111,25 @@ required:
   - reset-names
   - ports
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a09g057-csi2
+    then:
+      properties:
+        clocks:
+          maxItems: 2
+        clock-names:
+          maxItems: 2
+    else:
+      properties:
+        clocks:
+          minItems: 3
+        clock-names:
+          minItems: 3
+
 additionalProperties: false
 
 examples:
-- 
2.43.0

[PATCH v4 02/17] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block

[Tommaso Merciai]

Document the CSI-2 block which is part of CRU found in Renesas RZ/G3E
SoC.

The CSI-2 block on the RZ/G3E SoC is identical to one found on the
RZ/V2H(P) SoC.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
 - Fixed CRU_CMN_RSTB as suggested by LPinchart
 - Collected tags.

Changes since v3:
 - Collected tag.

 .../devicetree/bindings/media/renesas,rzg2l-csi2.yaml          | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
index 1f9ee37584b3..c5c511c9f0db 100644
--- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
@@ -24,6 +24,9 @@ properties:
               - renesas,r9a07g044-csi2 # RZ/G2{L,LC}
               - renesas,r9a07g054-csi2 # RZ/V2L
           - const: renesas,rzg2l-csi2
+      - items:
+          - const: renesas,r9a09g047-csi2 # RZ/G3E
+          - const: renesas,r9a09g057-csi2
       - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
 
   reg:
-- 
2.43.0

[PATCH v4 03/17] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC

[Tommaso Merciai]

The CRU block found on the Renesas RZ/G3E ("R9A09G047") SoC has five
interrupts:

 - image_conv:    image_conv irq
 - axi_mst_err:   AXI master error level irq
 - vd_addr_wend:  Video data AXI master addr 0 write end irq
 - sd_addr_wend:  Statistics data AXI master addr 0 write end irq
 - vsd_addr_wend: Video statistics data AXI master addr 0 write end irq

This IP has only one input port 'port@1' similar to the RZ/G2UL CRU.

Document the CRU block found on the Renesas RZ/G3E ("R9A09G047") SoC.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
 - Use oneOf for interrupts and interrupt-names
 - Handle interrupts and interrupt names base on soc variants

Changes since v2:
 - Collected tag.

 .../bindings/media/renesas,rzg2l-cru.yaml     | 65 +++++++++++++++----
 1 file changed, 54 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
index bc1245127025..47e18690fa57 100644
--- a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
@@ -17,24 +17,43 @@ description:
 
 properties:
   compatible:
-    items:
-      - enum:
-          - renesas,r9a07g043-cru       # RZ/G2UL
-          - renesas,r9a07g044-cru       # RZ/G2{L,LC}
-          - renesas,r9a07g054-cru       # RZ/V2L
-      - const: renesas,rzg2l-cru
+    oneOf:
+      - items:
+          - enum:
+              - renesas,r9a07g043-cru       # RZ/G2UL
+              - renesas,r9a07g044-cru       # RZ/G2{L,LC}
+              - renesas,r9a07g054-cru       # RZ/V2L
+          - const: renesas,rzg2l-cru
+      - const: renesas,r9a09g047-cru        # RZ/G3E
 
   reg:
     maxItems: 1
 
   interrupts:
-    maxItems: 3
+    oneOf:
+      - items:
+          - description: CRU Interrupt for image_conv
+          - description: CRU Interrupt for image_conv_err
+          - description: CRU AXI master error interrupt
+      - items:
+          - description: CRU Interrupt for image_conv
+          - description: CRU AXI master error interrupt
+          - description: CRU Video Data AXI Master Address 0 Write End interrupt
+          - description: CRU Statistics data AXI master addr 0 write end interrupt
+          - description: CRU Video statistics data AXI master addr 0 write end interrupt
 
   interrupt-names:
-    items:
-      - const: image_conv
-      - const: image_conv_err
-      - const: axi_mst_err
+    oneOf:
+      - items:
+          - const: image_conv
+          - const: image_conv_err
+          - const: axi_mst_err
+      - items:
+          - const: image_conv
+          - const: axi_mst_err
+          - const: vd_addr_wend
+          - const: sd_addr_wend
+          - const: vsd_addr_wend
 
   clocks:
     items:
@@ -109,6 +128,10 @@ allOf:
               - renesas,r9a07g054-cru
     then:
       properties:
+        interrupts:
+          maxItems: 3
+        interrupt-names:
+          maxItems: 3
         ports:
           required:
             - port@0
@@ -122,10 +145,30 @@ allOf:
               - renesas,r9a07g043-cru
     then:
       properties:
+        interrupts:
+          maxItems: 3
+        interrupt-names:
+          maxItems: 3
         ports:
           properties:
             port@0: false
+          required:
+            - port@1
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a09g047-cru
+    then:
+      properties:
+        interrupts:
+          minItems: 5
+        interrupt-names:
+          minItems: 5
+        ports:
+          properties:
+            port@0: false
           required:
             - port@1
 
-- 
2.43.0

[PATCH v4 04/17] media: rzg2l-cru: csi2: Use local variable for struct device in rzg2l_csi2_probe()

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Use a local variable for the struct device pointers. This increases code
readability with shortened lines.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
 - Fixed commit msg and commit body as suggested by LPinchart
 - Collected tags

 .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 31 ++++++++++---------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index 881e910dce02..948f1917b830 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -764,10 +764,11 @@ static const struct media_entity_operations rzg2l_csi2_entity_ops = {
 
 static int rzg2l_csi2_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct rzg2l_csi2 *csi2;
 	int ret;
 
-	csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
+	csi2 = devm_kzalloc(dev, sizeof(*csi2), GFP_KERNEL);
 	if (!csi2)
 		return -ENOMEM;
 
@@ -775,28 +776,28 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
 	if (IS_ERR(csi2->base))
 		return PTR_ERR(csi2->base);
 
-	csi2->cmn_rstb = devm_reset_control_get_exclusive(&pdev->dev, "cmn-rstb");
+	csi2->cmn_rstb = devm_reset_control_get_exclusive(dev, "cmn-rstb");
 	if (IS_ERR(csi2->cmn_rstb))
-		return dev_err_probe(&pdev->dev, PTR_ERR(csi2->cmn_rstb),
+		return dev_err_probe(dev, PTR_ERR(csi2->cmn_rstb),
 				     "Failed to get cpg cmn-rstb\n");
 
-	csi2->presetn = devm_reset_control_get_shared(&pdev->dev, "presetn");
+	csi2->presetn = devm_reset_control_get_shared(dev, "presetn");
 	if (IS_ERR(csi2->presetn))
-		return dev_err_probe(&pdev->dev, PTR_ERR(csi2->presetn),
+		return dev_err_probe(dev, PTR_ERR(csi2->presetn),
 				     "Failed to get cpg presetn\n");
 
-	csi2->sysclk = devm_clk_get(&pdev->dev, "system");
+	csi2->sysclk = devm_clk_get(dev, "system");
 	if (IS_ERR(csi2->sysclk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(csi2->sysclk),
+		return dev_err_probe(dev, PTR_ERR(csi2->sysclk),
 				     "Failed to get system clk\n");
 
-	csi2->vclk = devm_clk_get(&pdev->dev, "video");
+	csi2->vclk = devm_clk_get(dev, "video");
 	if (IS_ERR(csi2->vclk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(csi2->vclk),
+		return dev_err_probe(dev, PTR_ERR(csi2->vclk),
 				     "Failed to get video clock\n");
 	csi2->vclk_rate = clk_get_rate(csi2->vclk);
 
-	csi2->dev = &pdev->dev;
+	csi2->dev = dev;
 
 	platform_set_drvdata(pdev, csi2);
 
@@ -804,18 +805,18 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	pm_runtime_enable(&pdev->dev);
+	pm_runtime_enable(dev);
 
 	ret = rzg2l_validate_csi2_lanes(csi2);
 	if (ret)
 		goto error_pm;
 
-	csi2->subdev.dev = &pdev->dev;
+	csi2->subdev.dev = dev;
 	v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
 	csi2->subdev.internal_ops = &rzg2l_csi2_internal_ops;
-	v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
+	v4l2_set_subdevdata(&csi2->subdev, dev);
 	snprintf(csi2->subdev.name, sizeof(csi2->subdev.name),
-		 "csi-%s", dev_name(&pdev->dev));
+		 "csi-%s", dev_name(dev));
 	csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
 
 	csi2->subdev.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
@@ -852,7 +853,7 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
 	v4l2_async_nf_cleanup(&csi2->notifier);
 	media_entity_cleanup(&csi2->subdev.entity);
 error_pm:
-	pm_runtime_disable(&pdev->dev);
+	pm_runtime_disable(dev);
 
 	return ret;
 }
-- 
2.43.0

[PATCH v4 05/17] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable()

[Tommaso Merciai]

Use newly added devm_pm_runtime_enable() into rzg2l_csi2_probe() and
drop error path accordingly. Drop also unnecessary pm_runtime_disable()
from rzg2l_csi2_remove().

Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
 - Collected tags

Changes since v2:
 - Collected tags

 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index 948f1917b830..4ccf7c5ea58b 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -805,11 +805,13 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	pm_runtime_enable(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
 
 	ret = rzg2l_validate_csi2_lanes(csi2);
 	if (ret)
-		goto error_pm;
+		return ret;
 
 	csi2->subdev.dev = dev;
 	v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
@@ -834,7 +836,7 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
 	ret = media_entity_pads_init(&csi2->subdev.entity, ARRAY_SIZE(csi2->pads),
 				     csi2->pads);
 	if (ret)
-		goto error_pm;
+		return ret;
 
 	ret = v4l2_subdev_init_finalize(&csi2->subdev);
 	if (ret < 0)
@@ -852,8 +854,6 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
 	v4l2_async_nf_unregister(&csi2->notifier);
 	v4l2_async_nf_cleanup(&csi2->notifier);
 	media_entity_cleanup(&csi2->subdev.entity);
-error_pm:
-	pm_runtime_disable(dev);
 
 	return ret;
 }
@@ -867,7 +867,6 @@ static void rzg2l_csi2_remove(struct platform_device *pdev)
 	v4l2_async_unregister_subdev(&csi2->subdev);
 	v4l2_subdev_cleanup(&csi2->subdev);
 	media_entity_cleanup(&csi2->subdev.entity);
-	pm_runtime_disable(&pdev->dev);
 }
 
 static int rzg2l_csi2_pm_runtime_suspend(struct device *dev)
-- 
2.43.0

[PATCH v4 06/17] media: rzg2l-cru: rzg2l-core: Use local variable for struct device in rzg2l_cru_probe()

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Use a local variable for the struct device pointers. This increases code
readability with shortened lines.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
 - Fixed commit msg and commit body as suggested by LPinchart
 - Collected tags

 .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 29 ++++++++++---------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 89be584a4988..70fed0ce45ea 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -240,10 +240,11 @@ static int rzg2l_cru_media_init(struct rzg2l_cru_dev *cru)
 
 static int rzg2l_cru_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct rzg2l_cru_dev *cru;
 	int irq, ret;
 
-	cru = devm_kzalloc(&pdev->dev, sizeof(*cru), GFP_KERNEL);
+	cru = devm_kzalloc(dev, sizeof(*cru), GFP_KERNEL);
 	if (!cru)
 		return -ENOMEM;
 
@@ -251,32 +252,32 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
 	if (IS_ERR(cru->base))
 		return PTR_ERR(cru->base);
 
-	cru->presetn = devm_reset_control_get_shared(&pdev->dev, "presetn");
+	cru->presetn = devm_reset_control_get_shared(dev, "presetn");
 	if (IS_ERR(cru->presetn))
-		return dev_err_probe(&pdev->dev, PTR_ERR(cru->presetn),
+		return dev_err_probe(dev, PTR_ERR(cru->presetn),
 				     "Failed to get cpg presetn\n");
 
-	cru->aresetn = devm_reset_control_get_exclusive(&pdev->dev, "aresetn");
+	cru->aresetn = devm_reset_control_get_exclusive(dev, "aresetn");
 	if (IS_ERR(cru->aresetn))
-		return dev_err_probe(&pdev->dev, PTR_ERR(cru->aresetn),
+		return dev_err_probe(dev, PTR_ERR(cru->aresetn),
 				     "Failed to get cpg aresetn\n");
 
-	cru->vclk = devm_clk_get(&pdev->dev, "video");
+	cru->vclk = devm_clk_get(dev, "video");
 	if (IS_ERR(cru->vclk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(cru->vclk),
+		return dev_err_probe(dev, PTR_ERR(cru->vclk),
 				     "Failed to get video clock\n");
 
-	cru->dev = &pdev->dev;
-	cru->info = of_device_get_match_data(&pdev->dev);
+	cru->dev = dev;
+	cru->info = of_device_get_match_data(dev);
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return irq;
 
-	ret = devm_request_irq(&pdev->dev, irq, rzg2l_cru_irq, 0,
+	ret = devm_request_irq(dev, irq, rzg2l_cru_irq, 0,
 			       KBUILD_MODNAME, cru);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "failed to request irq\n");
+		return dev_err_probe(dev, ret, "failed to request irq\n");
 
 	platform_set_drvdata(pdev, cru);
 
@@ -285,8 +286,8 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
 		return ret;
 
 	cru->num_buf = RZG2L_CRU_HW_BUFFER_DEFAULT;
-	pm_suspend_ignore_children(&pdev->dev, true);
-	pm_runtime_enable(&pdev->dev);
+	pm_suspend_ignore_children(dev, true);
+	pm_runtime_enable(dev);
 
 	ret = rzg2l_cru_media_init(cru);
 	if (ret)
@@ -296,7 +297,7 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
 
 error_dma_unregister:
 	rzg2l_cru_dma_unregister(cru);
-	pm_runtime_disable(&pdev->dev);
+	pm_runtime_disable(dev);
 
 	return ret;
 }
-- 
2.43.0

[PATCH v4 07/17] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable()

[Tommaso Merciai]

Use newly added devm_pm_runtime_enable() into rzg2l_cru_probe() and
drop unnecessary pm_runtime_disable() from rzg2l_cru_probe() and
rzg2l_csi2_remove().

Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
 - Fixed DMA leak as suggested by LPinchart
 - Collected tags

Changes since v2:
 - Collected tags

 drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 70fed0ce45ea..eed9d2bd0841 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -287,7 +287,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
 
 	cru->num_buf = RZG2L_CRU_HW_BUFFER_DEFAULT;
 	pm_suspend_ignore_children(dev, true);
-	pm_runtime_enable(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		goto error_dma_unregister;
 
 	ret = rzg2l_cru_media_init(cru);
 	if (ret)
@@ -297,7 +299,6 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
 
 error_dma_unregister:
 	rzg2l_cru_dma_unregister(cru);
-	pm_runtime_disable(dev);
 
 	return ret;
 }
@@ -306,8 +307,6 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
 {
 	struct rzg2l_cru_dev *cru = platform_get_drvdata(pdev);
 
-	pm_runtime_disable(&pdev->dev);
-
 	v4l2_async_nf_unregister(&cru->notifier);
 	v4l2_async_nf_cleanup(&cru->notifier);
 
-- 
2.43.0

[PATCH v4 08/17] media: rzg2l-cru: csi2: Introduce SoC-specific D-PHY handling

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

In preparation for adding support for the RZ/V2H(P) SoC, where the D-PHY
differs from the existing RZ/G2L implementation, introduce a new
rzg2l_csi2_info structure. This structure provides function pointers for
SoC-specific D-PHY enable and disable operations.

Modify rzg2l_csi2_dphy_setting() to use these function pointers instead of
calling rzg2l_csi2_dphy_enable() and rzg2l_csi2_dphy_disable() directly.
Update the device match table to store the appropriate function pointers
for each compatible SoC.

This change prepares the driver for future extensions without affecting
the current functionality for RZ/G2L.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
 - Moved rzg2l_csi2_info below the definition of the rzg2l_csi2_dphy_enable()
   function as suggested by LPinchart
 - Collected tags

 .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 24 ++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index 4ccf7c5ea58b..4aa5d58dde5b 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -107,6 +107,7 @@ struct rzg2l_csi2 {
 	void __iomem *base;
 	struct reset_control *presetn;
 	struct reset_control *cmn_rstb;
+	const struct rzg2l_csi2_info *info;
 	struct clk *sysclk;
 	struct clk *vclk;
 	unsigned long vclk_rate;
@@ -123,6 +124,11 @@ struct rzg2l_csi2 {
 	bool dphy_enabled;
 };
 
+struct rzg2l_csi2_info {
+	int (*dphy_enable)(struct rzg2l_csi2 *csi2);
+	int (*dphy_disable)(struct rzg2l_csi2 *csi2);
+};
+
 struct rzg2l_csi2_timings {
 	u32 t_init;
 	u32 tclk_miss;
@@ -355,14 +361,19 @@ static int rzg2l_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
 	return ret;
 }
 
+static const struct rzg2l_csi2_info rzg2l_csi2_info = {
+	.dphy_enable = rzg2l_csi2_dphy_enable,
+	.dphy_disable = rzg2l_csi2_dphy_disable,
+};
+
 static int rzg2l_csi2_dphy_setting(struct v4l2_subdev *sd, bool on)
 {
 	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
 
 	if (on)
-		return rzg2l_csi2_dphy_enable(csi2);
+		return csi2->info->dphy_enable(csi2);
 
-	return rzg2l_csi2_dphy_disable(csi2);
+	return csi2->info->dphy_disable(csi2);
 }
 
 static int rzg2l_csi2_mipi_link_enable(struct rzg2l_csi2 *csi2)
@@ -772,6 +783,10 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
 	if (!csi2)
 		return -ENOMEM;
 
+	csi2->info = of_device_get_match_data(dev);
+	if (!csi2->info)
+		return dev_err_probe(dev, -EINVAL, "Failed to get OF match data\n");
+
 	csi2->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(csi2->base))
 		return PTR_ERR(csi2->base);
@@ -891,7 +906,10 @@ static const struct dev_pm_ops rzg2l_csi2_pm_ops = {
 };
 
 static const struct of_device_id rzg2l_csi2_of_table[] = {
-	{ .compatible = "renesas,rzg2l-csi2", },
+	{
+		.compatible = "renesas,rzg2l-csi2",
+		.data = &rzg2l_csi2_info,
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, rzg2l_csi2_of_table);
-- 
2.43.0

[PATCH v4 09/17] media: rzg2l-cru: csi2: Skip system clock for RZ/V2H(P) SoC

[Tommaso Merciai]

The RZ/V2H(P) SoC does not require a `system` clock for the CSI-2
interface. To accommodate this, introduce a `has_system_clk` bool flag
in the `rzg2l_csi2_info` structure and update the rzg2l_csi2_probe() to
conditionally request the clock only when needed.

This patch is in preparation for adding support for RZ/V2H(P) SoC.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
 - Added has_system_clk bool flag to the rzg2l_csi2_info structure to handle
   case where system clock is not required as suggested by LPinchart.
 - Fixed commit body and msg

 .../media/platform/renesas/rzg2l-cru/rzg2l-csi2.c    | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index 4aa5d58dde5b..e4781105eadc 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -127,6 +127,7 @@ struct rzg2l_csi2 {
 struct rzg2l_csi2_info {
 	int (*dphy_enable)(struct rzg2l_csi2 *csi2);
 	int (*dphy_disable)(struct rzg2l_csi2 *csi2);
+	bool has_system_clk;
 };
 
 struct rzg2l_csi2_timings {
@@ -364,6 +365,7 @@ static int rzg2l_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
 static const struct rzg2l_csi2_info rzg2l_csi2_info = {
 	.dphy_enable = rzg2l_csi2_dphy_enable,
 	.dphy_disable = rzg2l_csi2_dphy_disable,
+	.has_system_clk = true,
 };
 
 static int rzg2l_csi2_dphy_setting(struct v4l2_subdev *sd, bool on)
@@ -801,10 +803,12 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(csi2->presetn),
 				     "Failed to get cpg presetn\n");
 
-	csi2->sysclk = devm_clk_get(dev, "system");
-	if (IS_ERR(csi2->sysclk))
-		return dev_err_probe(dev, PTR_ERR(csi2->sysclk),
-				     "Failed to get system clk\n");
+	if (csi2->info->has_system_clk) {
+		csi2->sysclk = devm_clk_get(dev, "system");
+		if (IS_ERR(csi2->sysclk))
+			return dev_err_probe(dev, PTR_ERR(csi2->sysclk),
+					     "Failed to get system clk\n");
+	}
 
 	csi2->vclk = devm_clk_get(dev, "video");
 	if (IS_ERR(csi2->vclk))
-- 
2.43.0

[PATCH v4 10/17] media: rzg2l-cru: csi2: Add support for RZ/V2H(P) SoC

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The D-PHY on the RZ/V2H(P) SoC is different from the D-PHY on the RZ/G2L
SoC. To handle this difference, function pointers for D-PHY enable/disable
have been added, and the `struct rzg2l_csi2_info` pointer is passed as OF
data.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
 - Moved CRUm_SWAPCTL write of rzv2h_csi2_dphy_enable function under the error
   check as suggested by LPinchart.
 - Moved rzv2h_csi2_info after rzv2h_csi2_dphy_enable() as suggested by LPinchart
 - Collected tag.

 .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 95 +++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index e4781105eadc..9243306e2aa9 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -85,6 +85,15 @@
 					 CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
 					 CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1))
 
+/* DPHY registers on RZ/V2H(P) SoC */
+#define CRUm_S_TIMCTL			0x41c
+#define CRUm_S_TIMCTL_S_HSSETTLECTL(x)	((x) << 8)
+
+#define CRUm_S_DPHYCTL_MSB		0x434
+#define CRUm_S_DPHYCTL_MSB_DESKEW	BIT(1)
+
+#define CRUm_SWAPCTL			0x438
+
 #define VSRSTS_RETRIES			20
 
 #define RZG2L_CSI2_MIN_WIDTH		320
@@ -140,6 +149,30 @@ struct rzg2l_csi2_timings {
 	u32 max_hsfreq;
 };
 
+struct rzv2h_csi2_s_hssettlectl {
+	unsigned int hsfreq;
+	u16 s_hssettlectl;
+};
+
+static const struct rzv2h_csi2_s_hssettlectl rzv2h_s_hssettlectl[] = {
+	{   90,  1 }, {  130,  2 }, {  180,  3 },
+	{  220,  4 }, {  270,  5 }, {  310,  6 },
+	{  360,  7 }, {  400,  8 }, {  450,  9 },
+	{  490, 10 }, {  540, 11 }, {  580, 12 },
+	{  630, 13 }, {  670, 14 }, {  720, 15 },
+	{  760, 16 }, {  810, 17 }, {  850, 18 },
+	{  900, 19 }, {  940, 20 }, {  990, 21 },
+	{ 1030, 22 }, { 1080, 23 }, { 1120, 24 },
+	{ 1170, 25 }, { 1220, 26 }, { 1260, 27 },
+	{ 1310, 28 }, { 1350, 29 }, { 1400, 30 },
+	{ 1440, 31 }, { 1490, 32 }, { 1530, 33 },
+	{ 1580, 34 }, { 1620, 35 }, { 1670, 36 },
+	{ 1710, 37 }, { 1760, 38 }, { 1800, 39 },
+	{ 1850, 40 }, { 1890, 41 }, { 1940, 42 },
+	{ 1980, 43 }, { 2030, 44 }, { 2070, 45 },
+	{ 2100, 46 },
+};
+
 static const struct rzg2l_csi2_timings rzg2l_csi2_global_timings[] = {
 	{
 		.max_hsfreq = 80,
@@ -434,6 +467,64 @@ static int rzg2l_csi2_mipi_link_disable(struct rzg2l_csi2 *csi2)
 	return 0;
 }
 
+static int rzv2h_csi2_dphy_disable(struct rzg2l_csi2 *csi2)
+{
+	int ret;
+
+	/* Reset the CRU (D-PHY) */
+	ret = reset_control_assert(csi2->cmn_rstb);
+	if (ret)
+		return ret;
+
+	csi2->dphy_enabled = false;
+
+	return 0;
+}
+
+static int rzv2h_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
+{
+	unsigned int i;
+	u16 hssettle;
+	int mbps;
+
+	mbps = rzg2l_csi2_calc_mbps(csi2);
+	if (mbps < 0)
+		return mbps;
+
+	csi2->hsfreq = mbps;
+
+	for (i = 0; i < ARRAY_SIZE(rzv2h_s_hssettlectl); i++) {
+		if (csi2->hsfreq <= rzv2h_s_hssettlectl[i].hsfreq)
+			break;
+	}
+
+	if (i == ARRAY_SIZE(rzv2h_s_hssettlectl))
+		return -EINVAL;
+
+	rzg2l_csi2_write(csi2, CRUm_SWAPCTL, 0);
+
+	hssettle = rzv2h_s_hssettlectl[i].s_hssettlectl;
+	rzg2l_csi2_write(csi2, CRUm_S_TIMCTL,
+			 CRUm_S_TIMCTL_S_HSSETTLECTL(hssettle));
+
+	if (csi2->hsfreq > 1500)
+		rzg2l_csi2_set(csi2, CRUm_S_DPHYCTL_MSB,
+			       CRUm_S_DPHYCTL_MSB_DESKEW);
+	else
+		rzg2l_csi2_clr(csi2, CRUm_S_DPHYCTL_MSB,
+			       CRUm_S_DPHYCTL_MSB_DESKEW);
+
+	csi2->dphy_enabled = true;
+
+	return 0;
+}
+
+static const struct rzg2l_csi2_info rzv2h_csi2_info = {
+	.dphy_enable = rzv2h_csi2_dphy_enable,
+	.dphy_disable = rzv2h_csi2_dphy_disable,
+	.has_system_clk = false,
+};
+
 static int rzg2l_csi2_mipi_link_setting(struct v4l2_subdev *sd, bool on)
 {
 	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
@@ -910,6 +1001,10 @@ static const struct dev_pm_ops rzg2l_csi2_pm_ops = {
 };
 
 static const struct of_device_id rzg2l_csi2_of_table[] = {
+	{
+		.compatible = "renesas,r9a09g057-csi2",
+		.data = &rzv2h_csi2_info,
+	},
 	{
 		.compatible = "renesas,rzg2l-csi2",
 		.data = &rzg2l_csi2_info,
-- 
2.43.0

[PATCH v4 11/17] media: rzg2l-cru: Add register mapping support

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a
CRU-IP that is mostly identical to RZ/G2L but with different register
offsets and additional registers. Introduce a flexible register mapping
mechanism to handle these variations.

Define the `rzg2l_cru_info` structure to store register mappings and
pass it as part of the OF match data. Update the read/write functions
to check out-of-bound accesses and use indexed register offsets from
`rzg2l_cru_info`, ensuring compatibility across different SoC variants.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
 - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
   accesses as suggested by LPinchart.
 - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
 - Update commit body

 .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 46 ++++++++++++-
 .../renesas/rzg2l-cru/rzg2l-cru-regs.h        | 66 ++++++++++---------
 .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  4 ++
 .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 58 ++++++++++++++--
 4 files changed, 139 insertions(+), 35 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index eed9d2bd0841..abc2a979833a 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -22,6 +22,7 @@
 #include <media/v4l2-mc.h>
 
 #include "rzg2l-cru.h"
+#include "rzg2l-cru-regs.h"
 
 static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n)
 {
@@ -269,6 +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
 
 	cru->dev = dev;
 	cru->info = of_device_get_match_data(dev);
+	if (!cru->info)
+		return dev_err_probe(dev, -EINVAL,
+				     "Failed to get OF match data\n");
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
 	rzg2l_cru_dma_unregister(cru);
 }
 
+static const u16 rzg2l_cru_regs[] = {
+	[CRUnCTRL] = 0x0,
+	[CRUnIE] = 0x4,
+	[CRUnINTS] = 0x8,
+	[CRUnRST] = 0xc,
+	[AMnMB1ADDRL] = 0x100,
+	[AMnMB1ADDRH] = 0x104,
+	[AMnMB2ADDRL] = 0x108,
+	[AMnMB2ADDRH] = 0x10c,
+	[AMnMB3ADDRL] = 0x110,
+	[AMnMB3ADDRH] = 0x114,
+	[AMnMB4ADDRL] = 0x118,
+	[AMnMB4ADDRH] = 0x11c,
+	[AMnMB5ADDRL] = 0x120,
+	[AMnMB5ADDRH] = 0x124,
+	[AMnMB6ADDRL] = 0x128,
+	[AMnMB6ADDRH] = 0x12c,
+	[AMnMB7ADDRL] = 0x130,
+	[AMnMB7ADDRH] = 0x134,
+	[AMnMB8ADDRL] = 0x138,
+	[AMnMB8ADDRH] = 0x13c,
+	[AMnMBVALID] = 0x148,
+	[AMnMBS] = 0x14c,
+	[AMnAXIATTR] = 0x158,
+	[AMnFIFOPNTR] = 0x168,
+	[AMnAXISTP] = 0x174,
+	[AMnAXISTPACK] = 0x178,
+	[ICnEN] = 0x200,
+	[ICnMC] = 0x208,
+	[ICnMS] = 0x254,
+	[ICnDMR] = 0x26c,
+};
+
+static const struct rzg2l_cru_info rzgl2_cru_info = {
+	.regs = rzg2l_cru_regs,
+};
+
 static const struct of_device_id rzg2l_cru_of_id_table[] = {
-	{ .compatible = "renesas,rzg2l-cru", },
+	{
+		.compatible = "renesas,rzg2l-cru",
+		.data = &rzgl2_cru_info,
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, rzg2l_cru_of_id_table);
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
index 1c9f22118a5d..86c320286246 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
@@ -10,71 +10,77 @@
 
 /* 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_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))
+#define AMnMBxADDRL(x)			(AMnMB1ADDRL + (x) * 2)
 
 /* Memory Bank Base Address (Higher) Register for CRU Image Data */
-#define AMnMBxADDRH(x)			(0x104 + ((x) * 8))
+#define AMnMBxADDRH(x)			(AMnMB1ADDRH + (x) * 2)
 
-/* 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 Transfer Setting Register for CRU Image Data */
-#define AMnAXIATTR			0x158
 #define AMnAXIATTR_AXILEN_MASK		GENMASK(3, 0)
 #define AMnAXIATTR_AXILEN		(0xf)
 
-/* 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(x)			((x) << 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)
 
+enum rzg2l_cru_common_regs {
+	CRUnCTRL,	/* CRU Control */
+	CRUnIE,		/* CRU Interrupt Enable */
+	CRUnINTS,	/* CRU Interrupt Status */
+	CRUnRST, 	/* CRU Reset */
+	AMnMB1ADDRL,	/* Bank 1 Address (Lower) for CRU Image Data */
+	AMnMB1ADDRH,	/* Bank 1 Address (Higher) for CRU Image Data */
+	AMnMB2ADDRL,    /* Bank 2 Address (Lower) for CRU Image Data */
+	AMnMB2ADDRH,    /* Bank 2 Address (Higher) for CRU Image Data */
+	AMnMB3ADDRL,    /* Bank 3 Address (Lower) for CRU Image Data */
+	AMnMB3ADDRH,    /* Bank 3 Address (Higher) for CRU Image Data */
+	AMnMB4ADDRL,    /* Bank 4 Address (Lower) for CRU Image Data */
+	AMnMB4ADDRH,    /* Bank 4 Address (Higher) for CRU Image Data */
+	AMnMB5ADDRL,    /* Bank 5 Address (Lower) for CRU Image Data */
+	AMnMB5ADDRH,    /* Bank 5 Address (Higher) for CRU Image Data */
+	AMnMB6ADDRL,    /* Bank 6 Address (Lower) for CRU Image Data */
+	AMnMB6ADDRH,    /* Bank 6 Address (Higher) for CRU Image Data */
+	AMnMB7ADDRL,    /* Bank 7 Address (Lower) for CRU Image Data */
+	AMnMB7ADDRH,    /* Bank 7 Address (Higher) for CRU Image Data */
+	AMnMB8ADDRL,    /* Bank 8 Address (Lower) for CRU Image Data */
+	AMnMB8ADDRH,    /* Bank 8 Address (Higher) for CRU Image Data */
+	AMnMBVALID,	/* Memory Bank Enable for CRU Image Data */
+	AMnMBS,		/* Memory Bank Status for CRU Image Data */
+	AMnAXIATTR,	/* AXI Master Transfer Setting Register for CRU Image Data */
+	AMnFIFOPNTR,	/* AXI Master FIFO Pointer for CRU Image Data */
+	AMnAXISTP,	/* AXI Master Transfer Stop for CRU Image Data */
+	AMnAXISTPACK,	/* AXI Master Transfer Stop Status for CRU Image Data */
+	ICnEN,		/* CRU Image Processing Enable */
+	ICnMC,		/* CRU Image Processing Main Control */
+	ICnMS,		/* CRU Module Status */
+	ICnDMR,		/* CRU Data Output Mode */
+	RZG2L_CRU_MAX_REG,
+};
+
 #endif /* __RZG2L_CRU_REGS_H__ */
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index 8b898ce05b84..00c3f7458e20 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -80,6 +80,10 @@ struct rzg2l_cru_ip_format {
 	bool yuv;
 };
 
+struct rzg2l_cru_info {
+	const u16 *regs;
+};
+
 /**
  * struct rzg2l_cru_dev - Renesas CRU device structure
  * @dev:		(OF) device
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index cd69c8a686d3..792f0df51a4b 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -42,16 +42,66 @@ struct rzg2l_cru_buffer {
 /* -----------------------------------------------------------------------------
  * DMA operations
  */
-static void rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
+static void __rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
 {
-	iowrite32(value, cru->base + offset);
+	const u16 *regs = cru->info->regs;
+
+	/*
+	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
+	 * rest of the registers have valid offset being set in cru->info->regs.
+	 */
+	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
+	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
+		return;
+
+	iowrite32(value, cru->base + regs[offset]);
+}
+
+static u32 __rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
+{
+	const u16 *regs = cru->info->regs;
+
+	/*
+	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
+	 * rest of the registers have valid offset being set in cru->info->regs.
+	 */
+	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
+	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
+		return 0;
+
+	return ioread32(cru->base + regs[offset]);
 }
 
-static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
+static inline void
+__rzg2l_cru_write_constant(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
 {
-	return ioread32(cru->base + offset);
+	const u16 *regs = cru->info->regs;
+
+	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
+
+	iowrite32(value, cru->base + regs[offset]);
 }
 
+static inline u32
+__rzg2l_cru_read_constant(struct rzg2l_cru_dev *cru, u32 offset)
+{
+	const u16 *regs = cru->info->regs;
+
+	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
+
+	return ioread32(cru->base + regs[offset]);
+}
+
+#define rzg2l_cru_write(cru, offset, value) \
+	(__builtin_constant_p(offset) ? \
+	 __rzg2l_cru_write_constant(cru, offset, value) : \
+	 __rzg2l_cru_write(cru, offset, value))
+
+#define rzg2l_cru_read(cru, offset) \
+	(__builtin_constant_p(offset) ? \
+	 __rzg2l_cru_read_constant(cru, offset) : \
+	 __rzg2l_cru_read(cru, offset))
+
 /* Need to hold qlock before calling */
 static void return_unused_buffers(struct rzg2l_cru_dev *cru,
 				  enum vb2_buffer_state state)
-- 
2.43.0

[PATCH v4 12/17] media: rzg2l-cru: Pass resolution limits via OF data

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Pass `max_width` and `max_height` as part of the OF data to facilitate the
addition of support for RZ/G3E and RZ/V2H(P) SoCs. These SoCs have a
maximum resolution of 4096x4096 as compared to 2800x4095 on RZ/G2L SoC.
This change prepares the driver for easier integration of these SoCs by
defining the resolution limits in the `rzg2l_cru_info` structure.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
 - Collected tag.

 .../media/platform/renesas/rzg2l-cru/rzg2l-core.c   |  2 ++
 .../media/platform/renesas/rzg2l-cru/rzg2l-cru.h    |  4 ++--
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c | 13 +++++++++----
 .../media/platform/renesas/rzg2l-cru/rzg2l-video.c  |  5 +++--
 4 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index abc2a979833a..19f93b7fe6fb 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -355,6 +355,8 @@ static const u16 rzg2l_cru_regs[] = {
 };
 
 static const struct rzg2l_cru_info rzgl2_cru_info = {
+	.max_width = 2800,
+	.max_height = 4095,
 	.regs = rzg2l_cru_regs,
 };
 
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index 00c3f7458e20..6a621073948a 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -27,9 +27,7 @@
 #define RZG2L_CRU_CSI2_VCHANNEL		4
 
 #define RZG2L_CRU_MIN_INPUT_WIDTH	320
-#define RZG2L_CRU_MAX_INPUT_WIDTH	2800
 #define RZG2L_CRU_MIN_INPUT_HEIGHT	240
-#define RZG2L_CRU_MAX_INPUT_HEIGHT	4095
 
 enum rzg2l_csi2_pads {
 	RZG2L_CRU_IP_SINK = 0,
@@ -81,6 +79,8 @@ struct rzg2l_cru_ip_format {
 };
 
 struct rzg2l_cru_info {
+	unsigned int max_width;
+	unsigned int max_height;
 	const u16 *regs;
 };
 
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
index 76a2b451f1da..7836c7cd53dc 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
@@ -148,6 +148,8 @@ static int rzg2l_cru_ip_set_format(struct v4l2_subdev *sd,
 				   struct v4l2_subdev_state *state,
 				   struct v4l2_subdev_format *fmt)
 {
+	struct rzg2l_cru_dev *cru = v4l2_get_subdevdata(sd);
+	const struct rzg2l_cru_info *info = cru->info;
 	struct v4l2_mbus_framefmt *src_format;
 	struct v4l2_mbus_framefmt *sink_format;
 
@@ -170,9 +172,9 @@ static int rzg2l_cru_ip_set_format(struct v4l2_subdev *sd,
 	sink_format->ycbcr_enc = fmt->format.ycbcr_enc;
 	sink_format->quantization = fmt->format.quantization;
 	sink_format->width = clamp_t(u32, fmt->format.width,
-				     RZG2L_CRU_MIN_INPUT_WIDTH, RZG2L_CRU_MAX_INPUT_WIDTH);
+				     RZG2L_CRU_MIN_INPUT_WIDTH, info->max_width);
 	sink_format->height = clamp_t(u32, fmt->format.height,
-				      RZG2L_CRU_MIN_INPUT_HEIGHT, RZG2L_CRU_MAX_INPUT_HEIGHT);
+				      RZG2L_CRU_MIN_INPUT_HEIGHT, info->max_height);
 
 	fmt->format = *sink_format;
 
@@ -197,6 +199,9 @@ static int rzg2l_cru_ip_enum_frame_size(struct v4l2_subdev *sd,
 					struct v4l2_subdev_state *state,
 					struct v4l2_subdev_frame_size_enum *fse)
 {
+	struct rzg2l_cru_dev *cru = v4l2_get_subdevdata(sd);
+	const struct rzg2l_cru_info *info = cru->info;
+
 	if (fse->index != 0)
 		return -EINVAL;
 
@@ -205,8 +210,8 @@ static int rzg2l_cru_ip_enum_frame_size(struct v4l2_subdev *sd,
 
 	fse->min_width = RZG2L_CRU_MIN_INPUT_WIDTH;
 	fse->min_height = RZG2L_CRU_MIN_INPUT_HEIGHT;
-	fse->max_width = RZG2L_CRU_MAX_INPUT_WIDTH;
-	fse->max_height = RZG2L_CRU_MAX_INPUT_HEIGHT;
+	fse->max_width = info->max_width;
+	fse->max_height = info->max_height;
 
 	return 0;
 }
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 792f0df51a4b..93a105dec8f1 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -736,6 +736,7 @@ int rzg2l_cru_dma_register(struct rzg2l_cru_dev *cru)
 static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
 				   struct v4l2_pix_format *pix)
 {
+	const struct rzg2l_cru_info *info = cru->info;
 	const struct rzg2l_cru_ip_format *fmt;
 
 	fmt = rzg2l_cru_ip_format_to_fmt(pix->pixelformat);
@@ -758,8 +759,8 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
 	}
 
 	/* Limit to CRU capabilities */
-	v4l_bound_align_image(&pix->width, 320, RZG2L_CRU_MAX_INPUT_WIDTH, 1,
-			      &pix->height, 240, RZG2L_CRU_MAX_INPUT_HEIGHT, 2, 0);
+	v4l_bound_align_image(&pix->width, 320, info->max_width, 1,
+			      &pix->height, 240, info->max_height, 2, 0);
 
 	pix->bytesperline = pix->width * fmt->bpp;
 	pix->sizeimage = pix->bytesperline * pix->height;
-- 
2.43.0

[PATCH v4 13/17] media: rzg2l-cru: Add image_conv offset to OF data

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add `image_conv` field to the `rzg2l_cru_info` structure to store the
register offset for image conversion control. RZ/G2L uses `ICnMC`, while
RZ/G3E and RZ/V2H(P) use `ICnIPMC_C0`.

Update `rzg2l_cru_initialize_image_conv()` and `rzg2l_cru_csi2_setup()`
to use this `image_conv` offset from the OF data, facilitating future
support for RZ/G3E and RZ/V2H(P) SoCs.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 .../media/platform/renesas/rzg2l-cru/rzg2l-core.c  |  1 +
 .../media/platform/renesas/rzg2l-cru/rzg2l-cru.h   |  1 +
 .../media/platform/renesas/rzg2l-cru/rzg2l-video.c | 14 ++++++++------
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 19f93b7fe6fb..7e94ae803967 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -357,6 +357,7 @@ static const u16 rzg2l_cru_regs[] = {
 static const struct rzg2l_cru_info rzgl2_cru_info = {
 	.max_width = 2800,
 	.max_height = 4095,
+	.image_conv = ICnMC,
 	.regs = rzg2l_cru_regs,
 };
 
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index 6a621073948a..ca156772b949 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -81,6 +81,7 @@ struct rzg2l_cru_ip_format {
 struct rzg2l_cru_info {
 	unsigned int max_width;
 	unsigned int max_height;
+	u16 image_conv;
 	const u16 *regs;
 };
 
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 93a105dec8f1..5033c8d98639 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -246,20 +246,22 @@ static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
 				 const struct rzg2l_cru_ip_format *ip_fmt,
 				 u8 csi_vc)
 {
+	const struct rzg2l_cru_info *info = cru->info;
 	u32 icnmc = ICnMC_INF(ip_fmt->datatype);
 
-	icnmc |= (rzg2l_cru_read(cru, ICnMC) & ~ICnMC_INF_MASK);
+	icnmc |= (rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK);
 
 	/* Set virtual channel CSI2 */
 	icnmc |= ICnMC_VCSEL(csi_vc);
 
-	rzg2l_cru_write(cru, ICnMC, icnmc);
+	rzg2l_cru_write(cru, info->image_conv, icnmc);
 }
 
 static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
 					   struct v4l2_mbus_framefmt *ip_sd_fmt,
 					   u8 csi_vc)
 {
+	const struct rzg2l_cru_info *info = cru->info;
 	const struct rzg2l_cru_ip_format *cru_video_fmt;
 	const struct rzg2l_cru_ip_format *cru_ip_fmt;
 
@@ -276,11 +278,11 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
 
 	/* If input and output use same colorspace, do bypass mode */
 	if (cru_ip_fmt->yuv == cru_video_fmt->yuv)
-		rzg2l_cru_write(cru, ICnMC,
-				rzg2l_cru_read(cru, ICnMC) | ICnMC_CSCTHR);
+		rzg2l_cru_write(cru, info->image_conv,
+				rzg2l_cru_read(cru, info->image_conv) | ICnMC_CSCTHR);
 	else
-		rzg2l_cru_write(cru, ICnMC,
-				rzg2l_cru_read(cru, ICnMC) & (~ICnMC_CSCTHR));
+		rzg2l_cru_write(cru, info->image_conv,
+				rzg2l_cru_read(cru, info->image_conv) & (~ICnMC_CSCTHR));
 
 	/* Set output data format */
 	rzg2l_cru_write(cru, ICnDMR, cru_video_fmt->icndmr);
-- 
2.43.0

[PATCH v4 14/17] media: rzg2l-cru: Add IRQ handler to OF data

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add `irq_handler` to the `rzg2l_cru_info` structure and pass it as part of
the OF data. This prepares for supporting RZ/G3E and RZ/V2H(P) SoCs, which
require a different IRQ handler. Update the IRQ request code to use the
handler from the OF data.

Add `enable_interrupts` and `disable_interrupts` function pointers to the
`rzg2l_cru_info` structure and pass them as part of the OF data. This
prepares for supporting RZ/G3E and RZ/V2H(P) SoCs, which require different
interrupt configurations.

Implement `rzg2l_cru_enable_interrupts()` and
`rzg2l_cru_disable_interrupts()` functions and update the code to use them
instead of directly writing to interrupt registers.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
 - Squashed patch 15 and 14
 - Collected tag

 .../platform/renesas/rzg2l-cru/rzg2l-core.c   |  5 ++++-
 .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  8 ++++++++
 .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 19 ++++++++++++++-----
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 7e94ae803967..302f792cb415 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -278,7 +278,7 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	ret = devm_request_irq(dev, irq, rzg2l_cru_irq, 0,
+	ret = devm_request_irq(dev, irq, cru->info->irq_handler, 0,
 			       KBUILD_MODNAME, cru);
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to request irq\n");
@@ -359,6 +359,9 @@ static const struct rzg2l_cru_info rzgl2_cru_info = {
 	.max_height = 4095,
 	.image_conv = ICnMC,
 	.regs = rzg2l_cru_regs,
+	.irq_handler = rzg2l_cru_irq,
+	.enable_interrupts = rzg2l_cru_enable_interrupts,
+	.disable_interrupts = rzg2l_cru_disable_interrupts,
 };
 
 static const struct of_device_id rzg2l_cru_of_id_table[] = {
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index ca156772b949..3f694044d8cd 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -34,6 +34,8 @@ enum rzg2l_csi2_pads {
 	RZG2L_CRU_IP_SOURCE,
 };
 
+struct rzg2l_cru_dev;
+
 /**
  * enum rzg2l_cru_dma_state - DMA states
  * @RZG2L_CRU_DMA_STOPPED:   No operation in progress
@@ -83,6 +85,9 @@ struct rzg2l_cru_info {
 	unsigned int max_height;
 	u16 image_conv;
 	const u16 *regs;
+	irqreturn_t (*irq_handler)(int irq, void *data);
+	void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
+	void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
 };
 
 /**
@@ -177,4 +182,7 @@ const struct rzg2l_cru_ip_format *rzg2l_cru_ip_code_to_fmt(unsigned int code);
 const struct rzg2l_cru_ip_format *rzg2l_cru_ip_format_to_fmt(u32 format);
 const struct rzg2l_cru_ip_format *rzg2l_cru_ip_index_to_fmt(u32 index);
 
+void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
+void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
+
 #endif
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 5033c8d98639..8995aa254c17 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -300,8 +300,7 @@ void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
 	spin_lock_irqsave(&cru->qlock, flags);
 
 	/* Disable and clear the interrupt */
-	rzg2l_cru_write(cru, CRUnIE, 0);
-	rzg2l_cru_write(cru, CRUnINTS, 0x001F0F0F);
+	cru->info->disable_interrupts(cru);
 
 	/* Stop the operation of image conversion */
 	rzg2l_cru_write(cru, ICnEN, 0);
@@ -393,6 +392,17 @@ static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)
 	return fd.entry[0].bus.csi2.vc;
 }
 
+void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
+{
+	rzg2l_cru_write(cru, CRUnIE, CRUnIE_EFE);
+}
+
+void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru)
+{
+	rzg2l_cru_write(cru, CRUnIE, 0);
+	rzg2l_cru_write(cru, CRUnINTS, 0x001f000f);
+}
+
 int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
 {
 	struct v4l2_mbus_framefmt *fmt = rzg2l_cru_ip_get_src_fmt(cru);
@@ -414,8 +424,7 @@ int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
 	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);
+	cru->info->disable_interrupts(cru);
 
 	/* Initialize the AXI master */
 	rzg2l_cru_initialize_axi(cru);
@@ -428,7 +437,7 @@ int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
 	}
 
 	/* Enable interrupt */
-	rzg2l_cru_write(cru, CRUnIE, CRUnIE_EFE);
+	cru->info->enable_interrupts(cru);
 
 	/* Enable image processing reception */
 	rzg2l_cru_write(cru, ICnEN, ICnEN_ICEN);
-- 
2.43.0

[PATCH v4 15/17] media: rzg2l-cru: Add function pointer to check if FIFO is empty

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add a `fifo_empty` function pointer to the `rzg2l_cru_info` structure and
pass it as part of the OF data. On RZ/G3E and RZ/V2H(P) SoCs, checking if
the FIFO is empty requires a different register configuration.

Implement `rzg2l_fifo_empty()` and update the code to use it from the
function pointer.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
 - Fixed return of rzg2l_fifo_empty() as suggested by LPinchart
 - Collected tag

 .../platform/renesas/rzg2l-cru/rzg2l-core.c   |  1 +
 .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  3 +++
 .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 23 +++++++++++++------
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 302f792cb415..e4fb3e12d6bf 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -362,6 +362,7 @@ static const struct rzg2l_cru_info rzgl2_cru_info = {
 	.irq_handler = rzg2l_cru_irq,
 	.enable_interrupts = rzg2l_cru_enable_interrupts,
 	.disable_interrupts = rzg2l_cru_disable_interrupts,
+	.fifo_empty = rzg2l_fifo_empty,
 };
 
 static const struct of_device_id rzg2l_cru_of_id_table[] = {
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index 3f694044d8cd..2e17bfef43ce 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -88,6 +88,7 @@ struct rzg2l_cru_info {
 	irqreturn_t (*irq_handler)(int irq, void *data);
 	void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
 	void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
+	bool (*fifo_empty)(struct rzg2l_cru_dev *cru);
 };
 
 /**
@@ -185,4 +186,6 @@ const struct rzg2l_cru_ip_format *rzg2l_cru_ip_index_to_fmt(u32 index);
 void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
 void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
 
+bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru);
+
 #endif
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 8995aa254c17..83d7baa07dc7 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -290,9 +290,23 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
 	return 0;
 }
 
-void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
+bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru)
 {
 	u32 amnfifopntr, amnfifopntr_w, amnfifopntr_r_y;
+
+	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)
+		return true;
+
+	return amnfifopntr_w == amnfifopntr_r_y;
+}
+
+void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
+{
 	unsigned int retries = 0;
 	unsigned long flags;
 	u32 icnms;
@@ -320,12 +334,7 @@ void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
 
 	/* 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)
+		if (cru->info->fifo_empty(cru))
 			break;
 
 		usleep_range(10, 20);
-- 
2.43.0

[PATCH v4 16/17] media: rzg2l-cru: Add function pointer to configure CSI

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add a `csi_setup` function pointer to the `rzg2l_cru_info` structure and
pass it as part of the OF data. On RZ/G3E and RZ/V2H(P) SoCs, additional
register configurations are required compared to the RZ/G2L SoC.

Modify `rzg2l_cru_csi2_setup()` to be referenced through this function
pointer and update the code to use it accordingly.

This change is in preparation for adding support for RZ/G3E and RZ/V2H(P)
SoCs.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
 - Collected tag

 drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c  | 1 +
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h   | 6 ++++++
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 8 ++++----
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index e4fb3e12d6bf..3ae0cd83af16 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -363,6 +363,7 @@ static const struct rzg2l_cru_info rzgl2_cru_info = {
 	.enable_interrupts = rzg2l_cru_enable_interrupts,
 	.disable_interrupts = rzg2l_cru_disable_interrupts,
 	.fifo_empty = rzg2l_fifo_empty,
+	.csi_setup = rzg2l_cru_csi2_setup,
 };
 
 static const struct of_device_id rzg2l_cru_of_id_table[] = {
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index 2e17bfef43ce..ccaba5220f1c 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -89,6 +89,9 @@ struct rzg2l_cru_info {
 	void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
 	void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
 	bool (*fifo_empty)(struct rzg2l_cru_dev *cru);
+	void (*csi_setup)(struct rzg2l_cru_dev *cru,
+			  const struct rzg2l_cru_ip_format *ip_fmt,
+			  u8 csi_vc);
 };
 
 /**
@@ -187,5 +190,8 @@ void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
 void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
 
 bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru);
+void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
+			  const struct rzg2l_cru_ip_format *ip_fmt,
+			  u8 csi_vc);
 
 #endif
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 83d7baa07dc7..a3c4e2a0bef6 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -242,9 +242,9 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
 	rzg2l_cru_write(cru, AMnAXIATTR, amnaxiattr);
 }
 
-static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
-				 const struct rzg2l_cru_ip_format *ip_fmt,
-				 u8 csi_vc)
+void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
+			  const struct rzg2l_cru_ip_format *ip_fmt,
+			  u8 csi_vc)
 {
 	const struct rzg2l_cru_info *info = cru->info;
 	u32 icnmc = ICnMC_INF(ip_fmt->datatype);
@@ -266,7 +266,7 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
 	const struct rzg2l_cru_ip_format *cru_ip_fmt;
 
 	cru_ip_fmt = rzg2l_cru_ip_code_to_fmt(ip_sd_fmt->code);
-	rzg2l_cru_csi2_setup(cru, cru_ip_fmt, csi_vc);
+	info->csi_setup(cru, cru_ip_fmt, csi_vc);
 
 	/* Output format */
 	cru_video_fmt = rzg2l_cru_ip_format_to_fmt(cru->format.pixelformat);
-- 
2.43.0

[PATCH v4 17/17] media: rzg2l-cru: Add support for RZ/G3E SoC

[Tommaso Merciai]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The CRU block on the Renesas RZ/G3E SoC is similar to the one found on
the Renesas RZ/G2L SoC, with the following differences:

- Additional registers rzg3e_cru_regs.
- A different irq handler rzg3e_cru_irq.
- A different rzg3e_cru_csi2_setup.
- A different max input width.
- Additional stride register.

Introduce rzg3e_cru_info struct to handle differences between RZ/G2L
and RZ/G3E and related RZ/G3E functions:

 - rzg3e_cru_enable_interrupts()
 - rzg3e_cru_enable_interrupts()
 - rz3e_fifo_empty()
 - rzg3e_cru_csi2_setup()
 - rzg3e_cru_get_current_slot()

Add then support for the RZ/G3E SoC CRU block with the new compatible
string "renesas,r9a09g047-cru".

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
 - Use dma_addr_t with buf_addr directly instead of splitting that into
   cru->mem_banks (high and low address) as suggested by LPinchart.
 - Moved and improved stride adjustment into rzg2l_cru_format_align()
   as suggested by LPinchart.
 - Use csi_vc into rzg3e_cru_csi2_setup() instead of cru->svc_channel as
   suggested by LPinchart
 - Added has_stride field to handle soc differences as suggested by LPinchart.

Changes since v3:
 - Fixed kernel test robot warnings from rzg3e_cru_get_current_slot() and
   rzg3e_cru_irq()

 .../platform/renesas/rzg2l-cru/rzg2l-core.c   |  62 +++++++
 .../renesas/rzg2l-cru/rzg2l-cru-regs.h        |  25 +++
 .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  13 ++
 .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 171 +++++++++++++++++-
 4 files changed, 270 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 3ae0cd83af16..1356be14eda8 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -290,6 +290,12 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
 		return ret;
 
 	cru->num_buf = RZG2L_CRU_HW_BUFFER_DEFAULT;
+	cru->buf_addr = devm_kmalloc_array(dev, cru->num_buf,
+					   sizeof(dma_addr_t), GFP_KERNEL);
+	if (!cru->buf_addr)
+		return dev_err_probe(dev, -ENOMEM,
+				     "Failed to init buf addr\n");
+
 	pm_suspend_ignore_children(dev, true);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
@@ -321,6 +327,58 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
 	rzg2l_cru_dma_unregister(cru);
 }
 
+static const u16 rzg3e_cru_regs[] = {
+	[CRUnCTRL] = 0x0,
+	[CRUnIE] = 0x4,
+	[CRUnIE2] = 0x8,
+	[CRUnINTS] = 0xc,
+	[CRUnINTS2] = 0x10,
+	[CRUnRST] = 0x18,
+	[AMnMB1ADDRL] = 0x40,
+	[AMnMB1ADDRH] = 0x44,
+	[AMnMB2ADDRL] = 0x48,
+	[AMnMB2ADDRH] = 0x4c,
+	[AMnMB3ADDRL] = 0x50,
+	[AMnMB3ADDRH] = 0x54,
+	[AMnMB4ADDRL] = 0x58,
+	[AMnMB4ADDRH] = 0x5c,
+	[AMnMB5ADDRL] = 0x60,
+	[AMnMB5ADDRH] = 0x64,
+	[AMnMB6ADDRL] = 0x68,
+	[AMnMB6ADDRH] = 0x6c,
+	[AMnMB7ADDRL] = 0x70,
+	[AMnMB7ADDRH] = 0x74,
+	[AMnMB8ADDRL] = 0x78,
+	[AMnMB8ADDRH] = 0x7c,
+	[AMnMBVALID] = 0x88,
+	[AMnMADRSL] = 0x8c,
+	[AMnMADRSH] = 0x90,
+	[AMnAXIATTR] = 0xec,
+	[AMnFIFOPNTR] = 0xf8,
+	[AMnAXISTP] = 0x110,
+	[AMnAXISTPACK] = 0x114,
+	[AMnIS] = 0x128,
+	[ICnEN] = 0x1f0,
+	[ICnSVCNUM] = 0x1f8,
+	[ICnSVC] = 0x1fc,
+	[ICnIPMC_C0] = 0x200,
+	[ICnMS] = 0x2d8,
+	[ICnDMR] = 0x304,
+};
+
+static const struct rzg2l_cru_info rzg3e_cru_info = {
+	.max_width = 4095,
+	.max_height = 4095,
+	.image_conv = ICnIPMC_C0,
+	.has_stride = true,
+	.regs = rzg3e_cru_regs,
+	.irq_handler = rzg3e_cru_irq,
+	.enable_interrupts = rzg3e_cru_enable_interrupts,
+	.disable_interrupts = rzg3e_cru_disable_interrupts,
+	.fifo_empty = rz3e_fifo_empty,
+	.csi_setup = rzg3e_cru_csi2_setup,
+};
+
 static const u16 rzg2l_cru_regs[] = {
 	[CRUnCTRL] = 0x0,
 	[CRUnIE] = 0x4,
@@ -367,6 +425,10 @@ static const struct rzg2l_cru_info rzgl2_cru_info = {
 };
 
 static const struct of_device_id rzg2l_cru_of_id_table[] = {
+	{
+		.compatible = "renesas,r9a09g047-cru",
+		.data = &rzg3e_cru_info,
+	},
 	{
 		.compatible = "renesas,rzg2l-cru",
 		.data = &rzgl2_cru_info,
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
index 86c320286246..52324b076674 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
@@ -14,8 +14,13 @@
 
 #define CRUnIE_EFE			BIT(17)
 
+#define CRUnIE2_FSxE(x)			BIT(((x) * 3))
+#define CRUnIE2_FExE(x)			BIT(((x) * 3) + 1)
+
 #define CRUnINTS_SFS			BIT(16)
 
+#define CRUnINTS2_FSxS(x)		BIT(((x) * 3))
+
 #define CRUnRST_VRESETN			BIT(0)
 
 /* Memory Bank Base Address (Lower) Register for CRU Image Data */
@@ -32,7 +37,14 @@
 #define AMnAXIATTR_AXILEN		(0xf)
 
 #define AMnFIFOPNTR_FIFOWPNTR		GENMASK(7, 0)
+#define AMnFIFOPNTR_FIFOWPNTR_B0	AMnFIFOPNTR_FIFOWPNTR
+#define AMnFIFOPNTR_FIFOWPNTR_B1	GENMASK(15, 8)
 #define AMnFIFOPNTR_FIFORPNTR_Y		GENMASK(23, 16)
+#define AMnFIFOPNTR_FIFORPNTR_B0	AMnFIFOPNTR_FIFORPNTR_Y
+#define AMnFIFOPNTR_FIFORPNTR_B1	GENMASK(31, 24)
+
+#define AMnIS_IS_MASK			GENMASK(14, 7)
+#define AMnIS_IS(x)			((x) << 7)
 
 #define AMnAXISTP_AXI_STOP		BIT(0)
 
@@ -40,6 +52,11 @@
 
 #define ICnEN_ICEN			BIT(0)
 
+#define ICnSVC_SVC0(x)			(x)
+#define ICnSVC_SVC1(x)			((x) << 4)
+#define ICnSVC_SVC2(x)			((x) << 8)
+#define ICnSVC_SVC3(x)			((x) << 12)
+
 #define ICnMC_CSCTHR			BIT(5)
 #define ICnMC_INF(x)			((x) << 16)
 #define ICnMC_VCSEL(x)			((x) << 22)
@@ -52,7 +69,9 @@
 enum rzg2l_cru_common_regs {
 	CRUnCTRL,	/* CRU Control */
 	CRUnIE,		/* CRU Interrupt Enable */
+	CRUnIE2,	/* CRU Interrupt Enable(2) */
 	CRUnINTS,	/* CRU Interrupt Status */
+	CRUnINTS2,	/* CRU Interrupt Status(2) */
 	CRUnRST, 	/* CRU Reset */
 	AMnMB1ADDRL,	/* Bank 1 Address (Lower) for CRU Image Data */
 	AMnMB1ADDRH,	/* Bank 1 Address (Higher) for CRU Image Data */
@@ -72,12 +91,18 @@ enum rzg2l_cru_common_regs {
 	AMnMB8ADDRH,    /* Bank 8 Address (Higher) for CRU Image Data */
 	AMnMBVALID,	/* Memory Bank Enable for CRU Image Data */
 	AMnMBS,		/* Memory Bank Status for CRU Image Data */
+	AMnMADRSL,	/* VD Memory Address Lower Status Register */
+	AMnMADRSH,	/* VD Memory Address Higher Status Register */
 	AMnAXIATTR,	/* AXI Master Transfer Setting Register for CRU Image Data */
 	AMnFIFOPNTR,	/* AXI Master FIFO Pointer for CRU Image Data */
 	AMnAXISTP,	/* AXI Master Transfer Stop for CRU Image Data */
 	AMnAXISTPACK,	/* AXI Master Transfer Stop Status for CRU Image Data */
+	AMnIS,		/* Image Stride Setting Register */
 	ICnEN,		/* CRU Image Processing Enable */
+	ICnSVCNUM,	/* CRU SVC Number Register */
+	ICnSVC,		/* CRU VC Select Register */
 	ICnMC,		/* CRU Image Processing Main Control */
+	ICnIPMC_C0,	/* CRU Image Converter Main Control 0 */
 	ICnMS,		/* CRU Module Status */
 	ICnDMR,		/* CRU Data Output Mode */
 	RZG2L_CRU_MAX_REG,
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index ccaba5220f1c..d68d83340686 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -85,6 +85,7 @@ struct rzg2l_cru_info {
 	unsigned int max_height;
 	u16 image_conv;
 	const u16 *regs;
+	bool has_stride;
 	irqreturn_t (*irq_handler)(int irq, void *data);
 	void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
 	void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
@@ -108,6 +109,8 @@ struct rzg2l_cru_info {
  * @vdev:		V4L2 video device associated with CRU
  * @v4l2_dev:		V4L2 device
  * @num_buf:		Holds the current number of buffers enabled
+ * @svc_channel:	SVC0/1/2/3 to use for RZ/G3E
+ * @buf_addr:		Memory addresses where current video data is written.
  * @notifier:		V4L2 asynchronous subdevs notifier
  *
  * @ip:			Image processing subdev info
@@ -144,6 +147,9 @@ struct rzg2l_cru_dev {
 	struct v4l2_device v4l2_dev;
 	u8 num_buf;
 
+	u8 svc_channel;
+	dma_addr_t *buf_addr;
+
 	struct v4l2_async_notifier notifier;
 
 	struct rzg2l_cru_ip ip;
@@ -175,6 +181,7 @@ void rzg2l_cru_dma_unregister(struct rzg2l_cru_dev *cru);
 int rzg2l_cru_video_register(struct rzg2l_cru_dev *cru);
 void rzg2l_cru_video_unregister(struct rzg2l_cru_dev *cru);
 irqreturn_t rzg2l_cru_irq(int irq, void *data);
+irqreturn_t rzg3e_cru_irq(int irq, void *data);
 
 const struct v4l2_format_info *rzg2l_cru_format_from_pixel(u32 format);
 
@@ -188,10 +195,16 @@ const struct rzg2l_cru_ip_format *rzg2l_cru_ip_index_to_fmt(u32 index);
 
 void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
 void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
+void rzg3e_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
+void rzg3e_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
 
 bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru);
+bool rz3e_fifo_empty(struct rzg2l_cru_dev *cru);
 void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
 			  const struct rzg2l_cru_ip_format *ip_fmt,
 			  u8 csi_vc);
+void rzg3e_cru_csi2_setup(struct rzg2l_cru_dev *cru,
+			  const struct rzg2l_cru_ip_format *ip_fmt,
+			  u8 csi_vc);
 
 #endif
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index a3c4e2a0bef6..518420ad5586 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -31,6 +31,9 @@
 #define RZG2L_CRU_DEFAULT_FIELD		V4L2_FIELD_NONE
 #define RZG2L_CRU_DEFAULT_COLORSPACE	V4L2_COLORSPACE_SRGB
 
+#define RZG2L_CRU_STRIDE_MAX		32640
+#define RZG2L_CRU_STRIDE_ALIGN		128
+
 struct rzg2l_cru_buffer {
 	struct vb2_v4l2_buffer vb;
 	struct list_head list;
@@ -184,6 +187,8 @@ static void rzg2l_cru_set_slot_addr(struct rzg2l_cru_dev *cru,
 	/* Currently, we just use the buffer in 32 bits address */
 	rzg2l_cru_write(cru, AMnMBxADDRL(slot), addr);
 	rzg2l_cru_write(cru, AMnMBxADDRH(slot), 0);
+
+	cru->buf_addr[slot] = addr;
 }
 
 /*
@@ -224,6 +229,7 @@ static void rzg2l_cru_fill_hw_slot(struct rzg2l_cru_dev *cru, int slot)
 
 static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
 {
+	const struct rzg2l_cru_info *info = cru->info;
 	unsigned int slot;
 	u32 amnaxiattr;
 
@@ -236,12 +242,39 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
 	for (slot = 0; slot < cru->num_buf; slot++)
 		rzg2l_cru_fill_hw_slot(cru, slot);
 
+	if (info->has_stride) {
+		u32 stride = cru->format.bytesperline;
+		u32 amnis;
+
+		stride /= RZG2L_CRU_STRIDE_ALIGN;
+		amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
+		rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
+	}
+
 	/* Set AXI burst max length to recommended setting */
 	amnaxiattr = rzg2l_cru_read(cru, AMnAXIATTR) & ~AMnAXIATTR_AXILEN_MASK;
 	amnaxiattr |= AMnAXIATTR_AXILEN;
 	rzg2l_cru_write(cru, AMnAXIATTR, amnaxiattr);
 }
 
+void rzg3e_cru_csi2_setup(struct rzg2l_cru_dev *cru,
+			  const struct rzg2l_cru_ip_format *ip_fmt,
+			  u8 csi_vc)
+{
+	const struct rzg2l_cru_info *info = cru->info;
+	u32 icnmc = ICnMC_INF(ip_fmt->datatype);
+
+	icnmc |= (rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK);
+
+	/* Set virtual channel CSI2 */
+	icnmc |= ICnMC_VCSEL(csi_vc);
+
+	rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
+	rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
+			ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
+	rzg2l_cru_write(cru, info->image_conv, icnmc);
+}
+
 void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
 			  const struct rzg2l_cru_ip_format *ip_fmt,
 			  u8 csi_vc)
@@ -290,6 +323,19 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
 	return 0;
 }
 
+bool rz3e_fifo_empty(struct rzg2l_cru_dev *cru)
+{
+	u32 amnfifopntr = rzg2l_cru_read(cru, AMnFIFOPNTR);
+
+	if ((((amnfifopntr & AMnFIFOPNTR_FIFORPNTR_B1) >> 24) ==
+	     ((amnfifopntr & AMnFIFOPNTR_FIFOWPNTR_B1) >> 8)) &&
+	    (((amnfifopntr & AMnFIFOPNTR_FIFORPNTR_B0) >> 16) ==
+	     (amnfifopntr & AMnFIFOPNTR_FIFOWPNTR_B0)))
+		return true;
+
+	return false;
+}
+
 bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru)
 {
 	u32 amnfifopntr, amnfifopntr_w, amnfifopntr_r_y;
@@ -401,6 +447,20 @@ static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)
 	return fd.entry[0].bus.csi2.vc;
 }
 
+void rzg3e_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
+{
+	rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FSxE(cru->svc_channel));
+	rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FExE(cru->svc_channel));
+}
+
+void rzg3e_cru_disable_interrupts(struct rzg2l_cru_dev *cru)
+{
+	rzg2l_cru_write(cru, CRUnIE, 0);
+	rzg2l_cru_write(cru, CRUnIE2, 0);
+	rzg2l_cru_write(cru, CRUnINTS, rzg2l_cru_read(cru, CRUnINTS));
+	rzg2l_cru_write(cru, CRUnINTS2, rzg2l_cru_read(cru, CRUnINTS2));
+}
+
 void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
 {
 	rzg2l_cru_write(cru, CRUnIE, CRUnIE_EFE);
@@ -423,6 +483,7 @@ int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
 	if (ret < 0)
 		return ret;
 	csi_vc = ret;
+	cru->svc_channel = csi_vc;
 
 	spin_lock_irqsave(&cru->qlock, flags);
 
@@ -601,6 +662,107 @@ irqreturn_t rzg2l_cru_irq(int irq, void *data)
 	return IRQ_RETVAL(handled);
 }
 
+static int rzg3e_cru_get_current_slot(struct rzg2l_cru_dev *cru)
+{
+	u64 amnmadrs;
+	unsigned int slot;
+
+	/*
+	 * When AMnMADRSL is read, AMnMADRSH of the higher-order
+	 * address also latches the address.
+	 *
+	 * AMnMADRSH must be read after AMnMADRSL has been read.
+	 */
+	amnmadrs = rzg2l_cru_read(cru, AMnMADRSL);
+	amnmadrs |= ((u64)rzg2l_cru_read(cru, AMnMADRSH) << 32);
+
+	/* Ensure amnmadrs is within this buffer range */
+	for (slot = 0; slot < cru->num_buf; slot++)
+		if (amnmadrs >= cru->buf_addr[slot] &&
+		    amnmadrs < cru->buf_addr[slot] + cru->format.sizeimage)
+			return slot;
+
+	dev_err(cru->dev, "Invalid MB address 0x%llx (out of range)\n", amnmadrs);
+	return -EINVAL;
+}
+
+irqreturn_t rzg3e_cru_irq(int irq, void *data)
+{
+	struct rzg2l_cru_dev *cru = data;
+	unsigned int handled = 0;
+	unsigned long flags;
+	u32 irq_status;
+	int slot;
+
+	spin_lock_irqsave(&cru->qlock, flags);
+	irq_status = rzg2l_cru_read(cru, CRUnINTS2);
+	if (!irq_status)
+		goto done;
+
+	dev_dbg(cru->dev, "CRUnINTS2 0x%x\n", irq_status);
+
+	handled = 1;
+
+	rzg2l_cru_write(cru, CRUnINTS2, rzg2l_cru_read(cru, CRUnINTS2));
+
+	/* 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;
+	}
+
+	if (cru->state == RZG2L_CRU_DMA_STOPPING) {
+		if (irq_status & CRUnINTS2_FSxS(0) ||
+		    irq_status & CRUnINTS2_FSxS(1) ||
+		    irq_status & CRUnINTS2_FSxS(2) ||
+		    irq_status & CRUnINTS2_FSxS(3))
+			dev_dbg(cru->dev, "IRQ while state stopping\n");
+		goto done;
+	}
+
+	slot = rzg3e_cru_get_current_slot(cru);
+	if (slot < 0)
+		goto done;
+
+	dev_dbg(cru->dev, "Current written slot: %d\n", slot);
+	cru->buf_addr[slot] = 0;
+
+	/*
+	 * 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);
+}
+
 static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count)
 {
 	struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vq);
@@ -782,7 +944,14 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
 	v4l_bound_align_image(&pix->width, 320, info->max_width, 1,
 			      &pix->height, 240, info->max_height, 2, 0);
 
-	pix->bytesperline = pix->width * fmt->bpp;
+	if (info->has_stride) {
+		u32 stride = clamp(pix->bytesperline, pix->width * fmt->bpp,
+				   RZG2L_CRU_STRIDE_MAX);
+		pix->bytesperline = round_up(stride, RZG2L_CRU_STRIDE_ALIGN);
+	} else {
+		pix->bytesperline = pix->width * fmt->bpp;
+	}
+
 	pix->sizeimage = pix->bytesperline * pix->height;
 
 	dev_dbg(cru->dev, "Format %ux%u bpl: %u size: %u\n",
-- 
2.43.0

Re: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support

[kernel test robot]

Hi Tommaso,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.14-rc5 next-20250307]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Tommaso-Merciai/media-dt-bindings-renesas-rzg2l-csi2-Document-Renesas-RZ-V2H-P-SoC/20250304-002513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250303160834.3493507-12-tommaso.merciai.xr%40bp.renesas.com
patch subject: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support
config: sh-randconfig-r112-20250308 (https://download.01.org/0day-ci/archive/20250308/202503081652.V24IMXGL-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250308/202503081652.V24IMXGL-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503081652.V24IMXGL-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from <command-line>:
   drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c: In function '__rzg2l_cru_write_constant':
>> include/linux/compiler_types.h:542:45: error: call to '__compiletime_assert_304' declared with attribute error: BUILD_BUG_ON failed: offset >= RZG2L_CRU_MAX_REG
     542 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |                                             ^
   include/linux/compiler_types.h:523:25: note: in definition of macro '__compiletime_assert'
     523 |                         prefix ## suffix();                             \
         |                         ^~~~~~
   include/linux/compiler_types.h:542:9: note: in expansion of macro '_compiletime_assert'
     542 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
      50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
         |         ^~~~~~~~~~~~~~~~
   drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c:80:9: note: in expansion of macro 'BUILD_BUG_ON'
      80 |         BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
         |         ^~~~~~~~~~~~
--
   In file included from <command-line>:
   rzg2l-video.c: In function '__rzg2l_cru_write_constant':
>> include/linux/compiler_types.h:542:45: error: call to '__compiletime_assert_304' declared with attribute error: BUILD_BUG_ON failed: offset >= RZG2L_CRU_MAX_REG
     542 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |                                             ^
   include/linux/compiler_types.h:523:25: note: in definition of macro '__compiletime_assert'
     523 |                         prefix ## suffix();                             \
         |                         ^~~~~~
   include/linux/compiler_types.h:542:9: note: in expansion of macro '_compiletime_assert'
     542 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
      50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
         |         ^~~~~~~~~~~~~~~~
   rzg2l-video.c:80:9: note: in expansion of macro 'BUILD_BUG_ON'
      80 |         BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
         |         ^~~~~~~~~~~~


vim +/__compiletime_assert_304 +542 include/linux/compiler_types.h

eb5c2d4b45e3d2d Will Deacon 2020-07-21  528  
eb5c2d4b45e3d2d Will Deacon 2020-07-21  529  #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2d Will Deacon 2020-07-21  530  	__compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2d Will Deacon 2020-07-21  531  
eb5c2d4b45e3d2d Will Deacon 2020-07-21  532  /**
eb5c2d4b45e3d2d Will Deacon 2020-07-21  533   * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2d Will Deacon 2020-07-21  534   * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2d Will Deacon 2020-07-21  535   * @msg:       a message to emit if condition is false
eb5c2d4b45e3d2d Will Deacon 2020-07-21  536   *
eb5c2d4b45e3d2d Will Deacon 2020-07-21  537   * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2d Will Deacon 2020-07-21  538   * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2d Will Deacon 2020-07-21  539   * compiler has support to do so.
eb5c2d4b45e3d2d Will Deacon 2020-07-21  540   */
eb5c2d4b45e3d2d Will Deacon 2020-07-21  541  #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2d Will Deacon 2020-07-21 @542  	_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2d Will Deacon 2020-07-21  543  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

RE: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support

[Biju Das]

Hi Tommaso,

> -----Original Message-----
> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> Sent: 03 March 2025 16:08
> Subject: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support
> 
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a CRU-IP that is mostly identical
> to RZ/G2L but with different register offsets and additional registers. Introduce a flexible register
> mapping mechanism to handle these variations.
> 
> Define the `rzg2l_cru_info` structure to store register mappings and pass it as part of the OF match
> data. Update the read/write functions to check out-of-bound accesses and use indexed register offsets
> from `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> Changes since v2:
>  - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
>    accesses as suggested by LPinchart.
>  - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
>  - Update commit body
> 
>  .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 46 ++++++++++++-
>  .../renesas/rzg2l-cru/rzg2l-cru-regs.h        | 66 ++++++++++---------
>  .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  4 ++
>  .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 58 ++++++++++++++--
>  4 files changed, 139 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> index eed9d2bd0841..abc2a979833a 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> @@ -22,6 +22,7 @@
>  #include <media/v4l2-mc.h>
> 
>  #include "rzg2l-cru.h"
> +#include "rzg2l-cru-regs.h"
> 
>  static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n)  { @@ -269,6
> +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> 
>  	cru->dev = dev;
>  	cru->info = of_device_get_match_data(dev);
> +	if (!cru->info)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Failed to get OF match data\n");
> 
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0)
> @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
>  	rzg2l_cru_dma_unregister(cru);
>  }
> 
> +static const u16 rzg2l_cru_regs[] = {
> +	[CRUnCTRL] = 0x0,
> +	[CRUnIE] = 0x4,
> +	[CRUnINTS] = 0x8,
> +	[CRUnRST] = 0xc,
> +	[AMnMB1ADDRL] = 0x100,
> +	[AMnMB1ADDRH] = 0x104,
> +	[AMnMB2ADDRL] = 0x108,
> +	[AMnMB2ADDRH] = 0x10c,
> +	[AMnMB3ADDRL] = 0x110,
> +	[AMnMB3ADDRH] = 0x114,
> +	[AMnMB4ADDRL] = 0x118,
> +	[AMnMB4ADDRH] = 0x11c,
> +	[AMnMB5ADDRL] = 0x120,
> +	[AMnMB5ADDRH] = 0x124,
> +	[AMnMB6ADDRL] = 0x128,
> +	[AMnMB6ADDRH] = 0x12c,
> +	[AMnMB7ADDRL] = 0x130,
> +	[AMnMB7ADDRH] = 0x134,
> +	[AMnMB8ADDRL] = 0x138,
> +	[AMnMB8ADDRH] = 0x13c,
> +	[AMnMBVALID] = 0x148,
> +	[AMnMBS] = 0x14c,
> +	[AMnAXIATTR] = 0x158,
> +	[AMnFIFOPNTR] = 0x168,
> +	[AMnAXISTP] = 0x174,
> +	[AMnAXISTPACK] = 0x178,
> +	[ICnEN] = 0x200,
> +	[ICnMC] = 0x208,
> +	[ICnMS] = 0x254,
> +	[ICnDMR] = 0x26c,
> +};
> +
> +static const struct rzg2l_cru_info rzgl2_cru_info = {
> +	.regs = rzg2l_cru_regs,
> +};
> +
>  static const struct of_device_id rzg2l_cru_of_id_table[] = {
> -	{ .compatible = "renesas,rzg2l-cru", },
> +	{
> +		.compatible = "renesas,rzg2l-cru",
> +		.data = &rzgl2_cru_info,
> +	},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, rzg2l_cru_of_id_table); diff --git a/drivers/media/platform/renesas/rzg2l-
> cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> index 1c9f22118a5d..86c320286246 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> @@ -10,71 +10,77 @@
> 
>  /* 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_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))
> +#define AMnMBxADDRL(x)			(AMnMB1ADDRL + (x) * 2)
> 
>  /* Memory Bank Base Address (Higher) Register for CRU Image Data */
> -#define AMnMBxADDRH(x)			(0x104 + ((x) * 8))
> +#define AMnMBxADDRH(x)			(AMnMB1ADDRH + (x) * 2)
> 
> -/* 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 Transfer Setting Register for CRU Image Data */
> -#define AMnAXIATTR			0x158
>  #define AMnAXIATTR_AXILEN_MASK		GENMASK(3, 0)
>  #define AMnAXIATTR_AXILEN		(0xf)
> 
> -/* 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(x)			((x) << 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)
> 
> +enum rzg2l_cru_common_regs {
> +	CRUnCTRL,	/* CRU Control */
> +	CRUnIE,		/* CRU Interrupt Enable */
> +	CRUnINTS,	/* CRU Interrupt Status */
> +	CRUnRST, 	/* CRU Reset */
> +	AMnMB1ADDRL,	/* Bank 1 Address (Lower) for CRU Image Data */
> +	AMnMB1ADDRH,	/* Bank 1 Address (Higher) for CRU Image Data */
> +	AMnMB2ADDRL,    /* Bank 2 Address (Lower) for CRU Image Data */
> +	AMnMB2ADDRH,    /* Bank 2 Address (Higher) for CRU Image Data */
> +	AMnMB3ADDRL,    /* Bank 3 Address (Lower) for CRU Image Data */
> +	AMnMB3ADDRH,    /* Bank 3 Address (Higher) for CRU Image Data */
> +	AMnMB4ADDRL,    /* Bank 4 Address (Lower) for CRU Image Data */
> +	AMnMB4ADDRH,    /* Bank 4 Address (Higher) for CRU Image Data */
> +	AMnMB5ADDRL,    /* Bank 5 Address (Lower) for CRU Image Data */
> +	AMnMB5ADDRH,    /* Bank 5 Address (Higher) for CRU Image Data */
> +	AMnMB6ADDRL,    /* Bank 6 Address (Lower) for CRU Image Data */
> +	AMnMB6ADDRH,    /* Bank 6 Address (Higher) for CRU Image Data */
> +	AMnMB7ADDRL,    /* Bank 7 Address (Lower) for CRU Image Data */
> +	AMnMB7ADDRH,    /* Bank 7 Address (Higher) for CRU Image Data */
> +	AMnMB8ADDRL,    /* Bank 8 Address (Lower) for CRU Image Data */
> +	AMnMB8ADDRH,    /* Bank 8 Address (Higher) for CRU Image Data */
> +	AMnMBVALID,	/* Memory Bank Enable for CRU Image Data */
> +	AMnMBS,		/* Memory Bank Status for CRU Image Data */
> +	AMnAXIATTR,	/* AXI Master Transfer Setting Register for CRU Image Data */
> +	AMnFIFOPNTR,	/* AXI Master FIFO Pointer for CRU Image Data */
> +	AMnAXISTP,	/* AXI Master Transfer Stop for CRU Image Data */
> +	AMnAXISTPACK,	/* AXI Master Transfer Stop Status for CRU Image Data */
> +	ICnEN,		/* CRU Image Processing Enable */
> +	ICnMC,		/* CRU Image Processing Main Control */
> +	ICnMS,		/* CRU Module Status */
> +	ICnDMR,		/* CRU Data Output Mode */
> +	RZG2L_CRU_MAX_REG,
> +};
> +
>  #endif /* __RZG2L_CRU_REGS_H__ */
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> index 8b898ce05b84..00c3f7458e20 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> @@ -80,6 +80,10 @@ struct rzg2l_cru_ip_format {
>  	bool yuv;
>  };
> 
> +struct rzg2l_cru_info {
> +	const u16 *regs;
> +};
> +
>  /**
>   * struct rzg2l_cru_dev - Renesas CRU device structure
>   * @dev:		(OF) device
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index cd69c8a686d3..792f0df51a4b 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -42,16 +42,66 @@ struct rzg2l_cru_buffer {
>  /* -----------------------------------------------------------------------------
>   * DMA operations
>   */
> -static void rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> +static void __rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset,
> +u32 value)
>  {
> -	iowrite32(value, cru->base + offset);
> +	const u16 *regs = cru->info->regs;
> +
> +	/*
> +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> +	 * rest of the registers have valid offset being set in cru->info->regs.
> +	 */
> +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> +		return;
> +
> +	iowrite32(value, cru->base + regs[offset]); }
> +
> +static u32 __rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset) {
> +	const u16 *regs = cru->info->regs;
> +
> +	/*
> +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> +	 * rest of the registers have valid offset being set in cru->info->regs.
> +	 */
> +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> +		return 0;
> +
> +	return ioread32(cru->base + regs[offset]);
>  }
> 
> -static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
> +static inline void
> +__rzg2l_cru_write_constant(struct rzg2l_cru_dev *cru, u32 offset, u32
> +value)
>  {
> -	return ioread32(cru->base + offset);
> +	const u16 *regs = cru->info->regs;
> +
> +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> +
> +	iowrite32(value, cru->base + regs[offset]);
Do you need this code as the purpose is to test compile time constant and
It won't execute at run time?


>  }
> 
> +static inline u32
> +__rzg2l_cru_read_constant(struct rzg2l_cru_dev *cru, u32 offset) {
> +	const u16 *regs = cru->info->regs;
> +
> +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> +
> +	return ioread32(cru->base + regs[offset]); 

Do you need this code as the purpose is to test compile time constant and
It won't execute at run time?

Not sure, maybe adding an entry with MAX_ID in LUT,
that will avoid buffer overflows and you can take out
All out of bound array checks?

Cheers,
Biju

}
> +
> +#define rzg2l_cru_write(cru, offset, value) \
> +	(__builtin_constant_p(offset) ? \
> +	 __rzg2l_cru_write_constant(cru, offset, value) : \
> +	 __rzg2l_cru_write(cru, offset, value))
> +
> +#define rzg2l_cru_read(cru, offset) \
> +	(__builtin_constant_p(offset) ? \
> +	 __rzg2l_cru_read_constant(cru, offset) : \
> +	 __rzg2l_cru_read(cru, offset))
> +
>  /* Need to hold qlock before calling */  static void return_unused_buffers(struct rzg2l_cru_dev *cru,
>  				  enum vb2_buffer_state state)
> --
> 2.43.0

Re: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support

[Tommaso Merciai]

Hi Biju,
Thanks for your comment.

On Wed, Mar 12, 2025 at 01:37:25PM +0000, Biju Das wrote:
> Hi Tommaso,
> 
> > -----Original Message-----
> > From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > Sent: 03 March 2025 16:08
> > Subject: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support
> > 
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > 
> > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a CRU-IP that is mostly identical
> > to RZ/G2L but with different register offsets and additional registers. Introduce a flexible register
> > mapping mechanism to handle these variations.
> > 
> > Define the `rzg2l_cru_info` structure to store register mappings and pass it as part of the OF match
> > data. Update the read/write functions to check out-of-bound accesses and use indexed register offsets
> > from `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
> > 
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > Changes since v2:
> >  - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> >    accesses as suggested by LPinchart.
> >  - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> >  - Update commit body
> > 
> >  .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 46 ++++++++++++-
> >  .../renesas/rzg2l-cru/rzg2l-cru-regs.h        | 66 ++++++++++---------
> >  .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  4 ++
> >  .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 58 ++++++++++++++--
> >  4 files changed, 139 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > index eed9d2bd0841..abc2a979833a 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > @@ -22,6 +22,7 @@
> >  #include <media/v4l2-mc.h>
> > 
> >  #include "rzg2l-cru.h"
> > +#include "rzg2l-cru-regs.h"
> > 
> >  static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n)  { @@ -269,6
> > +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > 
> >  	cru->dev = dev;
> >  	cru->info = of_device_get_match_data(dev);
> > +	if (!cru->info)
> > +		return dev_err_probe(dev, -EINVAL,
> > +				     "Failed to get OF match data\n");
> > 
> >  	irq = platform_get_irq(pdev, 0);
> >  	if (irq < 0)
> > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> >  	rzg2l_cru_dma_unregister(cru);
> >  }
> > 
> > +static const u16 rzg2l_cru_regs[] = {
> > +	[CRUnCTRL] = 0x0,
> > +	[CRUnIE] = 0x4,
> > +	[CRUnINTS] = 0x8,
> > +	[CRUnRST] = 0xc,
> > +	[AMnMB1ADDRL] = 0x100,
> > +	[AMnMB1ADDRH] = 0x104,
> > +	[AMnMB2ADDRL] = 0x108,
> > +	[AMnMB2ADDRH] = 0x10c,
> > +	[AMnMB3ADDRL] = 0x110,
> > +	[AMnMB3ADDRH] = 0x114,
> > +	[AMnMB4ADDRL] = 0x118,
> > +	[AMnMB4ADDRH] = 0x11c,
> > +	[AMnMB5ADDRL] = 0x120,
> > +	[AMnMB5ADDRH] = 0x124,
> > +	[AMnMB6ADDRL] = 0x128,
> > +	[AMnMB6ADDRH] = 0x12c,
> > +	[AMnMB7ADDRL] = 0x130,
> > +	[AMnMB7ADDRH] = 0x134,
> > +	[AMnMB8ADDRL] = 0x138,
> > +	[AMnMB8ADDRH] = 0x13c,
> > +	[AMnMBVALID] = 0x148,
> > +	[AMnMBS] = 0x14c,
> > +	[AMnAXIATTR] = 0x158,
> > +	[AMnFIFOPNTR] = 0x168,
> > +	[AMnAXISTP] = 0x174,
> > +	[AMnAXISTPACK] = 0x178,
> > +	[ICnEN] = 0x200,
> > +	[ICnMC] = 0x208,
> > +	[ICnMS] = 0x254,
> > +	[ICnDMR] = 0x26c,
> > +};
> > +
> > +static const struct rzg2l_cru_info rzgl2_cru_info = {
> > +	.regs = rzg2l_cru_regs,
> > +};
> > +
> >  static const struct of_device_id rzg2l_cru_of_id_table[] = {
> > -	{ .compatible = "renesas,rzg2l-cru", },
> > +	{
> > +		.compatible = "renesas,rzg2l-cru",
> > +		.data = &rzgl2_cru_info,
> > +	},
> >  	{ /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, rzg2l_cru_of_id_table); diff --git a/drivers/media/platform/renesas/rzg2l-
> > cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > index 1c9f22118a5d..86c320286246 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > @@ -10,71 +10,77 @@
> > 
> >  /* 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_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))
> > +#define AMnMBxADDRL(x)			(AMnMB1ADDRL + (x) * 2)
> > 
> >  /* Memory Bank Base Address (Higher) Register for CRU Image Data */
> > -#define AMnMBxADDRH(x)			(0x104 + ((x) * 8))
> > +#define AMnMBxADDRH(x)			(AMnMB1ADDRH + (x) * 2)
> > 
> > -/* 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 Transfer Setting Register for CRU Image Data */
> > -#define AMnAXIATTR			0x158
> >  #define AMnAXIATTR_AXILEN_MASK		GENMASK(3, 0)
> >  #define AMnAXIATTR_AXILEN		(0xf)
> > 
> > -/* 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(x)			((x) << 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)
> > 
> > +enum rzg2l_cru_common_regs {
> > +	CRUnCTRL,	/* CRU Control */
> > +	CRUnIE,		/* CRU Interrupt Enable */
> > +	CRUnINTS,	/* CRU Interrupt Status */
> > +	CRUnRST, 	/* CRU Reset */
> > +	AMnMB1ADDRL,	/* Bank 1 Address (Lower) for CRU Image Data */
> > +	AMnMB1ADDRH,	/* Bank 1 Address (Higher) for CRU Image Data */
> > +	AMnMB2ADDRL,    /* Bank 2 Address (Lower) for CRU Image Data */
> > +	AMnMB2ADDRH,    /* Bank 2 Address (Higher) for CRU Image Data */
> > +	AMnMB3ADDRL,    /* Bank 3 Address (Lower) for CRU Image Data */
> > +	AMnMB3ADDRH,    /* Bank 3 Address (Higher) for CRU Image Data */
> > +	AMnMB4ADDRL,    /* Bank 4 Address (Lower) for CRU Image Data */
> > +	AMnMB4ADDRH,    /* Bank 4 Address (Higher) for CRU Image Data */
> > +	AMnMB5ADDRL,    /* Bank 5 Address (Lower) for CRU Image Data */
> > +	AMnMB5ADDRH,    /* Bank 5 Address (Higher) for CRU Image Data */
> > +	AMnMB6ADDRL,    /* Bank 6 Address (Lower) for CRU Image Data */
> > +	AMnMB6ADDRH,    /* Bank 6 Address (Higher) for CRU Image Data */
> > +	AMnMB7ADDRL,    /* Bank 7 Address (Lower) for CRU Image Data */
> > +	AMnMB7ADDRH,    /* Bank 7 Address (Higher) for CRU Image Data */
> > +	AMnMB8ADDRL,    /* Bank 8 Address (Lower) for CRU Image Data */
> > +	AMnMB8ADDRH,    /* Bank 8 Address (Higher) for CRU Image Data */
> > +	AMnMBVALID,	/* Memory Bank Enable for CRU Image Data */
> > +	AMnMBS,		/* Memory Bank Status for CRU Image Data */
> > +	AMnAXIATTR,	/* AXI Master Transfer Setting Register for CRU Image Data */
> > +	AMnFIFOPNTR,	/* AXI Master FIFO Pointer for CRU Image Data */
> > +	AMnAXISTP,	/* AXI Master Transfer Stop for CRU Image Data */
> > +	AMnAXISTPACK,	/* AXI Master Transfer Stop Status for CRU Image Data */
> > +	ICnEN,		/* CRU Image Processing Enable */
> > +	ICnMC,		/* CRU Image Processing Main Control */
> > +	ICnMS,		/* CRU Module Status */
> > +	ICnDMR,		/* CRU Data Output Mode */
> > +	RZG2L_CRU_MAX_REG,
> > +};
> > +
> >  #endif /* __RZG2L_CRU_REGS_H__ */
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > index 8b898ce05b84..00c3f7458e20 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > @@ -80,6 +80,10 @@ struct rzg2l_cru_ip_format {
> >  	bool yuv;
> >  };
> > 
> > +struct rzg2l_cru_info {
> > +	const u16 *regs;
> > +};
> > +
> >  /**
> >   * struct rzg2l_cru_dev - Renesas CRU device structure
> >   * @dev:		(OF) device
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > index cd69c8a686d3..792f0df51a4b 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > @@ -42,16 +42,66 @@ struct rzg2l_cru_buffer {
> >  /* -----------------------------------------------------------------------------
> >   * DMA operations
> >   */
> > -static void rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> > +static void __rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset,
> > +u32 value)
> >  {
> > -	iowrite32(value, cru->base + offset);
> > +	const u16 *regs = cru->info->regs;
> > +
> > +	/*
> > +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> > +	 * rest of the registers have valid offset being set in cru->info->regs.
> > +	 */
> > +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> > +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> > +		return;
> > +
> > +	iowrite32(value, cru->base + regs[offset]); }
> > +
> > +static u32 __rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset) {
> > +	const u16 *regs = cru->info->regs;
> > +
> > +	/*
> > +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> > +	 * rest of the registers have valid offset being set in cru->info->regs.
> > +	 */
> > +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> > +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> > +		return 0;
> > +
> > +	return ioread32(cru->base + regs[offset]);
> >  }
> > 
> > -static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
> > +static inline void
> > +__rzg2l_cru_write_constant(struct rzg2l_cru_dev *cru, u32 offset, u32
> > +value)
> >  {
> > -	return ioread32(cru->base + offset);
> > +	const u16 *regs = cru->info->regs;
> > +
> > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > +
> > +	iowrite32(value, cru->base + regs[offset]);
> Do you need this code as the purpose is to test compile time constant and
> It won't execute at run time?

It was suggested in a previous review.

I've done some investigation on the above bot issue here.
Using __always_inline for constant read/write issue seems solved.

I found this link: https://www.kernel.org/doc/local/inline.html

But tbh I'm not finding an example into the kernel that use both 
BUILD_BUG_ON and  __always_inline.

Laurent what do you think about? Do you have some hints?
Thanks in advance.

> 
> 
> >  }
> > 
> > +static inline u32
> > +__rzg2l_cru_read_constant(struct rzg2l_cru_dev *cru, u32 offset) {
> > +	const u16 *regs = cru->info->regs;
> > +
> > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > +
> > +	return ioread32(cru->base + regs[offset]); 
> 
> Do you need this code as the purpose is to test compile time constant and
> It won't execute at run time?
> 
> Not sure, maybe adding an entry with MAX_ID in LUT,
> that will avoid buffer overflows and you can take out
> All out of bound array checks?
> 
> Cheers,
> Biju
> 
> }
> > +
> > +#define rzg2l_cru_write(cru, offset, value) \
> > +	(__builtin_constant_p(offset) ? \
> > +	 __rzg2l_cru_write_constant(cru, offset, value) : \
> > +	 __rzg2l_cru_write(cru, offset, value))
> > +
> > +#define rzg2l_cru_read(cru, offset) \
> > +	(__builtin_constant_p(offset) ? \
> > +	 __rzg2l_cru_read_constant(cru, offset) : \
> > +	 __rzg2l_cru_read(cru, offset))
> > +
> >  /* Need to hold qlock before calling */  static void return_unused_buffers(struct rzg2l_cru_dev *cru,
> >  				  enum vb2_buffer_state state)
> > --
> > 2.43.0
> 

Regards,
Tommaso

Re: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support

[Laurent Pinchart]

Hi Tommaso,

Thanks for being patient (and reminding me about this). Apparently,
Embedded World is bad for e-mail backlogs.

On Thu, Mar 13, 2025 at 01:01:24PM +0100, Tommaso Merciai wrote:
> On Wed, Mar 12, 2025 at 01:37:25PM +0000, Biju Das wrote:
> > On 03 March 2025 16:08, Tommaso Merciai wrote:
> > > 
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > 
> > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a CRU-IP that is mostly identical
> > > to RZ/G2L but with different register offsets and additional registers. Introduce a flexible register
> > > mapping mechanism to handle these variations.
> > > 
> > > Define the `rzg2l_cru_info` structure to store register mappings and pass it as part of the OF match
> > > data. Update the read/write functions to check out-of-bound accesses and use indexed register offsets
> > > from `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
> > > 
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > > Changes since v2:
> > >  - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > >    accesses as suggested by LPinchart.
> > >  - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > >  - Update commit body
> > > 
> > >  .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 46 ++++++++++++-
> > >  .../renesas/rzg2l-cru/rzg2l-cru-regs.h        | 66 ++++++++++---------
> > >  .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  4 ++
> > >  .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 58 ++++++++++++++--
> > >  4 files changed, 139 insertions(+), 35 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > index eed9d2bd0841..abc2a979833a 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > @@ -22,6 +22,7 @@
> > >  #include <media/v4l2-mc.h>
> > > 
> > >  #include "rzg2l-cru.h"
> > > +#include "rzg2l-cru-regs.h"
> > > 
> > >  static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n)  { @@ -269,6
> > > +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > > 
> > >  	cru->dev = dev;
> > >  	cru->info = of_device_get_match_data(dev);
> > > +	if (!cru->info)
> > > +		return dev_err_probe(dev, -EINVAL,
> > > +				     "Failed to get OF match data\n");
> > > 
> > >  	irq = platform_get_irq(pdev, 0);
> > >  	if (irq < 0)
> > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > >  	rzg2l_cru_dma_unregister(cru);
> > >  }
> > > 
> > > +static const u16 rzg2l_cru_regs[] = {
> > > +	[CRUnCTRL] = 0x0,
> > > +	[CRUnIE] = 0x4,
> > > +	[CRUnINTS] = 0x8,
> > > +	[CRUnRST] = 0xc,
> > > +	[AMnMB1ADDRL] = 0x100,
> > > +	[AMnMB1ADDRH] = 0x104,
> > > +	[AMnMB2ADDRL] = 0x108,
> > > +	[AMnMB2ADDRH] = 0x10c,
> > > +	[AMnMB3ADDRL] = 0x110,
> > > +	[AMnMB3ADDRH] = 0x114,
> > > +	[AMnMB4ADDRL] = 0x118,
> > > +	[AMnMB4ADDRH] = 0x11c,
> > > +	[AMnMB5ADDRL] = 0x120,
> > > +	[AMnMB5ADDRH] = 0x124,
> > > +	[AMnMB6ADDRL] = 0x128,
> > > +	[AMnMB6ADDRH] = 0x12c,
> > > +	[AMnMB7ADDRL] = 0x130,
> > > +	[AMnMB7ADDRH] = 0x134,
> > > +	[AMnMB8ADDRL] = 0x138,
> > > +	[AMnMB8ADDRH] = 0x13c,
> > > +	[AMnMBVALID] = 0x148,
> > > +	[AMnMBS] = 0x14c,
> > > +	[AMnAXIATTR] = 0x158,
> > > +	[AMnFIFOPNTR] = 0x168,
> > > +	[AMnAXISTP] = 0x174,
> > > +	[AMnAXISTPACK] = 0x178,
> > > +	[ICnEN] = 0x200,
> > > +	[ICnMC] = 0x208,
> > > +	[ICnMS] = 0x254,
> > > +	[ICnDMR] = 0x26c,
> > > +};
> > > +
> > > +static const struct rzg2l_cru_info rzgl2_cru_info = {
> > > +	.regs = rzg2l_cru_regs,
> > > +};
> > > +
> > >  static const struct of_device_id rzg2l_cru_of_id_table[] = {
> > > -	{ .compatible = "renesas,rzg2l-cru", },
> > > +	{
> > > +		.compatible = "renesas,rzg2l-cru",
> > > +		.data = &rzgl2_cru_info,
> > > +	},
> > >  	{ /* sentinel */ }
> > >  };
> > >  MODULE_DEVICE_TABLE(of, rzg2l_cru_of_id_table); diff --git a/drivers/media/platform/renesas/rzg2l-
> > > cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > index 1c9f22118a5d..86c320286246 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > @@ -10,71 +10,77 @@
> > > 
> > >  /* 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_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))
> > > +#define AMnMBxADDRL(x)			(AMnMB1ADDRL + (x) * 2)
> > > 
> > >  /* Memory Bank Base Address (Higher) Register for CRU Image Data */
> > > -#define AMnMBxADDRH(x)			(0x104 + ((x) * 8))
> > > +#define AMnMBxADDRH(x)			(AMnMB1ADDRH + (x) * 2)
> > > 
> > > -/* 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 Transfer Setting Register for CRU Image Data */
> > > -#define AMnAXIATTR			0x158
> > >  #define AMnAXIATTR_AXILEN_MASK		GENMASK(3, 0)
> > >  #define AMnAXIATTR_AXILEN		(0xf)
> > > 
> > > -/* 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(x)			((x) << 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)
> > > 
> > > +enum rzg2l_cru_common_regs {
> > > +	CRUnCTRL,	/* CRU Control */
> > > +	CRUnIE,		/* CRU Interrupt Enable */
> > > +	CRUnINTS,	/* CRU Interrupt Status */
> > > +	CRUnRST, 	/* CRU Reset */
> > > +	AMnMB1ADDRL,	/* Bank 1 Address (Lower) for CRU Image Data */
> > > +	AMnMB1ADDRH,	/* Bank 1 Address (Higher) for CRU Image Data */
> > > +	AMnMB2ADDRL,    /* Bank 2 Address (Lower) for CRU Image Data */
> > > +	AMnMB2ADDRH,    /* Bank 2 Address (Higher) for CRU Image Data */
> > > +	AMnMB3ADDRL,    /* Bank 3 Address (Lower) for CRU Image Data */
> > > +	AMnMB3ADDRH,    /* Bank 3 Address (Higher) for CRU Image Data */
> > > +	AMnMB4ADDRL,    /* Bank 4 Address (Lower) for CRU Image Data */
> > > +	AMnMB4ADDRH,    /* Bank 4 Address (Higher) for CRU Image Data */
> > > +	AMnMB5ADDRL,    /* Bank 5 Address (Lower) for CRU Image Data */
> > > +	AMnMB5ADDRH,    /* Bank 5 Address (Higher) for CRU Image Data */
> > > +	AMnMB6ADDRL,    /* Bank 6 Address (Lower) for CRU Image Data */
> > > +	AMnMB6ADDRH,    /* Bank 6 Address (Higher) for CRU Image Data */
> > > +	AMnMB7ADDRL,    /* Bank 7 Address (Lower) for CRU Image Data */
> > > +	AMnMB7ADDRH,    /* Bank 7 Address (Higher) for CRU Image Data */
> > > +	AMnMB8ADDRL,    /* Bank 8 Address (Lower) for CRU Image Data */
> > > +	AMnMB8ADDRH,    /* Bank 8 Address (Higher) for CRU Image Data */
> > > +	AMnMBVALID,	/* Memory Bank Enable for CRU Image Data */
> > > +	AMnMBS,		/* Memory Bank Status for CRU Image Data */
> > > +	AMnAXIATTR,	/* AXI Master Transfer Setting Register for CRU Image Data */
> > > +	AMnFIFOPNTR,	/* AXI Master FIFO Pointer for CRU Image Data */
> > > +	AMnAXISTP,	/* AXI Master Transfer Stop for CRU Image Data */
> > > +	AMnAXISTPACK,	/* AXI Master Transfer Stop Status for CRU Image Data */
> > > +	ICnEN,		/* CRU Image Processing Enable */
> > > +	ICnMC,		/* CRU Image Processing Main Control */
> > > +	ICnMS,		/* CRU Module Status */
> > > +	ICnDMR,		/* CRU Data Output Mode */
> > > +	RZG2L_CRU_MAX_REG,
> > > +};
> > > +
> > >  #endif /* __RZG2L_CRU_REGS_H__ */
> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > index 8b898ce05b84..00c3f7458e20 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > @@ -80,6 +80,10 @@ struct rzg2l_cru_ip_format {
> > >  	bool yuv;
> > >  };
> > > 
> > > +struct rzg2l_cru_info {
> > > +	const u16 *regs;
> > > +};
> > > +
> > >  /**
> > >   * struct rzg2l_cru_dev - Renesas CRU device structure
> > >   * @dev:		(OF) device
> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > index cd69c8a686d3..792f0df51a4b 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > @@ -42,16 +42,66 @@ struct rzg2l_cru_buffer {
> > >  /* -----------------------------------------------------------------------------
> > >   * DMA operations
> > >   */
> > > -static void rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> > > +static void __rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset,
> > > +u32 value)
> > >  {
> > > -	iowrite32(value, cru->base + offset);
> > > +	const u16 *regs = cru->info->regs;
> > > +
> > > +	/*
> > > +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> > > +	 * rest of the registers have valid offset being set in cru->info->regs.
> > > +	 */
> > > +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> > > +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> > > +		return;
> > > +
> > > +	iowrite32(value, cru->base + regs[offset]); }
> > > +
> > > +static u32 __rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset) {
> > > +	const u16 *regs = cru->info->regs;
> > > +
> > > +	/*
> > > +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> > > +	 * rest of the registers have valid offset being set in cru->info->regs.
> > > +	 */
> > > +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> > > +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> > > +		return 0;
> > > +
> > > +	return ioread32(cru->base + regs[offset]);
> > >  }
> > > 
> > > -static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
> > > +static inline void
> > > +__rzg2l_cru_write_constant(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> > >  {
> > > -	return ioread32(cru->base + offset);
> > > +	const u16 *regs = cru->info->regs;
> > > +
> > > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > > +
> > > +	iowrite32(value, cru->base + regs[offset]);
> >
> > Do you need this code as the purpose is to test compile time constant and
> > It won't execute at run time?

Biju, I'm not sure to understan this comment.
__rzg2l_cru_write_constant() is called at runtime, with a compile-time
constant offset. The BUILD_BUG_ON() verifies at compile time that the
offset is valid, causing compilation errors if it isn't.

__rzg2l_cru_write(), on the other hand, is called when the offset is not
known at compile time, because it's computed dynamically. That's a small
subset of the calls. It needs to check the offset at runtime for
overflows.

What do you mean by "won't execute at runtime", and what code do you
think is not needed ?

> It was suggested in a previous review.
> 
> I've done some investigation on the above bot issue here.
> Using __always_inline for constant read/write issue seems solved.
> 
> I found this link: https://www.kernel.org/doc/local/inline.html
> 
> But tbh I'm not finding an example into the kernel that use both 
> BUILD_BUG_ON and  __always_inline.
> 
> Laurent what do you think about? Do you have some hints?
> Thanks in advance.

Do you mean that the compile-time assertions are caused by
__rzg2l_cru_write_constant() not being inlined ? The function could be
marked as __always_inline I suppose. Or the BUILD_BUG_ON() check could
be moved to the rzg2l_cru_write() macro.

> > >  }
> > > 
> > > +static inline u32
> > > +__rzg2l_cru_read_constant(struct rzg2l_cru_dev *cru, u32 offset) {
> > > +	const u16 *regs = cru->info->regs;
> > > +
> > > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > > +
> > > +	return ioread32(cru->base + regs[offset]); 
> > 
> > Do you need this code as the purpose is to test compile time constant and
> > It won't execute at run time?
> > 
> > Not sure, maybe adding an entry with MAX_ID in LUT,
> > that will avoid buffer overflows and you can take out
> > All out of bound array checks?
> > 
> > Cheers,
> > Biju
> > 
> > }
> > > +
> > > +#define rzg2l_cru_write(cru, offset, value) \
> > > +	(__builtin_constant_p(offset) ? \
> > > +	 __rzg2l_cru_write_constant(cru, offset, value) : \
> > > +	 __rzg2l_cru_write(cru, offset, value))
> > > +
> > > +#define rzg2l_cru_read(cru, offset) \
> > > +	(__builtin_constant_p(offset) ? \
> > > +	 __rzg2l_cru_read_constant(cru, offset) : \
> > > +	 __rzg2l_cru_read(cru, offset))
> > > +
> > >  /* Need to hold qlock before calling */  static void return_unused_buffers(struct rzg2l_cru_dev *cru,
> > >  				  enum vb2_buffer_state state)

-- 
Regards,

Laurent Pinchart

RE: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support

[Biju Das]

Hi Laurent,

Thanks for the feedback.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 27 March 2025 10:16
> Subject: Re: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support
> 
> Hi Tommaso,
> 
> Thanks for being patient (and reminding me about this). Apparently, Embedded World is bad for e-mail
> backlogs.
> 
> On Thu, Mar 13, 2025 at 01:01:24PM +0100, Tommaso Merciai wrote:
> > On Wed, Mar 12, 2025 at 01:37:25PM +0000, Biju Das wrote:
> > > On 03 March 2025 16:08, Tommaso Merciai wrote:
> > > >
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have
> > > > a CRU-IP that is mostly identical to RZ/G2L but with different
> > > > register offsets and additional registers. Introduce a flexible register mapping mechanism to
> handle these variations.
> > > >
> > > > Define the `rzg2l_cru_info` structure to store register mappings
> > > > and pass it as part of the OF match data. Update the read/write
> > > > functions to check out-of-bound accesses and use indexed register offsets from `rzg2l_cru_info`,
> ensuring compatibility across different SoC variants.
> > > >
> > > > Signed-off-by: Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > ---
> > > > Changes since v2:
> > > >  - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > >    accesses as suggested by LPinchart.
> > > >  - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > >  - Update commit body
> > > >
> > > >  .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 46 ++++++++++++-
> > > >  .../renesas/rzg2l-cru/rzg2l-cru-regs.h        | 66 ++++++++++---------
> > > >  .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  4 ++
> > > >  .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 58
> > > > ++++++++++++++--
> > > >  4 files changed, 139 insertions(+), 35 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > index eed9d2bd0841..abc2a979833a 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > @@ -22,6 +22,7 @@
> > > >  #include <media/v4l2-mc.h>
> > > >
> > > >  #include "rzg2l-cru.h"
> > > > +#include "rzg2l-cru-regs.h"
> > > >
> > > >  static inline struct rzg2l_cru_dev *notifier_to_cru(struct
> > > > v4l2_async_notifier *n)  { @@ -269,6
> > > > +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > > >
> > > >  	cru->dev = dev;
> > > >  	cru->info = of_device_get_match_data(dev);
> > > > +	if (!cru->info)
> > > > +		return dev_err_probe(dev, -EINVAL,
> > > > +				     "Failed to get OF match data\n");
> > > >
> > > >  	irq = platform_get_irq(pdev, 0);
> > > >  	if (irq < 0)
> > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > >  	rzg2l_cru_dma_unregister(cru);
> > > >  }
> > > >
> > > >
> > > > -static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
> > > > +static inline void
> > > > +__rzg2l_cru_write_constant(struct rzg2l_cru_dev *cru, u32 offset,
> > > > +u32 value)
> > > >  {
> > > > -	return ioread32(cru->base + offset);
> > > > +	const u16 *regs = cru->info->regs;
> > > > +
> > > > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > > > +
> > > > +	iowrite32(value, cru->base + regs[offset]);
> > >
> > > Do you need this code as the purpose is to test compile time
> > > constant and It won't execute at run time?
> 
> Biju, I'm not sure to understan this comment.
> __rzg2l_cru_write_constant() is called at runtime, with a compile-time constant offset. The
> BUILD_BUG_ON() verifies at compile time that the offset is valid, causing compilation errors if it
> isn't.

Thanks for explanation. Now got it. I don't see any drivers using this way. Hence the confusion.

> 
> __rzg2l_cru_write(), on the other hand, is called when the offset is not known at compile time,
> because it's computed dynamically. That's a small subset of the calls. It needs to check the offset at
> runtime for overflows.

OK.

> 
> What do you mean by "won't execute at runtime", and what code do you think is not needed ?

I got confused with BUILD_BUG_ON() on a frequently called function and the one without __rzg2l_cru_write()

Cheers,
Biju

Re: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support

[Tommaso Merciai]

Hi Laurent,

Thanks for your comment.

On Thu, Mar 27, 2025 at 12:15:54PM +0200, Laurent Pinchart wrote:
> Hi Tommaso,
> 
> Thanks for being patient (and reminding me about this). Apparently,
> Embedded World is bad for e-mail backlogs.

I can imagine.
I skipped the EW this year, hope you had fun there :)
No worries.

> 
> On Thu, Mar 13, 2025 at 01:01:24PM +0100, Tommaso Merciai wrote:
> > On Wed, Mar 12, 2025 at 01:37:25PM +0000, Biju Das wrote:
> > > On 03 March 2025 16:08, Tommaso Merciai wrote:
> > > > 
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > 
> > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a CRU-IP that is mostly identical
> > > > to RZ/G2L but with different register offsets and additional registers. Introduce a flexible register
> > > > mapping mechanism to handle these variations.
> > > > 
> > > > Define the `rzg2l_cru_info` structure to store register mappings and pass it as part of the OF match
> > > > data. Update the read/write functions to check out-of-bound accesses and use indexed register offsets
> > > > from `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
> > > > 
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > ---
> > > > Changes since v2:
> > > >  - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > >    accesses as suggested by LPinchart.
> > > >  - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > >  - Update commit body
> > > > 
> > > >  .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 46 ++++++++++++-
> > > >  .../renesas/rzg2l-cru/rzg2l-cru-regs.h        | 66 ++++++++++---------
> > > >  .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  4 ++
> > > >  .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 58 ++++++++++++++--
> > > >  4 files changed, 139 insertions(+), 35 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > index eed9d2bd0841..abc2a979833a 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > @@ -22,6 +22,7 @@
> > > >  #include <media/v4l2-mc.h>
> > > > 
> > > >  #include "rzg2l-cru.h"
> > > > +#include "rzg2l-cru-regs.h"
> > > > 
> > > >  static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n)  { @@ -269,6
> > > > +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > > > 
> > > >  	cru->dev = dev;
> > > >  	cru->info = of_device_get_match_data(dev);
> > > > +	if (!cru->info)
> > > > +		return dev_err_probe(dev, -EINVAL,
> > > > +				     "Failed to get OF match data\n");
> > > > 
> > > >  	irq = platform_get_irq(pdev, 0);
> > > >  	if (irq < 0)
> > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > >  	rzg2l_cru_dma_unregister(cru);
> > > >  }
> > > > 
> > > > +static const u16 rzg2l_cru_regs[] = {
> > > > +	[CRUnCTRL] = 0x0,
> > > > +	[CRUnIE] = 0x4,
> > > > +	[CRUnINTS] = 0x8,
> > > > +	[CRUnRST] = 0xc,
> > > > +	[AMnMB1ADDRL] = 0x100,
> > > > +	[AMnMB1ADDRH] = 0x104,
> > > > +	[AMnMB2ADDRL] = 0x108,
> > > > +	[AMnMB2ADDRH] = 0x10c,
> > > > +	[AMnMB3ADDRL] = 0x110,
> > > > +	[AMnMB3ADDRH] = 0x114,
> > > > +	[AMnMB4ADDRL] = 0x118,
> > > > +	[AMnMB4ADDRH] = 0x11c,
> > > > +	[AMnMB5ADDRL] = 0x120,
> > > > +	[AMnMB5ADDRH] = 0x124,
> > > > +	[AMnMB6ADDRL] = 0x128,
> > > > +	[AMnMB6ADDRH] = 0x12c,
> > > > +	[AMnMB7ADDRL] = 0x130,
> > > > +	[AMnMB7ADDRH] = 0x134,
> > > > +	[AMnMB8ADDRL] = 0x138,
> > > > +	[AMnMB8ADDRH] = 0x13c,
> > > > +	[AMnMBVALID] = 0x148,
> > > > +	[AMnMBS] = 0x14c,
> > > > +	[AMnAXIATTR] = 0x158,
> > > > +	[AMnFIFOPNTR] = 0x168,
> > > > +	[AMnAXISTP] = 0x174,
> > > > +	[AMnAXISTPACK] = 0x178,
> > > > +	[ICnEN] = 0x200,
> > > > +	[ICnMC] = 0x208,
> > > > +	[ICnMS] = 0x254,
> > > > +	[ICnDMR] = 0x26c,
> > > > +};
> > > > +
> > > > +static const struct rzg2l_cru_info rzgl2_cru_info = {
> > > > +	.regs = rzg2l_cru_regs,
> > > > +};
> > > > +
> > > >  static const struct of_device_id rzg2l_cru_of_id_table[] = {
> > > > -	{ .compatible = "renesas,rzg2l-cru", },
> > > > +	{
> > > > +		.compatible = "renesas,rzg2l-cru",
> > > > +		.data = &rzgl2_cru_info,
> > > > +	},
> > > >  	{ /* sentinel */ }
> > > >  };
> > > >  MODULE_DEVICE_TABLE(of, rzg2l_cru_of_id_table); diff --git a/drivers/media/platform/renesas/rzg2l-
> > > > cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > > index 1c9f22118a5d..86c320286246 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > > @@ -10,71 +10,77 @@
> > > > 
> > > >  /* 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_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))
> > > > +#define AMnMBxADDRL(x)			(AMnMB1ADDRL + (x) * 2)
> > > > 
> > > >  /* Memory Bank Base Address (Higher) Register for CRU Image Data */
> > > > -#define AMnMBxADDRH(x)			(0x104 + ((x) * 8))
> > > > +#define AMnMBxADDRH(x)			(AMnMB1ADDRH + (x) * 2)
> > > > 
> > > > -/* 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 Transfer Setting Register for CRU Image Data */
> > > > -#define AMnAXIATTR			0x158
> > > >  #define AMnAXIATTR_AXILEN_MASK		GENMASK(3, 0)
> > > >  #define AMnAXIATTR_AXILEN		(0xf)
> > > > 
> > > > -/* 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(x)			((x) << 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)
> > > > 
> > > > +enum rzg2l_cru_common_regs {
> > > > +	CRUnCTRL,	/* CRU Control */
> > > > +	CRUnIE,		/* CRU Interrupt Enable */
> > > > +	CRUnINTS,	/* CRU Interrupt Status */
> > > > +	CRUnRST, 	/* CRU Reset */
> > > > +	AMnMB1ADDRL,	/* Bank 1 Address (Lower) for CRU Image Data */
> > > > +	AMnMB1ADDRH,	/* Bank 1 Address (Higher) for CRU Image Data */
> > > > +	AMnMB2ADDRL,    /* Bank 2 Address (Lower) for CRU Image Data */
> > > > +	AMnMB2ADDRH,    /* Bank 2 Address (Higher) for CRU Image Data */
> > > > +	AMnMB3ADDRL,    /* Bank 3 Address (Lower) for CRU Image Data */
> > > > +	AMnMB3ADDRH,    /* Bank 3 Address (Higher) for CRU Image Data */
> > > > +	AMnMB4ADDRL,    /* Bank 4 Address (Lower) for CRU Image Data */
> > > > +	AMnMB4ADDRH,    /* Bank 4 Address (Higher) for CRU Image Data */
> > > > +	AMnMB5ADDRL,    /* Bank 5 Address (Lower) for CRU Image Data */
> > > > +	AMnMB5ADDRH,    /* Bank 5 Address (Higher) for CRU Image Data */
> > > > +	AMnMB6ADDRL,    /* Bank 6 Address (Lower) for CRU Image Data */
> > > > +	AMnMB6ADDRH,    /* Bank 6 Address (Higher) for CRU Image Data */
> > > > +	AMnMB7ADDRL,    /* Bank 7 Address (Lower) for CRU Image Data */
> > > > +	AMnMB7ADDRH,    /* Bank 7 Address (Higher) for CRU Image Data */
> > > > +	AMnMB8ADDRL,    /* Bank 8 Address (Lower) for CRU Image Data */
> > > > +	AMnMB8ADDRH,    /* Bank 8 Address (Higher) for CRU Image Data */
> > > > +	AMnMBVALID,	/* Memory Bank Enable for CRU Image Data */
> > > > +	AMnMBS,		/* Memory Bank Status for CRU Image Data */
> > > > +	AMnAXIATTR,	/* AXI Master Transfer Setting Register for CRU Image Data */
> > > > +	AMnFIFOPNTR,	/* AXI Master FIFO Pointer for CRU Image Data */
> > > > +	AMnAXISTP,	/* AXI Master Transfer Stop for CRU Image Data */
> > > > +	AMnAXISTPACK,	/* AXI Master Transfer Stop Status for CRU Image Data */
> > > > +	ICnEN,		/* CRU Image Processing Enable */
> > > > +	ICnMC,		/* CRU Image Processing Main Control */
> > > > +	ICnMS,		/* CRU Module Status */
> > > > +	ICnDMR,		/* CRU Data Output Mode */
> > > > +	RZG2L_CRU_MAX_REG,
> > > > +};
> > > > +
> > > >  #endif /* __RZG2L_CRU_REGS_H__ */
> > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > index 8b898ce05b84..00c3f7458e20 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > @@ -80,6 +80,10 @@ struct rzg2l_cru_ip_format {
> > > >  	bool yuv;
> > > >  };
> > > > 
> > > > +struct rzg2l_cru_info {
> > > > +	const u16 *regs;
> > > > +};
> > > > +
> > > >  /**
> > > >   * struct rzg2l_cru_dev - Renesas CRU device structure
> > > >   * @dev:		(OF) device
> > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > index cd69c8a686d3..792f0df51a4b 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > @@ -42,16 +42,66 @@ struct rzg2l_cru_buffer {
> > > >  /* -----------------------------------------------------------------------------
> > > >   * DMA operations
> > > >   */
> > > > -static void rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> > > > +static void __rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset,
> > > > +u32 value)
> > > >  {
> > > > -	iowrite32(value, cru->base + offset);
> > > > +	const u16 *regs = cru->info->regs;
> > > > +
> > > > +	/*
> > > > +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> > > > +	 * rest of the registers have valid offset being set in cru->info->regs.
> > > > +	 */
> > > > +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> > > > +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> > > > +		return;
> > > > +
> > > > +	iowrite32(value, cru->base + regs[offset]); }
> > > > +
> > > > +static u32 __rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset) {
> > > > +	const u16 *regs = cru->info->regs;
> > > > +
> > > > +	/*
> > > > +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> > > > +	 * rest of the registers have valid offset being set in cru->info->regs.
> > > > +	 */
> > > > +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> > > > +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> > > > +		return 0;
> > > > +
> > > > +	return ioread32(cru->base + regs[offset]);
> > > >  }
> > > > 
> > > > -static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
> > > > +static inline void
> > > > +__rzg2l_cru_write_constant(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> > > >  {
> > > > -	return ioread32(cru->base + offset);
> > > > +	const u16 *regs = cru->info->regs;
> > > > +
> > > > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > > > +
> > > > +	iowrite32(value, cru->base + regs[offset]);
> > >
> > > Do you need this code as the purpose is to test compile time constant and
> > > It won't execute at run time?
> 
> Biju, I'm not sure to understan this comment.
> __rzg2l_cru_write_constant() is called at runtime, with a compile-time
> constant offset. The BUILD_BUG_ON() verifies at compile time that the
> offset is valid, causing compilation errors if it isn't.
> 
> __rzg2l_cru_write(), on the other hand, is called when the offset is not
> known at compile time, because it's computed dynamically. That's a small
> subset of the calls. It needs to check the offset at runtime for
> overflows.
> 
> What do you mean by "won't execute at runtime", and what code do you
> think is not needed ?
> 
> > It was suggested in a previous review.
> > 
> > I've done some investigation on the above bot issue here.
> > Using __always_inline for constant read/write issue seems solved.
> > 
> > I found this link: https://www.kernel.org/doc/local/inline.html
> > 
> > But tbh I'm not finding an example into the kernel that use both 
> > BUILD_BUG_ON and  __always_inline.
> > 
> > Laurent what do you think about? Do you have some hints?
> > Thanks in advance.
> 
> Do you mean that the compile-time assertions are caused by
> __rzg2l_cru_write_constant() not being inlined ?

Seems yes.
Using __always_inline seems to solve the issue reported by the bot test.

> The function could be
> marked as __always_inline I suppose. Or the BUILD_BUG_ON() check could
> be moved to the rzg2l_cru_write() macro.

Mmm not sure that I completely got this way.

Actually we have:

#define rzg2l_cru_write(cru, offset, value) \
	(__builtin_constant_p(offset) ? \
	 __rzg2l_cru_write_constant(cru, offset, value) : \
	 __rzg2l_cru_write(cru, offset, value))

And BUILD_BUG_ON() can only be user on constant offset.

Thanks!


> 
> > > >  }
> > > > 
> > > > +static inline u32
> > > > +__rzg2l_cru_read_constant(struct rzg2l_cru_dev *cru, u32 offset) {
> > > > +	const u16 *regs = cru->info->regs;
> > > > +
> > > > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > > > +
> > > > +	return ioread32(cru->base + regs[offset]); 
> > > 
> > > Do you need this code as the purpose is to test compile time constant and
> > > It won't execute at run time?
> > > 
> > > Not sure, maybe adding an entry with MAX_ID in LUT,
> > > that will avoid buffer overflows and you can take out
> > > All out of bound array checks?
> > > 
> > > Cheers,
> > > Biju
> > > 
> > > }
> > > > +
> > > > +#define rzg2l_cru_write(cru, offset, value) \
> > > > +	(__builtin_constant_p(offset) ? \
> > > > +	 __rzg2l_cru_write_constant(cru, offset, value) : \
> > > > +	 __rzg2l_cru_write(cru, offset, value))
> > > > +
> > > > +#define rzg2l_cru_read(cru, offset) \
> > > > +	(__builtin_constant_p(offset) ? \
> > > > +	 __rzg2l_cru_read_constant(cru, offset) : \
> > > > +	 __rzg2l_cru_read(cru, offset))
> > > > +
> > > >  /* Need to hold qlock before calling */  static void return_unused_buffers(struct rzg2l_cru_dev *cru,
> > > >  				  enum vb2_buffer_state state)
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Thanks & Regards,
Tommaso

Re: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support

[Laurent Pinchart]

On Thu, Mar 27, 2025 at 05:45:39PM +0100, Tommaso Merciai wrote:
> Hi Laurent,
> 
> Thanks for your comment.
> 
> On Thu, Mar 27, 2025 at 12:15:54PM +0200, Laurent Pinchart wrote:
> > Hi Tommaso,
> > 
> > Thanks for being patient (and reminding me about this). Apparently,
> > Embedded World is bad for e-mail backlogs.
> 
> I can imagine.
> I skipped the EW this year, hope you had fun there :)
> No worries.
> 
> > 
> > On Thu, Mar 13, 2025 at 01:01:24PM +0100, Tommaso Merciai wrote:
> > > On Wed, Mar 12, 2025 at 01:37:25PM +0000, Biju Das wrote:
> > > > On 03 March 2025 16:08, Tommaso Merciai wrote:
> > > > > 
> > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > 
> > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a CRU-IP that is mostly identical
> > > > > to RZ/G2L but with different register offsets and additional registers. Introduce a flexible register
> > > > > mapping mechanism to handle these variations.
> > > > > 
> > > > > Define the `rzg2l_cru_info` structure to store register mappings and pass it as part of the OF match
> > > > > data. Update the read/write functions to check out-of-bound accesses and use indexed register offsets
> > > > > from `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
> > > > > 
> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > > ---
> > > > > Changes since v2:
> > > > >  - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > >    accesses as suggested by LPinchart.
> > > > >  - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > >  - Update commit body
> > > > > 
> > > > >  .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 46 ++++++++++++-
> > > > >  .../renesas/rzg2l-cru/rzg2l-cru-regs.h        | 66 ++++++++++---------
> > > > >  .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  4 ++
> > > > >  .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 58 ++++++++++++++--
> > > > >  4 files changed, 139 insertions(+), 35 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > index eed9d2bd0841..abc2a979833a 100644
> > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > @@ -22,6 +22,7 @@
> > > > >  #include <media/v4l2-mc.h>
> > > > > 
> > > > >  #include "rzg2l-cru.h"
> > > > > +#include "rzg2l-cru-regs.h"
> > > > > 
> > > > >  static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n)  { @@ -269,6
> > > > > +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > > > > 
> > > > >  	cru->dev = dev;
> > > > >  	cru->info = of_device_get_match_data(dev);
> > > > > +	if (!cru->info)
> > > > > +		return dev_err_probe(dev, -EINVAL,
> > > > > +				     "Failed to get OF match data\n");
> > > > > 
> > > > >  	irq = platform_get_irq(pdev, 0);
> > > > >  	if (irq < 0)
> > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > >  	rzg2l_cru_dma_unregister(cru);
> > > > >  }
> > > > > 
> > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > +	[CRUnCTRL] = 0x0,
> > > > > +	[CRUnIE] = 0x4,
> > > > > +	[CRUnINTS] = 0x8,
> > > > > +	[CRUnRST] = 0xc,
> > > > > +	[AMnMB1ADDRL] = 0x100,
> > > > > +	[AMnMB1ADDRH] = 0x104,
> > > > > +	[AMnMB2ADDRL] = 0x108,
> > > > > +	[AMnMB2ADDRH] = 0x10c,
> > > > > +	[AMnMB3ADDRL] = 0x110,
> > > > > +	[AMnMB3ADDRH] = 0x114,
> > > > > +	[AMnMB4ADDRL] = 0x118,
> > > > > +	[AMnMB4ADDRH] = 0x11c,
> > > > > +	[AMnMB5ADDRL] = 0x120,
> > > > > +	[AMnMB5ADDRH] = 0x124,
> > > > > +	[AMnMB6ADDRL] = 0x128,
> > > > > +	[AMnMB6ADDRH] = 0x12c,
> > > > > +	[AMnMB7ADDRL] = 0x130,
> > > > > +	[AMnMB7ADDRH] = 0x134,
> > > > > +	[AMnMB8ADDRL] = 0x138,
> > > > > +	[AMnMB8ADDRH] = 0x13c,
> > > > > +	[AMnMBVALID] = 0x148,
> > > > > +	[AMnMBS] = 0x14c,
> > > > > +	[AMnAXIATTR] = 0x158,
> > > > > +	[AMnFIFOPNTR] = 0x168,
> > > > > +	[AMnAXISTP] = 0x174,
> > > > > +	[AMnAXISTPACK] = 0x178,
> > > > > +	[ICnEN] = 0x200,
> > > > > +	[ICnMC] = 0x208,
> > > > > +	[ICnMS] = 0x254,
> > > > > +	[ICnDMR] = 0x26c,
> > > > > +};
> > > > > +
> > > > > +static const struct rzg2l_cru_info rzgl2_cru_info = {
> > > > > +	.regs = rzg2l_cru_regs,
> > > > > +};
> > > > > +
> > > > >  static const struct of_device_id rzg2l_cru_of_id_table[] = {
> > > > > -	{ .compatible = "renesas,rzg2l-cru", },
> > > > > +	{
> > > > > +		.compatible = "renesas,rzg2l-cru",
> > > > > +		.data = &rzgl2_cru_info,
> > > > > +	},
> > > > >  	{ /* sentinel */ }
> > > > >  };
> > > > >  MODULE_DEVICE_TABLE(of, rzg2l_cru_of_id_table); diff --git a/drivers/media/platform/renesas/rzg2l-
> > > > > cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > > > index 1c9f22118a5d..86c320286246 100644
> > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > > > @@ -10,71 +10,77 @@
> > > > > 
> > > > >  /* 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_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))
> > > > > +#define AMnMBxADDRL(x)			(AMnMB1ADDRL + (x) * 2)
> > > > > 
> > > > >  /* Memory Bank Base Address (Higher) Register for CRU Image Data */
> > > > > -#define AMnMBxADDRH(x)			(0x104 + ((x) * 8))
> > > > > +#define AMnMBxADDRH(x)			(AMnMB1ADDRH + (x) * 2)
> > > > > 
> > > > > -/* 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 Transfer Setting Register for CRU Image Data */
> > > > > -#define AMnAXIATTR			0x158
> > > > >  #define AMnAXIATTR_AXILEN_MASK		GENMASK(3, 0)
> > > > >  #define AMnAXIATTR_AXILEN		(0xf)
> > > > > 
> > > > > -/* 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(x)			((x) << 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)
> > > > > 
> > > > > +enum rzg2l_cru_common_regs {
> > > > > +	CRUnCTRL,	/* CRU Control */
> > > > > +	CRUnIE,		/* CRU Interrupt Enable */
> > > > > +	CRUnINTS,	/* CRU Interrupt Status */
> > > > > +	CRUnRST, 	/* CRU Reset */
> > > > > +	AMnMB1ADDRL,	/* Bank 1 Address (Lower) for CRU Image Data */
> > > > > +	AMnMB1ADDRH,	/* Bank 1 Address (Higher) for CRU Image Data */
> > > > > +	AMnMB2ADDRL,    /* Bank 2 Address (Lower) for CRU Image Data */
> > > > > +	AMnMB2ADDRH,    /* Bank 2 Address (Higher) for CRU Image Data */
> > > > > +	AMnMB3ADDRL,    /* Bank 3 Address (Lower) for CRU Image Data */
> > > > > +	AMnMB3ADDRH,    /* Bank 3 Address (Higher) for CRU Image Data */
> > > > > +	AMnMB4ADDRL,    /* Bank 4 Address (Lower) for CRU Image Data */
> > > > > +	AMnMB4ADDRH,    /* Bank 4 Address (Higher) for CRU Image Data */
> > > > > +	AMnMB5ADDRL,    /* Bank 5 Address (Lower) for CRU Image Data */
> > > > > +	AMnMB5ADDRH,    /* Bank 5 Address (Higher) for CRU Image Data */
> > > > > +	AMnMB6ADDRL,    /* Bank 6 Address (Lower) for CRU Image Data */
> > > > > +	AMnMB6ADDRH,    /* Bank 6 Address (Higher) for CRU Image Data */
> > > > > +	AMnMB7ADDRL,    /* Bank 7 Address (Lower) for CRU Image Data */
> > > > > +	AMnMB7ADDRH,    /* Bank 7 Address (Higher) for CRU Image Data */
> > > > > +	AMnMB8ADDRL,    /* Bank 8 Address (Lower) for CRU Image Data */
> > > > > +	AMnMB8ADDRH,    /* Bank 8 Address (Higher) for CRU Image Data */
> > > > > +	AMnMBVALID,	/* Memory Bank Enable for CRU Image Data */
> > > > > +	AMnMBS,		/* Memory Bank Status for CRU Image Data */
> > > > > +	AMnAXIATTR,	/* AXI Master Transfer Setting Register for CRU Image Data */
> > > > > +	AMnFIFOPNTR,	/* AXI Master FIFO Pointer for CRU Image Data */
> > > > > +	AMnAXISTP,	/* AXI Master Transfer Stop for CRU Image Data */
> > > > > +	AMnAXISTPACK,	/* AXI Master Transfer Stop Status for CRU Image Data */
> > > > > +	ICnEN,		/* CRU Image Processing Enable */
> > > > > +	ICnMC,		/* CRU Image Processing Main Control */
> > > > > +	ICnMS,		/* CRU Module Status */
> > > > > +	ICnDMR,		/* CRU Data Output Mode */
> > > > > +	RZG2L_CRU_MAX_REG,
> > > > > +};
> > > > > +
> > > > >  #endif /* __RZG2L_CRU_REGS_H__ */
> > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > > index 8b898ce05b84..00c3f7458e20 100644
> > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > > @@ -80,6 +80,10 @@ struct rzg2l_cru_ip_format {
> > > > >  	bool yuv;
> > > > >  };
> > > > > 
> > > > > +struct rzg2l_cru_info {
> > > > > +	const u16 *regs;
> > > > > +};
> > > > > +
> > > > >  /**
> > > > >   * struct rzg2l_cru_dev - Renesas CRU device structure
> > > > >   * @dev:		(OF) device
> > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > > index cd69c8a686d3..792f0df51a4b 100644
> > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > > @@ -42,16 +42,66 @@ struct rzg2l_cru_buffer {
> > > > >  /* -----------------------------------------------------------------------------
> > > > >   * DMA operations
> > > > >   */
> > > > > -static void rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> > > > > +static void __rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset,
> > > > > +u32 value)
> > > > >  {
> > > > > -	iowrite32(value, cru->base + offset);
> > > > > +	const u16 *regs = cru->info->regs;
> > > > > +
> > > > > +	/*
> > > > > +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> > > > > +	 * rest of the registers have valid offset being set in cru->info->regs.
> > > > > +	 */
> > > > > +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> > > > > +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> > > > > +		return;
> > > > > +
> > > > > +	iowrite32(value, cru->base + regs[offset]); }
> > > > > +
> > > > > +static u32 __rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset) {
> > > > > +	const u16 *regs = cru->info->regs;
> > > > > +
> > > > > +	/*
> > > > > +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> > > > > +	 * rest of the registers have valid offset being set in cru->info->regs.
> > > > > +	 */
> > > > > +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> > > > > +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> > > > > +		return 0;
> > > > > +
> > > > > +	return ioread32(cru->base + regs[offset]);
> > > > >  }
> > > > > 
> > > > > -static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
> > > > > +static inline void
> > > > > +__rzg2l_cru_write_constant(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> > > > >  {
> > > > > -	return ioread32(cru->base + offset);
> > > > > +	const u16 *regs = cru->info->regs;
> > > > > +
> > > > > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > > > > +
> > > > > +	iowrite32(value, cru->base + regs[offset]);
> > > >
> > > > Do you need this code as the purpose is to test compile time constant and
> > > > It won't execute at run time?
> > 
> > Biju, I'm not sure to understan this comment.
> > __rzg2l_cru_write_constant() is called at runtime, with a compile-time
> > constant offset. The BUILD_BUG_ON() verifies at compile time that the
> > offset is valid, causing compilation errors if it isn't.
> > 
> > __rzg2l_cru_write(), on the other hand, is called when the offset is not
> > known at compile time, because it's computed dynamically. That's a small
> > subset of the calls. It needs to check the offset at runtime for
> > overflows.
> > 
> > What do you mean by "won't execute at runtime", and what code do you
> > think is not needed ?
> > 
> > > It was suggested in a previous review.
> > > 
> > > I've done some investigation on the above bot issue here.
> > > Using __always_inline for constant read/write issue seems solved.
> > > 
> > > I found this link: https://www.kernel.org/doc/local/inline.html
> > > 
> > > But tbh I'm not finding an example into the kernel that use both 
> > > BUILD_BUG_ON and  __always_inline.
> > > 
> > > Laurent what do you think about? Do you have some hints?
> > > Thanks in advance.
> > 
> > Do you mean that the compile-time assertions are caused by
> > __rzg2l_cru_write_constant() not being inlined ?
> 
> Seems yes.
> Using __always_inline seems to solve the issue reported by the bot test.
> 
> > The function could be
> > marked as __always_inline I suppose. Or the BUILD_BUG_ON() check could
> > be moved to the rzg2l_cru_write() macro.
> 
> Mmm not sure that I completely got this way.
> 
> Actually we have:
> 
> #define rzg2l_cru_write(cru, offset, value) \
> 	(__builtin_constant_p(offset) ? \
> 	 __rzg2l_cru_write_constant(cru, offset, value) : \
> 	 __rzg2l_cru_write(cru, offset, value))
> 
> And BUILD_BUG_ON() can only be user on constant offset.

There seems to be quite a few examples of usage of __always_inline with
BUILD_BUG_ON(), so we can go that way. Otherwise, you could write
something like (untested)

#define rzg2l_cru_write(cru, offset, value)				\
({									\
	u32 __offset = (offset);					\
	if (__builtin_constant_p(__offset)) {				\
		BUILD_BUG_ON(__offset >= RZG2L_CRU_MAX_REG);		\
		__rzg2l_cru_write_constant(cru, __offset, value);	\
	} else {							\
		__rzg2l_cru_write(cru, __offset, value));		\
	}								\
})

> > > > >  }
> > > > > 
> > > > > +static inline u32
> > > > > +__rzg2l_cru_read_constant(struct rzg2l_cru_dev *cru, u32 offset) {
> > > > > +	const u16 *regs = cru->info->regs;
> > > > > +
> > > > > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > > > > +
> > > > > +	return ioread32(cru->base + regs[offset]); 
> > > > 
> > > > Do you need this code as the purpose is to test compile time constant and
> > > > It won't execute at run time?
> > > > 
> > > > Not sure, maybe adding an entry with MAX_ID in LUT,
> > > > that will avoid buffer overflows and you can take out
> > > > All out of bound array checks?
> > > > 
> > > > Cheers,
> > > > Biju
> > > > 
> > > > }
> > > > > +
> > > > > +#define rzg2l_cru_write(cru, offset, value) \
> > > > > +	(__builtin_constant_p(offset) ? \
> > > > > +	 __rzg2l_cru_write_constant(cru, offset, value) : \
> > > > > +	 __rzg2l_cru_write(cru, offset, value))
> > > > > +
> > > > > +#define rzg2l_cru_read(cru, offset) \
> > > > > +	(__builtin_constant_p(offset) ? \
> > > > > +	 __rzg2l_cru_read_constant(cru, offset) : \
> > > > > +	 __rzg2l_cru_read(cru, offset))
> > > > > +
> > > > >  /* Need to hold qlock before calling */  static void return_unused_buffers(struct rzg2l_cru_dev *cru,
> > > > >  				  enum vb2_buffer_state state)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 11/17] media: rzg2l-cru: Add register mapping support

[Tommaso Merciai]

Hi Laurent,

On Thu, Mar 27, 2025 at 07:06:59PM +0200, Laurent Pinchart wrote:
> On Thu, Mar 27, 2025 at 05:45:39PM +0100, Tommaso Merciai wrote:
> > Hi Laurent,
> > 
> > Thanks for your comment.
> > 
> > On Thu, Mar 27, 2025 at 12:15:54PM +0200, Laurent Pinchart wrote:
> > > Hi Tommaso,
> > > 
> > > Thanks for being patient (and reminding me about this). Apparently,
> > > Embedded World is bad for e-mail backlogs.
> > 
> > I can imagine.
> > I skipped the EW this year, hope you had fun there :)
> > No worries.
> > 
> > > 
> > > On Thu, Mar 13, 2025 at 01:01:24PM +0100, Tommaso Merciai wrote:
> > > > On Wed, Mar 12, 2025 at 01:37:25PM +0000, Biju Das wrote:
> > > > > On 03 March 2025 16:08, Tommaso Merciai wrote:
> > > > > > 
> > > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > 
> > > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a CRU-IP that is mostly identical
> > > > > > to RZ/G2L but with different register offsets and additional registers. Introduce a flexible register
> > > > > > mapping mechanism to handle these variations.
> > > > > > 
> > > > > > Define the `rzg2l_cru_info` structure to store register mappings and pass it as part of the OF match
> > > > > > data. Update the read/write functions to check out-of-bound accesses and use indexed register offsets
> > > > > > from `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
> > > > > > 
> > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > > > ---
> > > > > > Changes since v2:
> > > > > >  - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > > >    accesses as suggested by LPinchart.
> > > > > >  - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > > >  - Update commit body
> > > > > > 
> > > > > >  .../platform/renesas/rzg2l-cru/rzg2l-core.c   | 46 ++++++++++++-
> > > > > >  .../renesas/rzg2l-cru/rzg2l-cru-regs.h        | 66 ++++++++++---------
> > > > > >  .../platform/renesas/rzg2l-cru/rzg2l-cru.h    |  4 ++
> > > > > >  .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 58 ++++++++++++++--
> > > > > >  4 files changed, 139 insertions(+), 35 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > index eed9d2bd0841..abc2a979833a 100644
> > > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > @@ -22,6 +22,7 @@
> > > > > >  #include <media/v4l2-mc.h>
> > > > > > 
> > > > > >  #include "rzg2l-cru.h"
> > > > > > +#include "rzg2l-cru-regs.h"
> > > > > > 
> > > > > >  static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n)  { @@ -269,6
> > > > > > +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > > > > > 
> > > > > >  	cru->dev = dev;
> > > > > >  	cru->info = of_device_get_match_data(dev);
> > > > > > +	if (!cru->info)
> > > > > > +		return dev_err_probe(dev, -EINVAL,
> > > > > > +				     "Failed to get OF match data\n");
> > > > > > 
> > > > > >  	irq = platform_get_irq(pdev, 0);
> > > > > >  	if (irq < 0)
> > > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > > >  	rzg2l_cru_dma_unregister(cru);
> > > > > >  }
> > > > > > 
> > > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > > +	[CRUnCTRL] = 0x0,
> > > > > > +	[CRUnIE] = 0x4,
> > > > > > +	[CRUnINTS] = 0x8,
> > > > > > +	[CRUnRST] = 0xc,
> > > > > > +	[AMnMB1ADDRL] = 0x100,
> > > > > > +	[AMnMB1ADDRH] = 0x104,
> > > > > > +	[AMnMB2ADDRL] = 0x108,
> > > > > > +	[AMnMB2ADDRH] = 0x10c,
> > > > > > +	[AMnMB3ADDRL] = 0x110,
> > > > > > +	[AMnMB3ADDRH] = 0x114,
> > > > > > +	[AMnMB4ADDRL] = 0x118,
> > > > > > +	[AMnMB4ADDRH] = 0x11c,
> > > > > > +	[AMnMB5ADDRL] = 0x120,
> > > > > > +	[AMnMB5ADDRH] = 0x124,
> > > > > > +	[AMnMB6ADDRL] = 0x128,
> > > > > > +	[AMnMB6ADDRH] = 0x12c,
> > > > > > +	[AMnMB7ADDRL] = 0x130,
> > > > > > +	[AMnMB7ADDRH] = 0x134,
> > > > > > +	[AMnMB8ADDRL] = 0x138,
> > > > > > +	[AMnMB8ADDRH] = 0x13c,
> > > > > > +	[AMnMBVALID] = 0x148,
> > > > > > +	[AMnMBS] = 0x14c,
> > > > > > +	[AMnAXIATTR] = 0x158,
> > > > > > +	[AMnFIFOPNTR] = 0x168,
> > > > > > +	[AMnAXISTP] = 0x174,
> > > > > > +	[AMnAXISTPACK] = 0x178,
> > > > > > +	[ICnEN] = 0x200,
> > > > > > +	[ICnMC] = 0x208,
> > > > > > +	[ICnMS] = 0x254,
> > > > > > +	[ICnDMR] = 0x26c,
> > > > > > +};
> > > > > > +
> > > > > > +static const struct rzg2l_cru_info rzgl2_cru_info = {
> > > > > > +	.regs = rzg2l_cru_regs,
> > > > > > +};
> > > > > > +
> > > > > >  static const struct of_device_id rzg2l_cru_of_id_table[] = {
> > > > > > -	{ .compatible = "renesas,rzg2l-cru", },
> > > > > > +	{
> > > > > > +		.compatible = "renesas,rzg2l-cru",
> > > > > > +		.data = &rzgl2_cru_info,
> > > > > > +	},
> > > > > >  	{ /* sentinel */ }
> > > > > >  };
> > > > > >  MODULE_DEVICE_TABLE(of, rzg2l_cru_of_id_table); diff --git a/drivers/media/platform/renesas/rzg2l-
> > > > > > cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > > > > index 1c9f22118a5d..86c320286246 100644
> > > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> > > > > > @@ -10,71 +10,77 @@
> > > > > > 
> > > > > >  /* 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_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))
> > > > > > +#define AMnMBxADDRL(x)			(AMnMB1ADDRL + (x) * 2)
> > > > > > 
> > > > > >  /* Memory Bank Base Address (Higher) Register for CRU Image Data */
> > > > > > -#define AMnMBxADDRH(x)			(0x104 + ((x) * 8))
> > > > > > +#define AMnMBxADDRH(x)			(AMnMB1ADDRH + (x) * 2)
> > > > > > 
> > > > > > -/* 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 Transfer Setting Register for CRU Image Data */
> > > > > > -#define AMnAXIATTR			0x158
> > > > > >  #define AMnAXIATTR_AXILEN_MASK		GENMASK(3, 0)
> > > > > >  #define AMnAXIATTR_AXILEN		(0xf)
> > > > > > 
> > > > > > -/* 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(x)			((x) << 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)
> > > > > > 
> > > > > > +enum rzg2l_cru_common_regs {
> > > > > > +	CRUnCTRL,	/* CRU Control */
> > > > > > +	CRUnIE,		/* CRU Interrupt Enable */
> > > > > > +	CRUnINTS,	/* CRU Interrupt Status */
> > > > > > +	CRUnRST, 	/* CRU Reset */
> > > > > > +	AMnMB1ADDRL,	/* Bank 1 Address (Lower) for CRU Image Data */
> > > > > > +	AMnMB1ADDRH,	/* Bank 1 Address (Higher) for CRU Image Data */
> > > > > > +	AMnMB2ADDRL,    /* Bank 2 Address (Lower) for CRU Image Data */
> > > > > > +	AMnMB2ADDRH,    /* Bank 2 Address (Higher) for CRU Image Data */
> > > > > > +	AMnMB3ADDRL,    /* Bank 3 Address (Lower) for CRU Image Data */
> > > > > > +	AMnMB3ADDRH,    /* Bank 3 Address (Higher) for CRU Image Data */
> > > > > > +	AMnMB4ADDRL,    /* Bank 4 Address (Lower) for CRU Image Data */
> > > > > > +	AMnMB4ADDRH,    /* Bank 4 Address (Higher) for CRU Image Data */
> > > > > > +	AMnMB5ADDRL,    /* Bank 5 Address (Lower) for CRU Image Data */
> > > > > > +	AMnMB5ADDRH,    /* Bank 5 Address (Higher) for CRU Image Data */
> > > > > > +	AMnMB6ADDRL,    /* Bank 6 Address (Lower) for CRU Image Data */
> > > > > > +	AMnMB6ADDRH,    /* Bank 6 Address (Higher) for CRU Image Data */
> > > > > > +	AMnMB7ADDRL,    /* Bank 7 Address (Lower) for CRU Image Data */
> > > > > > +	AMnMB7ADDRH,    /* Bank 7 Address (Higher) for CRU Image Data */
> > > > > > +	AMnMB8ADDRL,    /* Bank 8 Address (Lower) for CRU Image Data */
> > > > > > +	AMnMB8ADDRH,    /* Bank 8 Address (Higher) for CRU Image Data */
> > > > > > +	AMnMBVALID,	/* Memory Bank Enable for CRU Image Data */
> > > > > > +	AMnMBS,		/* Memory Bank Status for CRU Image Data */
> > > > > > +	AMnAXIATTR,	/* AXI Master Transfer Setting Register for CRU Image Data */
> > > > > > +	AMnFIFOPNTR,	/* AXI Master FIFO Pointer for CRU Image Data */
> > > > > > +	AMnAXISTP,	/* AXI Master Transfer Stop for CRU Image Data */
> > > > > > +	AMnAXISTPACK,	/* AXI Master Transfer Stop Status for CRU Image Data */
> > > > > > +	ICnEN,		/* CRU Image Processing Enable */
> > > > > > +	ICnMC,		/* CRU Image Processing Main Control */
> > > > > > +	ICnMS,		/* CRU Module Status */
> > > > > > +	ICnDMR,		/* CRU Data Output Mode */
> > > > > > +	RZG2L_CRU_MAX_REG,
> > > > > > +};
> > > > > > +
> > > > > >  #endif /* __RZG2L_CRU_REGS_H__ */
> > > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > > > index 8b898ce05b84..00c3f7458e20 100644
> > > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > > > @@ -80,6 +80,10 @@ struct rzg2l_cru_ip_format {
> > > > > >  	bool yuv;
> > > > > >  };
> > > > > > 
> > > > > > +struct rzg2l_cru_info {
> > > > > > +	const u16 *regs;
> > > > > > +};
> > > > > > +
> > > > > >  /**
> > > > > >   * struct rzg2l_cru_dev - Renesas CRU device structure
> > > > > >   * @dev:		(OF) device
> > > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > > > index cd69c8a686d3..792f0df51a4b 100644
> > > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > > > @@ -42,16 +42,66 @@ struct rzg2l_cru_buffer {
> > > > > >  /* -----------------------------------------------------------------------------
> > > > > >   * DMA operations
> > > > > >   */
> > > > > > -static void rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> > > > > > +static void __rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset,
> > > > > > +u32 value)
> > > > > >  {
> > > > > > -	iowrite32(value, cru->base + offset);
> > > > > > +	const u16 *regs = cru->info->regs;
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> > > > > > +	 * rest of the registers have valid offset being set in cru->info->regs.
> > > > > > +	 */
> > > > > > +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> > > > > > +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> > > > > > +		return;
> > > > > > +
> > > > > > +	iowrite32(value, cru->base + regs[offset]); }
> > > > > > +
> > > > > > +static u32 __rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset) {
> > > > > > +	const u16 *regs = cru->info->regs;
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * CRUnCTRL is a first register on all CRU supported SoCs so validate
> > > > > > +	 * rest of the registers have valid offset being set in cru->info->regs.
> > > > > > +	 */
> > > > > > +	if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> > > > > > +	    WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> > > > > > +		return 0;
> > > > > > +
> > > > > > +	return ioread32(cru->base + regs[offset]);
> > > > > >  }
> > > > > > 
> > > > > > -static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
> > > > > > +static inline void
> > > > > > +__rzg2l_cru_write_constant(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> > > > > >  {
> > > > > > -	return ioread32(cru->base + offset);
> > > > > > +	const u16 *regs = cru->info->regs;
> > > > > > +
> > > > > > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > > > > > +
> > > > > > +	iowrite32(value, cru->base + regs[offset]);
> > > > >
> > > > > Do you need this code as the purpose is to test compile time constant and
> > > > > It won't execute at run time?
> > > 
> > > Biju, I'm not sure to understan this comment.
> > > __rzg2l_cru_write_constant() is called at runtime, with a compile-time
> > > constant offset. The BUILD_BUG_ON() verifies at compile time that the
> > > offset is valid, causing compilation errors if it isn't.
> > > 
> > > __rzg2l_cru_write(), on the other hand, is called when the offset is not
> > > known at compile time, because it's computed dynamically. That's a small
> > > subset of the calls. It needs to check the offset at runtime for
> > > overflows.
> > > 
> > > What do you mean by "won't execute at runtime", and what code do you
> > > think is not needed ?
> > > 
> > > > It was suggested in a previous review.
> > > > 
> > > > I've done some investigation on the above bot issue here.
> > > > Using __always_inline for constant read/write issue seems solved.
> > > > 
> > > > I found this link: https://www.kernel.org/doc/local/inline.html
> > > > 
> > > > But tbh I'm not finding an example into the kernel that use both 
> > > > BUILD_BUG_ON and  __always_inline.
> > > > 
> > > > Laurent what do you think about? Do you have some hints?
> > > > Thanks in advance.
> > > 
> > > Do you mean that the compile-time assertions are caused by
> > > __rzg2l_cru_write_constant() not being inlined ?
> > 
> > Seems yes.
> > Using __always_inline seems to solve the issue reported by the bot test.
> > 
> > > The function could be
> > > marked as __always_inline I suppose. Or the BUILD_BUG_ON() check could
> > > be moved to the rzg2l_cru_write() macro.
> > 
> > Mmm not sure that I completely got this way.
> > 
> > Actually we have:
> > 
> > #define rzg2l_cru_write(cru, offset, value) \
> > 	(__builtin_constant_p(offset) ? \
> > 	 __rzg2l_cru_write_constant(cru, offset, value) : \
> > 	 __rzg2l_cru_write(cru, offset, value))
> > 
> > And BUILD_BUG_ON() can only be user on constant offset.
> 
> There seems to be quite a few examples of usage of __always_inline with
> BUILD_BUG_ON(), so we can go that way. Otherwise, you could write
> something like (untested)
> 
> #define rzg2l_cru_write(cru, offset, value)				\
> ({									\
> 	u32 __offset = (offset);					\
> 	if (__builtin_constant_p(__offset)) {				\
> 		BUILD_BUG_ON(__offset >= RZG2L_CRU_MAX_REG);		\
> 		__rzg2l_cru_write_constant(cru, __offset, value);	\
> 	} else {							\
> 		__rzg2l_cru_write(cru, __offset, value));		\
> 	}								\
> })

Thanks for the hint.

> 
> > > > > >  }
> > > > > > 
> > > > > > +static inline u32
> > > > > > +__rzg2l_cru_read_constant(struct rzg2l_cru_dev *cru, u32 offset) {
> > > > > > +	const u16 *regs = cru->info->regs;
> > > > > > +
> > > > > > +	BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> > > > > > +
> > > > > > +	return ioread32(cru->base + regs[offset]); 
> > > > > 
> > > > > Do you need this code as the purpose is to test compile time constant and
> > > > > It won't execute at run time?
> > > > > 
> > > > > Not sure, maybe adding an entry with MAX_ID in LUT,
> > > > > that will avoid buffer overflows and you can take out
> > > > > All out of bound array checks?
> > > > > 
> > > > > Cheers,
> > > > > Biju
> > > > > 
> > > > > }
> > > > > > +
> > > > > > +#define rzg2l_cru_write(cru, offset, value) \
> > > > > > +	(__builtin_constant_p(offset) ? \
> > > > > > +	 __rzg2l_cru_write_constant(cru, offset, value) : \
> > > > > > +	 __rzg2l_cru_write(cru, offset, value))
> > > > > > +
> > > > > > +#define rzg2l_cru_read(cru, offset) \
> > > > > > +	(__builtin_constant_p(offset) ? \
> > > > > > +	 __rzg2l_cru_read_constant(cru, offset) : \
> > > > > > +	 __rzg2l_cru_read(cru, offset))
> > > > > > +
> > > > > >  /* Need to hold qlock before calling */  static void return_unused_buffers(struct rzg2l_cru_dev *cru,
> > > > > >  				  enum vb2_buffer_state state)
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Regards,
Tommaso