* [PATCH v5 01/17] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 02/17] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block Tommaso Merciai
` (15 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Rob Herring, Laurent Pinchart,
Tommaso Merciai, Mauro Carvalho Chehab, 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.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
- Dropped empty line as suggested by LPinchart
- Fixed minItems into else conditional block as suggested by RHerring
Changes since v2:
- Collected tags
- Fixed CRU_CMN_RSTB description as suggested by LPinchart
Changes since v3:
- Fixed CRU_CMN_RSTB description as suggested by GUytterhoeven
.../bindings/media/renesas,rzg2l-csi2.yaml | 59 ++++++++++++++-----
1 file changed, 44 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
index 7faa12fecd5bb..1f9ee37584b34 100644
--- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
@@ -17,12 +17,14 @@ description:
properties:
compatible:
- items:
- - enum:
- - renesas,r9a07g043-csi2 # RZ/G2UL
- - renesas,r9a07g044-csi2 # RZ/G2{L,LC}
- - renesas,r9a07g054-csi2 # RZ/V2L
- - const: renesas,rzg2l-csi2
+ oneOf:
+ - items:
+ - enum:
+ - renesas,r9a07g043-csi2 # RZ/G2UL
+ - renesas,r9a07g044-csi2 # RZ/G2{L,LC}
+ - renesas,r9a07g054-csi2 # RZ/V2L
+ - const: renesas,rzg2l-csi2
+ - const: renesas,r9a09g057-csi2 # RZ/V2H(P)
reg:
maxItems: 1
@@ -31,16 +33,24 @@ properties:
maxItems: 1
clocks:
- items:
- - description: Internal clock for connecting CRU and MIPI
- - description: CRU Main clock
- - description: CRU Register access clock
+ oneOf:
+ - items:
+ - description: Internal clock for connecting CRU and MIPI
+ - description: CRU Main clock
+ - description: CRU Register access clock
+ - items:
+ - description: CRU Main clock
+ - description: CRU Register access clock
clock-names:
- items:
- - const: system
- - const: video
- - const: apb
+ oneOf:
+ - items:
+ - const: system
+ - const: video
+ - const: apb
+ - items:
+ - const: video
+ - const: apb
power-domains:
maxItems: 1
@@ -48,7 +58,7 @@ properties:
resets:
items:
- description: CRU_PRESETN reset terminal
- - description: CRU_CMN_RSTB reset terminal
+ - description: D-PHY reset (CRU_CMN_RSTB or CRU_n_S_RESETN)
reset-names:
items:
@@ -101,6 +111,25 @@ required:
- reset-names
- ports
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: renesas,r9a09g057-csi2
+ then:
+ properties:
+ clocks:
+ maxItems: 2
+ clock-names:
+ maxItems: 2
+ else:
+ properties:
+ clocks:
+ minItems: 3
+ clock-names:
+ minItems: 3
+
additionalProperties: false
examples:
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 02/17] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 01/17] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 03/17] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC Tommaso Merciai
` (14 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Rob Herring,
Laurent Pinchart, Mauro Carvalho Chehab, 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.
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
- Fixed CRU_CMN_RSTB as suggested by LPinchart
- Collected tags.
Changes since v3:
- Collected tag.
.../devicetree/bindings/media/renesas,rzg2l-csi2.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
index 1f9ee37584b34..c5c511c9f0db2 100644
--- a/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-csi2.yaml
@@ -24,6 +24,9 @@ properties:
- renesas,r9a07g044-csi2 # RZ/G2{L,LC}
- renesas,r9a07g054-csi2 # RZ/V2L
- const: renesas,rzg2l-csi2
+ - items:
+ - const: renesas,r9a09g047-csi2 # RZ/G3E
+ - const: renesas,r9a09g057-csi2
- const: renesas,r9a09g057-csi2 # RZ/V2H(P)
reg:
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 03/17] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 01/17] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/V2H(P) SoC Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 02/17] media: dt-bindings: renesas,rzg2l-csi2: Document Renesas RZ/G3E CSI-2 block Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 04/17] media: rzg2l-cru: csi2: Use local variable for struct device in rzg2l_csi2_probe() Tommaso Merciai
` (13 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Rob Herring,
Mauro Carvalho Chehab, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel
The CRU block found on the Renesas RZ/G3E ("R9A09G047") SoC has five
interrupts:
- image_conv: image_conv irq
- axi_mst_err: AXI master error level irq
- vd_addr_wend: Video data AXI master addr 0 write end irq
- sd_addr_wend: Statistics data AXI master addr 0 write end irq
- vsd_addr_wend: Video statistics data AXI master addr 0 write end irq
This IP has only one input port 'port@1' similar to the RZ/G2UL CRU.
Document the CRU block found on the Renesas RZ/G3E ("R9A09G047") SoC.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
- Use oneOf for interrupts and interrupt-names
- Handle interrupts and interrupt names base on soc variants
Changes since v2:
- Collected tag.
.../bindings/media/renesas,rzg2l-cru.yaml | 65 +++++++++++++++----
1 file changed, 54 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
index bc1245127025e..47e18690fa570 100644
--- a/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,rzg2l-cru.yaml
@@ -17,24 +17,43 @@ description:
properties:
compatible:
- items:
- - enum:
- - renesas,r9a07g043-cru # RZ/G2UL
- - renesas,r9a07g044-cru # RZ/G2{L,LC}
- - renesas,r9a07g054-cru # RZ/V2L
- - const: renesas,rzg2l-cru
+ oneOf:
+ - items:
+ - enum:
+ - renesas,r9a07g043-cru # RZ/G2UL
+ - renesas,r9a07g044-cru # RZ/G2{L,LC}
+ - renesas,r9a07g054-cru # RZ/V2L
+ - const: renesas,rzg2l-cru
+ - const: renesas,r9a09g047-cru # RZ/G3E
reg:
maxItems: 1
interrupts:
- maxItems: 3
+ oneOf:
+ - items:
+ - description: CRU Interrupt for image_conv
+ - description: CRU Interrupt for image_conv_err
+ - description: CRU AXI master error interrupt
+ - items:
+ - description: CRU Interrupt for image_conv
+ - description: CRU AXI master error interrupt
+ - description: CRU Video Data AXI Master Address 0 Write End interrupt
+ - description: CRU Statistics data AXI master addr 0 write end interrupt
+ - description: CRU Video statistics data AXI master addr 0 write end interrupt
interrupt-names:
- items:
- - const: image_conv
- - const: image_conv_err
- - const: axi_mst_err
+ oneOf:
+ - items:
+ - const: image_conv
+ - const: image_conv_err
+ - const: axi_mst_err
+ - items:
+ - const: image_conv
+ - const: axi_mst_err
+ - const: vd_addr_wend
+ - const: sd_addr_wend
+ - const: vsd_addr_wend
clocks:
items:
@@ -109,6 +128,10 @@ allOf:
- renesas,r9a07g054-cru
then:
properties:
+ interrupts:
+ maxItems: 3
+ interrupt-names:
+ maxItems: 3
ports:
required:
- port@0
@@ -122,10 +145,30 @@ allOf:
- renesas,r9a07g043-cru
then:
properties:
+ interrupts:
+ maxItems: 3
+ interrupt-names:
+ maxItems: 3
ports:
properties:
port@0: false
+ required:
+ - port@1
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: renesas,r9a09g047-cru
+ then:
+ properties:
+ interrupts:
+ minItems: 5
+ interrupt-names:
+ minItems: 5
+ ports:
+ properties:
+ port@0: false
required:
- port@1
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 04/17] media: rzg2l-cru: csi2: Use local variable for struct device in rzg2l_csi2_probe()
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (2 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 03/17] media: dt-bindings: renesas,rzg2l-cru: Document Renesas RZ/G3E SoC Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 05/17] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable() Tommaso Merciai
` (12 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Laurent Pinchart, 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
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Use a local variable for the struct device pointers. This increases code
readability with shortened lines.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
- Fixed commit msg and commit body as suggested by LPinchart
- Collected tags
.../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 31 ++++++++++---------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index 881e910dce023..948f1917b830d 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -764,10 +764,11 @@ static const struct media_entity_operations rzg2l_csi2_entity_ops = {
static int rzg2l_csi2_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct rzg2l_csi2 *csi2;
int ret;
- csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
+ csi2 = devm_kzalloc(dev, sizeof(*csi2), GFP_KERNEL);
if (!csi2)
return -ENOMEM;
@@ -775,28 +776,28 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
if (IS_ERR(csi2->base))
return PTR_ERR(csi2->base);
- csi2->cmn_rstb = devm_reset_control_get_exclusive(&pdev->dev, "cmn-rstb");
+ csi2->cmn_rstb = devm_reset_control_get_exclusive(dev, "cmn-rstb");
if (IS_ERR(csi2->cmn_rstb))
- return dev_err_probe(&pdev->dev, PTR_ERR(csi2->cmn_rstb),
+ return dev_err_probe(dev, PTR_ERR(csi2->cmn_rstb),
"Failed to get cpg cmn-rstb\n");
- csi2->presetn = devm_reset_control_get_shared(&pdev->dev, "presetn");
+ csi2->presetn = devm_reset_control_get_shared(dev, "presetn");
if (IS_ERR(csi2->presetn))
- return dev_err_probe(&pdev->dev, PTR_ERR(csi2->presetn),
+ return dev_err_probe(dev, PTR_ERR(csi2->presetn),
"Failed to get cpg presetn\n");
- csi2->sysclk = devm_clk_get(&pdev->dev, "system");
+ csi2->sysclk = devm_clk_get(dev, "system");
if (IS_ERR(csi2->sysclk))
- return dev_err_probe(&pdev->dev, PTR_ERR(csi2->sysclk),
+ return dev_err_probe(dev, PTR_ERR(csi2->sysclk),
"Failed to get system clk\n");
- csi2->vclk = devm_clk_get(&pdev->dev, "video");
+ csi2->vclk = devm_clk_get(dev, "video");
if (IS_ERR(csi2->vclk))
- return dev_err_probe(&pdev->dev, PTR_ERR(csi2->vclk),
+ return dev_err_probe(dev, PTR_ERR(csi2->vclk),
"Failed to get video clock\n");
csi2->vclk_rate = clk_get_rate(csi2->vclk);
- csi2->dev = &pdev->dev;
+ csi2->dev = dev;
platform_set_drvdata(pdev, csi2);
@@ -804,18 +805,18 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
if (ret)
return ret;
- pm_runtime_enable(&pdev->dev);
+ pm_runtime_enable(dev);
ret = rzg2l_validate_csi2_lanes(csi2);
if (ret)
goto error_pm;
- csi2->subdev.dev = &pdev->dev;
+ csi2->subdev.dev = dev;
v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
csi2->subdev.internal_ops = &rzg2l_csi2_internal_ops;
- v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
+ v4l2_set_subdevdata(&csi2->subdev, dev);
snprintf(csi2->subdev.name, sizeof(csi2->subdev.name),
- "csi-%s", dev_name(&pdev->dev));
+ "csi-%s", dev_name(dev));
csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
csi2->subdev.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
@@ -852,7 +853,7 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
v4l2_async_nf_cleanup(&csi2->notifier);
media_entity_cleanup(&csi2->subdev.entity);
error_pm:
- pm_runtime_disable(&pdev->dev);
+ pm_runtime_disable(dev);
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 05/17] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable()
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (3 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 04/17] media: rzg2l-cru: csi2: Use local variable for struct device in rzg2l_csi2_probe() Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 06/17] media: rzg2l-cru: rzg2l-core: Use local variable for struct device in rzg2l_cru_probe() Tommaso Merciai
` (11 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Laurent Pinchart,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, 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().
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
- Collected tags
Changes since v2:
- Collected tags
drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index 948f1917b830d..4ccf7c5ea58b0 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -805,11 +805,13 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
if (ret)
return ret;
- pm_runtime_enable(dev);
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
ret = rzg2l_validate_csi2_lanes(csi2);
if (ret)
- goto error_pm;
+ return ret;
csi2->subdev.dev = dev;
v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
@@ -834,7 +836,7 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
ret = media_entity_pads_init(&csi2->subdev.entity, ARRAY_SIZE(csi2->pads),
csi2->pads);
if (ret)
- goto error_pm;
+ return ret;
ret = v4l2_subdev_init_finalize(&csi2->subdev);
if (ret < 0)
@@ -852,8 +854,6 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
v4l2_async_nf_unregister(&csi2->notifier);
v4l2_async_nf_cleanup(&csi2->notifier);
media_entity_cleanup(&csi2->subdev.entity);
-error_pm:
- pm_runtime_disable(dev);
return ret;
}
@@ -867,7 +867,6 @@ static void rzg2l_csi2_remove(struct platform_device *pdev)
v4l2_async_unregister_subdev(&csi2->subdev);
v4l2_subdev_cleanup(&csi2->subdev);
media_entity_cleanup(&csi2->subdev.entity);
- pm_runtime_disable(&pdev->dev);
}
static int rzg2l_csi2_pm_runtime_suspend(struct device *dev)
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 06/17] media: rzg2l-cru: rzg2l-core: Use local variable for struct device in rzg2l_cru_probe()
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (4 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 05/17] media: rzg2l-cru: csi2: Use devm_pm_runtime_enable() Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 07/17] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable() Tommaso Merciai
` (10 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Laurent Pinchart, 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
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Use a local variable for the struct device pointers. This increases code
readability with shortened lines.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
- Fixed commit msg and commit body as suggested by LPinchart
- Collected tags
.../platform/renesas/rzg2l-cru/rzg2l-core.c | 29 ++++++++++---------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 89be584a49885..70fed0ce45ea0 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -240,10 +240,11 @@ static int rzg2l_cru_media_init(struct rzg2l_cru_dev *cru)
static int rzg2l_cru_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct rzg2l_cru_dev *cru;
int irq, ret;
- cru = devm_kzalloc(&pdev->dev, sizeof(*cru), GFP_KERNEL);
+ cru = devm_kzalloc(dev, sizeof(*cru), GFP_KERNEL);
if (!cru)
return -ENOMEM;
@@ -251,32 +252,32 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
if (IS_ERR(cru->base))
return PTR_ERR(cru->base);
- cru->presetn = devm_reset_control_get_shared(&pdev->dev, "presetn");
+ cru->presetn = devm_reset_control_get_shared(dev, "presetn");
if (IS_ERR(cru->presetn))
- return dev_err_probe(&pdev->dev, PTR_ERR(cru->presetn),
+ return dev_err_probe(dev, PTR_ERR(cru->presetn),
"Failed to get cpg presetn\n");
- cru->aresetn = devm_reset_control_get_exclusive(&pdev->dev, "aresetn");
+ cru->aresetn = devm_reset_control_get_exclusive(dev, "aresetn");
if (IS_ERR(cru->aresetn))
- return dev_err_probe(&pdev->dev, PTR_ERR(cru->aresetn),
+ return dev_err_probe(dev, PTR_ERR(cru->aresetn),
"Failed to get cpg aresetn\n");
- cru->vclk = devm_clk_get(&pdev->dev, "video");
+ cru->vclk = devm_clk_get(dev, "video");
if (IS_ERR(cru->vclk))
- return dev_err_probe(&pdev->dev, PTR_ERR(cru->vclk),
+ return dev_err_probe(dev, PTR_ERR(cru->vclk),
"Failed to get video clock\n");
- cru->dev = &pdev->dev;
- cru->info = of_device_get_match_data(&pdev->dev);
+ cru->dev = dev;
+ cru->info = of_device_get_match_data(dev);
irq = platform_get_irq(pdev, 0);
if (irq < 0)
return irq;
- ret = devm_request_irq(&pdev->dev, irq, rzg2l_cru_irq, 0,
+ ret = devm_request_irq(dev, irq, rzg2l_cru_irq, 0,
KBUILD_MODNAME, cru);
if (ret)
- return dev_err_probe(&pdev->dev, ret, "failed to request irq\n");
+ return dev_err_probe(dev, ret, "failed to request irq\n");
platform_set_drvdata(pdev, cru);
@@ -285,8 +286,8 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
return ret;
cru->num_buf = RZG2L_CRU_HW_BUFFER_DEFAULT;
- pm_suspend_ignore_children(&pdev->dev, true);
- pm_runtime_enable(&pdev->dev);
+ pm_suspend_ignore_children(dev, true);
+ pm_runtime_enable(dev);
ret = rzg2l_cru_media_init(cru);
if (ret)
@@ -296,7 +297,7 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
error_dma_unregister:
rzg2l_cru_dma_unregister(cru);
- pm_runtime_disable(&pdev->dev);
+ pm_runtime_disable(dev);
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 07/17] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable()
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (5 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 06/17] media: rzg2l-cru: rzg2l-core: Use local variable for struct device in rzg2l_cru_probe() Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 08/17] media: rzg2l-cru: csi2: Introduce SoC-specific D-PHY handling Tommaso Merciai
` (9 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Tommaso Merciai, Laurent Pinchart,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, 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().
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v1:
- Fixed DMA leak as suggested by LPinchart
- Collected tags
Changes since v2:
- Collected tags
drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 70fed0ce45ea0..eed9d2bd08414 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -287,7 +287,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
cru->num_buf = RZG2L_CRU_HW_BUFFER_DEFAULT;
pm_suspend_ignore_children(dev, true);
- pm_runtime_enable(dev);
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ goto error_dma_unregister;
ret = rzg2l_cru_media_init(cru);
if (ret)
@@ -297,7 +299,6 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
error_dma_unregister:
rzg2l_cru_dma_unregister(cru);
- pm_runtime_disable(dev);
return ret;
}
@@ -306,8 +307,6 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
{
struct rzg2l_cru_dev *cru = platform_get_drvdata(pdev);
- pm_runtime_disable(&pdev->dev);
-
v4l2_async_nf_unregister(&cru->notifier);
v4l2_async_nf_cleanup(&cru->notifier);
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 08/17] media: rzg2l-cru: csi2: Introduce SoC-specific D-PHY handling
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (6 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 07/17] media: rzg2l-cru: rzg2l-core: Use devm_pm_runtime_enable() Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 09/17] media: rzg2l-cru: csi2: Skip system clock for RZ/V2H(P) SoC Tommaso Merciai
` (8 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Laurent Pinchart, Tommaso Merciai,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
In preparation for adding support for the RZ/V2H(P) SoC, where the D-PHY
differs from the existing RZ/G2L implementation, introduce a new
rzg2l_csi2_info structure. This structure provides function pointers for
SoC-specific D-PHY enable and disable operations.
Modify rzg2l_csi2_dphy_setting() to use these function pointers instead of
calling rzg2l_csi2_dphy_enable() and rzg2l_csi2_dphy_disable() directly.
Update the device match table to store the appropriate function pointers
for each compatible SoC.
This change prepares the driver for future extensions without affecting
the current functionality for RZ/G2L.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
- Moved rzg2l_csi2_info below the definition of the rzg2l_csi2_dphy_enable()
function as suggested by LPinchart
- Collected tags
.../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 24 ++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index 4ccf7c5ea58b0..4aa5d58dde5bd 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -107,6 +107,7 @@ struct rzg2l_csi2 {
void __iomem *base;
struct reset_control *presetn;
struct reset_control *cmn_rstb;
+ const struct rzg2l_csi2_info *info;
struct clk *sysclk;
struct clk *vclk;
unsigned long vclk_rate;
@@ -123,6 +124,11 @@ struct rzg2l_csi2 {
bool dphy_enabled;
};
+struct rzg2l_csi2_info {
+ int (*dphy_enable)(struct rzg2l_csi2 *csi2);
+ int (*dphy_disable)(struct rzg2l_csi2 *csi2);
+};
+
struct rzg2l_csi2_timings {
u32 t_init;
u32 tclk_miss;
@@ -355,14 +361,19 @@ static int rzg2l_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
return ret;
}
+static const struct rzg2l_csi2_info rzg2l_csi2_info = {
+ .dphy_enable = rzg2l_csi2_dphy_enable,
+ .dphy_disable = rzg2l_csi2_dphy_disable,
+};
+
static int rzg2l_csi2_dphy_setting(struct v4l2_subdev *sd, bool on)
{
struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
if (on)
- return rzg2l_csi2_dphy_enable(csi2);
+ return csi2->info->dphy_enable(csi2);
- return rzg2l_csi2_dphy_disable(csi2);
+ return csi2->info->dphy_disable(csi2);
}
static int rzg2l_csi2_mipi_link_enable(struct rzg2l_csi2 *csi2)
@@ -772,6 +783,10 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
if (!csi2)
return -ENOMEM;
+ csi2->info = of_device_get_match_data(dev);
+ if (!csi2->info)
+ return dev_err_probe(dev, -EINVAL, "Failed to get OF match data\n");
+
csi2->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(csi2->base))
return PTR_ERR(csi2->base);
@@ -891,7 +906,10 @@ static const struct dev_pm_ops rzg2l_csi2_pm_ops = {
};
static const struct of_device_id rzg2l_csi2_of_table[] = {
- { .compatible = "renesas,rzg2l-csi2", },
+ {
+ .compatible = "renesas,rzg2l-csi2",
+ .data = &rzg2l_csi2_info,
+ },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, rzg2l_csi2_of_table);
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 09/17] media: rzg2l-cru: csi2: Skip system clock for RZ/V2H(P) SoC
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (7 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 08/17] media: rzg2l-cru: csi2: Introduce SoC-specific D-PHY handling Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-04-02 0:44 ` Laurent Pinchart
2025-03-28 17:29 ` [PATCH v5 10/17] media: rzg2l-cru: csi2: Add support " Tommaso Merciai
` (7 subsequent siblings)
16 siblings, 1 reply; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 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
The RZ/V2H(P) SoC does not require a `system` clock for the CSI-2
interface. To accommodate this, introduce a `has_system_clk` bool flag
in the `rzg2l_csi2_info` structure and update the rzg2l_csi2_probe() to
conditionally request the clock only when needed.
This patch is in preparation for adding support for RZ/V2H(P) SoC.
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
- Added has_system_clk bool flag to the rzg2l_csi2_info structure to handle
case where system clock is not required as suggested by LPinchart.
- Fixed commit body and msg
.../media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index 4aa5d58dde5bd..e4781105eadc0 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -127,6 +127,7 @@ struct rzg2l_csi2 {
struct rzg2l_csi2_info {
int (*dphy_enable)(struct rzg2l_csi2 *csi2);
int (*dphy_disable)(struct rzg2l_csi2 *csi2);
+ bool has_system_clk;
};
struct rzg2l_csi2_timings {
@@ -364,6 +365,7 @@ static int rzg2l_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
static const struct rzg2l_csi2_info rzg2l_csi2_info = {
.dphy_enable = rzg2l_csi2_dphy_enable,
.dphy_disable = rzg2l_csi2_dphy_disable,
+ .has_system_clk = true,
};
static int rzg2l_csi2_dphy_setting(struct v4l2_subdev *sd, bool on)
@@ -801,10 +803,12 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(csi2->presetn),
"Failed to get cpg presetn\n");
- csi2->sysclk = devm_clk_get(dev, "system");
- if (IS_ERR(csi2->sysclk))
- return dev_err_probe(dev, PTR_ERR(csi2->sysclk),
- "Failed to get system clk\n");
+ if (csi2->info->has_system_clk) {
+ csi2->sysclk = devm_clk_get(dev, "system");
+ if (IS_ERR(csi2->sysclk))
+ return dev_err_probe(dev, PTR_ERR(csi2->sysclk),
+ "Failed to get system clk\n");
+ }
csi2->vclk = devm_clk_get(dev, "video");
if (IS_ERR(csi2->vclk))
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v5 09/17] media: rzg2l-cru: csi2: Skip system clock for RZ/V2H(P) SoC
2025-03-28 17:29 ` [PATCH v5 09/17] media: rzg2l-cru: csi2: Skip system clock for RZ/V2H(P) SoC Tommaso Merciai
@ 2025-04-02 0:44 ` Laurent Pinchart
0 siblings, 0 replies; 34+ messages in thread
From: Laurent Pinchart @ 2025-04-02 0:44 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.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 Tommaso,
Thank you for the patch.
On Fri, Mar 28, 2025 at 06:29:45PM +0100, Tommaso Merciai wrote:
> The RZ/V2H(P) SoC does not require a `system` clock for the CSI-2
> interface. To accommodate this, introduce a `has_system_clk` bool flag
> in the `rzg2l_csi2_info` structure and update the rzg2l_csi2_probe() to
> conditionally request the clock only when needed.
>
> This patch is in preparation for adding support for RZ/V2H(P) SoC.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> Changes since v2:
> - Added has_system_clk bool flag to the rzg2l_csi2_info structure to handle
> case where system clock is not required as suggested by LPinchart.
> - Fixed commit body and msg
>
> .../media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> index 4aa5d58dde5bd..e4781105eadc0 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> @@ -127,6 +127,7 @@ struct rzg2l_csi2 {
> struct rzg2l_csi2_info {
> int (*dphy_enable)(struct rzg2l_csi2 *csi2);
> int (*dphy_disable)(struct rzg2l_csi2 *csi2);
> + bool has_system_clk;
> };
>
> struct rzg2l_csi2_timings {
> @@ -364,6 +365,7 @@ static int rzg2l_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
> static const struct rzg2l_csi2_info rzg2l_csi2_info = {
> .dphy_enable = rzg2l_csi2_dphy_enable,
> .dphy_disable = rzg2l_csi2_dphy_disable,
> + .has_system_clk = true,
> };
>
> static int rzg2l_csi2_dphy_setting(struct v4l2_subdev *sd, bool on)
> @@ -801,10 +803,12 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
> return dev_err_probe(dev, PTR_ERR(csi2->presetn),
> "Failed to get cpg presetn\n");
>
> - csi2->sysclk = devm_clk_get(dev, "system");
> - if (IS_ERR(csi2->sysclk))
> - return dev_err_probe(dev, PTR_ERR(csi2->sysclk),
> - "Failed to get system clk\n");
> + if (csi2->info->has_system_clk) {
> + csi2->sysclk = devm_clk_get(dev, "system");
> + if (IS_ERR(csi2->sysclk))
> + return dev_err_probe(dev, PTR_ERR(csi2->sysclk),
> + "Failed to get system clk\n");
> + }
>
> csi2->vclk = devm_clk_get(dev, "video");
> if (IS_ERR(csi2->vclk))
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 10/17] media: rzg2l-cru: csi2: Add support for RZ/V2H(P) SoC
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (8 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 09/17] media: rzg2l-cru: csi2: Skip system clock for RZ/V2H(P) SoC Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support Tommaso Merciai
` (6 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Laurent Pinchart, 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
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
The D-PHY on the RZ/V2H(P) SoC is different from the D-PHY on the RZ/G2L
SoC. To handle this difference, function pointers for D-PHY enable/disable
have been added, and the `struct rzg2l_csi2_info` pointer is passed as OF
data.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
- Moved CRUm_SWAPCTL write of rzv2h_csi2_dphy_enable function under the error
check as suggested by LPinchart.
- Moved rzv2h_csi2_info after rzv2h_csi2_dphy_enable() as suggested by LPinchart
- Collected tag.
.../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 95 +++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index e4781105eadc0..9243306e2aa98 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -85,6 +85,15 @@
CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1))
+/* DPHY registers on RZ/V2H(P) SoC */
+#define CRUm_S_TIMCTL 0x41c
+#define CRUm_S_TIMCTL_S_HSSETTLECTL(x) ((x) << 8)
+
+#define CRUm_S_DPHYCTL_MSB 0x434
+#define CRUm_S_DPHYCTL_MSB_DESKEW BIT(1)
+
+#define CRUm_SWAPCTL 0x438
+
#define VSRSTS_RETRIES 20
#define RZG2L_CSI2_MIN_WIDTH 320
@@ -140,6 +149,30 @@ struct rzg2l_csi2_timings {
u32 max_hsfreq;
};
+struct rzv2h_csi2_s_hssettlectl {
+ unsigned int hsfreq;
+ u16 s_hssettlectl;
+};
+
+static const struct rzv2h_csi2_s_hssettlectl rzv2h_s_hssettlectl[] = {
+ { 90, 1 }, { 130, 2 }, { 180, 3 },
+ { 220, 4 }, { 270, 5 }, { 310, 6 },
+ { 360, 7 }, { 400, 8 }, { 450, 9 },
+ { 490, 10 }, { 540, 11 }, { 580, 12 },
+ { 630, 13 }, { 670, 14 }, { 720, 15 },
+ { 760, 16 }, { 810, 17 }, { 850, 18 },
+ { 900, 19 }, { 940, 20 }, { 990, 21 },
+ { 1030, 22 }, { 1080, 23 }, { 1120, 24 },
+ { 1170, 25 }, { 1220, 26 }, { 1260, 27 },
+ { 1310, 28 }, { 1350, 29 }, { 1400, 30 },
+ { 1440, 31 }, { 1490, 32 }, { 1530, 33 },
+ { 1580, 34 }, { 1620, 35 }, { 1670, 36 },
+ { 1710, 37 }, { 1760, 38 }, { 1800, 39 },
+ { 1850, 40 }, { 1890, 41 }, { 1940, 42 },
+ { 1980, 43 }, { 2030, 44 }, { 2070, 45 },
+ { 2100, 46 },
+};
+
static const struct rzg2l_csi2_timings rzg2l_csi2_global_timings[] = {
{
.max_hsfreq = 80,
@@ -434,6 +467,64 @@ static int rzg2l_csi2_mipi_link_disable(struct rzg2l_csi2 *csi2)
return 0;
}
+static int rzv2h_csi2_dphy_disable(struct rzg2l_csi2 *csi2)
+{
+ int ret;
+
+ /* Reset the CRU (D-PHY) */
+ ret = reset_control_assert(csi2->cmn_rstb);
+ if (ret)
+ return ret;
+
+ csi2->dphy_enabled = false;
+
+ return 0;
+}
+
+static int rzv2h_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
+{
+ unsigned int i;
+ u16 hssettle;
+ int mbps;
+
+ mbps = rzg2l_csi2_calc_mbps(csi2);
+ if (mbps < 0)
+ return mbps;
+
+ csi2->hsfreq = mbps;
+
+ for (i = 0; i < ARRAY_SIZE(rzv2h_s_hssettlectl); i++) {
+ if (csi2->hsfreq <= rzv2h_s_hssettlectl[i].hsfreq)
+ break;
+ }
+
+ if (i == ARRAY_SIZE(rzv2h_s_hssettlectl))
+ return -EINVAL;
+
+ rzg2l_csi2_write(csi2, CRUm_SWAPCTL, 0);
+
+ hssettle = rzv2h_s_hssettlectl[i].s_hssettlectl;
+ rzg2l_csi2_write(csi2, CRUm_S_TIMCTL,
+ CRUm_S_TIMCTL_S_HSSETTLECTL(hssettle));
+
+ if (csi2->hsfreq > 1500)
+ rzg2l_csi2_set(csi2, CRUm_S_DPHYCTL_MSB,
+ CRUm_S_DPHYCTL_MSB_DESKEW);
+ else
+ rzg2l_csi2_clr(csi2, CRUm_S_DPHYCTL_MSB,
+ CRUm_S_DPHYCTL_MSB_DESKEW);
+
+ csi2->dphy_enabled = true;
+
+ return 0;
+}
+
+static const struct rzg2l_csi2_info rzv2h_csi2_info = {
+ .dphy_enable = rzv2h_csi2_dphy_enable,
+ .dphy_disable = rzv2h_csi2_dphy_disable,
+ .has_system_clk = false,
+};
+
static int rzg2l_csi2_mipi_link_setting(struct v4l2_subdev *sd, bool on)
{
struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
@@ -910,6 +1001,10 @@ static const struct dev_pm_ops rzg2l_csi2_pm_ops = {
};
static const struct of_device_id rzg2l_csi2_of_table[] = {
+ {
+ .compatible = "renesas,r9a09g057-csi2",
+ .data = &rzv2h_csi2_info,
+ },
{
.compatible = "renesas,rzg2l-csi2",
.data = &rzg2l_csi2_info,
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (9 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 10/17] media: rzg2l-cru: csi2: Add support " Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-04-02 6:31 ` Biju Das
2025-03-28 17:29 ` [PATCH v5 12/17] media: rzg2l-cru: Pass resolution limits via OF data Tommaso Merciai
` (5 subsequent siblings)
16 siblings, 1 reply; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 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
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a
CRU-IP that is mostly identical to RZ/G2L but with different register
offsets and additional registers. Introduce a flexible register mapping
mechanism to handle these variations.
Define the `rzg2l_cru_info` structure to store register mappings and
pass it as part of the OF match data. Update the read/write functions
to check out-of-bound accesses and use indexed register offsets from
`rzg2l_cru_info`, ensuring compatibility across different SoC variants.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
- Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
accesses as suggested by LPinchart.
- Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
- Update commit body
Changes since v4:
- Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
as __always_inline
.../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
.../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
.../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
.../platform/renesas/rzg2l-cru/rzg2l-video.c | 58 ++++++++++++++--
4 files changed, 139 insertions(+), 35 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index eed9d2bd08414..abc2a979833aa 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -22,6 +22,7 @@
#include <media/v4l2-mc.h>
#include "rzg2l-cru.h"
+#include "rzg2l-cru-regs.h"
static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n)
{
@@ -269,6 +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
cru->dev = dev;
cru->info = of_device_get_match_data(dev);
+ if (!cru->info)
+ return dev_err_probe(dev, -EINVAL,
+ "Failed to get OF match data\n");
irq = platform_get_irq(pdev, 0);
if (irq < 0)
@@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
rzg2l_cru_dma_unregister(cru);
}
+static const u16 rzg2l_cru_regs[] = {
+ [CRUnCTRL] = 0x0,
+ [CRUnIE] = 0x4,
+ [CRUnINTS] = 0x8,
+ [CRUnRST] = 0xc,
+ [AMnMB1ADDRL] = 0x100,
+ [AMnMB1ADDRH] = 0x104,
+ [AMnMB2ADDRL] = 0x108,
+ [AMnMB2ADDRH] = 0x10c,
+ [AMnMB3ADDRL] = 0x110,
+ [AMnMB3ADDRH] = 0x114,
+ [AMnMB4ADDRL] = 0x118,
+ [AMnMB4ADDRH] = 0x11c,
+ [AMnMB5ADDRL] = 0x120,
+ [AMnMB5ADDRH] = 0x124,
+ [AMnMB6ADDRL] = 0x128,
+ [AMnMB6ADDRH] = 0x12c,
+ [AMnMB7ADDRL] = 0x130,
+ [AMnMB7ADDRH] = 0x134,
+ [AMnMB8ADDRL] = 0x138,
+ [AMnMB8ADDRH] = 0x13c,
+ [AMnMBVALID] = 0x148,
+ [AMnMBS] = 0x14c,
+ [AMnAXIATTR] = 0x158,
+ [AMnFIFOPNTR] = 0x168,
+ [AMnAXISTP] = 0x174,
+ [AMnAXISTPACK] = 0x178,
+ [ICnEN] = 0x200,
+ [ICnMC] = 0x208,
+ [ICnMS] = 0x254,
+ [ICnDMR] = 0x26c,
+};
+
+static const struct rzg2l_cru_info rzgl2_cru_info = {
+ .regs = rzg2l_cru_regs,
+};
+
static const struct of_device_id rzg2l_cru_of_id_table[] = {
- { .compatible = "renesas,rzg2l-cru", },
+ {
+ .compatible = "renesas,rzg2l-cru",
+ .data = &rzgl2_cru_info,
+ },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, rzg2l_cru_of_id_table);
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
index 1c9f22118a5d9..86c3202862465 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
@@ -10,71 +10,77 @@
/* HW CRU Registers Definition */
-/* CRU Control Register */
-#define CRUnCTRL 0x0
#define CRUnCTRL_VINSEL(x) ((x) << 0)
-/* CRU Interrupt Enable Register */
-#define CRUnIE 0x4
#define CRUnIE_EFE BIT(17)
-/* CRU Interrupt Status Register */
-#define CRUnINTS 0x8
#define CRUnINTS_SFS BIT(16)
-/* CRU Reset Register */
-#define CRUnRST 0xc
#define CRUnRST_VRESETN BIT(0)
/* Memory Bank Base Address (Lower) Register for CRU Image Data */
-#define AMnMBxADDRL(x) (0x100 + ((x) * 8))
+#define AMnMBxADDRL(x) (AMnMB1ADDRL + (x) * 2)
/* Memory Bank Base Address (Higher) Register for CRU Image Data */
-#define AMnMBxADDRH(x) (0x104 + ((x) * 8))
+#define AMnMBxADDRH(x) (AMnMB1ADDRH + (x) * 2)
-/* Memory Bank Enable Register for CRU Image Data */
-#define AMnMBVALID 0x148
#define AMnMBVALID_MBVALID(x) GENMASK(x, 0)
-/* Memory Bank Status Register for CRU Image Data */
-#define AMnMBS 0x14c
#define AMnMBS_MBSTS 0x7
-/* AXI Master Transfer Setting Register for CRU Image Data */
-#define AMnAXIATTR 0x158
#define AMnAXIATTR_AXILEN_MASK GENMASK(3, 0)
#define AMnAXIATTR_AXILEN (0xf)
-/* AXI Master FIFO Pointer Register for CRU Image Data */
-#define AMnFIFOPNTR 0x168
#define AMnFIFOPNTR_FIFOWPNTR GENMASK(7, 0)
#define AMnFIFOPNTR_FIFORPNTR_Y GENMASK(23, 16)
-/* AXI Master Transfer Stop Register for CRU Image Data */
-#define AMnAXISTP 0x174
#define AMnAXISTP_AXI_STOP BIT(0)
-/* AXI Master Transfer Stop Status Register for CRU Image Data */
-#define AMnAXISTPACK 0x178
#define AMnAXISTPACK_AXI_STOP_ACK BIT(0)
-/* CRU Image Processing Enable Register */
-#define ICnEN 0x200
#define ICnEN_ICEN BIT(0)
-/* CRU Image Processing Main Control Register */
-#define ICnMC 0x208
#define ICnMC_CSCTHR BIT(5)
#define ICnMC_INF(x) ((x) << 16)
#define ICnMC_VCSEL(x) ((x) << 22)
#define ICnMC_INF_MASK GENMASK(21, 16)
-/* CRU Module Status Register */
-#define ICnMS 0x254
#define ICnMS_IA BIT(2)
-/* CRU Data Output Mode Register */
-#define ICnDMR 0x26c
#define ICnDMR_YCMODE_UYVY (1 << 4)
+enum rzg2l_cru_common_regs {
+ CRUnCTRL, /* CRU Control */
+ CRUnIE, /* CRU Interrupt Enable */
+ CRUnINTS, /* CRU Interrupt Status */
+ CRUnRST, /* CRU Reset */
+ AMnMB1ADDRL, /* Bank 1 Address (Lower) for CRU Image Data */
+ AMnMB1ADDRH, /* Bank 1 Address (Higher) for CRU Image Data */
+ AMnMB2ADDRL, /* Bank 2 Address (Lower) for CRU Image Data */
+ AMnMB2ADDRH, /* Bank 2 Address (Higher) for CRU Image Data */
+ AMnMB3ADDRL, /* Bank 3 Address (Lower) for CRU Image Data */
+ AMnMB3ADDRH, /* Bank 3 Address (Higher) for CRU Image Data */
+ AMnMB4ADDRL, /* Bank 4 Address (Lower) for CRU Image Data */
+ AMnMB4ADDRH, /* Bank 4 Address (Higher) for CRU Image Data */
+ AMnMB5ADDRL, /* Bank 5 Address (Lower) for CRU Image Data */
+ AMnMB5ADDRH, /* Bank 5 Address (Higher) for CRU Image Data */
+ AMnMB6ADDRL, /* Bank 6 Address (Lower) for CRU Image Data */
+ AMnMB6ADDRH, /* Bank 6 Address (Higher) for CRU Image Data */
+ AMnMB7ADDRL, /* Bank 7 Address (Lower) for CRU Image Data */
+ AMnMB7ADDRH, /* Bank 7 Address (Higher) for CRU Image Data */
+ AMnMB8ADDRL, /* Bank 8 Address (Lower) for CRU Image Data */
+ AMnMB8ADDRH, /* Bank 8 Address (Higher) for CRU Image Data */
+ AMnMBVALID, /* Memory Bank Enable for CRU Image Data */
+ AMnMBS, /* Memory Bank Status for CRU Image Data */
+ AMnAXIATTR, /* AXI Master Transfer Setting Register for CRU Image Data */
+ AMnFIFOPNTR, /* AXI Master FIFO Pointer for CRU Image Data */
+ AMnAXISTP, /* AXI Master Transfer Stop for CRU Image Data */
+ AMnAXISTPACK, /* AXI Master Transfer Stop Status for CRU Image Data */
+ ICnEN, /* CRU Image Processing Enable */
+ ICnMC, /* CRU Image Processing Main Control */
+ ICnMS, /* CRU Module Status */
+ ICnDMR, /* CRU Data Output Mode */
+ RZG2L_CRU_MAX_REG,
+};
+
#endif /* __RZG2L_CRU_REGS_H__ */
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index 8b898ce05b847..00c3f7458e20a 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -80,6 +80,10 @@ struct rzg2l_cru_ip_format {
bool yuv;
};
+struct rzg2l_cru_info {
+ const u16 *regs;
+};
+
/**
* struct rzg2l_cru_dev - Renesas CRU device structure
* @dev: (OF) device
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index cd69c8a686d35..c82db80c33552 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -42,16 +42,66 @@ struct rzg2l_cru_buffer {
/* -----------------------------------------------------------------------------
* DMA operations
*/
-static void rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
+static void __rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
{
- iowrite32(value, cru->base + offset);
+ const u16 *regs = cru->info->regs;
+
+ /*
+ * CRUnCTRL is a first register on all CRU supported SoCs so validate
+ * rest of the registers have valid offset being set in cru->info->regs.
+ */
+ if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
+ WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
+ return;
+
+ iowrite32(value, cru->base + regs[offset]);
+}
+
+static u32 __rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
+{
+ const u16 *regs = cru->info->regs;
+
+ /*
+ * CRUnCTRL is a first register on all CRU supported SoCs so validate
+ * rest of the registers have valid offset being set in cru->info->regs.
+ */
+ if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
+ WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
+ return 0;
+
+ return ioread32(cru->base + regs[offset]);
}
-static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
+static __always_inline void
+__rzg2l_cru_write_constant(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
{
- return ioread32(cru->base + offset);
+ const u16 *regs = cru->info->regs;
+
+ BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
+
+ iowrite32(value, cru->base + regs[offset]);
}
+static __always_inline u32
+__rzg2l_cru_read_constant(struct rzg2l_cru_dev *cru, u32 offset)
+{
+ const u16 *regs = cru->info->regs;
+
+ BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
+
+ return ioread32(cru->base + regs[offset]);
+}
+
+#define rzg2l_cru_write(cru, offset, value) \
+ (__builtin_constant_p(offset) ? \
+ __rzg2l_cru_write_constant(cru, offset, value) : \
+ __rzg2l_cru_write(cru, offset, value))
+
+#define rzg2l_cru_read(cru, offset) \
+ (__builtin_constant_p(offset) ? \
+ __rzg2l_cru_read_constant(cru, offset) : \
+ __rzg2l_cru_read(cru, offset))
+
/* Need to hold qlock before calling */
static void return_unused_buffers(struct rzg2l_cru_dev *cru,
enum vb2_buffer_state state)
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* RE: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-03-28 17:29 ` [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support Tommaso Merciai
@ 2025-04-02 6:31 ` Biju Das
2025-04-02 7:35 ` Lad, Prabhakar
0 siblings, 1 reply; 34+ messages in thread
From: Biju Das @ 2025-04-02 6:31 UTC (permalink / raw)
To: Tommaso Merciai, Tommaso Merciai
Cc: linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, 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,
Thanks for the patch.
> -----Original Message-----
> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> Sent: 28 March 2025 17:30
> Subject: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a CRU-IP that is mostly identical
> to RZ/G2L but with different register offsets and additional registers. Introduce a flexible register
> mapping mechanism to handle these variations.
>
> Define the `rzg2l_cru_info` structure to store register mappings and pass it as part of the OF match
> data. Update the read/write functions to check out-of-bound accesses and use indexed register offsets
> from `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> Changes since v2:
> - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> accesses as suggested by LPinchart.
> - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> - Update commit body
>
> Changes since v4:
> - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> as __always_inline
>
> .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58 ++++++++++++++--
> 4 files changed, 139 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> index eed9d2bd08414..abc2a979833aa 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> @@ -22,6 +22,7 @@
> #include <media/v4l2-mc.h>
>
> #include "rzg2l-cru.h"
> +#include "rzg2l-cru-regs.h"
>
> static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n) { @@ -269,6
> +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
>
> cru->dev = dev;
> cru->info = of_device_get_match_data(dev);
> + if (!cru->info)
> + return dev_err_probe(dev, -EINVAL,
> + "Failed to get OF match data\n");
>
> irq = platform_get_irq(pdev, 0);
> if (irq < 0)
> @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> rzg2l_cru_dma_unregister(cru);
> }
>
> +static const u16 rzg2l_cru_regs[] = {
> + [CRUnCTRL] = 0x0,
> + [CRUnIE] = 0x4,
> + [CRUnINTS] = 0x8,
> + [CRUnRST] = 0xc,
> + [AMnMB1ADDRL] = 0x100,
> + [AMnMB1ADDRH] = 0x104,
> + [AMnMB2ADDRL] = 0x108,
> + [AMnMB2ADDRH] = 0x10c,
> + [AMnMB3ADDRL] = 0x110,
> + [AMnMB3ADDRH] = 0x114,
> + [AMnMB4ADDRL] = 0x118,
> + [AMnMB4ADDRH] = 0x11c,
> + [AMnMB5ADDRL] = 0x120,
> + [AMnMB5ADDRH] = 0x124,
> + [AMnMB6ADDRL] = 0x128,
> + [AMnMB6ADDRH] = 0x12c,
> + [AMnMB7ADDRL] = 0x130,
> + [AMnMB7ADDRH] = 0x134,
> + [AMnMB8ADDRL] = 0x138,
> + [AMnMB8ADDRH] = 0x13c,
> + [AMnMBVALID] = 0x148,
> + [AMnMBS] = 0x14c,
> + [AMnAXIATTR] = 0x158,
> + [AMnFIFOPNTR] = 0x168,
> + [AMnAXISTP] = 0x174,
> + [AMnAXISTPACK] = 0x178,
> + [ICnEN] = 0x200,
> + [ICnMC] = 0x208,
> + [ICnMS] = 0x254,
> + [ICnDMR] = 0x26c,
> +};
Do we need enum, can't we use struct instead with all these entries instead?
> +
> +static const struct rzg2l_cru_info rzgl2_cru_info = {
> + .regs = rzg2l_cru_regs,
> +};
For a single entry, why you need struct?
Cheers,
Biju
> +
> static const struct of_device_id rzg2l_cru_of_id_table[] = {
> - { .compatible = "renesas,rzg2l-cru", },
> + {
> + .compatible = "renesas,rzg2l-cru",
> + .data = &rzgl2_cru_info,
> + },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, rzg2l_cru_of_id_table); diff --git a/drivers/media/platform/renesas/rzg2l-
> cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> index 1c9f22118a5d9..86c3202862465 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> @@ -10,71 +10,77 @@
>
> /* HW CRU Registers Definition */
>
> -/* CRU Control Register */
> -#define CRUnCTRL 0x0
> #define CRUnCTRL_VINSEL(x) ((x) << 0)
>
> -/* CRU Interrupt Enable Register */
> -#define CRUnIE 0x4
> #define CRUnIE_EFE BIT(17)
>
> -/* CRU Interrupt Status Register */
> -#define CRUnINTS 0x8
> #define CRUnINTS_SFS BIT(16)
>
> -/* CRU Reset Register */
> -#define CRUnRST 0xc
> #define CRUnRST_VRESETN BIT(0)
>
> /* Memory Bank Base Address (Lower) Register for CRU Image Data */
> -#define AMnMBxADDRL(x) (0x100 + ((x) * 8))
> +#define AMnMBxADDRL(x) (AMnMB1ADDRL + (x) * 2)
>
> /* Memory Bank Base Address (Higher) Register for CRU Image Data */
> -#define AMnMBxADDRH(x) (0x104 + ((x) * 8))
> +#define AMnMBxADDRH(x) (AMnMB1ADDRH + (x) * 2)
>
> -/* Memory Bank Enable Register for CRU Image Data */
> -#define AMnMBVALID 0x148
> #define AMnMBVALID_MBVALID(x) GENMASK(x, 0)
>
> -/* Memory Bank Status Register for CRU Image Data */
> -#define AMnMBS 0x14c
> #define AMnMBS_MBSTS 0x7
>
> -/* AXI Master Transfer Setting Register for CRU Image Data */
> -#define AMnAXIATTR 0x158
> #define AMnAXIATTR_AXILEN_MASK GENMASK(3, 0)
> #define AMnAXIATTR_AXILEN (0xf)
>
> -/* AXI Master FIFO Pointer Register for CRU Image Data */
> -#define AMnFIFOPNTR 0x168
> #define AMnFIFOPNTR_FIFOWPNTR GENMASK(7, 0)
> #define AMnFIFOPNTR_FIFORPNTR_Y GENMASK(23, 16)
>
> -/* AXI Master Transfer Stop Register for CRU Image Data */
> -#define AMnAXISTP 0x174
> #define AMnAXISTP_AXI_STOP BIT(0)
>
> -/* AXI Master Transfer Stop Status Register for CRU Image Data */
> -#define AMnAXISTPACK 0x178
> #define AMnAXISTPACK_AXI_STOP_ACK BIT(0)
>
> -/* CRU Image Processing Enable Register */
> -#define ICnEN 0x200
> #define ICnEN_ICEN BIT(0)
>
> -/* CRU Image Processing Main Control Register */
> -#define ICnMC 0x208
> #define ICnMC_CSCTHR BIT(5)
> #define ICnMC_INF(x) ((x) << 16)
> #define ICnMC_VCSEL(x) ((x) << 22)
> #define ICnMC_INF_MASK GENMASK(21, 16)
>
> -/* CRU Module Status Register */
> -#define ICnMS 0x254
> #define ICnMS_IA BIT(2)
>
> -/* CRU Data Output Mode Register */
> -#define ICnDMR 0x26c
> #define ICnDMR_YCMODE_UYVY (1 << 4)
>
> +enum rzg2l_cru_common_regs {
> + CRUnCTRL, /* CRU Control */
> + CRUnIE, /* CRU Interrupt Enable */
> + CRUnINTS, /* CRU Interrupt Status */
> + CRUnRST, /* CRU Reset */
> + AMnMB1ADDRL, /* Bank 1 Address (Lower) for CRU Image Data */
> + AMnMB1ADDRH, /* Bank 1 Address (Higher) for CRU Image Data */
> + AMnMB2ADDRL, /* Bank 2 Address (Lower) for CRU Image Data */
> + AMnMB2ADDRH, /* Bank 2 Address (Higher) for CRU Image Data */
> + AMnMB3ADDRL, /* Bank 3 Address (Lower) for CRU Image Data */
> + AMnMB3ADDRH, /* Bank 3 Address (Higher) for CRU Image Data */
> + AMnMB4ADDRL, /* Bank 4 Address (Lower) for CRU Image Data */
> + AMnMB4ADDRH, /* Bank 4 Address (Higher) for CRU Image Data */
> + AMnMB5ADDRL, /* Bank 5 Address (Lower) for CRU Image Data */
> + AMnMB5ADDRH, /* Bank 5 Address (Higher) for CRU Image Data */
> + AMnMB6ADDRL, /* Bank 6 Address (Lower) for CRU Image Data */
> + AMnMB6ADDRH, /* Bank 6 Address (Higher) for CRU Image Data */
> + AMnMB7ADDRL, /* Bank 7 Address (Lower) for CRU Image Data */
> + AMnMB7ADDRH, /* Bank 7 Address (Higher) for CRU Image Data */
> + AMnMB8ADDRL, /* Bank 8 Address (Lower) for CRU Image Data */
> + AMnMB8ADDRH, /* Bank 8 Address (Higher) for CRU Image Data */
> + AMnMBVALID, /* Memory Bank Enable for CRU Image Data */
> + AMnMBS, /* Memory Bank Status for CRU Image Data */
> + AMnAXIATTR, /* AXI Master Transfer Setting Register for CRU Image Data */
> + AMnFIFOPNTR, /* AXI Master FIFO Pointer for CRU Image Data */
> + AMnAXISTP, /* AXI Master Transfer Stop for CRU Image Data */
> + AMnAXISTPACK, /* AXI Master Transfer Stop Status for CRU Image Data */
> + ICnEN, /* CRU Image Processing Enable */
> + ICnMC, /* CRU Image Processing Main Control */
> + ICnMS, /* CRU Module Status */
> + ICnDMR, /* CRU Data Output Mode */
> + RZG2L_CRU_MAX_REG,
> +};
> +
> #endif /* __RZG2L_CRU_REGS_H__ */
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> index 8b898ce05b847..00c3f7458e20a 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> @@ -80,6 +80,10 @@ struct rzg2l_cru_ip_format {
> bool yuv;
> };
>
> +struct rzg2l_cru_info {
> + const u16 *regs;
> +};
> +
> /**
> * struct rzg2l_cru_dev - Renesas CRU device structure
> * @dev: (OF) device
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index cd69c8a686d35..c82db80c33552 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -42,16 +42,66 @@ struct rzg2l_cru_buffer {
> /* -----------------------------------------------------------------------------
> * DMA operations
> */
> -static void rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset, u32 value)
> +static void __rzg2l_cru_write(struct rzg2l_cru_dev *cru, u32 offset,
> +u32 value)
> {
> - iowrite32(value, cru->base + offset);
> + const u16 *regs = cru->info->regs;
> +
> + /*
> + * CRUnCTRL is a first register on all CRU supported SoCs so validate
> + * rest of the registers have valid offset being set in cru->info->regs.
> + */
> + if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> + WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> + return;
> +
> + iowrite32(value, cru->base + regs[offset]); }
> +
> +static u32 __rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset) {
> + const u16 *regs = cru->info->regs;
> +
> + /*
> + * CRUnCTRL is a first register on all CRU supported SoCs so validate
> + * rest of the registers have valid offset being set in cru->info->regs.
> + */
> + if (WARN_ON(offset >= RZG2L_CRU_MAX_REG) ||
> + WARN_ON(offset != CRUnCTRL && regs[offset] == 0))
> + return 0;
> +
> + return ioread32(cru->base + regs[offset]);
> }
>
> -static u32 rzg2l_cru_read(struct rzg2l_cru_dev *cru, u32 offset)
> +static __always_inline void
> +__rzg2l_cru_write_constant(struct rzg2l_cru_dev *cru, u32 offset, u32
> +value)
> {
> - return ioread32(cru->base + offset);
> + const u16 *regs = cru->info->regs;
> +
> + BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> +
> + iowrite32(value, cru->base + regs[offset]);
> }
>
> +static __always_inline u32
> +__rzg2l_cru_read_constant(struct rzg2l_cru_dev *cru, u32 offset) {
> + const u16 *regs = cru->info->regs;
> +
> + BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG);
> +
> + return ioread32(cru->base + regs[offset]); }
> +
> +#define rzg2l_cru_write(cru, offset, value) \
> + (__builtin_constant_p(offset) ? \
> + __rzg2l_cru_write_constant(cru, offset, value) : \
> + __rzg2l_cru_write(cru, offset, value))
> +
> +#define rzg2l_cru_read(cru, offset) \
> + (__builtin_constant_p(offset) ? \
> + __rzg2l_cru_read_constant(cru, offset) : \
> + __rzg2l_cru_read(cru, offset))
> +
> /* Need to hold qlock before calling */ static void return_unused_buffers(struct rzg2l_cru_dev *cru,
> enum vb2_buffer_state state)
> --
> 2.43.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-02 6:31 ` Biju Das
@ 2025-04-02 7:35 ` Lad, Prabhakar
2025-04-02 8:19 ` Biju Das
0 siblings, 1 reply; 34+ messages in thread
From: Lad, Prabhakar @ 2025-04-02 7:35 UTC (permalink / raw)
To: Biju Das
Cc: Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, 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 Biju,
Thank you for the review.
On Wed, Apr 2, 2025 at 7:31 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Tommaso,
>
> Thanks for the patch.
>
> > -----Original Message-----
> > From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > Sent: 28 March 2025 17:30
> > Subject: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
> >
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a CRU-IP that is mostly identical
> > to RZ/G2L but with different register offsets and additional registers. Introduce a flexible register
> > mapping mechanism to handle these variations.
> >
> > Define the `rzg2l_cru_info` structure to store register mappings and pass it as part of the OF match
> > data. Update the read/write functions to check out-of-bound accesses and use indexed register offsets
> > from `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > Changes since v2:
> > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > accesses as suggested by LPinchart.
> > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > - Update commit body
> >
> > Changes since v4:
> > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > as __always_inline
> >
> > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58 ++++++++++++++--
> > 4 files changed, 139 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > index eed9d2bd08414..abc2a979833aa 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > @@ -22,6 +22,7 @@
> > #include <media/v4l2-mc.h>
> >
> > #include "rzg2l-cru.h"
> > +#include "rzg2l-cru-regs.h"
> >
> > static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n) { @@ -269,6
> > +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> >
> > cru->dev = dev;
> > cru->info = of_device_get_match_data(dev);
> > + if (!cru->info)
> > + return dev_err_probe(dev, -EINVAL,
> > + "Failed to get OF match data\n");
> >
> > irq = platform_get_irq(pdev, 0);
> > if (irq < 0)
> > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > rzg2l_cru_dma_unregister(cru);
> > }
> >
> > +static const u16 rzg2l_cru_regs[] = {
> > + [CRUnCTRL] = 0x0,
> > + [CRUnIE] = 0x4,
> > + [CRUnINTS] = 0x8,
> > + [CRUnRST] = 0xc,
> > + [AMnMB1ADDRL] = 0x100,
> > + [AMnMB1ADDRH] = 0x104,
> > + [AMnMB2ADDRL] = 0x108,
> > + [AMnMB2ADDRH] = 0x10c,
> > + [AMnMB3ADDRL] = 0x110,
> > + [AMnMB3ADDRH] = 0x114,
> > + [AMnMB4ADDRL] = 0x118,
> > + [AMnMB4ADDRH] = 0x11c,
> > + [AMnMB5ADDRL] = 0x120,
> > + [AMnMB5ADDRH] = 0x124,
> > + [AMnMB6ADDRL] = 0x128,
> > + [AMnMB6ADDRH] = 0x12c,
> > + [AMnMB7ADDRL] = 0x130,
> > + [AMnMB7ADDRH] = 0x134,
> > + [AMnMB8ADDRL] = 0x138,
> > + [AMnMB8ADDRH] = 0x13c,
> > + [AMnMBVALID] = 0x148,
> > + [AMnMBS] = 0x14c,
> > + [AMnAXIATTR] = 0x158,
> > + [AMnFIFOPNTR] = 0x168,
> > + [AMnAXISTP] = 0x174,
> > + [AMnAXISTPACK] = 0x178,
> > + [ICnEN] = 0x200,
> > + [ICnMC] = 0x208,
> > + [ICnMS] = 0x254,
> > + [ICnDMR] = 0x26c,
> > +};
>
> Do we need enum, can't we use struct instead with all these entries instead?
>
What benefit do you foresee when using struct? With the current
approach being used a minimal diff is generated when switched to
struct there will be lots of changes.
> > +
> > +static const struct rzg2l_cru_info rzgl2_cru_info = {
> > + .regs = rzg2l_cru_regs,
> > +};
>
> For a single entry, why you need struct?
>
This struct will grow further, see the later patches.
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-02 7:35 ` Lad, Prabhakar
@ 2025-04-02 8:19 ` Biju Das
2025-04-02 8:25 ` Lad, Prabhakar
0 siblings, 1 reply; 34+ messages in thread
From: Biju Das @ 2025-04-02 8:19 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, 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 Prabhakar,
> -----Original Message-----
> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> Sent: 02 April 2025 08:35
> Subject: Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
>
> Hi Biju,
>
> Thank you for the review.
>
> On Wed, Apr 2, 2025 at 7:31 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > Hi Tommaso,
> >
> > Thanks for the patch.
> >
> > > -----Original Message-----
> > > From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > Sent: 28 March 2025 17:30
> > > Subject: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping
> > > support
> > >
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a
> > > CRU-IP that is mostly identical to RZ/G2L but with different
> > > register offsets and additional registers. Introduce a flexible register mapping mechanism to
> handle these variations.
> > >
> > > Define the `rzg2l_cru_info` structure to store register mappings and
> > > pass it as part of the OF match data. Update the read/write
> > > functions to check out-of-bound accesses and use indexed register offsets from `rzg2l_cru_info`,
> ensuring compatibility across different SoC variants.
> > >
> > > Signed-off-by: Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > > Changes since v2:
> > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > accesses as suggested by LPinchart.
> > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > - Update commit body
> > >
> > > Changes since v4:
> > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > as __always_inline
> > >
> > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58 ++++++++++++++--
> > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > index eed9d2bd08414..abc2a979833aa 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > @@ -22,6 +22,7 @@
> > > #include <media/v4l2-mc.h>
> > >
> > > #include "rzg2l-cru.h"
> > > +#include "rzg2l-cru-regs.h"
> > >
> > > static inline struct rzg2l_cru_dev *notifier_to_cru(struct
> > > v4l2_async_notifier *n) { @@ -269,6
> > > +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > >
> > > cru->dev = dev;
> > > cru->info = of_device_get_match_data(dev);
> > > + if (!cru->info)
> > > + return dev_err_probe(dev, -EINVAL,
> > > + "Failed to get OF match data\n");
> > >
> > > irq = platform_get_irq(pdev, 0);
> > > if (irq < 0)
> > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > rzg2l_cru_dma_unregister(cru); }
> > >
> > > +static const u16 rzg2l_cru_regs[] = {
> > > + [CRUnCTRL] = 0x0,
> > > + [CRUnIE] = 0x4,
> > > + [CRUnINTS] = 0x8,
> > > + [CRUnRST] = 0xc,
> > > + [AMnMB1ADDRL] = 0x100,
> > > + [AMnMB1ADDRH] = 0x104,
> > > + [AMnMB2ADDRL] = 0x108,
> > > + [AMnMB2ADDRH] = 0x10c,
> > > + [AMnMB3ADDRL] = 0x110,
> > > + [AMnMB3ADDRH] = 0x114,
> > > + [AMnMB4ADDRL] = 0x118,
> > > + [AMnMB4ADDRH] = 0x11c,
> > > + [AMnMB5ADDRL] = 0x120,
> > > + [AMnMB5ADDRH] = 0x124,
> > > + [AMnMB6ADDRL] = 0x128,
> > > + [AMnMB6ADDRH] = 0x12c,
> > > + [AMnMB7ADDRL] = 0x130,
> > > + [AMnMB7ADDRH] = 0x134,
> > > + [AMnMB8ADDRL] = 0x138,
> > > + [AMnMB8ADDRH] = 0x13c,
> > > + [AMnMBVALID] = 0x148,
> > > + [AMnMBS] = 0x14c,
> > > + [AMnAXIATTR] = 0x158,
> > > + [AMnFIFOPNTR] = 0x168,
> > > + [AMnAXISTP] = 0x174,
> > > + [AMnAXISTPACK] = 0x178,
> > > + [ICnEN] = 0x200,
> > > + [ICnMC] = 0x208,
> > > + [ICnMS] = 0x254,
> > > + [ICnDMR] = 0x26c,
> > > +};
> >
> > Do we need enum, can't we use struct instead with all these entries instead?
> >
> What benefit do you foresee when using struct? With the current approach being used a minimal diff is
> generated when switched to struct there will be lots of changes.
The mapping is convinient when you want to iterate throught it. Here, if
you just want to access the offset value from its name, a structure
looks more appropriate.
See [1]
[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20250326122003.122976-15-biju.das.jz@bp.renesas.com/
Cheers,
Biju
>
> > > +
> > > +static const struct rzg2l_cru_info rzgl2_cru_info = {
> > > + .regs = rzg2l_cru_regs,
> > > +};
> >
> > For a single entry, why you need struct?
> >
> This struct will grow further, see the later patches.
>
> Cheers,
> Prabhakar
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-02 8:19 ` Biju Das
@ 2025-04-02 8:25 ` Lad, Prabhakar
2025-04-02 8:29 ` Biju Das
2025-04-02 9:26 ` Laurent Pinchart
0 siblings, 2 replies; 34+ messages in thread
From: Lad, Prabhakar @ 2025-04-02 8:25 UTC (permalink / raw)
To: Biju Das
Cc: Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, 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 Biju,
On Wed, Apr 2, 2025 at 9:20 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Prabhakar,
>
> > -----Original Message-----
> > From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> > Sent: 02 April 2025 08:35
> > Subject: Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
> >
> > Hi Biju,
> >
> > Thank you for the review.
> >
> > On Wed, Apr 2, 2025 at 7:31 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > >
> > > Hi Tommaso,
> > >
> > > Thanks for the patch.
> > >
> > > > -----Original Message-----
> > > > From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > Sent: 28 March 2025 17:30
> > > > Subject: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping
> > > > support
> > > >
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a
> > > > CRU-IP that is mostly identical to RZ/G2L but with different
> > > > register offsets and additional registers. Introduce a flexible register mapping mechanism to
> > handle these variations.
> > > >
> > > > Define the `rzg2l_cru_info` structure to store register mappings and
> > > > pass it as part of the OF match data. Update the read/write
> > > > functions to check out-of-bound accesses and use indexed register offsets from `rzg2l_cru_info`,
> > ensuring compatibility across different SoC variants.
> > > >
> > > > Signed-off-by: Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > ---
> > > > Changes since v2:
> > > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > accesses as suggested by LPinchart.
> > > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > - Update commit body
> > > >
> > > > Changes since v4:
> > > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > > as __always_inline
> > > >
> > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58 ++++++++++++++--
> > > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > index eed9d2bd08414..abc2a979833aa 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > @@ -22,6 +22,7 @@
> > > > #include <media/v4l2-mc.h>
> > > >
> > > > #include "rzg2l-cru.h"
> > > > +#include "rzg2l-cru-regs.h"
> > > >
> > > > static inline struct rzg2l_cru_dev *notifier_to_cru(struct
> > > > v4l2_async_notifier *n) { @@ -269,6
> > > > +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > > >
> > > > cru->dev = dev;
> > > > cru->info = of_device_get_match_data(dev);
> > > > + if (!cru->info)
> > > > + return dev_err_probe(dev, -EINVAL,
> > > > + "Failed to get OF match data\n");
> > > >
> > > > irq = platform_get_irq(pdev, 0);
> > > > if (irq < 0)
> > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > rzg2l_cru_dma_unregister(cru); }
> > > >
> > > > +static const u16 rzg2l_cru_regs[] = {
> > > > + [CRUnCTRL] = 0x0,
> > > > + [CRUnIE] = 0x4,
> > > > + [CRUnINTS] = 0x8,
> > > > + [CRUnRST] = 0xc,
> > > > + [AMnMB1ADDRL] = 0x100,
> > > > + [AMnMB1ADDRH] = 0x104,
> > > > + [AMnMB2ADDRL] = 0x108,
> > > > + [AMnMB2ADDRH] = 0x10c,
> > > > + [AMnMB3ADDRL] = 0x110,
> > > > + [AMnMB3ADDRH] = 0x114,
> > > > + [AMnMB4ADDRL] = 0x118,
> > > > + [AMnMB4ADDRH] = 0x11c,
> > > > + [AMnMB5ADDRL] = 0x120,
> > > > + [AMnMB5ADDRH] = 0x124,
> > > > + [AMnMB6ADDRL] = 0x128,
> > > > + [AMnMB6ADDRH] = 0x12c,
> > > > + [AMnMB7ADDRL] = 0x130,
> > > > + [AMnMB7ADDRH] = 0x134,
> > > > + [AMnMB8ADDRL] = 0x138,
> > > > + [AMnMB8ADDRH] = 0x13c,
> > > > + [AMnMBVALID] = 0x148,
> > > > + [AMnMBS] = 0x14c,
> > > > + [AMnAXIATTR] = 0x158,
> > > > + [AMnFIFOPNTR] = 0x168,
> > > > + [AMnAXISTP] = 0x174,
> > > > + [AMnAXISTPACK] = 0x178,
> > > > + [ICnEN] = 0x200,
> > > > + [ICnMC] = 0x208,
> > > > + [ICnMS] = 0x254,
> > > > + [ICnDMR] = 0x26c,
> > > > +};
> > >
> > > Do we need enum, can't we use struct instead with all these entries instead?
> > >
> > What benefit do you foresee when using struct? With the current approach being used a minimal diff is
> > generated when switched to struct there will be lots of changes.
>
> The mapping is convinient when you want to iterate throught it. Here, if
> you just want to access the offset value from its name, a structure
> looks more appropriate.
>
Thanks, as this patch has been reviewed by Laurent a couple of times
we will change this to struct If he insists.
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-02 8:25 ` Lad, Prabhakar
@ 2025-04-02 8:29 ` Biju Das
2025-04-02 9:26 ` Laurent Pinchart
1 sibling, 0 replies; 34+ messages in thread
From: Biju Das @ 2025-04-02 8:29 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, 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 Prabhakar,
> -----Original Message-----
> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> Sent: 02 April 2025 09:25
> Subject: Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
>
> Hi Biju,
>
> On Wed, Apr 2, 2025 at 9:20 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > Hi Prabhakar,
> >
> > > -----Original Message-----
> > > From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> > > Sent: 02 April 2025 08:35
> > > Subject: Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping
> > > support
> > >
> > > Hi Biju,
> > >
> > > Thank you for the review.
> > >
> > > On Wed, Apr 2, 2025 at 7:31 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > >
> > > > Hi Tommaso,
> > > >
> > > > Thanks for the patch.
> > > >
> > > > > -----Original Message-----
> > > > > From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > > Sent: 28 March 2025 17:30
> > > > > Subject: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping
> > > > > support
> > > > >
> > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > >
> > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which
> > > > > have a CRU-IP that is mostly identical to RZ/G2L but with
> > > > > different register offsets and additional registers. Introduce a
> > > > > flexible register mapping mechanism to
> > > handle these variations.
> > > > >
> > > > > Define the `rzg2l_cru_info` structure to store register mappings
> > > > > and pass it as part of the OF match data. Update the read/write
> > > > > functions to check out-of-bound accesses and use indexed
> > > > > register offsets from `rzg2l_cru_info`,
> > > ensuring compatibility across different SoC variants.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar
> > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > Signed-off-by: Tommaso Merciai
> > > > > <tommaso.merciai.xr@bp.renesas.com>
> > > > > ---
> > > > > Changes since v2:
> > > > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > > accesses as suggested by LPinchart.
> > > > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > > - Update commit body
> > > > >
> > > > > Changes since v4:
> > > > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > > > as __always_inline
> > > > >
> > > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58
> > > > > ++++++++++++++--
> > > > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > > > >
> > > > > diff --git
> > > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > index eed9d2bd08414..abc2a979833aa 100644
> > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > @@ -22,6 +22,7 @@
> > > > > #include <media/v4l2-mc.h>
> > > > >
> > > > > #include "rzg2l-cru.h"
> > > > > +#include "rzg2l-cru-regs.h"
> > > > >
> > > > > static inline struct rzg2l_cru_dev *notifier_to_cru(struct
> > > > > v4l2_async_notifier *n) { @@ -269,6
> > > > > +270,9 @@ static int rzg2l_cru_probe(struct platform_device
> > > > > +*pdev)
> > > > >
> > > > > cru->dev = dev;
> > > > > cru->info = of_device_get_match_data(dev);
> > > > > + if (!cru->info)
> > > > > + return dev_err_probe(dev, -EINVAL,
> > > > > + "Failed to get OF match
> > > > > + data\n");
> > > > >
> > > > > irq = platform_get_irq(pdev, 0);
> > > > > if (irq < 0)
> > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > > rzg2l_cru_dma_unregister(cru); }
> > > > >
> > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > + [CRUnCTRL] = 0x0,
> > > > > + [CRUnIE] = 0x4,
> > > > > + [CRUnINTS] = 0x8,
> > > > > + [CRUnRST] = 0xc,
> > > > > + [AMnMB1ADDRL] = 0x100,
> > > > > + [AMnMB1ADDRH] = 0x104,
> > > > > + [AMnMB2ADDRL] = 0x108,
> > > > > + [AMnMB2ADDRH] = 0x10c,
> > > > > + [AMnMB3ADDRL] = 0x110,
> > > > > + [AMnMB3ADDRH] = 0x114,
> > > > > + [AMnMB4ADDRL] = 0x118,
> > > > > + [AMnMB4ADDRH] = 0x11c,
> > > > > + [AMnMB5ADDRL] = 0x120,
> > > > > + [AMnMB5ADDRH] = 0x124,
> > > > > + [AMnMB6ADDRL] = 0x128,
> > > > > + [AMnMB6ADDRH] = 0x12c,
> > > > > + [AMnMB7ADDRL] = 0x130,
> > > > > + [AMnMB7ADDRH] = 0x134,
> > > > > + [AMnMB8ADDRL] = 0x138,
> > > > > + [AMnMB8ADDRH] = 0x13c,
> > > > > + [AMnMBVALID] = 0x148,
> > > > > + [AMnMBS] = 0x14c,
> > > > > + [AMnAXIATTR] = 0x158,
> > > > > + [AMnFIFOPNTR] = 0x168,
> > > > > + [AMnAXISTP] = 0x174,
> > > > > + [AMnAXISTPACK] = 0x178,
> > > > > + [ICnEN] = 0x200,
> > > > > + [ICnMC] = 0x208,
> > > > > + [ICnMS] = 0x254,
> > > > > + [ICnDMR] = 0x26c,
> > > > > +};
> > > >
> > > > Do we need enum, can't we use struct instead with all these entries instead?
> > > >
> > > What benefit do you foresee when using struct? With the current
> > > approach being used a minimal diff is generated when switched to struct there will be lots of
> changes.
> >
> > The mapping is convinient when you want to iterate throught it. Here,
> > if you just want to access the offset value from its name, a structure
> > looks more appropriate.
> >
> Thanks, as this patch has been reviewed by Laurent a couple of times we will change this to struct If
> he insists.
I just provided suggestion as Laurent reviewed all the patches in this series except this one and
I got this comment only 3 days back.
On the other hand, as with this approach there won't be any array bound check anymore with WARN_ON/BUILD_ON??
We can get rid of enums etc...
Cheers,
Biju
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-02 8:25 ` Lad, Prabhakar
2025-04-02 8:29 ` Biju Das
@ 2025-04-02 9:26 ` Laurent Pinchart
2025-04-02 9:36 ` Lad, Prabhakar
2025-04-02 9:39 ` Biju Das
1 sibling, 2 replies; 34+ messages in thread
From: Laurent Pinchart @ 2025-04-02 9:26 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Biju Das, Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Hans Verkuil, Uwe Kleine-König,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
On Wed, Apr 02, 2025 at 08:25:06AM +0000, Lad, Prabhakar wrote:
> On Wed, Apr 2, 2025 at 9:20 AM Biju Das wrote:
> > On 02 April 2025 08:35, Lad, Prabhakar wrote:
> > > On Wed, Apr 2, 2025 at 7:31 AM Biju Das wrote:
> > > > > On 28 March 2025 17:30, Tommaso Merciai wrote:
> > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > >
> > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a
> > > > > CRU-IP that is mostly identical to RZ/G2L but with different
> > > > > register offsets and additional registers. Introduce a
> > > > > flexible register mapping mechanism to handle these
> > > > > variations.
> > > > >
> > > > > Define the `rzg2l_cru_info` structure to store register mappings and
> > > > > pass it as part of the OF match data. Update the read/write
> > > > > functions to check out-of-bound accesses and use indexed
> > > > > register offsets from `rzg2l_cru_info`, ensuring compatibility
> > > > > across different SoC variants.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > > ---
> > > > > Changes since v2:
> > > > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > > accesses as suggested by LPinchart.
> > > > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > > - Update commit body
> > > > >
> > > > > Changes since v4:
> > > > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > > > as __always_inline
> > > > >
> > > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58 ++++++++++++++--
> > > > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > > > >
> > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > index eed9d2bd08414..abc2a979833aa 100644
> > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > @@ -22,6 +22,7 @@
> > > > > #include <media/v4l2-mc.h>
> > > > >
> > > > > #include "rzg2l-cru.h"
> > > > > +#include "rzg2l-cru-regs.h"
> > > > >
> > > > > static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n)
> > > > > {
> > > > > @@ -269,6 +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > > > >
> > > > > cru->dev = dev;
> > > > > cru->info = of_device_get_match_data(dev);
> > > > > + if (!cru->info)
> > > > > + return dev_err_probe(dev, -EINVAL,
> > > > > + "Failed to get OF match data\n");
> > > > >
> > > > > irq = platform_get_irq(pdev, 0);
> > > > > if (irq < 0)
> > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > > rzg2l_cru_dma_unregister(cru); }
> > > > >
> > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > + [CRUnCTRL] = 0x0,
> > > > > + [CRUnIE] = 0x4,
> > > > > + [CRUnINTS] = 0x8,
> > > > > + [CRUnRST] = 0xc,
> > > > > + [AMnMB1ADDRL] = 0x100,
> > > > > + [AMnMB1ADDRH] = 0x104,
> > > > > + [AMnMB2ADDRL] = 0x108,
> > > > > + [AMnMB2ADDRH] = 0x10c,
> > > > > + [AMnMB3ADDRL] = 0x110,
> > > > > + [AMnMB3ADDRH] = 0x114,
> > > > > + [AMnMB4ADDRL] = 0x118,
> > > > > + [AMnMB4ADDRH] = 0x11c,
> > > > > + [AMnMB5ADDRL] = 0x120,
> > > > > + [AMnMB5ADDRH] = 0x124,
> > > > > + [AMnMB6ADDRL] = 0x128,
> > > > > + [AMnMB6ADDRH] = 0x12c,
> > > > > + [AMnMB7ADDRL] = 0x130,
> > > > > + [AMnMB7ADDRH] = 0x134,
> > > > > + [AMnMB8ADDRL] = 0x138,
> > > > > + [AMnMB8ADDRH] = 0x13c,
> > > > > + [AMnMBVALID] = 0x148,
> > > > > + [AMnMBS] = 0x14c,
> > > > > + [AMnAXIATTR] = 0x158,
> > > > > + [AMnFIFOPNTR] = 0x168,
> > > > > + [AMnAXISTP] = 0x174,
> > > > > + [AMnAXISTPACK] = 0x178,
> > > > > + [ICnEN] = 0x200,
> > > > > + [ICnMC] = 0x208,
> > > > > + [ICnMS] = 0x254,
> > > > > + [ICnDMR] = 0x26c,
> > > > > +};
> > > >
> > > > Do we need enum, can't we use struct instead with all these entries instead?
> > > >
> > > What benefit do you foresee when using struct? With the current approach being used a minimal diff is
> > > generated when switched to struct there will be lots of changes.
> >
> > The mapping is convinient when you want to iterate throught it. Here, if
> > you just want to access the offset value from its name, a structure
> > looks more appropriate.
>
> Thanks, as this patch has been reviewed by Laurent a couple of times
> we will change this to struct If he insists.
How would a struct look like ? I'm not sure what is being proposed.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-02 9:26 ` Laurent Pinchart
@ 2025-04-02 9:36 ` Lad, Prabhakar
2025-04-02 9:39 ` Biju Das
1 sibling, 0 replies; 34+ messages in thread
From: Lad, Prabhakar @ 2025-04-02 9:36 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Biju Das, Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Hans Verkuil, Uwe Kleine-König,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Hi Laurent,
On Wed, Apr 2, 2025 at 10:26 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Wed, Apr 02, 2025 at 08:25:06AM +0000, Lad, Prabhakar wrote:
> > On Wed, Apr 2, 2025 at 9:20 AM Biju Das wrote:
> > > On 02 April 2025 08:35, Lad, Prabhakar wrote:
> > > > On Wed, Apr 2, 2025 at 7:31 AM Biju Das wrote:
> > > > > > On 28 March 2025 17:30, Tommaso Merciai wrote:
> > > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > >
> > > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which have a
> > > > > > CRU-IP that is mostly identical to RZ/G2L but with different
> > > > > > register offsets and additional registers. Introduce a
> > > > > > flexible register mapping mechanism to handle these
> > > > > > variations.
> > > > > >
> > > > > > Define the `rzg2l_cru_info` structure to store register mappings and
> > > > > > pass it as part of the OF match data. Update the read/write
> > > > > > functions to check out-of-bound accesses and use indexed
> > > > > > register offsets from `rzg2l_cru_info`, ensuring compatibility
> > > > > > across different SoC variants.
> > > > > >
> > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > > > ---
> > > > > > Changes since v2:
> > > > > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > > > accesses as suggested by LPinchart.
> > > > > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > > > - Update commit body
> > > > > >
> > > > > > Changes since v4:
> > > > > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > > > > as __always_inline
> > > > > >
> > > > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > > > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58 ++++++++++++++--
> > > > > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > index eed9d2bd08414..abc2a979833aa 100644
> > > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > @@ -22,6 +22,7 @@
> > > > > > #include <media/v4l2-mc.h>
> > > > > >
> > > > > > #include "rzg2l-cru.h"
> > > > > > +#include "rzg2l-cru-regs.h"
> > > > > >
> > > > > > static inline struct rzg2l_cru_dev *notifier_to_cru(struct v4l2_async_notifier *n)
> > > > > > {
> > > > > > @@ -269,6 +270,9 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > > > > >
> > > > > > cru->dev = dev;
> > > > > > cru->info = of_device_get_match_data(dev);
> > > > > > + if (!cru->info)
> > > > > > + return dev_err_probe(dev, -EINVAL,
> > > > > > + "Failed to get OF match data\n");
> > > > > >
> > > > > > irq = platform_get_irq(pdev, 0);
> > > > > > if (irq < 0)
> > > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > > > rzg2l_cru_dma_unregister(cru); }
> > > > > >
> > > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > > + [CRUnCTRL] = 0x0,
> > > > > > + [CRUnIE] = 0x4,
> > > > > > + [CRUnINTS] = 0x8,
> > > > > > + [CRUnRST] = 0xc,
> > > > > > + [AMnMB1ADDRL] = 0x100,
> > > > > > + [AMnMB1ADDRH] = 0x104,
> > > > > > + [AMnMB2ADDRL] = 0x108,
> > > > > > + [AMnMB2ADDRH] = 0x10c,
> > > > > > + [AMnMB3ADDRL] = 0x110,
> > > > > > + [AMnMB3ADDRH] = 0x114,
> > > > > > + [AMnMB4ADDRL] = 0x118,
> > > > > > + [AMnMB4ADDRH] = 0x11c,
> > > > > > + [AMnMB5ADDRL] = 0x120,
> > > > > > + [AMnMB5ADDRH] = 0x124,
> > > > > > + [AMnMB6ADDRL] = 0x128,
> > > > > > + [AMnMB6ADDRH] = 0x12c,
> > > > > > + [AMnMB7ADDRL] = 0x130,
> > > > > > + [AMnMB7ADDRH] = 0x134,
> > > > > > + [AMnMB8ADDRL] = 0x138,
> > > > > > + [AMnMB8ADDRH] = 0x13c,
> > > > > > + [AMnMBVALID] = 0x148,
> > > > > > + [AMnMBS] = 0x14c,
> > > > > > + [AMnAXIATTR] = 0x158,
> > > > > > + [AMnFIFOPNTR] = 0x168,
> > > > > > + [AMnAXISTP] = 0x174,
> > > > > > + [AMnAXISTPACK] = 0x178,
> > > > > > + [ICnEN] = 0x200,
> > > > > > + [ICnMC] = 0x208,
> > > > > > + [ICnMS] = 0x254,
> > > > > > + [ICnDMR] = 0x26c,
> > > > > > +};
> > > > >
> > > > > Do we need enum, can't we use struct instead with all these entries instead?
> > > > >
> > > > What benefit do you foresee when using struct? With the current approach being used a minimal diff is
> > > > generated when switched to struct there will be lots of changes.
> > >
> > > The mapping is convinient when you want to iterate throught it. Here, if
> > > you just want to access the offset value from its name, a structure
> > > looks more appropriate.
> >
> > Thanks, as this patch has been reviewed by Laurent a couple of times
> > we will change this to struct If he insists.
>
> How would a struct look like ? I'm not sure what is being proposed.
>
Something like below:
struct cru_regs {
u16 ctrl;
u16 ie;
u16 ints;
...
...
};
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-02 9:26 ` Laurent Pinchart
2025-04-02 9:36 ` Lad, Prabhakar
@ 2025-04-02 9:39 ` Biju Das
2025-04-07 16:55 ` Lad, Prabhakar
1 sibling, 1 reply; 34+ messages in thread
From: Biju Das @ 2025-04-02 9:39 UTC (permalink / raw)
To: laurent.pinchart, Lad, Prabhakar
Cc: Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Hans Verkuil, Uwe Kleine-König,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Hi Laurent,
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 April 2025 10:26
> Subject: Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
>
> On Wed, Apr 02, 2025 at 08:25:06AM +0000, Lad, Prabhakar wrote:
> > On Wed, Apr 2, 2025 at 9:20 AM Biju Das wrote:
> > > On 02 April 2025 08:35, Lad, Prabhakar wrote:
> > > > On Wed, Apr 2, 2025 at 7:31 AM Biju Das wrote:
> > > > > > On 28 March 2025 17:30, Tommaso Merciai wrote:
> > > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > >
> > > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which
> > > > > > have a CRU-IP that is mostly identical to RZ/G2L but with
> > > > > > different register offsets and additional registers. Introduce
> > > > > > a flexible register mapping mechanism to handle these
> > > > > > variations.
> > > > > >
> > > > > > Define the `rzg2l_cru_info` structure to store register
> > > > > > mappings and pass it as part of the OF match data. Update the
> > > > > > read/write functions to check out-of-bound accesses and use
> > > > > > indexed register offsets from `rzg2l_cru_info`, ensuring
> > > > > > compatibility across different SoC variants.
> > > > > >
> > > > > > Signed-off-by: Lad Prabhakar
> > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > Signed-off-by: Tommaso Merciai
> > > > > > <tommaso.merciai.xr@bp.renesas.com>
> > > > > > ---
> > > > > > Changes since v2:
> > > > > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > > > accesses as suggested by LPinchart.
> > > > > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > > > - Update commit body
> > > > > >
> > > > > > Changes since v4:
> > > > > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > > > > as __always_inline
> > > > > >
> > > > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > > > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58
> > > > > > ++++++++++++++--
> > > > > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > > > > >
> > > > > > diff --git
> > > > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > index eed9d2bd08414..abc2a979833aa 100644
> > > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > @@ -22,6 +22,7 @@
> > > > > > #include <media/v4l2-mc.h>
> > > > > >
> > > > > > #include "rzg2l-cru.h"
> > > > > > +#include "rzg2l-cru-regs.h"
> > > > > >
> > > > > > static inline struct rzg2l_cru_dev *notifier_to_cru(struct
> > > > > > v4l2_async_notifier *n) { @@ -269,6 +270,9 @@ static int
> > > > > > rzg2l_cru_probe(struct platform_device *pdev)
> > > > > >
> > > > > > cru->dev = dev;
> > > > > > cru->info = of_device_get_match_data(dev);
> > > > > > + if (!cru->info)
> > > > > > + return dev_err_probe(dev, -EINVAL,
> > > > > > + "Failed to get OF match
> > > > > > + data\n");
> > > > > >
> > > > > > irq = platform_get_irq(pdev, 0);
> > > > > > if (irq < 0)
> > > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > > > rzg2l_cru_dma_unregister(cru); }
> > > > > >
> > > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > > + [CRUnCTRL] = 0x0,
> > > > > > + [CRUnIE] = 0x4,
> > > > > > + [CRUnINTS] = 0x8,
> > > > > > + [CRUnRST] = 0xc,
> > > > > > + [AMnMB1ADDRL] = 0x100,
> > > > > > + [AMnMB1ADDRH] = 0x104,
> > > > > > + [AMnMB2ADDRL] = 0x108,
> > > > > > + [AMnMB2ADDRH] = 0x10c,
> > > > > > + [AMnMB3ADDRL] = 0x110,
> > > > > > + [AMnMB3ADDRH] = 0x114,
> > > > > > + [AMnMB4ADDRL] = 0x118,
> > > > > > + [AMnMB4ADDRH] = 0x11c,
> > > > > > + [AMnMB5ADDRL] = 0x120,
> > > > > > + [AMnMB5ADDRH] = 0x124,
> > > > > > + [AMnMB6ADDRL] = 0x128,
> > > > > > + [AMnMB6ADDRH] = 0x12c,
> > > > > > + [AMnMB7ADDRL] = 0x130,
> > > > > > + [AMnMB7ADDRH] = 0x134,
> > > > > > + [AMnMB8ADDRL] = 0x138,
> > > > > > + [AMnMB8ADDRH] = 0x13c,
> > > > > > + [AMnMBVALID] = 0x148,
> > > > > > + [AMnMBS] = 0x14c,
> > > > > > + [AMnAXIATTR] = 0x158,
> > > > > > + [AMnFIFOPNTR] = 0x168,
> > > > > > + [AMnAXISTP] = 0x174,
> > > > > > + [AMnAXISTPACK] = 0x178,
> > > > > > + [ICnEN] = 0x200,
> > > > > > + [ICnMC] = 0x208,
> > > > > > + [ICnMS] = 0x254,
> > > > > > + [ICnDMR] = 0x26c,
> > > > > > +};
> > > > >
> > > > > Do we need enum, can't we use struct instead with all these entries instead?
> > > > >
> > > > What benefit do you foresee when using struct? With the current
> > > > approach being used a minimal diff is generated when switched to struct there will be lots of
> changes.
> > >
> > > The mapping is convinient when you want to iterate throught it.
> > > Here, if you just want to access the offset value from its name, a
> > > structure looks more appropriate.
> >
> > Thanks, as this patch has been reviewed by Laurent a couple of times
> > we will change this to struct If he insists.
>
> How would a struct look like ? I'm not sure what is being proposed.
It will be
struct rzg2l_cru_regs {
u16 cru_n_ctrl;
u16 cru_n_ie;
u16 cru_n_ints;
u16 cru_n_rst;
};
static const struct rzg2l_cru_regs rzg2l_cru_regs = {
.cru_n_ctrl = 0x0,
.cru_n_ie = 0x4,
.cru_n_ints = 0x8,
.cru_n_rst = 0xc,
};
You can access it using info->regs->cru_n_ctrl instead of info->regs[CRUnCTRL]
This is proposal.
Cheers,
Biju
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-02 9:39 ` Biju Das
@ 2025-04-07 16:55 ` Lad, Prabhakar
2025-04-09 1:29 ` Laurent Pinchart
0 siblings, 1 reply; 34+ messages in thread
From: Lad, Prabhakar @ 2025-04-07 16:55 UTC (permalink / raw)
To: laurent.pinchart
Cc: Biju Das, Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Hans Verkuil, Uwe Kleine-König,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Hi Laurent,
On Wed, Apr 2, 2025 at 10:39 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Laurent,
>
> > -----Original Message-----
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Sent: 02 April 2025 10:26
> > Subject: Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
> >
> > On Wed, Apr 02, 2025 at 08:25:06AM +0000, Lad, Prabhakar wrote:
> > > On Wed, Apr 2, 2025 at 9:20 AM Biju Das wrote:
> > > > On 02 April 2025 08:35, Lad, Prabhakar wrote:
> > > > > On Wed, Apr 2, 2025 at 7:31 AM Biju Das wrote:
> > > > > > > On 28 March 2025 17:30, Tommaso Merciai wrote:
> > > > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > >
> > > > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which
> > > > > > > have a CRU-IP that is mostly identical to RZ/G2L but with
> > > > > > > different register offsets and additional registers. Introduce
> > > > > > > a flexible register mapping mechanism to handle these
> > > > > > > variations.
> > > > > > >
> > > > > > > Define the `rzg2l_cru_info` structure to store register
> > > > > > > mappings and pass it as part of the OF match data. Update the
> > > > > > > read/write functions to check out-of-bound accesses and use
> > > > > > > indexed register offsets from `rzg2l_cru_info`, ensuring
> > > > > > > compatibility across different SoC variants.
> > > > > > >
> > > > > > > Signed-off-by: Lad Prabhakar
> > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > Signed-off-by: Tommaso Merciai
> > > > > > > <tommaso.merciai.xr@bp.renesas.com>
> > > > > > > ---
> > > > > > > Changes since v2:
> > > > > > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > > > > accesses as suggested by LPinchart.
> > > > > > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > > > > - Update commit body
> > > > > > >
> > > > > > > Changes since v4:
> > > > > > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > > > > > as __always_inline
> > > > > > >
> > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > > > > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58
> > > > > > > ++++++++++++++--
> > > > > > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > > > > > >
> > > > > > > diff --git
> > > > > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > index eed9d2bd08414..abc2a979833aa 100644
> > > > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > @@ -22,6 +22,7 @@
> > > > > > > #include <media/v4l2-mc.h>
> > > > > > >
> > > > > > > #include "rzg2l-cru.h"
> > > > > > > +#include "rzg2l-cru-regs.h"
> > > > > > >
> > > > > > > static inline struct rzg2l_cru_dev *notifier_to_cru(struct
> > > > > > > v4l2_async_notifier *n) { @@ -269,6 +270,9 @@ static int
> > > > > > > rzg2l_cru_probe(struct platform_device *pdev)
> > > > > > >
> > > > > > > cru->dev = dev;
> > > > > > > cru->info = of_device_get_match_data(dev);
> > > > > > > + if (!cru->info)
> > > > > > > + return dev_err_probe(dev, -EINVAL,
> > > > > > > + "Failed to get OF match
> > > > > > > + data\n");
> > > > > > >
> > > > > > > irq = platform_get_irq(pdev, 0);
> > > > > > > if (irq < 0)
> > > > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > > > > rzg2l_cru_dma_unregister(cru); }
> > > > > > >
> > > > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > > > + [CRUnCTRL] = 0x0,
> > > > > > > + [CRUnIE] = 0x4,
> > > > > > > + [CRUnINTS] = 0x8,
> > > > > > > + [CRUnRST] = 0xc,
> > > > > > > + [AMnMB1ADDRL] = 0x100,
> > > > > > > + [AMnMB1ADDRH] = 0x104,
> > > > > > > + [AMnMB2ADDRL] = 0x108,
> > > > > > > + [AMnMB2ADDRH] = 0x10c,
> > > > > > > + [AMnMB3ADDRL] = 0x110,
> > > > > > > + [AMnMB3ADDRH] = 0x114,
> > > > > > > + [AMnMB4ADDRL] = 0x118,
> > > > > > > + [AMnMB4ADDRH] = 0x11c,
> > > > > > > + [AMnMB5ADDRL] = 0x120,
> > > > > > > + [AMnMB5ADDRH] = 0x124,
> > > > > > > + [AMnMB6ADDRL] = 0x128,
> > > > > > > + [AMnMB6ADDRH] = 0x12c,
> > > > > > > + [AMnMB7ADDRL] = 0x130,
> > > > > > > + [AMnMB7ADDRH] = 0x134,
> > > > > > > + [AMnMB8ADDRL] = 0x138,
> > > > > > > + [AMnMB8ADDRH] = 0x13c,
> > > > > > > + [AMnMBVALID] = 0x148,
> > > > > > > + [AMnMBS] = 0x14c,
> > > > > > > + [AMnAXIATTR] = 0x158,
> > > > > > > + [AMnFIFOPNTR] = 0x168,
> > > > > > > + [AMnAXISTP] = 0x174,
> > > > > > > + [AMnAXISTPACK] = 0x178,
> > > > > > > + [ICnEN] = 0x200,
> > > > > > > + [ICnMC] = 0x208,
> > > > > > > + [ICnMS] = 0x254,
> > > > > > > + [ICnDMR] = 0x26c,
> > > > > > > +};
> > > > > >
> > > > > > Do we need enum, can't we use struct instead with all these entries instead?
> > > > > >
> > > > > What benefit do you foresee when using struct? With the current
> > > > > approach being used a minimal diff is generated when switched to struct there will be lots of
> > changes.
> > > >
> > > > The mapping is convinient when you want to iterate throught it.
> > > > Here, if you just want to access the offset value from its name, a
> > > > structure looks more appropriate.
> > >
> > > Thanks, as this patch has been reviewed by Laurent a couple of times
> > > we will change this to struct If he insists.
> >
> > How would a struct look like ? I'm not sure what is being proposed.
>
>
> It will be
>
> struct rzg2l_cru_regs {
> u16 cru_n_ctrl;
> u16 cru_n_ie;
> u16 cru_n_ints;
> u16 cru_n_rst;
> };
>
> static const struct rzg2l_cru_regs rzg2l_cru_regs = {
> .cru_n_ctrl = 0x0,
> .cru_n_ie = 0x4,
> .cru_n_ints = 0x8,
> .cru_n_rst = 0xc,
> };
>
> You can access it using info->regs->cru_n_ctrl instead of info->regs[CRUnCTRL]
> This is proposal.
>
Are you OK with the above proposal?
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-07 16:55 ` Lad, Prabhakar
@ 2025-04-09 1:29 ` Laurent Pinchart
2025-04-09 7:25 ` Biju Das
0 siblings, 1 reply; 34+ messages in thread
From: Laurent Pinchart @ 2025-04-09 1:29 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Biju Das, Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Hans Verkuil, Uwe Kleine-König,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
On Mon, Apr 07, 2025 at 04:55:33PM +0000, Lad, Prabhakar wrote:
> On Wed, Apr 2, 2025 at 10:39 AM Biju Das wrote:
> > On 02 April 2025 10:26, Laurent Pinchart wrote:
> > > On Wed, Apr 02, 2025 at 08:25:06AM +0000, Lad, Prabhakar wrote:
> > > > On Wed, Apr 2, 2025 at 9:20 AM Biju Das wrote:
> > > > > On 02 April 2025 08:35, Lad, Prabhakar wrote:
> > > > > > On Wed, Apr 2, 2025 at 7:31 AM Biju Das wrote:
> > > > > > > > On 28 March 2025 17:30, Tommaso Merciai wrote:
> > > > > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > >
> > > > > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs, which
> > > > > > > > have a CRU-IP that is mostly identical to RZ/G2L but with
> > > > > > > > different register offsets and additional registers. Introduce
> > > > > > > > a flexible register mapping mechanism to handle these
> > > > > > > > variations.
> > > > > > > >
> > > > > > > > Define the `rzg2l_cru_info` structure to store register
> > > > > > > > mappings and pass it as part of the OF match data. Update the
> > > > > > > > read/write functions to check out-of-bound accesses and use
> > > > > > > > indexed register offsets from `rzg2l_cru_info`, ensuring
> > > > > > > > compatibility across different SoC variants.
> > > > > > > >
> > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > > > > > ---
> > > > > > > > Changes since v2:
> > > > > > > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > > > > > accesses as suggested by LPinchart.
> > > > > > > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > > > > > - Update commit body
> > > > > > > >
> > > > > > > > Changes since v4:
> > > > > > > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > > > > > > as __always_inline
> > > > > > > >
> > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > > > > > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58 ++++++++++++++--
> > > > > > > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > > > > > > >
> > > > > > > > diff --git
> > > > > > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > index eed9d2bd08414..abc2a979833aa 100644
> > > > > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > @@ -22,6 +22,7 @@
> > > > > > > > #include <media/v4l2-mc.h>
> > > > > > > >
> > > > > > > > #include "rzg2l-cru.h"
> > > > > > > > +#include "rzg2l-cru-regs.h"
> > > > > > > >
> > > > > > > > static inline struct rzg2l_cru_dev *notifier_to_cru(struct
> > > > > > > > v4l2_async_notifier *n) { @@ -269,6 +270,9 @@ static int
> > > > > > > > rzg2l_cru_probe(struct platform_device *pdev)
> > > > > > > >
> > > > > > > > cru->dev = dev;
> > > > > > > > cru->info = of_device_get_match_data(dev);
> > > > > > > > + if (!cru->info)
> > > > > > > > + return dev_err_probe(dev, -EINVAL,
> > > > > > > > + "Failed to get OF match data\n");
> > > > > > > >
> > > > > > > > irq = platform_get_irq(pdev, 0);
> > > > > > > > if (irq < 0)
> > > > > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > > > > > rzg2l_cru_dma_unregister(cru); }
> > > > > > > >
> > > > > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > > > > + [CRUnCTRL] = 0x0,
> > > > > > > > + [CRUnIE] = 0x4,
> > > > > > > > + [CRUnINTS] = 0x8,
> > > > > > > > + [CRUnRST] = 0xc,
> > > > > > > > + [AMnMB1ADDRL] = 0x100,
> > > > > > > > + [AMnMB1ADDRH] = 0x104,
> > > > > > > > + [AMnMB2ADDRL] = 0x108,
> > > > > > > > + [AMnMB2ADDRH] = 0x10c,
> > > > > > > > + [AMnMB3ADDRL] = 0x110,
> > > > > > > > + [AMnMB3ADDRH] = 0x114,
> > > > > > > > + [AMnMB4ADDRL] = 0x118,
> > > > > > > > + [AMnMB4ADDRH] = 0x11c,
> > > > > > > > + [AMnMB5ADDRL] = 0x120,
> > > > > > > > + [AMnMB5ADDRH] = 0x124,
> > > > > > > > + [AMnMB6ADDRL] = 0x128,
> > > > > > > > + [AMnMB6ADDRH] = 0x12c,
> > > > > > > > + [AMnMB7ADDRL] = 0x130,
> > > > > > > > + [AMnMB7ADDRH] = 0x134,
> > > > > > > > + [AMnMB8ADDRL] = 0x138,
> > > > > > > > + [AMnMB8ADDRH] = 0x13c,
> > > > > > > > + [AMnMBVALID] = 0x148,
> > > > > > > > + [AMnMBS] = 0x14c,
> > > > > > > > + [AMnAXIATTR] = 0x158,
> > > > > > > > + [AMnFIFOPNTR] = 0x168,
> > > > > > > > + [AMnAXISTP] = 0x174,
> > > > > > > > + [AMnAXISTPACK] = 0x178,
> > > > > > > > + [ICnEN] = 0x200,
> > > > > > > > + [ICnMC] = 0x208,
> > > > > > > > + [ICnMS] = 0x254,
> > > > > > > > + [ICnDMR] = 0x26c,
> > > > > > > > +};
> > > > > > >
> > > > > > > Do we need enum, can't we use struct instead with all these entries instead?
> > > > > > >
> > > > > > What benefit do you foresee when using struct? With the current
> > > > > > approach being used a minimal diff is generated when
> > > > > > switched to struct there will be lots of changes.
> > > > >
> > > > > The mapping is convinient when you want to iterate throught it.
> > > > > Here, if you just want to access the offset value from its name, a
> > > > > structure looks more appropriate.
> > > >
> > > > Thanks, as this patch has been reviewed by Laurent a couple of times
> > > > we will change this to struct If he insists.
> > >
> > > How would a struct look like ? I'm not sure what is being proposed.
> >
> > It will be
> >
> > struct rzg2l_cru_regs {
> > u16 cru_n_ctrl;
> > u16 cru_n_ie;
> > u16 cru_n_ints;
> > u16 cru_n_rst;
> > };
> >
> > static const struct rzg2l_cru_regs rzg2l_cru_regs = {
> > .cru_n_ctrl = 0x0,
> > .cru_n_ie = 0x4,
> > .cru_n_ints = 0x8,
> > .cru_n_rst = 0xc,
> > };
> >
> > You can access it using info->regs->cru_n_ctrl instead of info->regs[CRUnCTRL]
> > This is proposal.
>
> Are you OK with the above proposal?
I may be missing something, but I don't see why this would be a
significantly better option. It seems it would make the callers more
complex, and decrease readability.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 34+ messages in thread* RE: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-09 1:29 ` Laurent Pinchart
@ 2025-04-09 7:25 ` Biju Das
2025-04-09 10:33 ` Tommaso Merciai
2025-04-09 11:08 ` Laurent Pinchart
0 siblings, 2 replies; 34+ messages in thread
From: Biju Das @ 2025-04-09 7:25 UTC (permalink / raw)
To: laurent.pinchart, Lad, Prabhakar
Cc: Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Hans Verkuil, Uwe Kleine-König,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Hi Laurent,
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 09 April 2025 02:29
> Subject: Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
>
> On Mon, Apr 07, 2025 at 04:55:33PM +0000, Lad, Prabhakar wrote:
> > On Wed, Apr 2, 2025 at 10:39 AM Biju Das wrote:
> > > On 02 April 2025 10:26, Laurent Pinchart wrote:
> > > > On Wed, Apr 02, 2025 at 08:25:06AM +0000, Lad, Prabhakar wrote:
> > > > > On Wed, Apr 2, 2025 at 9:20 AM Biju Das wrote:
> > > > > > On 02 April 2025 08:35, Lad, Prabhakar wrote:
> > > > > > > On Wed, Apr 2, 2025 at 7:31 AM Biju Das wrote:
> > > > > > > > > On 28 March 2025 17:30, Tommaso Merciai wrote:
> > > > > > > > > From: Lad Prabhakar
> > > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > > >
> > > > > > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs,
> > > > > > > > > which have a CRU-IP that is mostly identical to RZ/G2L
> > > > > > > > > but with different register offsets and additional
> > > > > > > > > registers. Introduce a flexible register mapping
> > > > > > > > > mechanism to handle these variations.
> > > > > > > > >
> > > > > > > > > Define the `rzg2l_cru_info` structure to store register
> > > > > > > > > mappings and pass it as part of the OF match data.
> > > > > > > > > Update the read/write functions to check out-of-bound
> > > > > > > > > accesses and use indexed register offsets from
> > > > > > > > > `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Lad Prabhakar
> > > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > > > Signed-off-by: Tommaso Merciai
> > > > > > > > > <tommaso.merciai.xr@bp.renesas.com>
> > > > > > > > > ---
> > > > > > > > > Changes since v2:
> > > > > > > > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > > > > > > accesses as suggested by LPinchart.
> > > > > > > > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > > > > > > - Update commit body
> > > > > > > > >
> > > > > > > > > Changes since v4:
> > > > > > > > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > > > > > > > as __always_inline
> > > > > > > > >
> > > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > > > > > > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58
> > > > > > > > > ++++++++++++++--
> > > > > > > > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git
> > > > > > > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > > index eed9d2bd08414..abc2a979833aa 100644
> > > > > > > > > ---
> > > > > > > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cor
> > > > > > > > > +++ e.c
> > > > > > > > > @@ -22,6 +22,7 @@
> > > > > > > > > #include <media/v4l2-mc.h>
> > > > > > > > >
> > > > > > > > > #include "rzg2l-cru.h"
> > > > > > > > > +#include "rzg2l-cru-regs.h"
> > > > > > > > >
> > > > > > > > > static inline struct rzg2l_cru_dev
> > > > > > > > > *notifier_to_cru(struct v4l2_async_notifier *n) { @@
> > > > > > > > > -269,6 +270,9 @@ static int rzg2l_cru_probe(struct
> > > > > > > > > platform_device *pdev)
> > > > > > > > >
> > > > > > > > > cru->dev = dev;
> > > > > > > > > cru->info = of_device_get_match_data(dev);
> > > > > > > > > + if (!cru->info)
> > > > > > > > > + return dev_err_probe(dev, -EINVAL,
> > > > > > > > > + "Failed to get OF
> > > > > > > > > + match data\n");
> > > > > > > > >
> > > > > > > > > irq = platform_get_irq(pdev, 0);
> > > > > > > > > if (irq < 0)
> > > > > > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > > > > > > rzg2l_cru_dma_unregister(cru); }
> > > > > > > > >
> > > > > > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > > > > > + [CRUnCTRL] = 0x0,
> > > > > > > > > + [CRUnIE] = 0x4,
> > > > > > > > > + [CRUnINTS] = 0x8,
> > > > > > > > > + [CRUnRST] = 0xc,
> > > > > > > > > + [AMnMB1ADDRL] = 0x100,
> > > > > > > > > + [AMnMB1ADDRH] = 0x104,
> > > > > > > > > + [AMnMB2ADDRL] = 0x108,
> > > > > > > > > + [AMnMB2ADDRH] = 0x10c,
> > > > > > > > > + [AMnMB3ADDRL] = 0x110,
> > > > > > > > > + [AMnMB3ADDRH] = 0x114,
> > > > > > > > > + [AMnMB4ADDRL] = 0x118,
> > > > > > > > > + [AMnMB4ADDRH] = 0x11c,
> > > > > > > > > + [AMnMB5ADDRL] = 0x120,
> > > > > > > > > + [AMnMB5ADDRH] = 0x124,
> > > > > > > > > + [AMnMB6ADDRL] = 0x128,
> > > > > > > > > + [AMnMB6ADDRH] = 0x12c,
> > > > > > > > > + [AMnMB7ADDRL] = 0x130,
> > > > > > > > > + [AMnMB7ADDRH] = 0x134,
> > > > > > > > > + [AMnMB8ADDRL] = 0x138,
> > > > > > > > > + [AMnMB8ADDRH] = 0x13c,
> > > > > > > > > + [AMnMBVALID] = 0x148,
> > > > > > > > > + [AMnMBS] = 0x14c,
> > > > > > > > > + [AMnAXIATTR] = 0x158,
> > > > > > > > > + [AMnFIFOPNTR] = 0x168,
> > > > > > > > > + [AMnAXISTP] = 0x174,
> > > > > > > > > + [AMnAXISTPACK] = 0x178,
> > > > > > > > > + [ICnEN] = 0x200,
> > > > > > > > > + [ICnMC] = 0x208,
> > > > > > > > > + [ICnMS] = 0x254,
> > > > > > > > > + [ICnDMR] = 0x26c,
> > > > > > > > > +};
> > > > > > > >
> > > > > > > > Do we need enum, can't we use struct instead with all these entries instead?
> > > > > > > >
> > > > > > > What benefit do you foresee when using struct? With the
> > > > > > > current approach being used a minimal diff is generated when
> > > > > > > switched to struct there will be lots of changes.
> > > > > >
> > > > > > The mapping is convinient when you want to iterate throught it.
> > > > > > Here, if you just want to access the offset value from its
> > > > > > name, a structure looks more appropriate.
> > > > >
> > > > > Thanks, as this patch has been reviewed by Laurent a couple of
> > > > > times we will change this to struct If he insists.
> > > >
> > > > How would a struct look like ? I'm not sure what is being proposed.
> > >
> > > It will be
> > >
> > > struct rzg2l_cru_regs {
> > > u16 cru_n_ctrl;
> > > u16 cru_n_ie;
> > > u16 cru_n_ints;
> > > u16 cru_n_rst;
> > > };
> > >
> > > static const struct rzg2l_cru_regs rzg2l_cru_regs = {
> > > .cru_n_ctrl = 0x0,
> > > .cru_n_ie = 0x4,
> > > .cru_n_ints = 0x8,
> > > .cru_n_rst = 0xc,
> > > };
> > >
> > > You can access it using info->regs->cru_n_ctrl instead of
> > > info->regs[CRUnCTRL] This is proposal.
> >
> > Are you OK with the above proposal?
>
> I may be missing something, but I don't see why this would be a significantly better option. It seems
> it would make the callers more complex, and decrease readability.
Basically,
I guess sruct will allow us to avoid (WARN_ON(offset >= RZG2L_CRU_MAX_REG) and
BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG); checks as there is no array, so there is no
buffer overrun condition and also we can drop enum aswell.
So, if using struct decreases readability and makes the code complex,
then current patch is fine.
Cheers,
Biju
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-09 7:25 ` Biju Das
@ 2025-04-09 10:33 ` Tommaso Merciai
2025-04-09 11:08 ` Laurent Pinchart
1 sibling, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-04-09 10:33 UTC (permalink / raw)
To: Biju Das
Cc: laurent.pinchart, Lad, Prabhakar, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Hans Verkuil, Uwe Kleine-König,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Hi Laurent, Biju, Prabhakar,
Thanks all for the inputs!
On Wed, Apr 09, 2025 at 07:25:43AM +0000, Biju Das wrote:
> Hi Laurent,
>
> > -----Original Message-----
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Sent: 09 April 2025 02:29
> > Subject: Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
> >
> > On Mon, Apr 07, 2025 at 04:55:33PM +0000, Lad, Prabhakar wrote:
> > > On Wed, Apr 2, 2025 at 10:39 AM Biju Das wrote:
> > > > On 02 April 2025 10:26, Laurent Pinchart wrote:
> > > > > On Wed, Apr 02, 2025 at 08:25:06AM +0000, Lad, Prabhakar wrote:
> > > > > > On Wed, Apr 2, 2025 at 9:20 AM Biju Das wrote:
> > > > > > > On 02 April 2025 08:35, Lad, Prabhakar wrote:
> > > > > > > > On Wed, Apr 2, 2025 at 7:31 AM Biju Das wrote:
> > > > > > > > > > On 28 March 2025 17:30, Tommaso Merciai wrote:
> > > > > > > > > > From: Lad Prabhakar
> > > > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > > > >
> > > > > > > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs,
> > > > > > > > > > which have a CRU-IP that is mostly identical to RZ/G2L
> > > > > > > > > > but with different register offsets and additional
> > > > > > > > > > registers. Introduce a flexible register mapping
> > > > > > > > > > mechanism to handle these variations.
> > > > > > > > > >
> > > > > > > > > > Define the `rzg2l_cru_info` structure to store register
> > > > > > > > > > mappings and pass it as part of the OF match data.
> > > > > > > > > > Update the read/write functions to check out-of-bound
> > > > > > > > > > accesses and use indexed register offsets from
> > > > > > > > > > `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Lad Prabhakar
> > > > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > > > > Signed-off-by: Tommaso Merciai
> > > > > > > > > > <tommaso.merciai.xr@bp.renesas.com>
> > > > > > > > > > ---
> > > > > > > > > > Changes since v2:
> > > > > > > > > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > > > > > > > accesses as suggested by LPinchart.
> > > > > > > > > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > > > > > > > - Update commit body
> > > > > > > > > >
> > > > > > > > > > Changes since v4:
> > > > > > > > > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > > > > > > > > as __always_inline
> > > > > > > > > >
> > > > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > > > > > > > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58
> > > > > > > > > > ++++++++++++++--
> > > > > > > > > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git
> > > > > > > > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > > > index eed9d2bd08414..abc2a979833aa 100644
> > > > > > > > > > ---
> > > > > > > > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cor
> > > > > > > > > > +++ e.c
> > > > > > > > > > @@ -22,6 +22,7 @@
> > > > > > > > > > #include <media/v4l2-mc.h>
> > > > > > > > > >
> > > > > > > > > > #include "rzg2l-cru.h"
> > > > > > > > > > +#include "rzg2l-cru-regs.h"
> > > > > > > > > >
> > > > > > > > > > static inline struct rzg2l_cru_dev
> > > > > > > > > > *notifier_to_cru(struct v4l2_async_notifier *n) { @@
> > > > > > > > > > -269,6 +270,9 @@ static int rzg2l_cru_probe(struct
> > > > > > > > > > platform_device *pdev)
> > > > > > > > > >
> > > > > > > > > > cru->dev = dev;
> > > > > > > > > > cru->info = of_device_get_match_data(dev);
> > > > > > > > > > + if (!cru->info)
> > > > > > > > > > + return dev_err_probe(dev, -EINVAL,
> > > > > > > > > > + "Failed to get OF
> > > > > > > > > > + match data\n");
> > > > > > > > > >
> > > > > > > > > > irq = platform_get_irq(pdev, 0);
> > > > > > > > > > if (irq < 0)
> > > > > > > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > > > > > > > rzg2l_cru_dma_unregister(cru); }
> > > > > > > > > >
> > > > > > > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > > > > > > + [CRUnCTRL] = 0x0,
> > > > > > > > > > + [CRUnIE] = 0x4,
> > > > > > > > > > + [CRUnINTS] = 0x8,
> > > > > > > > > > + [CRUnRST] = 0xc,
> > > > > > > > > > + [AMnMB1ADDRL] = 0x100,
> > > > > > > > > > + [AMnMB1ADDRH] = 0x104,
> > > > > > > > > > + [AMnMB2ADDRL] = 0x108,
> > > > > > > > > > + [AMnMB2ADDRH] = 0x10c,
> > > > > > > > > > + [AMnMB3ADDRL] = 0x110,
> > > > > > > > > > + [AMnMB3ADDRH] = 0x114,
> > > > > > > > > > + [AMnMB4ADDRL] = 0x118,
> > > > > > > > > > + [AMnMB4ADDRH] = 0x11c,
> > > > > > > > > > + [AMnMB5ADDRL] = 0x120,
> > > > > > > > > > + [AMnMB5ADDRH] = 0x124,
> > > > > > > > > > + [AMnMB6ADDRL] = 0x128,
> > > > > > > > > > + [AMnMB6ADDRH] = 0x12c,
> > > > > > > > > > + [AMnMB7ADDRL] = 0x130,
> > > > > > > > > > + [AMnMB7ADDRH] = 0x134,
> > > > > > > > > > + [AMnMB8ADDRL] = 0x138,
> > > > > > > > > > + [AMnMB8ADDRH] = 0x13c,
> > > > > > > > > > + [AMnMBVALID] = 0x148,
> > > > > > > > > > + [AMnMBS] = 0x14c,
> > > > > > > > > > + [AMnAXIATTR] = 0x158,
> > > > > > > > > > + [AMnFIFOPNTR] = 0x168,
> > > > > > > > > > + [AMnAXISTP] = 0x174,
> > > > > > > > > > + [AMnAXISTPACK] = 0x178,
> > > > > > > > > > + [ICnEN] = 0x200,
> > > > > > > > > > + [ICnMC] = 0x208,
> > > > > > > > > > + [ICnMS] = 0x254,
> > > > > > > > > > + [ICnDMR] = 0x26c,
> > > > > > > > > > +};
> > > > > > > > >
> > > > > > > > > Do we need enum, can't we use struct instead with all these entries instead?
> > > > > > > > >
> > > > > > > > What benefit do you foresee when using struct? With the
> > > > > > > > current approach being used a minimal diff is generated when
> > > > > > > > switched to struct there will be lots of changes.
> > > > > > >
> > > > > > > The mapping is convinient when you want to iterate throught it.
> > > > > > > Here, if you just want to access the offset value from its
> > > > > > > name, a structure looks more appropriate.
> > > > > >
> > > > > > Thanks, as this patch has been reviewed by Laurent a couple of
> > > > > > times we will change this to struct If he insists.
> > > > >
> > > > > How would a struct look like ? I'm not sure what is being proposed.
> > > >
> > > > It will be
> > > >
> > > > struct rzg2l_cru_regs {
> > > > u16 cru_n_ctrl;
> > > > u16 cru_n_ie;
> > > > u16 cru_n_ints;
> > > > u16 cru_n_rst;
> > > > };
> > > >
> > > > static const struct rzg2l_cru_regs rzg2l_cru_regs = {
> > > > .cru_n_ctrl = 0x0,
> > > > .cru_n_ie = 0x4,
> > > > .cru_n_ints = 0x8,
> > > > .cru_n_rst = 0xc,
> > > > };
> > > >
> > > > You can access it using info->regs->cru_n_ctrl instead of
> > > > info->regs[CRUnCTRL] This is proposal.
> > >
> > > Are you OK with the above proposal?
> >
> > I may be missing something, but I don't see why this would be a significantly better option. It seems
> > it would make the callers more complex, and decrease readability.
>
>
> Basically,
> I guess sruct will allow us to avoid (WARN_ON(offset >= RZG2L_CRU_MAX_REG) and
> BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG); checks as there is no array, so there is no
> buffer overrun condition and also we can drop enum aswell.
>
> So, if using struct decreases readability and makes the code complex,
> then current patch is fine.
For v6 I'm going to keep the current registers mapping implementation
adding fixes suggested by Laurent.
Thanks & Regards,
Tommaso
>
> Cheers,
> Biju
>
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support
2025-04-09 7:25 ` Biju Das
2025-04-09 10:33 ` Tommaso Merciai
@ 2025-04-09 11:08 ` Laurent Pinchart
1 sibling, 0 replies; 34+ messages in thread
From: Laurent Pinchart @ 2025-04-09 11:08 UTC (permalink / raw)
To: Biju Das
Cc: Lad, Prabhakar, Tommaso Merciai, Tommaso Merciai,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
Prabhakar Mahadev Lad, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Hans Verkuil, Uwe Kleine-König,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Hi Biju,
On Wed, Apr 09, 2025 at 07:25:43AM +0000, Biju Das wrote:
> On 09 April 2025 02:29, Laurent Pinchart wrote:
> > On Mon, Apr 07, 2025 at 04:55:33PM +0000, Lad, Prabhakar wrote:
> > > On Wed, Apr 2, 2025 at 10:39 AM Biju Das wrote:
> > > > On 02 April 2025 10:26, Laurent Pinchart wrote:
> > > > > On Wed, Apr 02, 2025 at 08:25:06AM +0000, Lad, Prabhakar wrote:
> > > > > > On Wed, Apr 2, 2025 at 9:20 AM Biju Das wrote:
> > > > > > > On 02 April 2025 08:35, Lad, Prabhakar wrote:
> > > > > > > > On Wed, Apr 2, 2025 at 7:31 AM Biju Das wrote:
> > > > > > > > > > On 28 March 2025 17:30, Tommaso Merciai wrote:
> > > > > > > > > > From: Lad Prabhakar
> > > > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > > > >
> > > > > > > > > > Prepare for adding support for RZ/G3E and RZ/V2HP SoCs,
> > > > > > > > > > which have a CRU-IP that is mostly identical to RZ/G2L
> > > > > > > > > > but with different register offsets and additional
> > > > > > > > > > registers. Introduce a flexible register mapping
> > > > > > > > > > mechanism to handle these variations.
> > > > > > > > > >
> > > > > > > > > > Define the `rzg2l_cru_info` structure to store register
> > > > > > > > > > mappings and pass it as part of the OF match data.
> > > > > > > > > > Update the read/write functions to check out-of-bound
> > > > > > > > > > accesses and use indexed register offsets from
> > > > > > > > > > `rzg2l_cru_info`, ensuring compatibility across different SoC variants.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Lad Prabhakar
> > > > > > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > > > > Signed-off-by: Tommaso Merciai
> > > > > > > > > > <tommaso.merciai.xr@bp.renesas.com>
> > > > > > > > > > ---
> > > > > > > > > > Changes since v2:
> > > > > > > > > > - Implemented new rzg2l_cru_write/read() that now are checking out-of-bound
> > > > > > > > > > accesses as suggested by LPinchart.
> > > > > > > > > > - Fixed AMnMBxADDRL() and AMnMBxADDRH() as suggested by LPinchart.
> > > > > > > > > > - Update commit body
> > > > > > > > > >
> > > > > > > > > > Changes since v4:
> > > > > > > > > > - Mark __rzg2l_cru_write_constant/__rzg2l_cru_read_constant
> > > > > > > > > > as __always_inline
> > > > > > > > > >
> > > > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-core.c | 46 ++++++++++++-
> > > > > > > > > > .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 66 ++++++++++---------
> > > > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++
> > > > > > > > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 58
> > > > > > > > > > ++++++++++++++--
> > > > > > > > > > 4 files changed, 139 insertions(+), 35 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git
> > > > > > > > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > > > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > > > index eed9d2bd08414..abc2a979833aa 100644
> > > > > > > > > > ---
> > > > > > > > > > a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > > > > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cor
> > > > > > > > > > +++ e.c
> > > > > > > > > > @@ -22,6 +22,7 @@
> > > > > > > > > > #include <media/v4l2-mc.h>
> > > > > > > > > >
> > > > > > > > > > #include "rzg2l-cru.h"
> > > > > > > > > > +#include "rzg2l-cru-regs.h"
> > > > > > > > > >
> > > > > > > > > > static inline struct rzg2l_cru_dev
> > > > > > > > > > *notifier_to_cru(struct v4l2_async_notifier *n) { @@
> > > > > > > > > > -269,6 +270,9 @@ static int rzg2l_cru_probe(struct
> > > > > > > > > > platform_device *pdev)
> > > > > > > > > >
> > > > > > > > > > cru->dev = dev;
> > > > > > > > > > cru->info = of_device_get_match_data(dev);
> > > > > > > > > > + if (!cru->info)
> > > > > > > > > > + return dev_err_probe(dev, -EINVAL,
> > > > > > > > > > + "Failed to get OF
> > > > > > > > > > + match data\n");
> > > > > > > > > >
> > > > > > > > > > irq = platform_get_irq(pdev, 0);
> > > > > > > > > > if (irq < 0)
> > > > > > > > > > @@ -317,8 +321,48 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> > > > > > > > > > rzg2l_cru_dma_unregister(cru); }
> > > > > > > > > >
> > > > > > > > > > +static const u16 rzg2l_cru_regs[] = {
> > > > > > > > > > + [CRUnCTRL] = 0x0,
> > > > > > > > > > + [CRUnIE] = 0x4,
> > > > > > > > > > + [CRUnINTS] = 0x8,
> > > > > > > > > > + [CRUnRST] = 0xc,
> > > > > > > > > > + [AMnMB1ADDRL] = 0x100,
> > > > > > > > > > + [AMnMB1ADDRH] = 0x104,
> > > > > > > > > > + [AMnMB2ADDRL] = 0x108,
> > > > > > > > > > + [AMnMB2ADDRH] = 0x10c,
> > > > > > > > > > + [AMnMB3ADDRL] = 0x110,
> > > > > > > > > > + [AMnMB3ADDRH] = 0x114,
> > > > > > > > > > + [AMnMB4ADDRL] = 0x118,
> > > > > > > > > > + [AMnMB4ADDRH] = 0x11c,
> > > > > > > > > > + [AMnMB5ADDRL] = 0x120,
> > > > > > > > > > + [AMnMB5ADDRH] = 0x124,
> > > > > > > > > > + [AMnMB6ADDRL] = 0x128,
> > > > > > > > > > + [AMnMB6ADDRH] = 0x12c,
> > > > > > > > > > + [AMnMB7ADDRL] = 0x130,
> > > > > > > > > > + [AMnMB7ADDRH] = 0x134,
> > > > > > > > > > + [AMnMB8ADDRL] = 0x138,
> > > > > > > > > > + [AMnMB8ADDRH] = 0x13c,
> > > > > > > > > > + [AMnMBVALID] = 0x148,
> > > > > > > > > > + [AMnMBS] = 0x14c,
> > > > > > > > > > + [AMnAXIATTR] = 0x158,
> > > > > > > > > > + [AMnFIFOPNTR] = 0x168,
> > > > > > > > > > + [AMnAXISTP] = 0x174,
> > > > > > > > > > + [AMnAXISTPACK] = 0x178,
> > > > > > > > > > + [ICnEN] = 0x200,
> > > > > > > > > > + [ICnMC] = 0x208,
> > > > > > > > > > + [ICnMS] = 0x254,
> > > > > > > > > > + [ICnDMR] = 0x26c,
> > > > > > > > > > +};
> > > > > > > > >
> > > > > > > > > Do we need enum, can't we use struct instead with all these entries instead?
> > > > > > > > >
> > > > > > > > What benefit do you foresee when using struct? With the
> > > > > > > > current approach being used a minimal diff is generated when
> > > > > > > > switched to struct there will be lots of changes.
> > > > > > >
> > > > > > > The mapping is convinient when you want to iterate throught it.
> > > > > > > Here, if you just want to access the offset value from its
> > > > > > > name, a structure looks more appropriate.
> > > > > >
> > > > > > Thanks, as this patch has been reviewed by Laurent a couple of
> > > > > > times we will change this to struct If he insists.
> > > > >
> > > > > How would a struct look like ? I'm not sure what is being proposed.
> > > >
> > > > It will be
> > > >
> > > > struct rzg2l_cru_regs {
> > > > u16 cru_n_ctrl;
> > > > u16 cru_n_ie;
> > > > u16 cru_n_ints;
> > > > u16 cru_n_rst;
> > > > };
> > > >
> > > > static const struct rzg2l_cru_regs rzg2l_cru_regs = {
> > > > .cru_n_ctrl = 0x0,
> > > > .cru_n_ie = 0x4,
> > > > .cru_n_ints = 0x8,
> > > > .cru_n_rst = 0xc,
> > > > };
> > > >
> > > > You can access it using info->regs->cru_n_ctrl instead of
> > > > info->regs[CRUnCTRL] This is proposal.
> > >
> > > Are you OK with the above proposal?
> >
> > I may be missing something, but I don't see why this would be a significantly better option. It seems
> > it would make the callers more complex, and decrease readability.
>
> Basically,
> I guess sruct will allow us to avoid (WARN_ON(offset >= RZG2L_CRU_MAX_REG) and
> BUILD_BUG_ON(offset >= RZG2L_CRU_MAX_REG); checks as there is no array, so there is no
> buffer overrun condition and also we can drop enum aswell.
That's right fpr the BUILD_BUG_ON(), but I don't think that would be an
improvement. The BUILD_BUG_ON() catches at compile time the issues that
are comparable to mistyping a struct field name, so incorrect code
patterns will result in compile-time errors in both cases. The WARN_ON()
is there for cases where the offset needs to be dynamically computed,
and if we wanted to have similar safeguards with a struct, we would also
need a runtime check.
Using a struct would also prevent (I think) implementing additional
safeguards. Not registers exist on all platforms (hence the need for
this mapping mechanism), and it would be nice to catch attempts to
access a register that does not exist. With the existing series, we
could quite easily add a check in the read and write functions to ensure
the regs array entry is not zero (except when the register is CRUnCTRL).
With a struct-based approach, the read/write functions would get the
offset but wouldn't know which register is being accessed, so the same
logic can't be implemented.
Last but not least, I think the current approach is more readable in the
callers: lines are shorter, and register names match the documentation,
including being uppercase.
> So, if using struct decreases readability and makes the code complex,
> then current patch is fine.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 12/17] media: rzg2l-cru: Pass resolution limits via OF data
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (10 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 11/17] media: rzg2l-cru: Add register mapping support Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 13/17] media: rzg2l-cru: Add image_conv offset to " Tommaso Merciai
` (4 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Laurent Pinchart, Tommaso Merciai,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Pass `max_width` and `max_height` as part of the OF data to facilitate the
addition of support for RZ/G3E and RZ/V2H(P) SoCs. These SoCs have a
maximum resolution of 4096x4096 as compared to 2800x4095 on RZ/G2L SoC.
This change prepares the driver for easier integration of these SoCs by
defining the resolution limits in the `rzg2l_cru_info` structure.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
- Collected tag.
.../media/platform/renesas/rzg2l-cru/rzg2l-core.c | 2 ++
.../media/platform/renesas/rzg2l-cru/rzg2l-cru.h | 4 ++--
drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c | 13 +++++++++----
.../media/platform/renesas/rzg2l-cru/rzg2l-video.c | 5 +++--
4 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index abc2a979833aa..19f93b7fe6fb9 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -355,6 +355,8 @@ static const u16 rzg2l_cru_regs[] = {
};
static const struct rzg2l_cru_info rzgl2_cru_info = {
+ .max_width = 2800,
+ .max_height = 4095,
.regs = rzg2l_cru_regs,
};
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index 00c3f7458e20a..6a621073948aa 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -27,9 +27,7 @@
#define RZG2L_CRU_CSI2_VCHANNEL 4
#define RZG2L_CRU_MIN_INPUT_WIDTH 320
-#define RZG2L_CRU_MAX_INPUT_WIDTH 2800
#define RZG2L_CRU_MIN_INPUT_HEIGHT 240
-#define RZG2L_CRU_MAX_INPUT_HEIGHT 4095
enum rzg2l_csi2_pads {
RZG2L_CRU_IP_SINK = 0,
@@ -81,6 +79,8 @@ struct rzg2l_cru_ip_format {
};
struct rzg2l_cru_info {
+ unsigned int max_width;
+ unsigned int max_height;
const u16 *regs;
};
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
index 76a2b451f1daf..7836c7cd53dc3 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
@@ -148,6 +148,8 @@ static int rzg2l_cru_ip_set_format(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *fmt)
{
+ struct rzg2l_cru_dev *cru = v4l2_get_subdevdata(sd);
+ const struct rzg2l_cru_info *info = cru->info;
struct v4l2_mbus_framefmt *src_format;
struct v4l2_mbus_framefmt *sink_format;
@@ -170,9 +172,9 @@ static int rzg2l_cru_ip_set_format(struct v4l2_subdev *sd,
sink_format->ycbcr_enc = fmt->format.ycbcr_enc;
sink_format->quantization = fmt->format.quantization;
sink_format->width = clamp_t(u32, fmt->format.width,
- RZG2L_CRU_MIN_INPUT_WIDTH, RZG2L_CRU_MAX_INPUT_WIDTH);
+ RZG2L_CRU_MIN_INPUT_WIDTH, info->max_width);
sink_format->height = clamp_t(u32, fmt->format.height,
- RZG2L_CRU_MIN_INPUT_HEIGHT, RZG2L_CRU_MAX_INPUT_HEIGHT);
+ RZG2L_CRU_MIN_INPUT_HEIGHT, info->max_height);
fmt->format = *sink_format;
@@ -197,6 +199,9 @@ static int rzg2l_cru_ip_enum_frame_size(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_frame_size_enum *fse)
{
+ struct rzg2l_cru_dev *cru = v4l2_get_subdevdata(sd);
+ const struct rzg2l_cru_info *info = cru->info;
+
if (fse->index != 0)
return -EINVAL;
@@ -205,8 +210,8 @@ static int rzg2l_cru_ip_enum_frame_size(struct v4l2_subdev *sd,
fse->min_width = RZG2L_CRU_MIN_INPUT_WIDTH;
fse->min_height = RZG2L_CRU_MIN_INPUT_HEIGHT;
- fse->max_width = RZG2L_CRU_MAX_INPUT_WIDTH;
- fse->max_height = RZG2L_CRU_MAX_INPUT_HEIGHT;
+ fse->max_width = info->max_width;
+ fse->max_height = info->max_height;
return 0;
}
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index c82db80c33552..395c4d3d0f0fa 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -736,6 +736,7 @@ int rzg2l_cru_dma_register(struct rzg2l_cru_dev *cru)
static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
struct v4l2_pix_format *pix)
{
+ const struct rzg2l_cru_info *info = cru->info;
const struct rzg2l_cru_ip_format *fmt;
fmt = rzg2l_cru_ip_format_to_fmt(pix->pixelformat);
@@ -758,8 +759,8 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
}
/* Limit to CRU capabilities */
- v4l_bound_align_image(&pix->width, 320, RZG2L_CRU_MAX_INPUT_WIDTH, 1,
- &pix->height, 240, RZG2L_CRU_MAX_INPUT_HEIGHT, 2, 0);
+ v4l_bound_align_image(&pix->width, 320, info->max_width, 1,
+ &pix->height, 240, info->max_height, 2, 0);
pix->bytesperline = pix->width * fmt->bpp;
pix->sizeimage = pix->bytesperline * pix->height;
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 13/17] media: rzg2l-cru: Add image_conv offset to OF data
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (11 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 12/17] media: rzg2l-cru: Pass resolution limits via OF data Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-04-02 0:47 ` Laurent Pinchart
2025-03-28 17:29 ` [PATCH v5 14/17] media: rzg2l-cru: Add IRQ handler " Tommaso Merciai
` (3 subsequent siblings)
16 siblings, 1 reply; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 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, Sakari Ailus, devicetree, linux-kernel
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Add `image_conv` field to the `rzg2l_cru_info` structure to store the
register offset for image conversion control. RZ/G2L uses `ICnMC`, while
RZ/G3E and RZ/V2H(P) use `ICnIPMC_C0`.
Update `rzg2l_cru_initialize_image_conv()` and `rzg2l_cru_csi2_setup()`
to use this `image_conv` offset from the OF data, facilitating future
support for RZ/G3E and RZ/V2H(P) SoCs.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
.../media/platform/renesas/rzg2l-cru/rzg2l-core.c | 1 +
.../media/platform/renesas/rzg2l-cru/rzg2l-cru.h | 1 +
.../media/platform/renesas/rzg2l-cru/rzg2l-video.c | 14 ++++++++------
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 19f93b7fe6fb9..7e94ae8039677 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -357,6 +357,7 @@ static const u16 rzg2l_cru_regs[] = {
static const struct rzg2l_cru_info rzgl2_cru_info = {
.max_width = 2800,
.max_height = 4095,
+ .image_conv = ICnMC,
.regs = rzg2l_cru_regs,
};
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index 6a621073948aa..ca156772b949b 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -81,6 +81,7 @@ struct rzg2l_cru_ip_format {
struct rzg2l_cru_info {
unsigned int max_width;
unsigned int max_height;
+ u16 image_conv;
const u16 *regs;
};
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 395c4d3d0f0fa..e13f633a687b2 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -246,20 +246,22 @@ static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
const struct rzg2l_cru_ip_format *ip_fmt,
u8 csi_vc)
{
+ const struct rzg2l_cru_info *info = cru->info;
u32 icnmc = ICnMC_INF(ip_fmt->datatype);
- icnmc |= (rzg2l_cru_read(cru, ICnMC) & ~ICnMC_INF_MASK);
+ icnmc |= (rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK);
/* Set virtual channel CSI2 */
icnmc |= ICnMC_VCSEL(csi_vc);
- rzg2l_cru_write(cru, ICnMC, icnmc);
+ rzg2l_cru_write(cru, info->image_conv, icnmc);
}
static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
struct v4l2_mbus_framefmt *ip_sd_fmt,
u8 csi_vc)
{
+ const struct rzg2l_cru_info *info = cru->info;
const struct rzg2l_cru_ip_format *cru_video_fmt;
const struct rzg2l_cru_ip_format *cru_ip_fmt;
@@ -276,11 +278,11 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
/* If input and output use same colorspace, do bypass mode */
if (cru_ip_fmt->yuv == cru_video_fmt->yuv)
- rzg2l_cru_write(cru, ICnMC,
- rzg2l_cru_read(cru, ICnMC) | ICnMC_CSCTHR);
+ rzg2l_cru_write(cru, info->image_conv,
+ rzg2l_cru_read(cru, info->image_conv) | ICnMC_CSCTHR);
else
- rzg2l_cru_write(cru, ICnMC,
- rzg2l_cru_read(cru, ICnMC) & (~ICnMC_CSCTHR));
+ rzg2l_cru_write(cru, info->image_conv,
+ rzg2l_cru_read(cru, info->image_conv) & (~ICnMC_CSCTHR));
/* Set output data format */
rzg2l_cru_write(cru, ICnDMR, cru_video_fmt->icndmr);
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v5 13/17] media: rzg2l-cru: Add image_conv offset to OF data
2025-03-28 17:29 ` [PATCH v5 13/17] media: rzg2l-cru: Add image_conv offset to " Tommaso Merciai
@ 2025-04-02 0:47 ` Laurent Pinchart
0 siblings, 0 replies; 34+ messages in thread
From: Laurent Pinchart @ 2025-04-02 0:47 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.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, Sakari Ailus,
devicetree, linux-kernel
Hi Tommaso,
Thank you for the patch.
On Fri, Mar 28, 2025 at 06:29:49PM +0100, Tommaso Merciai wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add `image_conv` field to the `rzg2l_cru_info` structure to store the
> register offset for image conversion control. RZ/G2L uses `ICnMC`, while
> RZ/G3E and RZ/V2H(P) use `ICnIPMC_C0`.
>
> Update `rzg2l_cru_initialize_image_conv()` and `rzg2l_cru_csi2_setup()`
> to use this `image_conv` offset from the OF data, facilitating future
> support for RZ/G3E and RZ/V2H(P) SoCs.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> .../media/platform/renesas/rzg2l-cru/rzg2l-core.c | 1 +
> .../media/platform/renesas/rzg2l-cru/rzg2l-cru.h | 1 +
> .../media/platform/renesas/rzg2l-cru/rzg2l-video.c | 14 ++++++++------
> 3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> index 19f93b7fe6fb9..7e94ae8039677 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> @@ -357,6 +357,7 @@ static const u16 rzg2l_cru_regs[] = {
> static const struct rzg2l_cru_info rzgl2_cru_info = {
> .max_width = 2800,
> .max_height = 4095,
> + .image_conv = ICnMC,
> .regs = rzg2l_cru_regs,
> };
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> index 6a621073948aa..ca156772b949b 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> @@ -81,6 +81,7 @@ struct rzg2l_cru_ip_format {
> struct rzg2l_cru_info {
> unsigned int max_width;
> unsigned int max_height;
> + u16 image_conv;
> const u16 *regs;
> };
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 395c4d3d0f0fa..e13f633a687b2 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -246,20 +246,22 @@ static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
> const struct rzg2l_cru_ip_format *ip_fmt,
> u8 csi_vc)
> {
> + const struct rzg2l_cru_info *info = cru->info;
> u32 icnmc = ICnMC_INF(ip_fmt->datatype);
>
> - icnmc |= (rzg2l_cru_read(cru, ICnMC) & ~ICnMC_INF_MASK);
> + icnmc |= (rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK);
While at it you can drop the outer parentheses.
>
> /* Set virtual channel CSI2 */
> icnmc |= ICnMC_VCSEL(csi_vc);
>
> - rzg2l_cru_write(cru, ICnMC, icnmc);
> + rzg2l_cru_write(cru, info->image_conv, icnmc);
> }
>
> static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
> struct v4l2_mbus_framefmt *ip_sd_fmt,
> u8 csi_vc)
> {
> + const struct rzg2l_cru_info *info = cru->info;
> const struct rzg2l_cru_ip_format *cru_video_fmt;
> const struct rzg2l_cru_ip_format *cru_ip_fmt;
>
> @@ -276,11 +278,11 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
>
> /* If input and output use same colorspace, do bypass mode */
> if (cru_ip_fmt->yuv == cru_video_fmt->yuv)
> - rzg2l_cru_write(cru, ICnMC,
> - rzg2l_cru_read(cru, ICnMC) | ICnMC_CSCTHR);
> + rzg2l_cru_write(cru, info->image_conv,
> + rzg2l_cru_read(cru, info->image_conv) | ICnMC_CSCTHR);
> else
> - rzg2l_cru_write(cru, ICnMC,
> - rzg2l_cru_read(cru, ICnMC) & (~ICnMC_CSCTHR));
> + rzg2l_cru_write(cru, info->image_conv,
> + rzg2l_cru_read(cru, info->image_conv) & (~ICnMC_CSCTHR));
And here you can drop the parentheses around ~ICnMC_CSCTHR.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> /* Set output data format */
> rzg2l_cru_write(cru, ICnDMR, cru_video_fmt->icndmr);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 14/17] media: rzg2l-cru: Add IRQ handler to OF data
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (12 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 13/17] media: rzg2l-cru: Add image_conv offset to " Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 15/17] media: rzg2l-cru: Add function pointer to check if FIFO is empty Tommaso Merciai
` (2 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Laurent Pinchart, Tommaso Merciai,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Add `irq_handler` to the `rzg2l_cru_info` structure and pass it as part of
the OF data. This prepares for supporting RZ/G3E and RZ/V2H(P) SoCs, which
require a different IRQ handler. Update the IRQ request code to use the
handler from the OF data.
Add `enable_interrupts` and `disable_interrupts` function pointers to the
`rzg2l_cru_info` structure and pass them as part of the OF data. This
prepares for supporting RZ/G3E and RZ/V2H(P) SoCs, which require different
interrupt configurations.
Implement `rzg2l_cru_enable_interrupts()` and
`rzg2l_cru_disable_interrupts()` functions and update the code to use them
instead of directly writing to interrupt registers.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
- Squashed patch 15 and 14
- Collected tag
.../platform/renesas/rzg2l-cru/rzg2l-core.c | 5 ++++-
.../platform/renesas/rzg2l-cru/rzg2l-cru.h | 8 ++++++++
.../platform/renesas/rzg2l-cru/rzg2l-video.c | 19 ++++++++++++++-----
3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 7e94ae8039677..302f792cb4159 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -278,7 +278,7 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
if (irq < 0)
return irq;
- ret = devm_request_irq(dev, irq, rzg2l_cru_irq, 0,
+ ret = devm_request_irq(dev, irq, cru->info->irq_handler, 0,
KBUILD_MODNAME, cru);
if (ret)
return dev_err_probe(dev, ret, "failed to request irq\n");
@@ -359,6 +359,9 @@ static const struct rzg2l_cru_info rzgl2_cru_info = {
.max_height = 4095,
.image_conv = ICnMC,
.regs = rzg2l_cru_regs,
+ .irq_handler = rzg2l_cru_irq,
+ .enable_interrupts = rzg2l_cru_enable_interrupts,
+ .disable_interrupts = rzg2l_cru_disable_interrupts,
};
static const struct of_device_id rzg2l_cru_of_id_table[] = {
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index ca156772b949b..3f694044d8cd1 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -34,6 +34,8 @@ enum rzg2l_csi2_pads {
RZG2L_CRU_IP_SOURCE,
};
+struct rzg2l_cru_dev;
+
/**
* enum rzg2l_cru_dma_state - DMA states
* @RZG2L_CRU_DMA_STOPPED: No operation in progress
@@ -83,6 +85,9 @@ struct rzg2l_cru_info {
unsigned int max_height;
u16 image_conv;
const u16 *regs;
+ irqreturn_t (*irq_handler)(int irq, void *data);
+ void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
+ void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
};
/**
@@ -177,4 +182,7 @@ const struct rzg2l_cru_ip_format *rzg2l_cru_ip_code_to_fmt(unsigned int code);
const struct rzg2l_cru_ip_format *rzg2l_cru_ip_format_to_fmt(u32 format);
const struct rzg2l_cru_ip_format *rzg2l_cru_ip_index_to_fmt(u32 index);
+void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
+void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
+
#endif
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index e13f633a687b2..3bfb30a61d9b4 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -300,8 +300,7 @@ void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
spin_lock_irqsave(&cru->qlock, flags);
/* Disable and clear the interrupt */
- rzg2l_cru_write(cru, CRUnIE, 0);
- rzg2l_cru_write(cru, CRUnINTS, 0x001F0F0F);
+ cru->info->disable_interrupts(cru);
/* Stop the operation of image conversion */
rzg2l_cru_write(cru, ICnEN, 0);
@@ -393,6 +392,17 @@ static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)
return fd.entry[0].bus.csi2.vc;
}
+void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
+{
+ rzg2l_cru_write(cru, CRUnIE, CRUnIE_EFE);
+}
+
+void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru)
+{
+ rzg2l_cru_write(cru, CRUnIE, 0);
+ rzg2l_cru_write(cru, CRUnINTS, 0x001f000f);
+}
+
int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
{
struct v4l2_mbus_framefmt *fmt = rzg2l_cru_ip_get_src_fmt(cru);
@@ -414,8 +424,7 @@ int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
rzg2l_cru_write(cru, CRUnRST, CRUnRST_VRESETN);
/* Disable and clear the interrupt before using */
- rzg2l_cru_write(cru, CRUnIE, 0);
- rzg2l_cru_write(cru, CRUnINTS, 0x001f000f);
+ cru->info->disable_interrupts(cru);
/* Initialize the AXI master */
rzg2l_cru_initialize_axi(cru);
@@ -428,7 +437,7 @@ int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
}
/* Enable interrupt */
- rzg2l_cru_write(cru, CRUnIE, CRUnIE_EFE);
+ cru->info->enable_interrupts(cru);
/* Enable image processing reception */
rzg2l_cru_write(cru, ICnEN, ICnEN_ICEN);
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 15/17] media: rzg2l-cru: Add function pointer to check if FIFO is empty
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (13 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 14/17] media: rzg2l-cru: Add IRQ handler " Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 16/17] media: rzg2l-cru: Add function pointer to configure CSI Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 17/17] media: rzg2l-cru: Add support for RZ/G3E SoC Tommaso Merciai
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Laurent Pinchart, Tommaso Merciai,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Add a `fifo_empty` function pointer to the `rzg2l_cru_info` structure and
pass it as part of the OF data. On RZ/G3E and RZ/V2H(P) SoCs, checking if
the FIFO is empty requires a different register configuration.
Implement `rzg2l_fifo_empty()` and update the code to use it from the
function pointer.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
- Fixed return of rzg2l_fifo_empty() as suggested by LPinchart
- Collected tag
.../platform/renesas/rzg2l-cru/rzg2l-core.c | 1 +
.../platform/renesas/rzg2l-cru/rzg2l-cru.h | 3 +++
.../platform/renesas/rzg2l-cru/rzg2l-video.c | 23 +++++++++++++------
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 302f792cb4159..e4fb3e12d6bfc 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -362,6 +362,7 @@ static const struct rzg2l_cru_info rzgl2_cru_info = {
.irq_handler = rzg2l_cru_irq,
.enable_interrupts = rzg2l_cru_enable_interrupts,
.disable_interrupts = rzg2l_cru_disable_interrupts,
+ .fifo_empty = rzg2l_fifo_empty,
};
static const struct of_device_id rzg2l_cru_of_id_table[] = {
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index 3f694044d8cd1..2e17bfef43ce6 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -88,6 +88,7 @@ struct rzg2l_cru_info {
irqreturn_t (*irq_handler)(int irq, void *data);
void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
+ bool (*fifo_empty)(struct rzg2l_cru_dev *cru);
};
/**
@@ -185,4 +186,6 @@ const struct rzg2l_cru_ip_format *rzg2l_cru_ip_index_to_fmt(u32 index);
void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
+bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru);
+
#endif
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 3bfb30a61d9b4..31848dc463381 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -290,9 +290,23 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
return 0;
}
-void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
+bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru)
{
u32 amnfifopntr, amnfifopntr_w, amnfifopntr_r_y;
+
+ amnfifopntr = rzg2l_cru_read(cru, AMnFIFOPNTR);
+
+ amnfifopntr_w = amnfifopntr & AMnFIFOPNTR_FIFOWPNTR;
+ amnfifopntr_r_y =
+ (amnfifopntr & AMnFIFOPNTR_FIFORPNTR_Y) >> 16;
+ if (amnfifopntr_w == amnfifopntr_r_y)
+ return true;
+
+ return amnfifopntr_w == amnfifopntr_r_y;
+}
+
+void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
+{
unsigned int retries = 0;
unsigned long flags;
u32 icnms;
@@ -320,12 +334,7 @@ void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
/* Wait until the FIFO becomes empty */
for (retries = 5; retries > 0; retries--) {
- amnfifopntr = rzg2l_cru_read(cru, AMnFIFOPNTR);
-
- amnfifopntr_w = amnfifopntr & AMnFIFOPNTR_FIFOWPNTR;
- amnfifopntr_r_y =
- (amnfifopntr & AMnFIFOPNTR_FIFORPNTR_Y) >> 16;
- if (amnfifopntr_w == amnfifopntr_r_y)
+ if (cru->info->fifo_empty(cru))
break;
usleep_range(10, 20);
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 16/17] media: rzg2l-cru: Add function pointer to configure CSI
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (14 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 15/17] media: rzg2l-cru: Add function pointer to check if FIFO is empty Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-03-28 17:29 ` [PATCH v5 17/17] media: rzg2l-cru: Add support for RZ/G3E SoC Tommaso Merciai
16 siblings, 0 replies; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 UTC (permalink / raw)
To: tomm.merciai
Cc: linux-renesas-soc, linux-media, biju.das.jz,
prabhakar.mahadev-lad.rj, Laurent Pinchart, Tommaso Merciai,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, Magnus Damm, Hans Verkuil,
Uwe Kleine-König, devicetree, linux-kernel
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Add a `csi_setup` function pointer to the `rzg2l_cru_info` structure and
pass it as part of the OF data. On RZ/G3E and RZ/V2H(P) SoCs, additional
register configurations are required compared to the RZ/G2L SoC.
Modify `rzg2l_cru_csi2_setup()` to be referenced through this function
pointer and update the code to use it accordingly.
This change is in preparation for adding support for RZ/G3E and RZ/V2H(P)
SoCs.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
- Collected tag
drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c | 1 +
drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h | 6 ++++++
drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 8 ++++----
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index e4fb3e12d6bfc..3ae0cd83af164 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -363,6 +363,7 @@ static const struct rzg2l_cru_info rzgl2_cru_info = {
.enable_interrupts = rzg2l_cru_enable_interrupts,
.disable_interrupts = rzg2l_cru_disable_interrupts,
.fifo_empty = rzg2l_fifo_empty,
+ .csi_setup = rzg2l_cru_csi2_setup,
};
static const struct of_device_id rzg2l_cru_of_id_table[] = {
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index 2e17bfef43ce6..ccaba5220f1c8 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -89,6 +89,9 @@ struct rzg2l_cru_info {
void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
bool (*fifo_empty)(struct rzg2l_cru_dev *cru);
+ void (*csi_setup)(struct rzg2l_cru_dev *cru,
+ const struct rzg2l_cru_ip_format *ip_fmt,
+ u8 csi_vc);
};
/**
@@ -187,5 +190,8 @@ void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru);
+void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
+ const struct rzg2l_cru_ip_format *ip_fmt,
+ u8 csi_vc);
#endif
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 31848dc463381..748a0855b3245 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -242,9 +242,9 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
rzg2l_cru_write(cru, AMnAXIATTR, amnaxiattr);
}
-static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
- const struct rzg2l_cru_ip_format *ip_fmt,
- u8 csi_vc)
+void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
+ const struct rzg2l_cru_ip_format *ip_fmt,
+ u8 csi_vc)
{
const struct rzg2l_cru_info *info = cru->info;
u32 icnmc = ICnMC_INF(ip_fmt->datatype);
@@ -266,7 +266,7 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
const struct rzg2l_cru_ip_format *cru_ip_fmt;
cru_ip_fmt = rzg2l_cru_ip_code_to_fmt(ip_sd_fmt->code);
- rzg2l_cru_csi2_setup(cru, cru_ip_fmt, csi_vc);
+ info->csi_setup(cru, cru_ip_fmt, csi_vc);
/* Output format */
cru_video_fmt = rzg2l_cru_ip_format_to_fmt(cru->format.pixelformat);
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v5 17/17] media: rzg2l-cru: Add support for RZ/G3E SoC
2025-03-28 17:29 [PATCH v5 00/17] media: rzg2l-cru: Add support for RZ/G3E (CSI2, CRU) Tommaso Merciai
` (15 preceding siblings ...)
2025-03-28 17:29 ` [PATCH v5 16/17] media: rzg2l-cru: Add function pointer to configure CSI Tommaso Merciai
@ 2025-03-28 17:29 ` Tommaso Merciai
2025-04-02 1:14 ` Laurent Pinchart
16 siblings, 1 reply; 34+ messages in thread
From: Tommaso Merciai @ 2025-03-28 17:29 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
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
The CRU block on the Renesas RZ/G3E SoC is similar to the one found on
the Renesas RZ/G2L SoC, with the following differences:
- Additional registers rzg3e_cru_regs.
- A different irq handler rzg3e_cru_irq.
- A different rzg3e_cru_csi2_setup.
- A different max input width.
- Additional stride register.
Introduce rzg3e_cru_info struct to handle differences between RZ/G2L
and RZ/G3E and related RZ/G3E functions:
- rzg3e_cru_enable_interrupts()
- rzg3e_cru_enable_interrupts()
- rz3e_fifo_empty()
- rzg3e_cru_csi2_setup()
- rzg3e_cru_get_current_slot()
Add then support for the RZ/G3E SoC CRU block with the new compatible
string "renesas,r9a09g047-cru".
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
Changes since v2:
- Use dma_addr_t with buf_addr directly instead of splitting that into
cru->mem_banks (high and low address) as suggested by LPinchart.
- Moved and improved stride adjustment into rzg2l_cru_format_align()
as suggested by LPinchart.
- Use csi_vc into rzg3e_cru_csi2_setup() instead of cru->svc_channel as
suggested by LPinchart
- Added has_stride field to handle soc differences as suggested by LPinchart.
Changes since v3:
- Fixed kernel test robot warnings from rzg3e_cru_get_current_slot() and
rzg3e_cru_irq()
.../platform/renesas/rzg2l-cru/rzg2l-core.c | 62 +++++++
.../renesas/rzg2l-cru/rzg2l-cru-regs.h | 25 +++
.../platform/renesas/rzg2l-cru/rzg2l-cru.h | 13 ++
.../platform/renesas/rzg2l-cru/rzg2l-video.c | 171 +++++++++++++++++-
4 files changed, 270 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 3ae0cd83af164..1356be14eda8a 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -290,6 +290,12 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
return ret;
cru->num_buf = RZG2L_CRU_HW_BUFFER_DEFAULT;
+ cru->buf_addr = devm_kmalloc_array(dev, cru->num_buf,
+ sizeof(dma_addr_t), GFP_KERNEL);
+ if (!cru->buf_addr)
+ return dev_err_probe(dev, -ENOMEM,
+ "Failed to init buf addr\n");
+
pm_suspend_ignore_children(dev, true);
ret = devm_pm_runtime_enable(dev);
if (ret)
@@ -321,6 +327,58 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
rzg2l_cru_dma_unregister(cru);
}
+static const u16 rzg3e_cru_regs[] = {
+ [CRUnCTRL] = 0x0,
+ [CRUnIE] = 0x4,
+ [CRUnIE2] = 0x8,
+ [CRUnINTS] = 0xc,
+ [CRUnINTS2] = 0x10,
+ [CRUnRST] = 0x18,
+ [AMnMB1ADDRL] = 0x40,
+ [AMnMB1ADDRH] = 0x44,
+ [AMnMB2ADDRL] = 0x48,
+ [AMnMB2ADDRH] = 0x4c,
+ [AMnMB3ADDRL] = 0x50,
+ [AMnMB3ADDRH] = 0x54,
+ [AMnMB4ADDRL] = 0x58,
+ [AMnMB4ADDRH] = 0x5c,
+ [AMnMB5ADDRL] = 0x60,
+ [AMnMB5ADDRH] = 0x64,
+ [AMnMB6ADDRL] = 0x68,
+ [AMnMB6ADDRH] = 0x6c,
+ [AMnMB7ADDRL] = 0x70,
+ [AMnMB7ADDRH] = 0x74,
+ [AMnMB8ADDRL] = 0x78,
+ [AMnMB8ADDRH] = 0x7c,
+ [AMnMBVALID] = 0x88,
+ [AMnMADRSL] = 0x8c,
+ [AMnMADRSH] = 0x90,
+ [AMnAXIATTR] = 0xec,
+ [AMnFIFOPNTR] = 0xf8,
+ [AMnAXISTP] = 0x110,
+ [AMnAXISTPACK] = 0x114,
+ [AMnIS] = 0x128,
+ [ICnEN] = 0x1f0,
+ [ICnSVCNUM] = 0x1f8,
+ [ICnSVC] = 0x1fc,
+ [ICnIPMC_C0] = 0x200,
+ [ICnMS] = 0x2d8,
+ [ICnDMR] = 0x304,
+};
+
+static const struct rzg2l_cru_info rzg3e_cru_info = {
+ .max_width = 4095,
+ .max_height = 4095,
+ .image_conv = ICnIPMC_C0,
+ .has_stride = true,
+ .regs = rzg3e_cru_regs,
+ .irq_handler = rzg3e_cru_irq,
+ .enable_interrupts = rzg3e_cru_enable_interrupts,
+ .disable_interrupts = rzg3e_cru_disable_interrupts,
+ .fifo_empty = rz3e_fifo_empty,
+ .csi_setup = rzg3e_cru_csi2_setup,
+};
+
static const u16 rzg2l_cru_regs[] = {
[CRUnCTRL] = 0x0,
[CRUnIE] = 0x4,
@@ -367,6 +425,10 @@ static const struct rzg2l_cru_info rzgl2_cru_info = {
};
static const struct of_device_id rzg2l_cru_of_id_table[] = {
+ {
+ .compatible = "renesas,r9a09g047-cru",
+ .data = &rzg3e_cru_info,
+ },
{
.compatible = "renesas,rzg2l-cru",
.data = &rzgl2_cru_info,
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
index 86c3202862465..52324b076674b 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
@@ -14,8 +14,13 @@
#define CRUnIE_EFE BIT(17)
+#define CRUnIE2_FSxE(x) BIT(((x) * 3))
+#define CRUnIE2_FExE(x) BIT(((x) * 3) + 1)
+
#define CRUnINTS_SFS BIT(16)
+#define CRUnINTS2_FSxS(x) BIT(((x) * 3))
+
#define CRUnRST_VRESETN BIT(0)
/* Memory Bank Base Address (Lower) Register for CRU Image Data */
@@ -32,7 +37,14 @@
#define AMnAXIATTR_AXILEN (0xf)
#define AMnFIFOPNTR_FIFOWPNTR GENMASK(7, 0)
+#define AMnFIFOPNTR_FIFOWPNTR_B0 AMnFIFOPNTR_FIFOWPNTR
+#define AMnFIFOPNTR_FIFOWPNTR_B1 GENMASK(15, 8)
#define AMnFIFOPNTR_FIFORPNTR_Y GENMASK(23, 16)
+#define AMnFIFOPNTR_FIFORPNTR_B0 AMnFIFOPNTR_FIFORPNTR_Y
+#define AMnFIFOPNTR_FIFORPNTR_B1 GENMASK(31, 24)
+
+#define AMnIS_IS_MASK GENMASK(14, 7)
+#define AMnIS_IS(x) ((x) << 7)
#define AMnAXISTP_AXI_STOP BIT(0)
@@ -40,6 +52,11 @@
#define ICnEN_ICEN BIT(0)
+#define ICnSVC_SVC0(x) (x)
+#define ICnSVC_SVC1(x) ((x) << 4)
+#define ICnSVC_SVC2(x) ((x) << 8)
+#define ICnSVC_SVC3(x) ((x) << 12)
+
#define ICnMC_CSCTHR BIT(5)
#define ICnMC_INF(x) ((x) << 16)
#define ICnMC_VCSEL(x) ((x) << 22)
@@ -52,7 +69,9 @@
enum rzg2l_cru_common_regs {
CRUnCTRL, /* CRU Control */
CRUnIE, /* CRU Interrupt Enable */
+ CRUnIE2, /* CRU Interrupt Enable(2) */
CRUnINTS, /* CRU Interrupt Status */
+ CRUnINTS2, /* CRU Interrupt Status(2) */
CRUnRST, /* CRU Reset */
AMnMB1ADDRL, /* Bank 1 Address (Lower) for CRU Image Data */
AMnMB1ADDRH, /* Bank 1 Address (Higher) for CRU Image Data */
@@ -72,12 +91,18 @@ enum rzg2l_cru_common_regs {
AMnMB8ADDRH, /* Bank 8 Address (Higher) for CRU Image Data */
AMnMBVALID, /* Memory Bank Enable for CRU Image Data */
AMnMBS, /* Memory Bank Status for CRU Image Data */
+ AMnMADRSL, /* VD Memory Address Lower Status Register */
+ AMnMADRSH, /* VD Memory Address Higher Status Register */
AMnAXIATTR, /* AXI Master Transfer Setting Register for CRU Image Data */
AMnFIFOPNTR, /* AXI Master FIFO Pointer for CRU Image Data */
AMnAXISTP, /* AXI Master Transfer Stop for CRU Image Data */
AMnAXISTPACK, /* AXI Master Transfer Stop Status for CRU Image Data */
+ AMnIS, /* Image Stride Setting Register */
ICnEN, /* CRU Image Processing Enable */
+ ICnSVCNUM, /* CRU SVC Number Register */
+ ICnSVC, /* CRU VC Select Register */
ICnMC, /* CRU Image Processing Main Control */
+ ICnIPMC_C0, /* CRU Image Converter Main Control 0 */
ICnMS, /* CRU Module Status */
ICnDMR, /* CRU Data Output Mode */
RZG2L_CRU_MAX_REG,
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index ccaba5220f1c8..d68d833406865 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -85,6 +85,7 @@ struct rzg2l_cru_info {
unsigned int max_height;
u16 image_conv;
const u16 *regs;
+ bool has_stride;
irqreturn_t (*irq_handler)(int irq, void *data);
void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
@@ -108,6 +109,8 @@ struct rzg2l_cru_info {
* @vdev: V4L2 video device associated with CRU
* @v4l2_dev: V4L2 device
* @num_buf: Holds the current number of buffers enabled
+ * @svc_channel: SVC0/1/2/3 to use for RZ/G3E
+ * @buf_addr: Memory addresses where current video data is written.
* @notifier: V4L2 asynchronous subdevs notifier
*
* @ip: Image processing subdev info
@@ -144,6 +147,9 @@ struct rzg2l_cru_dev {
struct v4l2_device v4l2_dev;
u8 num_buf;
+ u8 svc_channel;
+ dma_addr_t *buf_addr;
+
struct v4l2_async_notifier notifier;
struct rzg2l_cru_ip ip;
@@ -175,6 +181,7 @@ void rzg2l_cru_dma_unregister(struct rzg2l_cru_dev *cru);
int rzg2l_cru_video_register(struct rzg2l_cru_dev *cru);
void rzg2l_cru_video_unregister(struct rzg2l_cru_dev *cru);
irqreturn_t rzg2l_cru_irq(int irq, void *data);
+irqreturn_t rzg3e_cru_irq(int irq, void *data);
const struct v4l2_format_info *rzg2l_cru_format_from_pixel(u32 format);
@@ -188,10 +195,16 @@ const struct rzg2l_cru_ip_format *rzg2l_cru_ip_index_to_fmt(u32 index);
void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
+void rzg3e_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
+void rzg3e_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru);
+bool rz3e_fifo_empty(struct rzg2l_cru_dev *cru);
void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
const struct rzg2l_cru_ip_format *ip_fmt,
u8 csi_vc);
+void rzg3e_cru_csi2_setup(struct rzg2l_cru_dev *cru,
+ const struct rzg2l_cru_ip_format *ip_fmt,
+ u8 csi_vc);
#endif
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 748a0855b3245..da6d13e80a45a 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -31,6 +31,9 @@
#define RZG2L_CRU_DEFAULT_FIELD V4L2_FIELD_NONE
#define RZG2L_CRU_DEFAULT_COLORSPACE V4L2_COLORSPACE_SRGB
+#define RZG2L_CRU_STRIDE_MAX 32640
+#define RZG2L_CRU_STRIDE_ALIGN 128
+
struct rzg2l_cru_buffer {
struct vb2_v4l2_buffer vb;
struct list_head list;
@@ -184,6 +187,8 @@ static void rzg2l_cru_set_slot_addr(struct rzg2l_cru_dev *cru,
/* Currently, we just use the buffer in 32 bits address */
rzg2l_cru_write(cru, AMnMBxADDRL(slot), addr);
rzg2l_cru_write(cru, AMnMBxADDRH(slot), 0);
+
+ cru->buf_addr[slot] = addr;
}
/*
@@ -224,6 +229,7 @@ static void rzg2l_cru_fill_hw_slot(struct rzg2l_cru_dev *cru, int slot)
static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
{
+ const struct rzg2l_cru_info *info = cru->info;
unsigned int slot;
u32 amnaxiattr;
@@ -236,12 +242,39 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
for (slot = 0; slot < cru->num_buf; slot++)
rzg2l_cru_fill_hw_slot(cru, slot);
+ if (info->has_stride) {
+ u32 stride = cru->format.bytesperline;
+ u32 amnis;
+
+ stride /= RZG2L_CRU_STRIDE_ALIGN;
+ amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
+ rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
+ }
+
/* Set AXI burst max length to recommended setting */
amnaxiattr = rzg2l_cru_read(cru, AMnAXIATTR) & ~AMnAXIATTR_AXILEN_MASK;
amnaxiattr |= AMnAXIATTR_AXILEN;
rzg2l_cru_write(cru, AMnAXIATTR, amnaxiattr);
}
+void rzg3e_cru_csi2_setup(struct rzg2l_cru_dev *cru,
+ const struct rzg2l_cru_ip_format *ip_fmt,
+ u8 csi_vc)
+{
+ const struct rzg2l_cru_info *info = cru->info;
+ u32 icnmc = ICnMC_INF(ip_fmt->datatype);
+
+ icnmc |= (rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK);
+
+ /* Set virtual channel CSI2 */
+ icnmc |= ICnMC_VCSEL(csi_vc);
+
+ rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
+ rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
+ ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
+ rzg2l_cru_write(cru, info->image_conv, icnmc);
+}
+
void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
const struct rzg2l_cru_ip_format *ip_fmt,
u8 csi_vc)
@@ -290,6 +323,19 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
return 0;
}
+bool rz3e_fifo_empty(struct rzg2l_cru_dev *cru)
+{
+ u32 amnfifopntr = rzg2l_cru_read(cru, AMnFIFOPNTR);
+
+ if ((((amnfifopntr & AMnFIFOPNTR_FIFORPNTR_B1) >> 24) ==
+ ((amnfifopntr & AMnFIFOPNTR_FIFOWPNTR_B1) >> 8)) &&
+ (((amnfifopntr & AMnFIFOPNTR_FIFORPNTR_B0) >> 16) ==
+ (amnfifopntr & AMnFIFOPNTR_FIFOWPNTR_B0)))
+ return true;
+
+ return false;
+}
+
bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru)
{
u32 amnfifopntr, amnfifopntr_w, amnfifopntr_r_y;
@@ -401,6 +447,20 @@ static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)
return fd.entry[0].bus.csi2.vc;
}
+void rzg3e_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
+{
+ rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FSxE(cru->svc_channel));
+ rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FExE(cru->svc_channel));
+}
+
+void rzg3e_cru_disable_interrupts(struct rzg2l_cru_dev *cru)
+{
+ rzg2l_cru_write(cru, CRUnIE, 0);
+ rzg2l_cru_write(cru, CRUnIE2, 0);
+ rzg2l_cru_write(cru, CRUnINTS, rzg2l_cru_read(cru, CRUnINTS));
+ rzg2l_cru_write(cru, CRUnINTS2, rzg2l_cru_read(cru, CRUnINTS2));
+}
+
void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
{
rzg2l_cru_write(cru, CRUnIE, CRUnIE_EFE);
@@ -423,6 +483,7 @@ int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
if (ret < 0)
return ret;
csi_vc = ret;
+ cru->svc_channel = csi_vc;
spin_lock_irqsave(&cru->qlock, flags);
@@ -601,6 +662,107 @@ irqreturn_t rzg2l_cru_irq(int irq, void *data)
return IRQ_RETVAL(handled);
}
+static int rzg3e_cru_get_current_slot(struct rzg2l_cru_dev *cru)
+{
+ u64 amnmadrs;
+ int slot;
+
+ /*
+ * When AMnMADRSL is read, AMnMADRSH of the higher-order
+ * address also latches the address.
+ *
+ * AMnMADRSH must be read after AMnMADRSL has been read.
+ */
+ amnmadrs = rzg2l_cru_read(cru, AMnMADRSL);
+ amnmadrs |= ((u64)rzg2l_cru_read(cru, AMnMADRSH) << 32);
+
+ /* Ensure amnmadrs is within this buffer range */
+ for (slot = 0; slot < cru->num_buf; slot++)
+ if (amnmadrs >= cru->buf_addr[slot] &&
+ amnmadrs < cru->buf_addr[slot] + cru->format.sizeimage)
+ return slot;
+
+ dev_err(cru->dev, "Invalid MB address 0x%llx (out of range)\n", amnmadrs);
+ return -EINVAL;
+}
+
+irqreturn_t rzg3e_cru_irq(int irq, void *data)
+{
+ struct rzg2l_cru_dev *cru = data;
+ unsigned int handled = 0;
+ unsigned long flags;
+ u32 irq_status;
+ int slot;
+
+ spin_lock_irqsave(&cru->qlock, flags);
+ irq_status = rzg2l_cru_read(cru, CRUnINTS2);
+ if (!irq_status)
+ goto done;
+
+ dev_dbg(cru->dev, "CRUnINTS2 0x%x\n", irq_status);
+
+ handled = 1;
+
+ rzg2l_cru_write(cru, CRUnINTS2, rzg2l_cru_read(cru, CRUnINTS2));
+
+ /* Nothing to do if capture status is 'RZG2L_CRU_DMA_STOPPED' */
+ if (cru->state == RZG2L_CRU_DMA_STOPPED) {
+ dev_dbg(cru->dev, "IRQ while state stopped\n");
+ goto done;
+ }
+
+ if (cru->state == RZG2L_CRU_DMA_STOPPING) {
+ if (irq_status & CRUnINTS2_FSxS(0) ||
+ irq_status & CRUnINTS2_FSxS(1) ||
+ irq_status & CRUnINTS2_FSxS(2) ||
+ irq_status & CRUnINTS2_FSxS(3))
+ dev_dbg(cru->dev, "IRQ while state stopping\n");
+ goto done;
+ }
+
+ slot = rzg3e_cru_get_current_slot(cru);
+ if (slot < 0)
+ goto done;
+
+ dev_dbg(cru->dev, "Current written slot: %d\n", slot);
+ cru->buf_addr[slot] = 0;
+
+ /*
+ * To hand buffers back in a known order to userspace start
+ * to capture first from slot 0.
+ */
+ if (cru->state == RZG2L_CRU_DMA_STARTING) {
+ if (slot != 0) {
+ dev_dbg(cru->dev, "Starting sync slot: %d\n", slot);
+ goto done;
+ }
+ dev_dbg(cru->dev, "Capture start synced!\n");
+ cru->state = RZG2L_CRU_DMA_RUNNING;
+ }
+
+ /* Capture frame */
+ if (cru->queue_buf[slot]) {
+ cru->queue_buf[slot]->field = cru->format.field;
+ cru->queue_buf[slot]->sequence = cru->sequence;
+ cru->queue_buf[slot]->vb2_buf.timestamp = ktime_get_ns();
+ vb2_buffer_done(&cru->queue_buf[slot]->vb2_buf,
+ VB2_BUF_STATE_DONE);
+ cru->queue_buf[slot] = NULL;
+ } else {
+ /* Scratch buffer was used, dropping frame. */
+ dev_dbg(cru->dev, "Dropping frame %u\n", cru->sequence);
+ }
+
+ cru->sequence++;
+
+ /* Prepare for next frame */
+ rzg2l_cru_fill_hw_slot(cru, slot);
+
+done:
+ spin_unlock_irqrestore(&cru->qlock, flags);
+ return IRQ_RETVAL(handled);
+}
+
static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count)
{
struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vq);
@@ -782,7 +944,14 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
v4l_bound_align_image(&pix->width, 320, info->max_width, 1,
&pix->height, 240, info->max_height, 2, 0);
- pix->bytesperline = pix->width * fmt->bpp;
+ if (info->has_stride) {
+ u32 stride = clamp(pix->bytesperline, pix->width * fmt->bpp,
+ RZG2L_CRU_STRIDE_MAX);
+ pix->bytesperline = round_up(stride, RZG2L_CRU_STRIDE_ALIGN);
+ } else {
+ pix->bytesperline = pix->width * fmt->bpp;
+ }
+
pix->sizeimage = pix->bytesperline * pix->height;
dev_dbg(cru->dev, "Format %ux%u bpl: %u size: %u\n",
--
2.43.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v5 17/17] media: rzg2l-cru: Add support for RZ/G3E SoC
2025-03-28 17:29 ` [PATCH v5 17/17] media: rzg2l-cru: Add support for RZ/G3E SoC Tommaso Merciai
@ 2025-04-02 1:14 ` Laurent Pinchart
0 siblings, 0 replies; 34+ messages in thread
From: Laurent Pinchart @ 2025-04-02 1:14 UTC (permalink / raw)
To: Tommaso Merciai
Cc: tomm.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 Tommaso,
Thank you for the patch.
On Fri, Mar 28, 2025 at 06:29:53PM +0100, Tommaso Merciai wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> The CRU block on the Renesas RZ/G3E SoC is similar to the one found on
> the Renesas RZ/G2L SoC, with the following differences:
>
> - Additional registers rzg3e_cru_regs.
> - A different irq handler rzg3e_cru_irq.
> - A different rzg3e_cru_csi2_setup.
> - A different max input width.
> - Additional stride register.
>
> Introduce rzg3e_cru_info struct to handle differences between RZ/G2L
> and RZ/G3E and related RZ/G3E functions:
>
> - rzg3e_cru_enable_interrupts()
> - rzg3e_cru_enable_interrupts()
> - rz3e_fifo_empty()
> - rzg3e_cru_csi2_setup()
> - rzg3e_cru_get_current_slot()
>
> Add then support for the RZ/G3E SoC CRU block with the new compatible
> string "renesas,r9a09g047-cru".
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> Changes since v2:
> - Use dma_addr_t with buf_addr directly instead of splitting that into
> cru->mem_banks (high and low address) as suggested by LPinchart.
> - Moved and improved stride adjustment into rzg2l_cru_format_align()
> as suggested by LPinchart.
> - Use csi_vc into rzg3e_cru_csi2_setup() instead of cru->svc_channel as
> suggested by LPinchart
> - Added has_stride field to handle soc differences as suggested by LPinchart.
>
> Changes since v3:
> - Fixed kernel test robot warnings from rzg3e_cru_get_current_slot() and
> rzg3e_cru_irq()
>
> .../platform/renesas/rzg2l-cru/rzg2l-core.c | 62 +++++++
> .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 25 +++
> .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 13 ++
> .../platform/renesas/rzg2l-cru/rzg2l-video.c | 171 +++++++++++++++++-
> 4 files changed, 270 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> index 3ae0cd83af164..1356be14eda8a 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> @@ -290,6 +290,12 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> return ret;
>
> cru->num_buf = RZG2L_CRU_HW_BUFFER_DEFAULT;
> + cru->buf_addr = devm_kmalloc_array(dev, cru->num_buf,
> + sizeof(dma_addr_t), GFP_KERNEL);
The number of entries is constant. Do you need to allocate this
dynamically ?
> + if (!cru->buf_addr)
> + return dev_err_probe(dev, -ENOMEM,
> + "Failed to init buf addr\n");
s/init/allocate/
but hopefully we can drop this error check by making cru->buf_addr a
fixed-size array.
> +
> pm_suspend_ignore_children(dev, true);
> ret = devm_pm_runtime_enable(dev);
> if (ret)
> @@ -321,6 +327,58 @@ static void rzg2l_cru_remove(struct platform_device *pdev)
> rzg2l_cru_dma_unregister(cru);
> }
>
> +static const u16 rzg3e_cru_regs[] = {
> + [CRUnCTRL] = 0x0,
> + [CRUnIE] = 0x4,
> + [CRUnIE2] = 0x8,
> + [CRUnINTS] = 0xc,
> + [CRUnINTS2] = 0x10,
> + [CRUnRST] = 0x18,
> + [AMnMB1ADDRL] = 0x40,
> + [AMnMB1ADDRH] = 0x44,
> + [AMnMB2ADDRL] = 0x48,
> + [AMnMB2ADDRH] = 0x4c,
> + [AMnMB3ADDRL] = 0x50,
> + [AMnMB3ADDRH] = 0x54,
> + [AMnMB4ADDRL] = 0x58,
> + [AMnMB4ADDRH] = 0x5c,
> + [AMnMB5ADDRL] = 0x60,
> + [AMnMB5ADDRH] = 0x64,
> + [AMnMB6ADDRL] = 0x68,
> + [AMnMB6ADDRH] = 0x6c,
> + [AMnMB7ADDRL] = 0x70,
> + [AMnMB7ADDRH] = 0x74,
> + [AMnMB8ADDRL] = 0x78,
> + [AMnMB8ADDRH] = 0x7c,
> + [AMnMBVALID] = 0x88,
> + [AMnMADRSL] = 0x8c,
> + [AMnMADRSH] = 0x90,
> + [AMnAXIATTR] = 0xec,
> + [AMnFIFOPNTR] = 0xf8,
> + [AMnAXISTP] = 0x110,
> + [AMnAXISTPACK] = 0x114,
> + [AMnIS] = 0x128,
> + [ICnEN] = 0x1f0,
> + [ICnSVCNUM] = 0x1f8,
> + [ICnSVC] = 0x1fc,
> + [ICnIPMC_C0] = 0x200,
> + [ICnMS] = 0x2d8,
> + [ICnDMR] = 0x304,
> +};
> +
> +static const struct rzg2l_cru_info rzg3e_cru_info = {
> + .max_width = 4095,
> + .max_height = 4095,
> + .image_conv = ICnIPMC_C0,
> + .has_stride = true,
> + .regs = rzg3e_cru_regs,
> + .irq_handler = rzg3e_cru_irq,
> + .enable_interrupts = rzg3e_cru_enable_interrupts,
> + .disable_interrupts = rzg3e_cru_disable_interrupts,
> + .fifo_empty = rz3e_fifo_empty,
> + .csi_setup = rzg3e_cru_csi2_setup,
> +};
> +
> static const u16 rzg2l_cru_regs[] = {
> [CRUnCTRL] = 0x0,
> [CRUnIE] = 0x4,
> @@ -367,6 +425,10 @@ static const struct rzg2l_cru_info rzgl2_cru_info = {
> };
>
> static const struct of_device_id rzg2l_cru_of_id_table[] = {
> + {
> + .compatible = "renesas,r9a09g047-cru",
> + .data = &rzg3e_cru_info,
> + },
> {
> .compatible = "renesas,rzg2l-cru",
> .data = &rzgl2_cru_info,
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> index 86c3202862465..52324b076674b 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h
> @@ -14,8 +14,13 @@
>
> #define CRUnIE_EFE BIT(17)
>
> +#define CRUnIE2_FSxE(x) BIT(((x) * 3))
> +#define CRUnIE2_FExE(x) BIT(((x) * 3) + 1)
> +
> #define CRUnINTS_SFS BIT(16)
>
> +#define CRUnINTS2_FSxS(x) BIT(((x) * 3))
> +
> #define CRUnRST_VRESETN BIT(0)
>
> /* Memory Bank Base Address (Lower) Register for CRU Image Data */
> @@ -32,7 +37,14 @@
> #define AMnAXIATTR_AXILEN (0xf)
>
> #define AMnFIFOPNTR_FIFOWPNTR GENMASK(7, 0)
> +#define AMnFIFOPNTR_FIFOWPNTR_B0 AMnFIFOPNTR_FIFOWPNTR
> +#define AMnFIFOPNTR_FIFOWPNTR_B1 GENMASK(15, 8)
> #define AMnFIFOPNTR_FIFORPNTR_Y GENMASK(23, 16)
> +#define AMnFIFOPNTR_FIFORPNTR_B0 AMnFIFOPNTR_FIFORPNTR_Y
> +#define AMnFIFOPNTR_FIFORPNTR_B1 GENMASK(31, 24)
> +
> +#define AMnIS_IS_MASK GENMASK(14, 7)
> +#define AMnIS_IS(x) ((x) << 7)
>
> #define AMnAXISTP_AXI_STOP BIT(0)
>
> @@ -40,6 +52,11 @@
>
> #define ICnEN_ICEN BIT(0)
>
> +#define ICnSVC_SVC0(x) (x)
> +#define ICnSVC_SVC1(x) ((x) << 4)
> +#define ICnSVC_SVC2(x) ((x) << 8)
> +#define ICnSVC_SVC3(x) ((x) << 12)
> +
> #define ICnMC_CSCTHR BIT(5)
> #define ICnMC_INF(x) ((x) << 16)
> #define ICnMC_VCSEL(x) ((x) << 22)
> @@ -52,7 +69,9 @@
> enum rzg2l_cru_common_regs {
> CRUnCTRL, /* CRU Control */
> CRUnIE, /* CRU Interrupt Enable */
> + CRUnIE2, /* CRU Interrupt Enable(2) */
> CRUnINTS, /* CRU Interrupt Status */
> + CRUnINTS2, /* CRU Interrupt Status(2) */
> CRUnRST, /* CRU Reset */
> AMnMB1ADDRL, /* Bank 1 Address (Lower) for CRU Image Data */
> AMnMB1ADDRH, /* Bank 1 Address (Higher) for CRU Image Data */
> @@ -72,12 +91,18 @@ enum rzg2l_cru_common_regs {
> AMnMB8ADDRH, /* Bank 8 Address (Higher) for CRU Image Data */
> AMnMBVALID, /* Memory Bank Enable for CRU Image Data */
> AMnMBS, /* Memory Bank Status for CRU Image Data */
> + AMnMADRSL, /* VD Memory Address Lower Status Register */
> + AMnMADRSH, /* VD Memory Address Higher Status Register */
> AMnAXIATTR, /* AXI Master Transfer Setting Register for CRU Image Data */
> AMnFIFOPNTR, /* AXI Master FIFO Pointer for CRU Image Data */
> AMnAXISTP, /* AXI Master Transfer Stop for CRU Image Data */
> AMnAXISTPACK, /* AXI Master Transfer Stop Status for CRU Image Data */
> + AMnIS, /* Image Stride Setting Register */
> ICnEN, /* CRU Image Processing Enable */
> + ICnSVCNUM, /* CRU SVC Number Register */
> + ICnSVC, /* CRU VC Select Register */
> ICnMC, /* CRU Image Processing Main Control */
> + ICnIPMC_C0, /* CRU Image Converter Main Control 0 */
> ICnMS, /* CRU Module Status */
> ICnDMR, /* CRU Data Output Mode */
> RZG2L_CRU_MAX_REG,
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> index ccaba5220f1c8..d68d833406865 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> @@ -85,6 +85,7 @@ struct rzg2l_cru_info {
> unsigned int max_height;
> u16 image_conv;
> const u16 *regs;
> + bool has_stride;
> irqreturn_t (*irq_handler)(int irq, void *data);
> void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
> void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
> @@ -108,6 +109,8 @@ struct rzg2l_cru_info {
> * @vdev: V4L2 video device associated with CRU
> * @v4l2_dev: V4L2 device
> * @num_buf: Holds the current number of buffers enabled
> + * @svc_channel: SVC0/1/2/3 to use for RZ/G3E
> + * @buf_addr: Memory addresses where current video data is written.
> * @notifier: V4L2 asynchronous subdevs notifier
> *
> * @ip: Image processing subdev info
> @@ -144,6 +147,9 @@ struct rzg2l_cru_dev {
> struct v4l2_device v4l2_dev;
> u8 num_buf;
>
> + u8 svc_channel;
> + dma_addr_t *buf_addr;
> +
> struct v4l2_async_notifier notifier;
>
> struct rzg2l_cru_ip ip;
> @@ -175,6 +181,7 @@ void rzg2l_cru_dma_unregister(struct rzg2l_cru_dev *cru);
> int rzg2l_cru_video_register(struct rzg2l_cru_dev *cru);
> void rzg2l_cru_video_unregister(struct rzg2l_cru_dev *cru);
> irqreturn_t rzg2l_cru_irq(int irq, void *data);
> +irqreturn_t rzg3e_cru_irq(int irq, void *data);
>
> const struct v4l2_format_info *rzg2l_cru_format_from_pixel(u32 format);
>
> @@ -188,10 +195,16 @@ const struct rzg2l_cru_ip_format *rzg2l_cru_ip_index_to_fmt(u32 index);
>
> void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
> void rzg2l_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
> +void rzg3e_cru_enable_interrupts(struct rzg2l_cru_dev *cru);
> +void rzg3e_cru_disable_interrupts(struct rzg2l_cru_dev *cru);
>
> bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru);
> +bool rz3e_fifo_empty(struct rzg2l_cru_dev *cru);
> void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
> const struct rzg2l_cru_ip_format *ip_fmt,
> u8 csi_vc);
> +void rzg3e_cru_csi2_setup(struct rzg2l_cru_dev *cru,
> + const struct rzg2l_cru_ip_format *ip_fmt,
> + u8 csi_vc);
>
> #endif
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 748a0855b3245..da6d13e80a45a 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -31,6 +31,9 @@
> #define RZG2L_CRU_DEFAULT_FIELD V4L2_FIELD_NONE
> #define RZG2L_CRU_DEFAULT_COLORSPACE V4L2_COLORSPACE_SRGB
>
> +#define RZG2L_CRU_STRIDE_MAX 32640
> +#define RZG2L_CRU_STRIDE_ALIGN 128
> +
> struct rzg2l_cru_buffer {
> struct vb2_v4l2_buffer vb;
> struct list_head list;
> @@ -184,6 +187,8 @@ static void rzg2l_cru_set_slot_addr(struct rzg2l_cru_dev *cru,
> /* Currently, we just use the buffer in 32 bits address */
> rzg2l_cru_write(cru, AMnMBxADDRL(slot), addr);
> rzg2l_cru_write(cru, AMnMBxADDRH(slot), 0);
> +
> + cru->buf_addr[slot] = addr;
> }
>
> /*
> @@ -224,6 +229,7 @@ static void rzg2l_cru_fill_hw_slot(struct rzg2l_cru_dev *cru, int slot)
>
> static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
> {
> + const struct rzg2l_cru_info *info = cru->info;
> unsigned int slot;
> u32 amnaxiattr;
>
> @@ -236,12 +242,39 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
> for (slot = 0; slot < cru->num_buf; slot++)
> rzg2l_cru_fill_hw_slot(cru, slot);
>
> + if (info->has_stride) {
> + u32 stride = cru->format.bytesperline;
> + u32 amnis;
> +
> + stride /= RZG2L_CRU_STRIDE_ALIGN;
> + amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
> + rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
> + }
> +
> /* Set AXI burst max length to recommended setting */
> amnaxiattr = rzg2l_cru_read(cru, AMnAXIATTR) & ~AMnAXIATTR_AXILEN_MASK;
> amnaxiattr |= AMnAXIATTR_AXILEN;
> rzg2l_cru_write(cru, AMnAXIATTR, amnaxiattr);
> }
>
> +void rzg3e_cru_csi2_setup(struct rzg2l_cru_dev *cru,
> + const struct rzg2l_cru_ip_format *ip_fmt,
> + u8 csi_vc)
> +{
> + const struct rzg2l_cru_info *info = cru->info;
> + u32 icnmc = ICnMC_INF(ip_fmt->datatype);
> +
> + icnmc |= (rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK);
> +
> + /* Set virtual channel CSI2 */
> + icnmc |= ICnMC_VCSEL(csi_vc);
> +
> + rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
> + rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
> + ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
> + rzg2l_cru_write(cru, info->image_conv, icnmc);
> +}
> +
> void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
> const struct rzg2l_cru_ip_format *ip_fmt,
> u8 csi_vc)
> @@ -290,6 +323,19 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru,
> return 0;
> }
>
> +bool rz3e_fifo_empty(struct rzg2l_cru_dev *cru)
> +{
> + u32 amnfifopntr = rzg2l_cru_read(cru, AMnFIFOPNTR);
> +
> + if ((((amnfifopntr & AMnFIFOPNTR_FIFORPNTR_B1) >> 24) ==
> + ((amnfifopntr & AMnFIFOPNTR_FIFOWPNTR_B1) >> 8)) &&
> + (((amnfifopntr & AMnFIFOPNTR_FIFORPNTR_B0) >> 16) ==
> + (amnfifopntr & AMnFIFOPNTR_FIFOWPNTR_B0)))
> + return true;
> +
> + return false;
> +}
> +
> bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru)
> {
> u32 amnfifopntr, amnfifopntr_w, amnfifopntr_r_y;
> @@ -401,6 +447,20 @@ static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)
> return fd.entry[0].bus.csi2.vc;
> }
>
> +void rzg3e_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
> +{
> + rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FSxE(cru->svc_channel));
> + rzg2l_cru_write(cru, CRUnIE2, CRUnIE2_FExE(cru->svc_channel));
> +}
> +
> +void rzg3e_cru_disable_interrupts(struct rzg2l_cru_dev *cru)
> +{
> + rzg2l_cru_write(cru, CRUnIE, 0);
> + rzg2l_cru_write(cru, CRUnIE2, 0);
> + rzg2l_cru_write(cru, CRUnINTS, rzg2l_cru_read(cru, CRUnINTS));
> + rzg2l_cru_write(cru, CRUnINTS2, rzg2l_cru_read(cru, CRUnINTS2));
> +}
> +
> void rzg2l_cru_enable_interrupts(struct rzg2l_cru_dev *cru)
> {
> rzg2l_cru_write(cru, CRUnIE, CRUnIE_EFE);
> @@ -423,6 +483,7 @@ int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
> if (ret < 0)
> return ret;
> csi_vc = ret;
> + cru->svc_channel = csi_vc;
>
> spin_lock_irqsave(&cru->qlock, flags);
>
> @@ -601,6 +662,107 @@ irqreturn_t rzg2l_cru_irq(int irq, void *data)
> return IRQ_RETVAL(handled);
> }
>
> +static int rzg3e_cru_get_current_slot(struct rzg2l_cru_dev *cru)
> +{
> + u64 amnmadrs;
> + int slot;
> +
> + /*
> + * When AMnMADRSL is read, AMnMADRSH of the higher-order
> + * address also latches the address.
> + *
> + * AMnMADRSH must be read after AMnMADRSL has been read.
> + */
> + amnmadrs = rzg2l_cru_read(cru, AMnMADRSL);
> + amnmadrs |= ((u64)rzg2l_cru_read(cru, AMnMADRSH) << 32);
Drop the outer parentheses.
> +
> + /* Ensure amnmadrs is within this buffer range */
> + for (slot = 0; slot < cru->num_buf; slot++)
> + if (amnmadrs >= cru->buf_addr[slot] &&
> + amnmadrs < cru->buf_addr[slot] + cru->format.sizeimage)
> + return slot;
I would use curly braces for the for statement.
> +
> + dev_err(cru->dev, "Invalid MB address 0x%llx (out of range)\n", amnmadrs);
> + return -EINVAL;
> +}
> +
> +irqreturn_t rzg3e_cru_irq(int irq, void *data)
> +{
> + struct rzg2l_cru_dev *cru = data;
> + unsigned int handled = 0;
> + unsigned long flags;
> + u32 irq_status;
> + int slot;
> +
> + spin_lock_irqsave(&cru->qlock, flags);
Use a scoped guard to avoid the gotos.
> + irq_status = rzg2l_cru_read(cru, CRUnINTS2);
> + if (!irq_status)
> + goto done;
You will be able to return directly here, and drop the handled variable.
> +
> + dev_dbg(cru->dev, "CRUnINTS2 0x%x\n", irq_status);
> +
> + handled = 1;
> +
> + rzg2l_cru_write(cru, CRUnINTS2, rzg2l_cru_read(cru, CRUnINTS2));
> +
> + /* Nothing to do if capture status is 'RZG2L_CRU_DMA_STOPPED' */
> + if (cru->state == RZG2L_CRU_DMA_STOPPED) {
> + dev_dbg(cru->dev, "IRQ while state stopped\n");
> + goto done;
> + }
> +
> + if (cru->state == RZG2L_CRU_DMA_STOPPING) {
> + if (irq_status & CRUnINTS2_FSxS(0) ||
> + irq_status & CRUnINTS2_FSxS(1) ||
> + irq_status & CRUnINTS2_FSxS(2) ||
> + irq_status & CRUnINTS2_FSxS(3))
> + dev_dbg(cru->dev, "IRQ while state stopping\n");
> + goto done;
> + }
> +
> + slot = rzg3e_cru_get_current_slot(cru);
> + if (slot < 0)
> + goto done;
> +
> + dev_dbg(cru->dev, "Current written slot: %d\n", slot);
> + cru->buf_addr[slot] = 0;
Have you tested operation with buffer queue underruns, when userspace
doesn't requeue buffers fast enough and the driver runs out of buffers ?
> +
> + /*
> + * To hand buffers back in a known order to userspace start
> + * to capture first from slot 0.
> + */
> + if (cru->state == RZG2L_CRU_DMA_STARTING) {
> + if (slot != 0) {
> + dev_dbg(cru->dev, "Starting sync slot: %d\n", slot);
> + goto done;
> + }
> + dev_dbg(cru->dev, "Capture start synced!\n");
> + cru->state = RZG2L_CRU_DMA_RUNNING;
> + }
> +
> + /* Capture frame */
> + if (cru->queue_buf[slot]) {
The queue shouldn't be an array, but a linked list. This isn't an issue
introduced by this series, so it can be fixed on top.
> + cru->queue_buf[slot]->field = cru->format.field;
Use a local variable for the queue entry.
struct vb2_v4l2_buffer *buf = cru->queue_buf[slot];
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> + cru->queue_buf[slot]->sequence = cru->sequence;
> + cru->queue_buf[slot]->vb2_buf.timestamp = ktime_get_ns();
> + vb2_buffer_done(&cru->queue_buf[slot]->vb2_buf,
> + VB2_BUF_STATE_DONE);
> + cru->queue_buf[slot] = NULL;
> + } else {
> + /* Scratch buffer was used, dropping frame. */
> + dev_dbg(cru->dev, "Dropping frame %u\n", cru->sequence);
> + }
> +
> + cru->sequence++;
> +
> + /* Prepare for next frame */
> + rzg2l_cru_fill_hw_slot(cru, slot);
> +
> +done:
> + spin_unlock_irqrestore(&cru->qlock, flags);
> + return IRQ_RETVAL(handled);
> +}
> +
> static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count)
> {
> struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vq);
> @@ -782,7 +944,14 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
> v4l_bound_align_image(&pix->width, 320, info->max_width, 1,
> &pix->height, 240, info->max_height, 2, 0);
>
> - pix->bytesperline = pix->width * fmt->bpp;
> + if (info->has_stride) {
> + u32 stride = clamp(pix->bytesperline, pix->width * fmt->bpp,
> + RZG2L_CRU_STRIDE_MAX);
> + pix->bytesperline = round_up(stride, RZG2L_CRU_STRIDE_ALIGN);
> + } else {
> + pix->bytesperline = pix->width * fmt->bpp;
> + }
> +
> pix->sizeimage = pix->bytesperline * pix->height;
>
> dev_dbg(cru->dev, "Format %ux%u bpl: %u size: %u\n",
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 34+ messages in thread