Re: [PATCH v2 1/7] dt-bindings: media: renesas,isp: Add ISP core function block

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Niklas,

Thank you for the patch.

On Mon, Apr 21, 2025 at 01:12:34PM +0200, Niklas Söderlund wrote:
> Some R-Car ISP instances have in addition to the channel selector (CS)
> an ISP core (CORE )to perform operations on an image stream. The core

s/ )/) /

> function is mapped to a different memory region and have a separate

s/have/has/

> interrupt then CS, extend the bindings to allow describing this.

s/then/than/

> 
> On the same SoC different instances of the ISP IP may have, or not have,
> the CORE functionality. The CS function on all instances on the SoC are
> the same and the documentation describes the full ISP (CS + CORE) as a
> single IP block. Where instances not having the CORE function simple
> lacking the functionality to modify the image data. There dependencies

s/simple lacking/simply lack/

s/There/There are/ ? Or did you mean something else ?

> on the CS functionality while operating the CORE functionality.
> 
> In order for the ISP core to function in memory-to-memory mode it needs
> to be feed input data from a Streaming Bridge interface. This interface
> is provided thru the VSP-X device. Add an optional new property
> "renesas,vspx" to provide a phandle to describe this relationship.
> 
> While adding mandatory reg-names and interrupt-names breaks existing
> bindings the driver itself remains backward compatible and provides CS
> functionality if a single unnamed reg and interrupt property is present.
> Furthermore all existing users of the bindings are updated in following
> work to add these new mandatory properties.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> * Changes since v1
> - Extend the commit message to make it explicit that different ISP
>   instances on the same SoC (same compatible value) can have, or not
>   have, a CORE function block attached.
> - Update documentation for renesas,vspx property.
> - Update example to cover all new properties.
> ---
>  .../bindings/media/renesas,isp.yaml           | 63 ++++++++++++++++---
>  1 file changed, 55 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> index c4de4555b753..927be02347e5 100644
> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> @@ -25,19 +25,55 @@ 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
>  
>    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
> +      functionality. The Streaming Bridge is responsible for feeding image
> +      and configuration data to the ISP when operating in memory-to-memory
> +      mode.
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -103,10 +139,14 @@ properties:
>  required:
>    - compatible
>    - reg
> +  - reg-names
>    - interrupts
> +  - interrupt-names
>    - clocks
> +  - clock-names
>    - power-domains
>    - resets
> +  - reset-names
>    - ports
>  
>  additionalProperties: false
> @@ -119,11 +159,18 @@ examples:
>  
>      isp1: isp@fed20000 {
>              compatible = "renesas,r8a779a0-isp", "renesas,rcar-gen4-isp";
> -            reg = <0xfed20000 0x10000>;
> -            interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> -            clocks = <&cpg CPG_MOD 613>;
> +            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>, <&cpg CPG_MOD 17>;
> +            clock-names = "cs", "core";
>              power-domains = <&sysc R8A779A0_PD_A3ISP01>;
> -            resets = <&cpg 613>;
> +            resets = <&cpg 613>, <&cpg 17>;
> +            reset-names = "cs", "core";
> +
> +            renesas,vspx = <&vspx1>;
>  
>              ports {
>                      #address-cells = <1>;

-- 
Regards,

Laurent Pinchart