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

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

On Wed, Apr 23, 2025 at 01:24:25PM +0200, Geert Uytterhoeven wrote:
> On Mon, 21 Apr 2025 at 13:12, 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
> > function is mapped to a different memory region and have a separate
> > interrupt then CS, extend the bindings to allow describing this.
> >
> > 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
> > 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>
> > ---
> > * 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.
> 
> Thanks for the update!
> 
> > --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> 
> > @@ -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>;
> 
> IThe second size should be 0x100000.
> 
> > +            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>;
> 
> With the above and the wording issues pointed out by Laurent fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Once we get a review from the DT bindings maintainers for this patch,
I'll take the next version addressing the small issues in my tree for
1/7 and 5/7 to 7/7. I'll let Geert merge 2/7 to 4/7.

-- 
Regards,

Laurent Pinchart