| Message ID | 20250519145754.454005-1-dan.scally@ideasonboard.com |
|---|---|
| Headers | show |
| Series | Add Input Video Control Block driver for RZ/V2H | expand |
On Mon, May 19, 2025 at 03:57:52PM GMT, 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. A nit, subject: drop second/last, redundant "bindings for". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > .../bindings/media/renesas,rzv2h-ivc.yaml | 99 +++++++++++++++++++ This fails testing - expect Rob's bot report (or check DT patchwork) - thus limited review. > 1 file changed, 99 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/renesas,rzv2h-ivc.yaml Filename matching compatible. You can also explain exception in the commit msg. > > 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 000000000000..29d522f7d365 > ... > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - resets > + - reset-names > + - port > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/renesas,r9a09g057-cpg.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + ivc: ivc@16040000 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation video-codec? video-encoder? video-engine? Or anything reasonable you find. Drop unused label > + compatible = "renesas,r9a09g057-ivc"; > + reg = <0x16040000 0x230>; > + > + clocks = <&cpg CPG_MOD R9A09G057_ISP0_PCLK>, > + <&cpg CPG_MOD R9A09G057_ISP0_VIN_ACLK>, > + <&cpg CPG_MOD R9A09G057_ISP0_SCLK>; Misaligned, should be aligned to < (see DTS coding style). > + clock-names = "pclk", "vin_aclk", "sclk"; > + > + resets = <&cpg R9A09G057_ISP_0_PRESETN>, > + <&cpg R9A09G057_ISP_0_VIN_ARESETN>, Same here > + <&cpg R9A09G057_ISP_0_ISP_SRESETN>; > + reset-names = "presetn", "vin_aresetn", "sresetn"; > + > + interrupts = <GIC_SPI 861 IRQ_TYPE_EDGE_RISING>; > + > + status = "okay"; Drop These are nits, so normally would qualify for review, but maybe I missed something since it fails testing. Best regards, Krzysztof
Hi Daniel, Thanks for the patch. > -----Original Message----- > From: Daniel Scally <dan.scally@ideasonboard.com> > Subject: [PATCH 1/3] dt-bindings: media: Add bindings for the RZ/V2H IVC block > > 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@ideasonboard.com> > --- > .../bindings/media/renesas,rzv2h-ivc.yaml | 99 +++++++++++++++++++ > 1 file changed, 99 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/renesas,rzv2h-ivc.yaml > > 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 000000000000..29d522f7d365 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/renesas,rzv2h-ivc.yaml > @@ -0,0 +1,99 @@ > +# 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@ideasonboard.com> > + > +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,r9a09g057-ivc > + > + 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 > + - 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 > + > + port: > + $ref: /schemas/graph.yaml#/properties/port > + description: Output parallel video bus > + > + properties: > + endpoint: > + $ref: /schemas/graph.yaml#/properties/endpoint > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names - power-domains ?? > + - resets > + - reset-names > + - port > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/renesas,r9a09g057-cpg.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + ivc: ivc@16040000 { > + compatible = "renesas,r9a09g057-ivc"; > + reg = <0x16040000 0x230>; > + > + clocks = <&cpg CPG_MOD R9A09G057_ISP0_PCLK>, > + <&cpg CPG_MOD R9A09G057_ISP0_VIN_ACLK>, > + <&cpg CPG_MOD R9A09G057_ISP0_SCLK>; > + clock-names = "pclk", "vin_aclk", "sclk"; power-domains?? Cheers, Biju > + > + resets = <&cpg R9A09G057_ISP_0_PRESETN>, > + <&cpg R9A09G057_ISP_0_VIN_ARESETN>, > + <&cpg R9A09G057_ISP_0_ISP_SRESETN>; > + reset-names = "presetn", "vin_aresetn", "sresetn"; > + > + interrupts = <GIC_SPI 861 IRQ_TYPE_EDGE_RISING>; > + > + status = "okay"; > + > + port { > + ivc_out: endpoint { > + remote-endpoint = <&isp_in>; > + }; > + }; > + }; > +... > -- > 2.34.1 >