Hi Michael, On Mon, Feb 24, 2025 at 11:21:41AM +0100, Michael Riesch wrote: > 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... :-( Is this something that needs to be set? Is there a default, for instance? If there's documentation available, it'd be nice to refer to that from somewhere, I'm not sure if the driver or DT bindings would be better. Probably the driver. > > >> + > >> + 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. If this is some custom interface, you should not say it's MIPI CSI-2 (even though MIPI CSI-2 data could be transported on top). > >> + vicap_mipi: port@1 { > >> + reg = <1>; > > > > Where is the endpoint? > > I'll add the endpoint in the example. Thank you. -- Kind regards, Sakari Ailus
