* [PATCH 0/8] media: rzg2l-cru: Document RZ/G3E (CSI2, CRU)
@ 2025-02-10 11:45 Tommaso Merciai
2025-02-10 11:45 ` [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets Tommaso Merciai
` (7 more replies)
0 siblings, 8 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-10 11:45 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel
Dear All,
In preparation of supporting the CRU/CSI2 IPs found into the Renesas RZ/G3E
SoC, this series adds support for CRU0 clocks and resets, adds also dt-bindings
for CRU and CSI2 IP and adds some fixes into rzg2l-csi2 and rzg2l-core drivers.
The series was tested in a out of tree branch with the following media
pipeline:
imx219 -> rzg3e CSI2 -> rzg3e CRU
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:SRGGB8_1X8/1920x1080 field:none]
<- "imx219 0-0010":0 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:SRGGB8_1X8/1920x1080 field:none]
-> "cru-ip-16000000.video":0 [ENABLED,IMMUTABLE]
- entity 4: imx219 0-0010 (1 pad, 1 link, 0 routes)
type V4L2 subdev subtype Sensor flags 0
device node name /dev/v4l-subdev1
pad0: Source
[stream:0 fmt:SRGGB8_1X8/1920x1080 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range
crop.bounds:(8,8)/3280x2464
crop:(688,160)/1920x2160]
-> "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:SRGGB8_1X8/1920x1080 field:none]
<- "csi-16000400.csi2":1 [ENABLED,IMMUTABLE]
pad1: Source
[stream:0 fmt:SRGGB8_1X8/1920x1080 field:none]
-> "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
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 rzg2l_cru device /dev/video0:
Driver Info:
Driver name : rzg2l_cru
Card type : RZG2L_CRU
Bus info : platform:16000000.video
Driver version : 6.14.0
Capabilities : 0xa4200001
Serial :
Bus info : platform:16000000.video
Media version : 6.14.0
Hardware revision: 0x00000000 (0)
Driver version : 6.14.0
Interface Info:
ID : 0x03000013
Type : V4L Video
Entity Info:
ID : 0x00000011 (17)
Name : CRU output
Function : V4L2 I/O
Pad 0x01000012 : 0: Sink, Must Connect
Link 0x02000017: from remote pad 0x100000a of entity 'cru-ip-16000000.video' (Video Pixel Formatter): Data, Enabled, Immutable
Required ioctls:
test MC information (see 'Media Driver Info' above): OK
test VIDIOC_QUERYCAP: OK
test invalid ioctls: OK
Allow for multiple opens:
test second /dev/video0 open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK
Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
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
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 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 (Input 0):
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 (Input 0):
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK (Not Supported)
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK
Codec ioctls (Input 0):
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 (Input 0):
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test CREATE_BUFS maximum buffers: OK
test VIDIOC_EXPBUF: OK
test Requests: OK (Not Supported)
Total for rzg2l_cru device /dev/video0: 47, Succeeded: 47, Failed: 0, Warnings: 0
root@smarc-rzg3e:~# v4l2-compliance -d /dev/v4l-subdev0
v4l2-compliance SHA: 4aee01a02792 2023-12-12 21:40:38
Compliance test for device /dev/v4l-subdev0:
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-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-subdev2
v4l2-compliance 1.26.1-5142, 64 [ 1509.489660] cru-ip-16000000.video: ================= START STATUS =================
bits, 64-bit tim[ 1509.498500] 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 (3):
media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
media: rzg2l-cru: csi2: Use temporary variable for struct device in
rzg2l_csi2_probe()
media: rzg2l-cru: rzg2l-core: Use temporary variable for struct device
in rzg2l_cru_probe()
Tommaso Merciai (5):
clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets
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()
.../bindings/media/renesas,rzg2l-cru.yaml | 33 ++++++---
.../bindings/media/renesas,rzg2l-csi2.yaml | 67 ++++++++++++++-----
drivers/clk/renesas/r9a09g047-cpg.c | 24 +++++++
.../platform/renesas/rzg2l-cru/rzg2l-core.c | 32 ++++-----
.../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 38 +++++------
5 files changed, 133 insertions(+), 61 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets
2025-02-10 11:45 [PATCH 0/8] media: rzg2l-cru: Document RZ/G3E (CSI2, CRU) Tommaso Merciai
@ 2025-02-10 11:45 ` Tommaso Merciai
2025-02-10 11:54 ` Biju Das
2025-02-20 14:36 ` Geert Uytterhoeven
2025-02-10 11:45 ` [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC Tommaso Merciai
` (6 subsequent siblings)
7 siblings, 2 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-10 11:45 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Magnus Damm,
devicetree, linux-kernel, linux-clk
Add support for CRU0 clocks and resets along with the corresponding
divider.
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
drivers/clk/renesas/r9a09g047-cpg.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/clk/renesas/r9a09g047-cpg.c b/drivers/clk/renesas/r9a09g047-cpg.c
index 51fd24c20ed5..5d02031219d8 100644
--- a/drivers/clk/renesas/r9a09g047-cpg.c
+++ b/drivers/clk/renesas/r9a09g047-cpg.c
@@ -28,6 +28,7 @@ enum clk_ids {
CLK_PLLCLN,
CLK_PLLDTY,
CLK_PLLCA55,
+ CLK_PLLVDO,
/* Internal Core Clocks */
CLK_PLLCM33_DIV16,
@@ -35,7 +36,10 @@ enum clk_ids {
CLK_PLLCLN_DIV8,
CLK_PLLCLN_DIV16,
CLK_PLLDTY_ACPU,
+ CLK_PLLDTY_ACPU_DIV2,
CLK_PLLDTY_ACPU_DIV4,
+ CLK_PLLDTY_DIV16,
+ CLK_PLLVDO_CRU0,
/* Module Clocks */
MOD_CLK_BASE,
@@ -49,6 +53,12 @@ static const struct clk_div_table dtable_1_8[] = {
{0, 0},
};
+static const struct clk_div_table dtable_2_4[] = {
+ {0, 2},
+ {1, 4},
+ {0, 0},
+};
+
static const struct clk_div_table dtable_2_64[] = {
{0, 2},
{1, 4},
@@ -69,6 +79,7 @@ static const struct cpg_core_clk r9a09g047_core_clks[] __initconst = {
DEF_FIXED(".pllcln", CLK_PLLCLN, CLK_QEXTAL, 200, 3),
DEF_FIXED(".plldty", CLK_PLLDTY, CLK_QEXTAL, 200, 3),
DEF_PLL(".pllca55", CLK_PLLCA55, CLK_QEXTAL, PLL_CONF(0x64)),
+ DEF_FIXED(".pllvdo", CLK_PLLVDO, CLK_QEXTAL, 105, 2),
/* Internal Core Clocks */
DEF_FIXED(".pllcm33_div16", CLK_PLLCM33_DIV16, CLK_PLLCM33, 1, 16),
@@ -78,7 +89,11 @@ static const struct cpg_core_clk r9a09g047_core_clks[] __initconst = {
DEF_FIXED(".pllcln_div16", CLK_PLLCLN_DIV16, CLK_PLLCLN, 1, 16),
DEF_DDIV(".plldty_acpu", CLK_PLLDTY_ACPU, CLK_PLLDTY, CDDIV0_DIVCTL2, dtable_2_64),
+ DEF_FIXED(".plldty_acpu_div2", CLK_PLLDTY_ACPU_DIV2, CLK_PLLDTY_ACPU, 1, 2),
DEF_FIXED(".plldty_acpu_div4", CLK_PLLDTY_ACPU_DIV4, CLK_PLLDTY_ACPU, 1, 4),
+ DEF_FIXED(".plldty_div16", CLK_PLLDTY_DIV16, CLK_PLLDTY, 1, 16),
+
+ DEF_DDIV(".pllvdo_cru0", CLK_PLLVDO_CRU0, CLK_PLLVDO, CDDIV3_DIVCTL3, dtable_2_4),
/* Core Clocks */
DEF_FIXED("sys_0_pclk", R9A09G047_SYS_0_PCLK, CLK_QEXTAL, 1, 1),
@@ -154,6 +169,12 @@ static const struct rzv2h_mod_clk r9a09g047_mod_clks[] __initconst = {
BUS_MSTOP(8, BIT(4))),
DEF_MOD("sdhi_2_aclk", CLK_PLLDTY_ACPU_DIV4, 10, 14, 5, 14,
BUS_MSTOP(8, BIT(4))),
+ DEF_MOD("cru_0_aclk", CLK_PLLDTY_ACPU_DIV2, 13, 2, 6, 18,
+ BUS_MSTOP(9, BIT(4))),
+ DEF_MOD_NO_PM("cru_0_vclk", CLK_PLLVDO_CRU0, 13, 3, 6, 19,
+ BUS_MSTOP(9, BIT(4))),
+ DEF_MOD("cru_0_pclk", CLK_PLLDTY_DIV16, 13, 4, 6, 20,
+ BUS_MSTOP(9, BIT(4))),
};
static const struct rzv2h_reset r9a09g047_resets[] __initconst = {
@@ -177,6 +198,9 @@ static const struct rzv2h_reset r9a09g047_resets[] __initconst = {
DEF_RST(10, 7, 4, 24), /* SDHI_0_IXRST */
DEF_RST(10, 8, 4, 25), /* SDHI_1_IXRST */
DEF_RST(10, 9, 4, 26), /* SDHI_2_IXRST */
+ DEF_RST(12, 5, 5, 22), /* CRU_0_PRESETN */
+ DEF_RST(12, 6, 5, 23), /* CRU_0_ARESETN */
+ DEF_RST(12, 7, 5, 24), /* CRU_0_S_RESETN */
};
const struct rzv2h_cpg_info r9a09g047_cpg_info __initconst = {
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-02-10 11:45 [PATCH 0/8] media: rzg2l-cru: Document RZ/G3E (CSI2, CRU) Tommaso Merciai
2025-02-10 11:45 ` [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets Tommaso Merciai
@ 2025-02-10 11:45 ` Tommaso Merciai
2025-02-14 0:29 ` Laurent Pinchart
2025-02-19 14:52 ` Rob Herring
2025-02-10 11:45 ` [PATCH 3/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block Tommaso Merciai
` (5 subsequent siblings)
7 siblings, 2 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-10 11:45 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel
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.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
.../bindings/media/renesas,rzg2l-csi2.yaml | 63 ++++++++++++++-----
1 file changed, 48 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..0d07c55a3f35 100644
--- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
@@ -17,12 +17,15 @@ 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 +34,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 +59,7 @@ properties:
resets:
items:
- description: CRU_PRESETN reset terminal
- - description: CRU_CMN_RSTB reset terminal
+ - description: CRU_CMN_RSTB reset terminal or D-PHY reset
reset-names:
items:
@@ -101,6 +112,28 @@ 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:
+ maxItems: 3
+
+ clock-names:
+ maxItems: 3
+
additionalProperties: false
examples:
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 3/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block
2025-02-10 11:45 [PATCH 0/8] media: rzg2l-cru: Document RZ/G3E (CSI2, CRU) Tommaso Merciai
2025-02-10 11:45 ` [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets Tommaso Merciai
2025-02-10 11:45 ` [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC Tommaso Merciai
@ 2025-02-10 11:45 ` Tommaso Merciai
2025-02-13 8:56 ` Krzysztof Kozlowski
2025-02-10 11:45 ` [PATCH 4/8] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC Tommaso Merciai
` (4 subsequent siblings)
7 siblings, 1 reply; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-10 11:45 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel
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.
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
.../devicetree/bindings/media/renesas,rzg2l-csi2.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
index 0d07c55a3f35..ecc620e9ca52 100644
--- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
@@ -25,6 +25,10 @@ properties:
- 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.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 4/8] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC
2025-02-10 11:45 [PATCH 0/8] media: rzg2l-cru: Document RZ/G3E (CSI2, CRU) Tommaso Merciai
` (2 preceding siblings ...)
2025-02-10 11:45 ` [PATCH 3/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block Tommaso Merciai
@ 2025-02-10 11:45 ` Tommaso Merciai
2025-02-10 13:47 ` Rob Herring (Arm)
2025-02-14 0:45 ` Laurent Pinchart
2025-02-10 11:45 ` [PATCH 5/8] media: rzg2l-cru: csi2: Use temporary variable for struct device in rzg2l_csi2_probe() Tommaso Merciai
` (3 subsequent siblings)
7 siblings, 2 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-10 11:45 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel
The CRU block found on the Renesas RZ/G3E ("R9A09G047") SoC has five
interrups:
- 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.
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
.../bindings/media/renesas,rzg2l-cru.yaml | 33 ++++++++++++-------
1 file changed, 22 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..7e4a7ed56378 100644
--- a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
@@ -17,24 +17,34 @@ 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
+ maxItems: 5
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:
@@ -120,6 +130,7 @@ allOf:
contains:
enum:
- renesas,r9a07g043-cru
+ - renesas,r9a09g047-cru
then:
properties:
ports:
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 5/8] media: rzg2l-cru: csi2: Use temporary variable for struct device in rzg2l_csi2_probe()
2025-02-10 11:45 [PATCH 0/8] media: rzg2l-cru: Document RZ/G3E (CSI2, CRU) Tommaso Merciai
` (3 preceding siblings ...)
2025-02-10 11:45 ` [PATCH 4/8] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC Tommaso Merciai
@ 2025-02-10 11:45 ` Tommaso Merciai
2025-02-10 11:55 ` Biju Das
2025-02-14 0:47 ` Laurent Pinchart
2025-02-10 11:45 ` [PATCH 6/8] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable() Tommaso Merciai
` (2 subsequent siblings)
7 siblings, 2 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-10 11:45 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Geert Uytterhoeven, Magnus Damm, Laurent Pinchart, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Use a temporary variable for the struct device pointers to avoid
dereferencing.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
.../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.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 6/8] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable()
2025-02-10 11:45 [PATCH 0/8] media: rzg2l-cru: Document RZ/G3E (CSI2, CRU) Tommaso Merciai
` (4 preceding siblings ...)
2025-02-10 11:45 ` [PATCH 5/8] media: rzg2l-cru: csi2: Use temporary variable for struct device in rzg2l_csi2_probe() Tommaso Merciai
@ 2025-02-10 11:45 ` Tommaso Merciai
2025-02-10 11:56 ` Biju Das
2025-02-14 0:48 ` Laurent Pinchart
2025-02-10 11:45 ` [PATCH 7/8] media: rzg2l-cru: rzg2l-core: Use temporary variable for struct device in rzg2l_cru_probe() Tommaso Merciai
2025-02-10 11:45 ` [PATCH 8/8] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable() Tommaso Merciai
7 siblings, 2 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-10 11:45 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Laurent Pinchart, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
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().
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
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.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 7/8] media: rzg2l-cru: rzg2l-core: Use temporary variable for struct device in rzg2l_cru_probe()
2025-02-10 11:45 [PATCH 0/8] media: rzg2l-cru: Document RZ/G3E (CSI2, CRU) Tommaso Merciai
` (5 preceding siblings ...)
2025-02-10 11:45 ` [PATCH 6/8] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable() Tommaso Merciai
@ 2025-02-10 11:45 ` Tommaso Merciai
2025-02-10 11:56 ` Biju Das
2025-02-14 0:49 ` Laurent Pinchart
2025-02-10 11:45 ` [PATCH 8/8] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable() Tommaso Merciai
7 siblings, 2 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-10 11:45 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Geert Uytterhoeven, Magnus Damm, Laurent Pinchart, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Use a temporary variable for the struct device pointers to avoid
dereferencing.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
.../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.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 8/8] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable()
2025-02-10 11:45 [PATCH 0/8] media: rzg2l-cru: Document RZ/G3E (CSI2, CRU) Tommaso Merciai
` (6 preceding siblings ...)
2025-02-10 11:45 ` [PATCH 7/8] media: rzg2l-cru: rzg2l-core: Use temporary variable for struct device in rzg2l_cru_probe() Tommaso Merciai
@ 2025-02-10 11:45 ` Tommaso Merciai
2025-02-10 11:57 ` Biju Das
2025-02-14 0:50 ` Laurent Pinchart
7 siblings, 2 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-10 11:45 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Laurent Pinchart, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
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().
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
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..5548b328d970 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)
+ return ret;
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.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* RE: [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets
2025-02-10 11:45 ` [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets Tommaso Merciai
@ 2025-02-10 11:54 ` Biju Das
2025-02-20 14:39 ` Geert Uytterhoeven
2025-02-20 14:36 ` Geert Uytterhoeven
1 sibling, 1 reply; 36+ messages in thread
From: Biju Das @ 2025-02-10 11:54 UTC (permalink / raw)
To: Tommaso Merciai, Tommaso Merciai
Cc: linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Magnus Damm,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-clk@vger.kernel.org
Hi Tommaso Merciai,
> -----Original Message-----
> From: Tommaso Merciai <tomm.merciai@gmail.com>
> Sent: 10 February 2025 11:46
> Subject: [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets
>
> Add support for CRU0 clocks and resets along with the corresponding divider.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> drivers/clk/renesas/r9a09g047-cpg.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/clk/renesas/r9a09g047-cpg.c b/drivers/clk/renesas/r9a09g047-cpg.c
> index 51fd24c20ed5..5d02031219d8 100644
> --- a/drivers/clk/renesas/r9a09g047-cpg.c
> +++ b/drivers/clk/renesas/r9a09g047-cpg.c
> @@ -28,6 +28,7 @@ enum clk_ids {
> CLK_PLLCLN,
> CLK_PLLDTY,
> CLK_PLLCA55,
> + CLK_PLLVDO,
>
> /* Internal Core Clocks */
> CLK_PLLCM33_DIV16,
> @@ -35,7 +36,10 @@ enum clk_ids {
> CLK_PLLCLN_DIV8,
> CLK_PLLCLN_DIV16,
> CLK_PLLDTY_ACPU,
> + CLK_PLLDTY_ACPU_DIV2,
> CLK_PLLDTY_ACPU_DIV4,
> + CLK_PLLDTY_DIV16,
> + CLK_PLLVDO_CRU0,
>
> /* Module Clocks */
> MOD_CLK_BASE,
> @@ -49,6 +53,12 @@ static const struct clk_div_table dtable_1_8[] = {
> {0, 0},
> };
>
> +static const struct clk_div_table dtable_2_4[] = {
> + {0, 2},
> + {1, 4},
> + {0, 0},
Not sure {0, 2}, {1, 4}, {0, 0}, to make lines shorter?
Cheers,
Biju
> +};
> +
> static const struct clk_div_table dtable_2_64[] = {
> {0, 2},
> {1, 4},
> @@ -69,6 +79,7 @@ static const struct cpg_core_clk r9a09g047_core_clks[] __initconst = {
> DEF_FIXED(".pllcln", CLK_PLLCLN, CLK_QEXTAL, 200, 3),
> DEF_FIXED(".plldty", CLK_PLLDTY, CLK_QEXTAL, 200, 3),
> DEF_PLL(".pllca55", CLK_PLLCA55, CLK_QEXTAL, PLL_CONF(0x64)),
> + DEF_FIXED(".pllvdo", CLK_PLLVDO, CLK_QEXTAL, 105, 2),
>
> /* Internal Core Clocks */
> DEF_FIXED(".pllcm33_div16", CLK_PLLCM33_DIV16, CLK_PLLCM33, 1, 16), @@ -78,7 +89,11 @@ static
> const struct cpg_core_clk r9a09g047_core_clks[] __initconst = {
> DEF_FIXED(".pllcln_div16", CLK_PLLCLN_DIV16, CLK_PLLCLN, 1, 16),
>
> DEF_DDIV(".plldty_acpu", CLK_PLLDTY_ACPU, CLK_PLLDTY, CDDIV0_DIVCTL2, dtable_2_64),
> + DEF_FIXED(".plldty_acpu_div2", CLK_PLLDTY_ACPU_DIV2, CLK_PLLDTY_ACPU,
> +1, 2),
> DEF_FIXED(".plldty_acpu_div4", CLK_PLLDTY_ACPU_DIV4, CLK_PLLDTY_ACPU, 1, 4),
> + DEF_FIXED(".plldty_div16", CLK_PLLDTY_DIV16, CLK_PLLDTY, 1, 16),
> +
> + DEF_DDIV(".pllvdo_cru0", CLK_PLLVDO_CRU0, CLK_PLLVDO, CDDIV3_DIVCTL3,
> +dtable_2_4),
>
> /* Core Clocks */
> DEF_FIXED("sys_0_pclk", R9A09G047_SYS_0_PCLK, CLK_QEXTAL, 1, 1), @@ -154,6 +169,12 @@ static
> const struct rzv2h_mod_clk r9a09g047_mod_clks[] __initconst = {
> BUS_MSTOP(8, BIT(4))),
> DEF_MOD("sdhi_2_aclk", CLK_PLLDTY_ACPU_DIV4, 10, 14, 5, 14,
> BUS_MSTOP(8, BIT(4))),
> + DEF_MOD("cru_0_aclk", CLK_PLLDTY_ACPU_DIV2, 13, 2, 6, 18,
> + BUS_MSTOP(9, BIT(4))),
> + DEF_MOD_NO_PM("cru_0_vclk", CLK_PLLVDO_CRU0, 13, 3, 6, 19,
> + BUS_MSTOP(9, BIT(4))),
> + DEF_MOD("cru_0_pclk", CLK_PLLDTY_DIV16, 13, 4, 6, 20,
> + BUS_MSTOP(9, BIT(4))),
> };
>
> static const struct rzv2h_reset r9a09g047_resets[] __initconst = { @@ -177,6 +198,9 @@ static const
> struct rzv2h_reset r9a09g047_resets[] __initconst = {
> DEF_RST(10, 7, 4, 24), /* SDHI_0_IXRST */
> DEF_RST(10, 8, 4, 25), /* SDHI_1_IXRST */
> DEF_RST(10, 9, 4, 26), /* SDHI_2_IXRST */
> + DEF_RST(12, 5, 5, 22), /* CRU_0_PRESETN */
> + DEF_RST(12, 6, 5, 23), /* CRU_0_ARESETN */
> + DEF_RST(12, 7, 5, 24), /* CRU_0_S_RESETN */
> };
>
> const struct rzv2h_cpg_info r9a09g047_cpg_info __initconst = {
> --
> 2.34.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH 5/8] media: rzg2l-cru: csi2: Use temporary variable for struct device in rzg2l_csi2_probe()
2025-02-10 11:45 ` [PATCH 5/8] media: rzg2l-cru: csi2: Use temporary variable for struct device in rzg2l_csi2_probe() Tommaso Merciai
@ 2025-02-10 11:55 ` Biju Das
2025-02-14 0:47 ` Laurent Pinchart
1 sibling, 0 replies; 36+ messages in thread
From: Biju Das @ 2025-02-10 11:55 UTC (permalink / raw)
To: Tommaso Merciai, Tommaso Merciai
Cc: linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Geert Uytterhoeven, Magnus Damm, Laurent Pinchart, Hans Verkuil,
Uwe Kleine-König, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi Tommaso Merciai,
> -----Original Message-----
> From: Tommaso Merciai <tomm.merciai@gmail.com>
> Sent: 10 February 2025 11:46
> Subject: [PATCH 5/8] media: rzg2l-cru: csi2: Use temporary variable for struct device in
> rzg2l_csi2_probe()
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Use a temporary variable for the struct device pointers to avoid dereferencing.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Cheers,
Biju
> ---
> .../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.34.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH 6/8] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable()
2025-02-10 11:45 ` [PATCH 6/8] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable() Tommaso Merciai
@ 2025-02-10 11:56 ` Biju Das
2025-02-14 0:48 ` Laurent Pinchart
1 sibling, 0 replies; 36+ messages in thread
From: Biju Das @ 2025-02-10 11:56 UTC (permalink / raw)
To: Tommaso Merciai, Tommaso Merciai
Cc: linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Laurent Pinchart, Hans Verkuil,
Uwe Kleine-König, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi Tommaso Merciai,
> -----Original Message-----
> From: Tommaso Merciai <tomm.merciai@gmail.com>
> Sent: 10 February 2025 11:46
> Subject: [PATCH 6/8] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable()
>
> 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().
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Cheers,
Biju
> ---
> 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.34.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH 7/8] media: rzg2l-cru: rzg2l-core: Use temporary variable for struct device in rzg2l_cru_probe()
2025-02-10 11:45 ` [PATCH 7/8] media: rzg2l-cru: rzg2l-core: Use temporary variable for struct device in rzg2l_cru_probe() Tommaso Merciai
@ 2025-02-10 11:56 ` Biju Das
2025-02-14 0:49 ` Laurent Pinchart
1 sibling, 0 replies; 36+ messages in thread
From: Biju Das @ 2025-02-10 11:56 UTC (permalink / raw)
To: Tommaso Merciai, Tommaso Merciai
Cc: linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Geert Uytterhoeven, Magnus Damm, Laurent Pinchart, Hans Verkuil,
Uwe Kleine-König, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi Tommaso Merciai,
> -----Original Message-----
> From: Tommaso Merciai <tomm.merciai@gmail.com>
> Sent: 10 February 2025 11:46
> Subject: [PATCH 7/8] media: rzg2l-cru: rzg2l-core: Use temporary variable for struct device in
> rzg2l_cru_probe()
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Use a temporary variable for the struct device pointers to avoid dereferencing.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Cheers,
Biju
> ---
> .../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.34.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH 8/8] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable()
2025-02-10 11:45 ` [PATCH 8/8] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable() Tommaso Merciai
@ 2025-02-10 11:57 ` Biju Das
2025-02-14 0:50 ` Laurent Pinchart
1 sibling, 0 replies; 36+ messages in thread
From: Biju Das @ 2025-02-10 11:57 UTC (permalink / raw)
To: Tommaso Merciai, Tommaso Merciai
Cc: linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Laurent Pinchart, Hans Verkuil,
Uwe Kleine-König, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi Tommaso Merciai,
> -----Original Message-----
> From: Tommaso Merciai <tomm.merciai@gmail.com>
> Sent: 10 February 2025 11:46
> Subject: [PATCH 8/8] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable()
>
> 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().
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Cheers,
Biju
> ---
> 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..5548b328d970 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)
> + return ret;
>
> 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.34.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 4/8] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC
2025-02-10 11:45 ` [PATCH 4/8] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC Tommaso Merciai
@ 2025-02-10 13:47 ` Rob Herring (Arm)
2025-02-14 0:45 ` Laurent Pinchart
1 sibling, 0 replies; 36+ messages in thread
From: Rob Herring (Arm) @ 2025-02-10 13:47 UTC (permalink / raw)
To: Tommaso Merciai
Cc: linux-kernel, prabhakar.mahadev-lad.rj, Krzysztof Kozlowski,
Conor Dooley, linux-renesas-soc, devicetree, Tommaso Merciai,
Geert Uytterhoeven, Mauro Carvalho Chehab, biju.das.jz,
Magnus Damm, linux-media
On Mon, 10 Feb 2025 12:45:36 +0100, Tommaso Merciai wrote:
> The CRU block found on the Renesas RZ/G3E ("R9A09G047") SoC has five
> interrups:
>
> - 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.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> .../bindings/media/renesas,rzg2l-cru.yaml | 33 ++++++++++++-------
> 1 file changed, 22 insertions(+), 11 deletions(-)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.example.dtb: video@10830000: interrupts: [[0, 167, 4], [0, 168, 4], [0, 169, 4]] is too short
from schema $id: http://devicetree.org/schemas/media/renesas,rzg2l-cru.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250210114540.524790-5-tommaso.merciai.xr@bp.renesas.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block
2025-02-10 11:45 ` [PATCH 3/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block Tommaso Merciai
@ 2025-02-13 8:56 ` Krzysztof Kozlowski
2025-02-13 15:59 ` Tommaso Merciai
0 siblings, 1 reply; 36+ messages in thread
From: Krzysztof Kozlowski @ 2025-02-13 8:56 UTC (permalink / raw)
To: Tommaso Merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel
On Mon, Feb 10, 2025 at 12:45:35PM +0100, Tommaso Merciai wrote:
> 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.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
From/SoB mismatch.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 3/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block
2025-02-13 8:56 ` Krzysztof Kozlowski
@ 2025-02-13 15:59 ` Tommaso Merciai
0 siblings, 0 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-13 15:59 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, devicetree, linux-kernel
Hi Krzysztof,
Thanks for your comment.
On Thu, Feb 13, 2025 at 09:56:58AM +0100, Krzysztof Kozlowski wrote:
> On Mon, Feb 10, 2025 at 12:45:35PM +0100, Tommaso Merciai wrote:
> > 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.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
>
> From/SoB mismatch.
Ouch sorry, I miss this part on my new setup.
I'll fix that on v2 same of dt-bindings failure on 4/8.
>
> Best regards,
> Krzysztof
>
Thanks & Regards,
Tommaso
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-02-10 11:45 ` [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC Tommaso Merciai
@ 2025-02-14 0:29 ` Laurent Pinchart
2025-02-18 11:55 ` Tommaso Merciai
2025-02-19 14:51 ` Rob Herring
2025-02-19 14:52 ` Rob Herring
1 sibling, 2 replies; 36+ messages in thread
From: Laurent Pinchart @ 2025-02-14 0:29 UTC (permalink / raw)
To: Tommaso Merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel
Hi Tommaso, Prabhakar,
Thank you for the patch.
On Mon, Feb 10, 2025 at 12:45:34PM +0100, Tommaso Merciai wrote:
> 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.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> .../bindings/media/renesas,rzg2l-csi2.yaml | 63 ++++++++++++++-----
> 1 file changed, 48 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..0d07c55a3f35 100644
> --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> @@ -17,12 +17,15 @@ 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
> +
I'd drop the empty line.
> + - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
>
> reg:
> maxItems: 1
> @@ -31,16 +34,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
I would move the clocks and clock-names definitions to the conditional
below. Otherwise I think a device tree that has two clocks only but
incorrectly uses "system" and "video" instead of "video" and "apb" will
validate.
>
> power-domains:
> maxItems: 1
> @@ -48,7 +59,7 @@ properties:
> resets:
> items:
> - description: CRU_PRESETN reset terminal
> - - description: CRU_CMN_RSTB reset terminal
> + - description: CRU_CMN_RSTB reset terminal or D-PHY reset
>
> reset-names:
> items:
> @@ -101,6 +112,28 @@ 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:
> + maxItems: 3
> +
> + clock-names:
> + maxItems: 3
> +
> additionalProperties: false
>
> examples:
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 4/8] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC
2025-02-10 11:45 ` [PATCH 4/8] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC Tommaso Merciai
2025-02-10 13:47 ` Rob Herring (Arm)
@ 2025-02-14 0:45 ` Laurent Pinchart
2025-02-18 15:51 ` Tommaso Merciai
1 sibling, 1 reply; 36+ messages in thread
From: Laurent Pinchart @ 2025-02-14 0:45 UTC (permalink / raw)
To: Tommaso Merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel
Hi Tommaso,
Thank you for the patch.
On Mon, Feb 10, 2025 at 12:45:36PM +0100, Tommaso Merciai wrote:
> The CRU block found on the Renesas RZ/G3E ("R9A09G047") SoC has five
> interrups:
>
> - 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.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> .../bindings/media/renesas,rzg2l-cru.yaml | 33 ++++++++++++-------
> 1 file changed, 22 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..7e4a7ed56378 100644
> --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> @@ -17,24 +17,34 @@ 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
> + maxItems: 5
>
> 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
This should move to a conditional block.
>
> clocks:
> items:
> @@ -120,6 +130,7 @@ allOf:
> contains:
> enum:
> - renesas,r9a07g043-cru
> + - renesas,r9a09g047-cru
> then:
> properties:
> ports:
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 5/8] media: rzg2l-cru: csi2: Use temporary variable for struct device in rzg2l_csi2_probe()
2025-02-10 11:45 ` [PATCH 5/8] media: rzg2l-cru: csi2: Use temporary variable for struct device in rzg2l_csi2_probe() Tommaso Merciai
2025-02-10 11:55 ` Biju Das
@ 2025-02-14 0:47 ` Laurent Pinchart
2025-02-18 12:12 ` Tommaso Merciai
1 sibling, 1 reply; 36+ messages in thread
From: Laurent Pinchart @ 2025-02-14 0:47 UTC (permalink / raw)
To: Tommaso Merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Geert Uytterhoeven, Magnus Damm, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
Hi Tommaso, Prabhakar,
Thank you for the patch.
On Mon, Feb 10, 2025 at 12:45:37PM +0100, Tommaso Merciai wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Use a temporary variable for the struct device pointers to avoid
s/temporary/local/ (in the subject message too).
> dereferencing.
The only advantage of this is shortened lines. That's a coding style
preference, and I'm fine with this patch, but the commit message should
mention this, not "avoid dereferencing".
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> .../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;
> }
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 6/8] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable()
2025-02-10 11:45 ` [PATCH 6/8] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable() Tommaso Merciai
2025-02-10 11:56 ` Biju Das
@ 2025-02-14 0:48 ` Laurent Pinchart
1 sibling, 0 replies; 36+ messages in thread
From: Laurent Pinchart @ 2025-02-14 0:48 UTC (permalink / raw)
To: Tommaso Merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
Hi Tommaso,
Thank you for the patch.
On Mon, Feb 10, 2025 at 12:45:38PM +0100, Tommaso Merciai wrote:
> 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().
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> 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)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 7/8] media: rzg2l-cru: rzg2l-core: Use temporary variable for struct device in rzg2l_cru_probe()
2025-02-10 11:45 ` [PATCH 7/8] media: rzg2l-cru: rzg2l-core: Use temporary variable for struct device in rzg2l_cru_probe() Tommaso Merciai
2025-02-10 11:56 ` Biju Das
@ 2025-02-14 0:49 ` Laurent Pinchart
1 sibling, 0 replies; 36+ messages in thread
From: Laurent Pinchart @ 2025-02-14 0:49 UTC (permalink / raw)
To: Tommaso Merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Geert Uytterhoeven, Magnus Damm, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
Hi Tommaso, Prabhakar,
Thank you for the patch.
On Mon, Feb 10, 2025 at 12:45:39PM +0100, Tommaso Merciai wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Use a temporary variable for the struct device pointers to avoid
> dereferencing.
Same comments as for 5/8.
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>
> ---
> .../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;
> }
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 8/8] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable()
2025-02-10 11:45 ` [PATCH 8/8] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable() Tommaso Merciai
2025-02-10 11:57 ` Biju Das
@ 2025-02-14 0:50 ` Laurent Pinchart
2025-02-18 12:21 ` Tommaso Merciai
1 sibling, 1 reply; 36+ messages in thread
From: Laurent Pinchart @ 2025-02-14 0:50 UTC (permalink / raw)
To: Tommaso Merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
Hi Tommaso,
Thank you for the patch.
On Mon, Feb 10, 2025 at 12:45:40PM +0100, Tommaso Merciai wrote:
> 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().
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> 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..5548b328d970 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)
> + return ret;
Leaking DMA.
>
> 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);
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-02-14 0:29 ` Laurent Pinchart
@ 2025-02-18 11:55 ` Tommaso Merciai
2025-02-18 12:35 ` Laurent Pinchart
2025-02-19 14:51 ` Rob Herring
1 sibling, 1 reply; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-18 11:55 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, devicetree, linux-kernel
Hi Laurent,
Thanks for your review.
On Fri, Feb 14, 2025 at 02:29:51AM +0200, Laurent Pinchart wrote:
> Hi Tommaso, Prabhakar,
>
> Thank you for the patch.
>
> On Mon, Feb 10, 2025 at 12:45:34PM +0100, Tommaso Merciai wrote:
> > 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.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > .../bindings/media/renesas,rzg2l-csi2.yaml | 63 ++++++++++++++-----
> > 1 file changed, 48 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..0d07c55a3f35 100644
> > --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > @@ -17,12 +17,15 @@ 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
> > +
>
> I'd drop the empty line.
I'll drop this line in v2, thanks.
>
> > + - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
> >
> > reg:
> > maxItems: 1
> > @@ -31,16 +34,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
>
> I would move the clocks and clock-names definitions to the conditional
> below. Otherwise I think a device tree that has two clocks only but
> incorrectly uses "system" and "video" instead of "video" and "apb" will
> validate.
Agreed. Taking as reference your work done on renesas,fcp.yaml.
What about the following?
clocks: true
clock-names: true
Then move the clocks and clock-names below as you suggested
into the conditional block:
allOf:
- if:
properties:
compatible:
contains:
const: renesas,r9a09g057-csi2
then:
properties:
clocks:
items:
- description: CRU Main clock
- description: CRU Register access clock
clock-names:
items:
- const: video
- const: apb
else:
properties:
clocks:
items:
- description: Internal clock for connecting CRU and MIPI
- description: CRU Main clock
- description: CRU Register access clock
clock-names:
items:
- const: system
- const: video
- const: apb
Thanks in advance.
>
> >
> > power-domains:
> > maxItems: 1
> > @@ -48,7 +59,7 @@ properties:
> > resets:
> > items:
> > - description: CRU_PRESETN reset terminal
> > - - description: CRU_CMN_RSTB reset terminal
> > + - description: CRU_CMN_RSTB reset terminal or D-PHY reset
> >
> > reset-names:
> > items:
> > @@ -101,6 +112,28 @@ 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:
> > + maxItems: 3
> > +
> > + clock-names:
> > + maxItems: 3
> > +
> > additionalProperties: false
> >
> > examples:
>
> --
> Regards,
>
> Laurent Pinchart
Thanks & Regards,
Tommaso
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 5/8] media: rzg2l-cru: csi2: Use temporary variable for struct device in rzg2l_csi2_probe()
2025-02-14 0:47 ` Laurent Pinchart
@ 2025-02-18 12:12 ` Tommaso Merciai
0 siblings, 0 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-18 12:12 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Geert Uytterhoeven, Magnus Damm, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
Hi Laurent,
Thanks for your review.
On Fri, Feb 14, 2025 at 02:47:29AM +0200, Laurent Pinchart wrote:
> Hi Tommaso, Prabhakar,
>
> Thank you for the patch.
>
> On Mon, Feb 10, 2025 at 12:45:37PM +0100, Tommaso Merciai wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Use a temporary variable for the struct device pointers to avoid
>
> s/temporary/local/ (in the subject message too).
I'll fix this in v2.
Thanks.
>
> > dereferencing.
>
> The only advantage of this is shortened lines. That's a coding style
> preference, and I'm fine with this patch, but the commit message should
> mention this, not "avoid dereferencing".
I will go for:
Use a local variable for the struct device pointers. This increases code
readability with shortened lines.
Thanks.
>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > .../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;
> > }
> >
>
> --
> Regards,
>
> Laurent Pinchart
Thanks & Regards,
Tommaso
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 8/8] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable()
2025-02-14 0:50 ` Laurent Pinchart
@ 2025-02-18 12:21 ` Tommaso Merciai
0 siblings, 0 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-18 12:21 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Hans Verkuil, Uwe Kleine-König, devicetree,
linux-kernel
Hi Laurent,
Thanks for your review.
On Fri, Feb 14, 2025 at 02:50:59AM +0200, Laurent Pinchart wrote:
> Hi Tommaso,
>
> Thank you for the patch.
>
> On Mon, Feb 10, 2025 at 12:45:40PM +0100, Tommaso Merciai wrote:
> > 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().
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > 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..5548b328d970 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)
> > + return ret;
>
> Leaking DMA.
Ouch, thanks!
Will fix this in v2.
>
> >
> > 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);
> >
>
> --
> Regards,
>
> Laurent Pinchart
Thanks & Regards,
Tommaso
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-02-18 11:55 ` Tommaso Merciai
@ 2025-02-18 12:35 ` Laurent Pinchart
2025-02-18 13:37 ` Tommaso Merciai
0 siblings, 1 reply; 36+ messages in thread
From: Laurent Pinchart @ 2025-02-18 12:35 UTC (permalink / raw)
To: Tommaso Merciai
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, devicetree, linux-kernel
On Tue, Feb 18, 2025 at 12:55:22PM +0100, Tommaso Merciai wrote:
> On Fri, Feb 14, 2025 at 02:29:51AM +0200, Laurent Pinchart wrote:
> > On Mon, Feb 10, 2025 at 12:45:34PM +0100, Tommaso Merciai wrote:
> > > 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.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > > .../bindings/media/renesas,rzg2l-csi2.yaml | 63 ++++++++++++++-----
> > > 1 file changed, 48 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..0d07c55a3f35 100644
> > > --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > > @@ -17,12 +17,15 @@ 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
> > > +
> >
> > I'd drop the empty line.
>
> I'll drop this line in v2, thanks.
>
> > > + - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
> > >
> > > reg:
> > > maxItems: 1
> > > @@ -31,16 +34,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
> >
> > I would move the clocks and clock-names definitions to the conditional
> > below. Otherwise I think a device tree that has two clocks only but
> > incorrectly uses "system" and "video" instead of "video" and "apb" will
> > validate.
>
> Agreed. Taking as reference your work done on renesas,fcp.yaml.
> What about the following?
>
> clocks: true
> clock-names: true
>
> Then move the clocks and clock-names below as you suggested
> into the conditional block:
>
> allOf:
> - if:
> properties:
> compatible:
> contains:
> const: renesas,r9a09g057-csi2
> then:
> properties:
> clocks:
> items:
> - description: CRU Main clock
> - description: CRU Register access clock
> clock-names:
> items:
> - const: video
> - const: apb
>
> else:
> properties:
> clocks:
> items:
> - description: Internal clock for connecting CRU and MIPI
> - description: CRU Main clock
> - description: CRU Register access clock
> clock-names:
> items:
> - const: system
> - const: video
> - const: apb
>
> Thanks in advance.
I do like that, but I think Krzysztof wasn't entirely happy with it (it
could be a separate but similar issue though, I don't recall the
details). I'd recommend checking with him (he's on CC, so he will
probably reply unless the mail gets buried in a mailbox that I am sure
fills up quite quickly).
> > >
> > > power-domains:
> > > maxItems: 1
> > > @@ -48,7 +59,7 @@ properties:
> > > resets:
> > > items:
> > > - description: CRU_PRESETN reset terminal
> > > - - description: CRU_CMN_RSTB reset terminal
> > > + - description: CRU_CMN_RSTB reset terminal or D-PHY reset
> > >
> > > reset-names:
> > > items:
> > > @@ -101,6 +112,28 @@ 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:
> > > + maxItems: 3
> > > +
> > > + clock-names:
> > > + maxItems: 3
> > > +
> > > additionalProperties: false
> > >
> > > examples:
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-02-18 12:35 ` Laurent Pinchart
@ 2025-02-18 13:37 ` Tommaso Merciai
0 siblings, 0 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-18 13:37 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, devicetree, linux-kernel
Hi Laurent,
On Tue, Feb 18, 2025 at 02:35:03PM +0200, Laurent Pinchart wrote:
> On Tue, Feb 18, 2025 at 12:55:22PM +0100, Tommaso Merciai wrote:
> > On Fri, Feb 14, 2025 at 02:29:51AM +0200, Laurent Pinchart wrote:
> > > On Mon, Feb 10, 2025 at 12:45:34PM +0100, Tommaso Merciai wrote:
> > > > 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.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > ---
> > > > .../bindings/media/renesas,rzg2l-csi2.yaml | 63 ++++++++++++++-----
> > > > 1 file changed, 48 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..0d07c55a3f35 100644
> > > > --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > > > @@ -17,12 +17,15 @@ 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
> > > > +
> > >
> > > I'd drop the empty line.
> >
> > I'll drop this line in v2, thanks.
> >
> > > > + - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
> > > >
> > > > reg:
> > > > maxItems: 1
> > > > @@ -31,16 +34,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
> > >
> > > I would move the clocks and clock-names definitions to the conditional
> > > below. Otherwise I think a device tree that has two clocks only but
> > > incorrectly uses "system" and "video" instead of "video" and "apb" will
> > > validate.
> >
> > Agreed. Taking as reference your work done on renesas,fcp.yaml.
> > What about the following?
> >
> > clocks: true
> > clock-names: true
> >
> > Then move the clocks and clock-names below as you suggested
> > into the conditional block:
> >
> > allOf:
> > - if:
> > properties:
> > compatible:
> > contains:
> > const: renesas,r9a09g057-csi2
> > then:
> > properties:
> > clocks:
> > items:
> > - description: CRU Main clock
> > - description: CRU Register access clock
> > clock-names:
> > items:
> > - const: video
> > - const: apb
> >
> > else:
> > properties:
> > clocks:
> > items:
> > - description: Internal clock for connecting CRU and MIPI
> > - description: CRU Main clock
> > - description: CRU Register access clock
> > clock-names:
> > items:
> > - const: system
> > - const: video
> > - const: apb
> >
> > Thanks in advance.
>
> I do like that, but I think Krzysztof wasn't entirely happy with it (it
> could be a separate but similar issue though, I don't recall the
> details). I'd recommend checking with him (he's on CC, so he will
> probably reply unless the mail gets buried in a mailbox that I am sure
> fills up quite quickly).
I read a bit of the conversation you have on renesas,fcp.
If I'm not missing something the suggestion was to use (in my case):
clocks:
minItems: 2
maxItems: 3
clock-names:
minItems: 2
maxItems: 3
Instead of:
clocks: true
clock-names: true
Please correct me if I'm missing somenthing.
Thanks in advance.
>
> > > >
> > > > power-domains:
> > > > maxItems: 1
> > > > @@ -48,7 +59,7 @@ properties:
> > > > resets:
> > > > items:
> > > > - description: CRU_PRESETN reset terminal
> > > > - - description: CRU_CMN_RSTB reset terminal
> > > > + - description: CRU_CMN_RSTB reset terminal or D-PHY reset
> > > >
> > > > reset-names:
> > > > items:
> > > > @@ -101,6 +112,28 @@ 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:
> > > > + maxItems: 3
> > > > +
> > > > + clock-names:
> > > > + maxItems: 3
> > > > +
> > > > additionalProperties: false
> > > >
> > > > examples:
>
> --
> Regards,
>
> Laurent Pinchart
Thanks & Regards,
Tommaso
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 4/8] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC
2025-02-14 0:45 ` Laurent Pinchart
@ 2025-02-18 15:51 ` Tommaso Merciai
0 siblings, 0 replies; 36+ messages in thread
From: Tommaso Merciai @ 2025-02-18 15:51 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, devicetree, linux-kernel
Hi Laurent,
Thanks for your review.
On Fri, Feb 14, 2025 at 02:45:00AM +0200, Laurent Pinchart wrote:
> Hi Tommaso,
>
> Thank you for the patch.
>
> On Mon, Feb 10, 2025 at 12:45:36PM +0100, Tommaso Merciai wrote:
> > The CRU block found on the Renesas RZ/G3E ("R9A09G047") SoC has five
> > interrups:
> >
> > - 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.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > .../bindings/media/renesas,rzg2l-cru.yaml | 33 ++++++++++++-------
> > 1 file changed, 22 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..7e4a7ed56378 100644
> > --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
> > @@ -17,24 +17,34 @@ 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
> > + maxItems: 5
> >
> > 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
>
> This should move to a conditional block.
I think here we can do similar to patch 2/8.
What about setting here:
interrupts:
minItems: 3
maxItems: 5
interrupt-names:
minItems: 3
maxItems: 5
Then move interrupts and interrupt-names into
the conditional block:
allOf:
- if:
properties:
compatible:
contains:
enum:
- renesas,r9a07g044-cru
- renesas,r9a07g054-cru
then:
properties:
interrupts:
minItems: 3
maxItems: 3
interrupt-names:
items:
- const: image_conv
- const: image_conv_err
- const: axi_mst_err
ports:
required:
- port@0
- port@1
- if:
properties:
compatible:
contains:
enum:
- renesas,r9a07g043-cru
then:
properties:
interrupts:
minItems: 3
maxItems: 3
interrupt-names:
items:
- const: image_conv
- const: image_conv_err
- const: axi_mst_err
ports:
properties:
port@0: false
required:
- port@1
- if:
properties:
compatible:
contains:
const: renesas,r9a09g047-cru
then:
properties:
interrupts:
minItems: 5
maxItems: 5
interrupt-names:
items:
- const: image_conv
- const: axi_mst_err
- const: vd_addr_wend
- const: sd_addr_wend
- const: vsd_addr_wend
ports:
properties:
port@0: false
required:
- port@1
>
> >
> > clocks:
> > items:
> > @@ -120,6 +130,7 @@ allOf:
> > contains:
> > enum:
> > - renesas,r9a07g043-cru
> > + - renesas,r9a09g047-cru
> > then:
> > properties:
> > ports:
>
> --
> Regards,
>
> Laurent Pinchart
Thanks & Regards,
Tommaso
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-02-14 0:29 ` Laurent Pinchart
2025-02-18 11:55 ` Tommaso Merciai
@ 2025-02-19 14:51 ` Rob Herring
2025-02-19 15:12 ` Laurent Pinchart
1 sibling, 1 reply; 36+ messages in thread
From: Rob Herring @ 2025-02-19 14:51 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, devicetree, linux-kernel
On Fri, Feb 14, 2025 at 02:29:51AM +0200, Laurent Pinchart wrote:
> Hi Tommaso, Prabhakar,
>
> Thank you for the patch.
>
> On Mon, Feb 10, 2025 at 12:45:34PM +0100, Tommaso Merciai wrote:
> > 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.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > .../bindings/media/renesas,rzg2l-csi2.yaml | 63 ++++++++++++++-----
> > 1 file changed, 48 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..0d07c55a3f35 100644
> > --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > @@ -17,12 +17,15 @@ 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
> > +
>
> I'd drop the empty line.
>
> > + - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
> >
> > reg:
> > maxItems: 1
> > @@ -31,16 +34,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
>
> I would move the clocks and clock-names definitions to the conditional
> below. Otherwise I think a device tree that has two clocks only but
> incorrectly uses "system" and "video" instead of "video" and "apb" will
> validate.
No, that wouldn't be allowed. The preference is to have it like this
because it discourages creating more variations. If the names are all
defined in if/then schema, then you can just add a new one with any
names you want. Though if the variations become such a mess, then
defining them in the if/then schemas would probably be better.
It would be better if 'clocks' could be reworked to avoid the 'oneOf'
though (oneOf == poor error messages). It just needs a 'minItems: 2'
added and the descriptions reworded for both cases.
Rob
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-02-10 11:45 ` [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC Tommaso Merciai
2025-02-14 0:29 ` Laurent Pinchart
@ 2025-02-19 14:52 ` Rob Herring
1 sibling, 0 replies; 36+ messages in thread
From: Rob Herring @ 2025-02-19 14:52 UTC (permalink / raw)
To: Tommaso Merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, devicetree, linux-kernel
On Mon, Feb 10, 2025 at 12:45:34PM +0100, Tommaso Merciai wrote:
> 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.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> .../bindings/media/renesas,rzg2l-csi2.yaml | 63 ++++++++++++++-----
> 1 file changed, 48 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..0d07c55a3f35 100644
> --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> @@ -17,12 +17,15 @@ 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 +34,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 +59,7 @@ properties:
> resets:
> items:
> - description: CRU_PRESETN reset terminal
> - - description: CRU_CMN_RSTB reset terminal
> + - description: CRU_CMN_RSTB reset terminal or D-PHY reset
>
> reset-names:
> items:
> @@ -101,6 +112,28 @@ required:
> - reset-names
> - ports
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: renesas,r9a09g057-csi2
> + then:
> + properties:
> + clocks:
> + maxItems: 2
> +
> + clock-names:
> + maxItems: 2
These are correct, but...
> +
> + else:
> + properties:
> + clocks:
> + maxItems: 3
> +
> + clock-names:
> + maxItems: 3
3 is already the max. You need 'minItems' here.
Rob
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-02-19 14:51 ` Rob Herring
@ 2025-02-19 15:12 ` Laurent Pinchart
2025-02-19 20:56 ` Rob Herring
0 siblings, 1 reply; 36+ messages in thread
From: Laurent Pinchart @ 2025-02-19 15:12 UTC (permalink / raw)
To: Rob Herring
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, devicetree, linux-kernel
On Wed, Feb 19, 2025 at 08:51:39AM -0600, Rob Herring wrote:
> On Fri, Feb 14, 2025 at 02:29:51AM +0200, Laurent Pinchart wrote:
> > Hi Tommaso, Prabhakar,
> >
> > Thank you for the patch.
> >
> > On Mon, Feb 10, 2025 at 12:45:34PM +0100, Tommaso Merciai wrote:
> > > 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.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > > .../bindings/media/renesas,rzg2l-csi2.yaml | 63 ++++++++++++++-----
> > > 1 file changed, 48 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..0d07c55a3f35 100644
> > > --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > > @@ -17,12 +17,15 @@ 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
> > > +
> >
> > I'd drop the empty line.
> >
> > > + - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
> > >
> > > reg:
> > > maxItems: 1
> > > @@ -31,16 +34,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
> >
> > I would move the clocks and clock-names definitions to the conditional
> > below. Otherwise I think a device tree that has two clocks only but
> > incorrectly uses "system" and "video" instead of "video" and "apb" will
> > validate.
>
> No, that wouldn't be allowed. The preference is to have it like this
> because it discourages creating more variations. If the names are all
> defined in if/then schema, then you can just add a new one with any
> names you want. Though if the variations become such a mess, then
> defining them in the if/then schemas would probably be better.
>
> It would be better if 'clocks' could be reworked to avoid the 'oneOf'
> though (oneOf == poor error messages). It just needs a 'minItems: 2'
> added and the descriptions reworded for both cases.
Don't the items in clocks need to match the items in clock-names ? We
can't reorder clock-names items as that would be an ABI breakage, so we
can't reorder clocks items either.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-02-19 15:12 ` Laurent Pinchart
@ 2025-02-19 20:56 ` Rob Herring
2025-02-19 21:17 ` Laurent Pinchart
0 siblings, 1 reply; 36+ messages in thread
From: Rob Herring @ 2025-02-19 20:56 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, devicetree, linux-kernel
On Wed, Feb 19, 2025 at 05:12:37PM +0200, Laurent Pinchart wrote:
> On Wed, Feb 19, 2025 at 08:51:39AM -0600, Rob Herring wrote:
> > On Fri, Feb 14, 2025 at 02:29:51AM +0200, Laurent Pinchart wrote:
> > > Hi Tommaso, Prabhakar,
> > >
> > > Thank you for the patch.
> > >
> > > On Mon, Feb 10, 2025 at 12:45:34PM +0100, Tommaso Merciai wrote:
> > > > 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.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > ---
> > > > .../bindings/media/renesas,rzg2l-csi2.yaml | 63 ++++++++++++++-----
> > > > 1 file changed, 48 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..0d07c55a3f35 100644
> > > > --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > > > @@ -17,12 +17,15 @@ 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
> > > > +
> > >
> > > I'd drop the empty line.
> > >
> > > > + - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
> > > >
> > > > reg:
> > > > maxItems: 1
> > > > @@ -31,16 +34,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
> > >
> > > I would move the clocks and clock-names definitions to the conditional
> > > below. Otherwise I think a device tree that has two clocks only but
> > > incorrectly uses "system" and "video" instead of "video" and "apb" will
> > > validate.
> >
> > No, that wouldn't be allowed. The preference is to have it like this
> > because it discourages creating more variations. If the names are all
> > defined in if/then schema, then you can just add a new one with any
> > names you want. Though if the variations become such a mess, then
> > defining them in the if/then schemas would probably be better.
> >
> > It would be better if 'clocks' could be reworked to avoid the 'oneOf'
> > though (oneOf == poor error messages). It just needs a 'minItems: 2'
> > added and the descriptions reworded for both cases.
>
> Don't the items in clocks need to match the items in clock-names ? We
> can't reorder clock-names items as that would be an ABI breakage, so we
> can't reorder clocks items either.
Validation wise, the only thing we check is is it 2 or 3 entries. No way
to enforce it's the right clock. The description is just for humans. So
you could just put "(optional)" on the first entry though there is no
way in json-schema to really do that. If you prefer as-is with the
oneOf, that's fine too.
Rob
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-02-19 20:56 ` Rob Herring
@ 2025-02-19 21:17 ` Laurent Pinchart
0 siblings, 0 replies; 36+ messages in thread
From: Laurent Pinchart @ 2025-02-19 21:17 UTC (permalink / raw)
To: Rob Herring
Cc: Tommaso Merciai, linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, devicetree, linux-kernel
On Wed, Feb 19, 2025 at 02:56:20PM -0600, Rob Herring wrote:
> On Wed, Feb 19, 2025 at 05:12:37PM +0200, Laurent Pinchart wrote:
> > On Wed, Feb 19, 2025 at 08:51:39AM -0600, Rob Herring wrote:
> > > On Fri, Feb 14, 2025 at 02:29:51AM +0200, Laurent Pinchart wrote:
> > > > Hi Tommaso, Prabhakar,
> > > >
> > > > Thank you for the patch.
> > > >
> > > > On Mon, Feb 10, 2025 at 12:45:34PM +0100, Tommaso Merciai wrote:
> > > > > 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.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > > ---
> > > > > .../bindings/media/renesas,rzg2l-csi2.yaml | 63 ++++++++++++++-----
> > > > > 1 file changed, 48 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..0d07c55a3f35 100644
> > > > > --- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > > > > +++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
> > > > > @@ -17,12 +17,15 @@ 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
> > > > > +
> > > >
> > > > I'd drop the empty line.
> > > >
> > > > > + - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
> > > > >
> > > > > reg:
> > > > > maxItems: 1
> > > > > @@ -31,16 +34,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
> > > >
> > > > I would move the clocks and clock-names definitions to the conditional
> > > > below. Otherwise I think a device tree that has two clocks only but
> > > > incorrectly uses "system" and "video" instead of "video" and "apb" will
> > > > validate.
> > >
> > > No, that wouldn't be allowed. The preference is to have it like this
> > > because it discourages creating more variations. If the names are all
> > > defined in if/then schema, then you can just add a new one with any
> > > names you want. Though if the variations become such a mess, then
> > > defining them in the if/then schemas would probably be better.
> > >
> > > It would be better if 'clocks' could be reworked to avoid the 'oneOf'
> > > though (oneOf == poor error messages). It just needs a 'minItems: 2'
> > > added and the descriptions reworded for both cases.
> >
> > Don't the items in clocks need to match the items in clock-names ? We
> > can't reorder clock-names items as that would be an ABI breakage, so we
> > can't reorder clocks items either.
>
> Validation wise, the only thing we check is is it 2 or 3 entries. No way
> to enforce it's the right clock. The description is just for humans. So
> you could just put "(optional)" on the first entry though there is no
> way in json-schema to really do that. If you prefer as-is with the
> oneOf, that's fine too.
Adding "(optional)" would work for me in this case I think, even if I
think it could be a bit confusing. I'm OK either way.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets
2025-02-10 11:45 ` [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets Tommaso Merciai
2025-02-10 11:54 ` Biju Das
@ 2025-02-20 14:36 ` Geert Uytterhoeven
1 sibling, 0 replies; 36+ messages in thread
From: Geert Uytterhoeven @ 2025-02-20 14:36 UTC (permalink / raw)
To: Tommaso Merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
Stephen Boyd, Magnus Damm, devicetree, linux-kernel, linux-clk
On Mon, 10 Feb 2025 at 12:46, Tommaso Merciai <tomm.merciai@gmail.com> wrote:
> Add support for CRU0 clocks and resets along with the corresponding
> divider.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk for v6.15.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets
2025-02-10 11:54 ` Biju Das
@ 2025-02-20 14:39 ` Geert Uytterhoeven
0 siblings, 0 replies; 36+ messages in thread
From: Geert Uytterhoeven @ 2025-02-20 14:39 UTC (permalink / raw)
To: Biju Das
Cc: Tommaso Merciai, linux-renesas-soc@vger.kernel.org,
linux-media@vger.kernel.org, Prabhakar Mahadev Lad,
Tommaso Merciai, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
Stephen Boyd, Magnus Damm, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org
Hi Biju,
On Mon, 10 Feb 2025 at 12:54, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > From: Tommaso Merciai <tomm.merciai@gmail.com>
> > Sent: 10 February 2025 11:46
> > Subject: [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets
> >
> > Add support for CRU0 clocks and resets along with the corresponding divider.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > --- a/drivers/clk/renesas/r9a09g047-cpg.c
> > +++ b/drivers/clk/renesas/r9a09g047-cpg.c
> > @@ -49,6 +53,12 @@ static const struct clk_div_table dtable_1_8[] = {
> > {0, 0},
> > };
> >
> > +static const struct clk_div_table dtable_2_4[] = {
> > + {0, 2},
> > + {1, 4},
> > + {0, 0},
>
> Not sure {0, 2}, {1, 4}, {0, 0}, to make lines shorter?
All SoCs from the RZ/G2L and RZ/V2H families use this formatting style:
git grep -wW clk_div_table -- drivers/clk/renesas/r9a0*
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2025-02-20 14:39 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-10 11:45 [PATCH 0/8] media: rzg2l-cru: Document RZ/G3E (CSI2, CRU) Tommaso Merciai
2025-02-10 11:45 ` [PATCH 1/8] clk: renesas: r9a09g047: Add support for CRU0 clocks, and resets Tommaso Merciai
2025-02-10 11:54 ` Biju Das
2025-02-20 14:39 ` Geert Uytterhoeven
2025-02-20 14:36 ` Geert Uytterhoeven
2025-02-10 11:45 ` [PATCH 2/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC Tommaso Merciai
2025-02-14 0:29 ` Laurent Pinchart
2025-02-18 11:55 ` Tommaso Merciai
2025-02-18 12:35 ` Laurent Pinchart
2025-02-18 13:37 ` Tommaso Merciai
2025-02-19 14:51 ` Rob Herring
2025-02-19 15:12 ` Laurent Pinchart
2025-02-19 20:56 ` Rob Herring
2025-02-19 21:17 ` Laurent Pinchart
2025-02-19 14:52 ` Rob Herring
2025-02-10 11:45 ` [PATCH 3/8] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block Tommaso Merciai
2025-02-13 8:56 ` Krzysztof Kozlowski
2025-02-13 15:59 ` Tommaso Merciai
2025-02-10 11:45 ` [PATCH 4/8] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC Tommaso Merciai
2025-02-10 13:47 ` Rob Herring (Arm)
2025-02-14 0:45 ` Laurent Pinchart
2025-02-18 15:51 ` Tommaso Merciai
2025-02-10 11:45 ` [PATCH 5/8] media: rzg2l-cru: csi2: Use temporary variable for struct device in rzg2l_csi2_probe() Tommaso Merciai
2025-02-10 11:55 ` Biju Das
2025-02-14 0:47 ` Laurent Pinchart
2025-02-18 12:12 ` Tommaso Merciai
2025-02-10 11:45 ` [PATCH 6/8] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable() Tommaso Merciai
2025-02-10 11:56 ` Biju Das
2025-02-14 0:48 ` Laurent Pinchart
2025-02-10 11:45 ` [PATCH 7/8] media: rzg2l-cru: rzg2l-core: Use temporary variable for struct device in rzg2l_cru_probe() Tommaso Merciai
2025-02-10 11:56 ` Biju Das
2025-02-14 0:49 ` Laurent Pinchart
2025-02-10 11:45 ` [PATCH 8/8] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable() Tommaso Merciai
2025-02-10 11:57 ` Biju Das
2025-02-14 0:50 ` Laurent Pinchart
2025-02-18 12:21 ` Tommaso Merciai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).