Hi Sakari, Thanks for the review. On 2/21/25 15:10, Sakari Ailus wrote: > Hi Michael, > > Thanks for the update. > > On Wed, Feb 19, 2025 at 11:16:34AM +0100, Michael Riesch wrote: >> Add documentation for the Rockchip RK3568 Video Capture (VICAP) unit. >> >> Signed-off-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx> >> --- >> .../bindings/media/rockchip,rk3568-vicap.yaml | 168 +++++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 169 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml b/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml >> new file mode 100644 >> index 000000000000..3dc15efeb32e >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml >> @@ -0,0 +1,168 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/rockchip,rk3568-vicap.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Rockchip RK3568 Video Capture (VICAP) >> + >> +maintainers: >> + - Michael Riesch <michael.riesch@xxxxxxxxxxxxxx> >> + >> +description: >> + The Rockchip RK3568 Video Capture (VICAP) block features a digital video >> + port (DVP, a parallel video interface) and a MIPI CSI-2 port. It receives >> + the data from camera sensors, video decoders, or other companion ICs and >> + transfers it into system main memory by AXI bus. >> + >> +properties: >> + compatible: >> + const: rockchip,rk3568-vicap >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + items: >> + - description: ACLK >> + - description: HCLK >> + - description: DCLK >> + - description: ICLK >> + >> + clock-names: >> + items: >> + - const: aclk >> + - const: hclk >> + - const: dclk >> + - const: iclk >> + >> + rockchip,cif-clk-delaynum: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + minimum: 0 >> + maximum: 127 >> + description: >> + Delay the DVP path clock input to align the sampling phase, only valid >> + in dual edge sampling mode. > > I suppose there's further documentation on this somewhere else? A reference > would be nice. I like your optimism :-) No, I am afraid this single sentence is all the the RK3568 TRM has to say about it. I can add a reference to the TRM page, but everyone who actually follows this reference will be disappointed... >> + >> + iommus: >> + maxItems: 1 >> + >> + resets: >> + items: >> + - description: ARST >> + - description: HRST >> + - description: DRST >> + - description: PRST >> + - description: IRST >> + >> + reset-names: >> + items: >> + - const: arst >> + - const: hrst >> + - const: drst >> + - const: prst >> + - const: irst >> + >> + rockchip,grf: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: >> + Phandle to general register file used for video input block control. >> + >> + power-domains: >> + maxItems: 1 >> + >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + >> + properties: >> + port@0: >> + $ref: /schemas/graph.yaml#/$defs/port-base >> + unevaluatedProperties: false >> + description: >> + The digital video port (DVP, a parallel video interface). >> + >> + properties: >> + endpoint: >> + $ref: video-interfaces.yaml# >> + unevaluatedProperties: false >> + >> + properties: >> + bus-type: >> + enum: [5, 6] >> + >> + required: >> + - bus-type >> + >> + port@1: >> + $ref: /schemas/graph.yaml#/$defs/port-base >> + unevaluatedProperties: false >> + description: The MIPI CSI-2 port. >> + >> + properties: >> + endpoint: >> + $ref: video-interfaces.yaml# >> + unevaluatedProperties: false > > Don't you need things like data-lanes here? Or is this a single lane > receiver? This may be a bit confusing, and I probably should extend the description a bit. This port/endpoint faces the MIPI CSI Host, which has its own driver provided in patch 6. The connection in between is a link with some internal format. Hence, no properties required. This is the same issue as the one discussed in the other thread, since the other end of this connection is discussed there. I'll fix the issue on both ends using Rob's suggestion. >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - clocks >> + - ports >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/clock/rk3568-cru.h> >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/interrupt-controller/irq.h> >> + #include <dt-bindings/power/rk3568-power.h> >> + #include <dt-bindings/media/video-interfaces.h> >> + >> + parent { >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + vicap: video-capture@fdfe0000 { >> + compatible = "rockchip,rk3568-vicap"; >> + reg = <0x0 0xfdfe0000 0x0 0x200>; >> + interrupts = <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>; >> + assigned-clocks = <&cru DCLK_VICAP>; >> + assigned-clock-rates = <300000000>; >> + clocks = <&cru ACLK_VICAP>, <&cru HCLK_VICAP>, >> + <&cru DCLK_VICAP>, <&cru ICLK_VICAP_G>; >> + clock-names = "aclk", "hclk", "dclk", "iclk"; >> + iommus = <&vicap_mmu>; >> + power-domains = <&power RK3568_PD_VI>; >> + resets = <&cru SRST_A_VICAP>, <&cru SRST_H_VICAP>, >> + <&cru SRST_D_VICAP>, <&cru SRST_P_VICAP>, >> + <&cru SRST_I_VICAP>; >> + reset-names = "arst", "hrst", "drst", "prst", "irst"; >> + rockchip,grf = <&grf>; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + vicap_dvp: port@0 { >> + reg = <0>; >> + >> + vicap_dvp_input: endpoint { >> + bus-type = <MEDIA_BUS_TYPE_BT656>; >> + bus-width = <16>; >> + pclk-sample = <MEDIA_PCLK_SAMPLE_DUAL_EDGE>; >> + remote-endpoint = <&it6801_output>; >> + }; >> + }; >> + >> + vicap_mipi: port@1 { >> + reg = <1>; > > Where is the endpoint? I'll add the endpoint in the example. Regards, Michael > >> + }; >> + }; >> + }; >> + }; >> +... >> diff --git a/MAINTAINERS b/MAINTAINERS >> index bbfaf35d50c6..cd8fa1afe5eb 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -20407,6 +20407,7 @@ M: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx> >> L: linux-media@xxxxxxxxxxxxxxx >> S: Maintained >> F: Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml >> +F: Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml >> >> ROCKCHIP CRYPTO DRIVERS >> M: Corentin Labbe <clabbe@xxxxxxxxxxxx> >> >
