On Sat, Mar 15, 2025 at 04:27:02PM +0100, Niklas Söderlund wrote:
> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> index c4de4555b753..de9bc739e084 100644
> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> @@ -25,19 +25,54 @@ properties:
> - renesas,r8a779h0-isp # V4M
> - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
> reg:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> +
> + reg-names:
> + minItems: 1
> + items:
> + - const: cs
> + - const: core
All of this and further must be restricted per compatible. Otherwise
commit msg should explain why one SoC can have it different on different
boards.
>
> interrupts:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> +
> + interrupt-names:
> + minItems: 1
> + items:
> + - const: cs
> + - const: core
>
> clocks:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> +
> + clock-names:
> + minItems: 1
> + items:
> + - const: cs
> + - const: core
>
> power-domains:
> maxItems: 1
>
> resets:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> +
> + reset-names:
> + minItems: 1
> + items:
> + - const: cs
> + - const: core
> +
> + renesas,vspx:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + A phandle to the companion VSPX responsible for the Streaming Bridge
But what does this device needs it for?
> + functionality. This property is not mandatory and not all ISP devices
> + have one attached.
Drop last sentence, redundant. Instead disallow it (renesas,vspx: false)
for all the variants not having VSPX.
>
> ports:
> $ref: /schemas/graph.yaml#/properties/ports
> @@ -103,10 +138,14 @@ properties:
> required:
> - compatible
> - reg
> + - reg-names
> - interrupts
> + - interrupt-names
> - clocks
> + - clock-names
> - power-domains
> - resets
> + - reset-names
> - ports
>
> additionalProperties: false
> @@ -119,11 +158,16 @@ examples:
>
> isp1: isp@fed20000 {
> compatible = "renesas,r8a779a0-isp", "renesas,rcar-gen4-isp";
> - reg = <0xfed20000 0x10000>;
> - interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0xfed20000 0x10000>, <0xfee00000 0x10000>;
> + reg-names = "cs", "core";
> + interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "cs", "core";
> clocks = <&cpg CPG_MOD 613>;
> + clock-names = "cs";
Why no core? The names feel inconsistent. If your block has "core" reg
for the "ISP core" sublock, why there is no "ISP core" clock for that
subblock?
Best regards,
Krzysztof
