Re: [PATCH v4 2/8] media: dt-bindings: add rockchip rk3588 vicap

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch removes the `items` definitions from the `clocks` and `resets` properties but fails to make `clock-names` and `reset-names` required.
- [Low] The `ports` node uses `additionalProperties: false` alongside a `$ref`, which violates the dt-schema meta-schema and will cause `dt_binding_check` to fail.
--

commit 9cfb752de7e6c8e4e818ef36f9918a2d0f7ec9d8
Author: Michael Riesch <michael.riesch@collabora.com>

media: dt-bindings: add rockchip rk3588 vicap

Add documentation for the Rockchip RK3588 Video Capture (VICAP) unit. To
that end, make the existing rockchip,rk3568-vicap documentation more
general and introduce variant specific constraints.

> diff --git a/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml b/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml
> index 18cd0a5a53181..897ed00c239bd 100644
> --- a/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml

[ ... ]

> @@ -26,37 +32,23 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    items:
> -      - description: ACLK
> -      - description: HCLK
> -      - description: DCLK
> -      - description: ICLK
> +    minItems: 4
> +    maxItems: 5
>  
>    clock-names:
> -    items:
> -      - const: aclk
> -      - const: hclk
> -      - const: dclk
> -      - const: iclk
> +    minItems: 4
> +    maxItems: 5
>  
>    iommus:
>      maxItems: 1
>  
>    resets:
> -    items:
> -      - description: ARST
> -      - description: HRST
> -      - description: DRST
> -      - description: PRST
> -      - description: IRST
> +    minItems: 5
> +    maxItems: 9

Since the items lists are removed here to allow variant-specific counts,
does this leave the sequence of clocks and resets unvalidated if a device
tree omits clock-names and reset-names?

Because clock-names and reset-names are not added to the required list,
device trees can omit them entirely.

Could clock-names and reset-names be added to the required list, or could
the items lists be moved into the respective if/then blocks to ensure the
expected sequences are preserved?

[ ... ]

> @@ -67,8 +59,15 @@ properties:
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> +    additionalProperties: false

Does using additionalProperties: false alongside a $ref here violate the
dt-schema meta-schema?

The dt-schema meta-schema forbids using additionalProperties when a $ref is
present on the same level, as it does not account for properties evaluated
by the reference. This results in a build-time validation failure when
running make dt_binding_check.

Should this be changed to unevaluatedProperties: false instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-rk3588-vicap-v4-0-6a6cd6f7c90b@collabora.com?part=2