Hi Krzysztof, Michael, On Fri, Mar 07, 2025 at 08:51:54AM +0100, Krzysztof Kozlowski wrote: > On Thu, Mar 06, 2025 at 05:56:04PM +0100, Michael Riesch wrote: > > Add documentation for the Rockchip RK3568 Video Capture (VICAP) unit. > > > > Signed-off-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx> > > subject: only one media prefix, the first > > A nit, subject: drop second/last, redundant "bindings". 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 > > > --- > > .../bindings/media/rockchip,rk3568-vicap.yaml | 169 +++++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 170 insertions(+) > > > > ... > > > + 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. Delay is zero by default and can be adjusted > > + optionally. > > default: 0 And this is technically specific to the DVP port (0). Should (or could?) it be located there? > > > + > > + 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#/properties/port > > + description: Internal port connected to a MIPI CSI-2 host. > > + > > + properties: > > + endpoint: > > + $ref: video-interfaces.yaml# > > + unevaluatedProperties: false > > Hm, does it actually work? graph/port does not allow any other > properties. You should use graph/port-base and probably still narrow > lanes for both of port@0 and port@1. I'd list the relevant properties for both DVP and CSI-2, either as mandatory or with defaults (could be reasonable for DVP signal polarities but not e.g. on number of CSI-2 lanes). > > > > + > > +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 { > > soc { > > > + #address-cells = <2>; > > + #size-cells = <2>; > > Best regards, > Krzysztof > -- Kind regards, Sakari Ailus
