Hi Krzysztof - thanks for comments
On 24/06/2025 14:16, Krzysztof Kozlowski wrote:
On 24/06/2025 14:35, Daniel Scally wrote:
The RZ/V2H SoC has a block called the Input Video Control block which
feeds image data into the Image Signal Processor. Add dt bindings to
describe the IVC.
Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
---
Changes in v2:
- compatible matches filename
This is not what I asked.
Ah, you meant the other way round. Ok.
- Added power-domains
- Aligned clock and reset entries on opening "<"
- Removed status = "okay"; from example
---
.../bindings/media/renesas,rzv2h-ivc.yaml | 103 +++++++++++++++++++++
1 file changed, 103 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/renesas,rzv2h-ivc.yaml b/Documentation/devicetree/bindings/media/renesas,rzv2h-ivc.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..ee0849eeeaba840cf43c81d69449c631ad1c6a6a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,rzv2h-ivc.yaml
@@ -0,0 +1,103 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/renesas,rzv2h-ivc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/V2H Input Video Control Block
+
+maintainers:
+ - Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
+
+description:
+ The IVC block is a module that takes video frames from memory and feeds them
+ to the Image Signal Processor for processing.
+
+properties:
+ compatible:
+ const: renesas,rzv2h-ivc
No, you cannot use generic compatibles. See writing bindings (or
basically every review from DT maintainers).
OK...to throw a spanner into this, I think there's actually two soc codes with the IP; r9a09g057h44
and r9a09g057h48. renesas,rzg2l-cru.yaml and renesas,rzg2l-csi2.yaml seem to use the 'generic' code
in a way that suggests this:
compatible:
items:
- enum:
- renesas,r9a09g057h44-ivc
- renesas,r9a09g057h48-ivc
- const: renesas,rzv2h-ivc
And then the filename stays as it is, but the example-schema.yaml to me suggests this, without the
'generic' code there at all:
compatible:
items:
- enum:
- renesas,r9a09g057h48-ivc
- const: renesas,r9a09g057h44-ivc
Is the latter one right?
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Input Video Control block register access clock
+ - description: Video input data AXI bus clock
+ - description: ISP system clock
+
+ clock-names:
+ items:
+ - const: pclk
+ - const: vin_aclk
aclk
vin is redundant. This cannot be anything else than vin.
Fair enough; there's also a clock that the documentation calls "reg_aclk" that goes to the ISP core
and I've been calling that one just "aclk" so the "vin_" was to distinguish them...but maybe I
should just stop trying to follow the documentation's names; how about "reg", "axi" and "isp" for
both the clock and reset names?
+ - const: sclk
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ items:
+ - description: Input Video Control block register access reset
+ - description: Video input data AXI bus reset
+ - description: ISP core reset
+
+ reset-names:
+ items:
+ - const: presetn
+ - const: vin_aresetn
+ - const: sresetn
s/reset// because it is redundant... which would point to pretty
pointless names. Look at your description - one is core or registers,
second is vin and other is isp-core... although then I wonder why ISP
input resets ISP core?
The two IP blocks share the ISP system clock / ISP core reset and also Video input data AXI bus
clock / reset.
Thanks
Dan
Best regards,
Krzysztof
