Re: [PATCH v5 03/11] media: dt-bindings: media: add bindings for rockchip rk3568 vicap

Krzysztof Kozlowski
2025-03-07T07:53:49Z

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@wolfvision.net>

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

> +
> + 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.

> +
> +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