[PATCH 0/7] rcar-isp: Prepare for ISP core support

[PATCH 0/7] rcar-isp: Prepare for ISP core support

[Niklas Söderlund]

Hello,

This series prepares for adding support for the ISP core functionality 
found on some R-Car ISP instances. No core support is however added in 
this series, the focus is to get the easy changes out of the way to 
avoid conflicts of fixes and new features being added in parallel on top 
of this.

Patch 1/7 extends the dt-bindings to allow describing both the CSISP and 
ISPCORE blocks. Patch 2/7, 3/7 and 4/7 updates the existing bindings to 
match the new style. While the change breaks the dt-bindings the driver 
is compatible with both styles.

Patch 5/7 prepares for the addition of the ISP core functions that will 
span multiple files by moving the driver implementation from a single C 
file to a directory where it can grow. The intent is to get this out of 
the way without bikeshedding the real ISP core work so fixes and such 
can be based on the new file structure as early as possible.

Patch 6/7 and 7/7 prepares the driver for dealing with two regions for 
when the ISP core work is integrated.

There is no functional gain in this series apart from correctly 
describing the hardware in dt.

Niklas Söderlund (7):
  dt-bindings: media: renesas,isp: Add ISP core function block
  arm64: dts: renesas: r8a779a0: Add ISP core function block
  arm64: dts: renesas: r8a779g0: Add ISP core function block
  arm64: dts: renesas: r8a779h0: Add ISP core function block
  media: rcar-isp: Move driver to own directory
  media: rcar-isp: Rename base register variable
  media: rcar-isp: Parse named cs memory region

 .../bindings/media/renesas,isp.yaml           | 56 +++++++++++++++--
 MAINTAINERS                                   |  2 +-
 arch/arm64/boot/dts/renesas/r8a779a0.dtsi     | 60 ++++++++++++++-----
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi     | 30 +++++++---
 arch/arm64/boot/dts/renesas/r8a779h0.dtsi     | 21 +++++--
 drivers/media/platform/renesas/Kconfig        | 18 +-----
 drivers/media/platform/renesas/Makefile       |  2 +-
 .../media/platform/renesas/rcar-isp/Kconfig   | 17 ++++++
 .../media/platform/renesas/rcar-isp/Makefile  |  4 ++
 .../renesas/{rcar-isp.c => rcar-isp/csisp.c}  | 56 ++++++++++-------
 10 files changed, 189 insertions(+), 77 deletions(-)
 create mode 100644 drivers/media/platform/renesas/rcar-isp/Kconfig
 create mode 100644 drivers/media/platform/renesas/rcar-isp/Makefile
 rename drivers/media/platform/renesas/{rcar-isp.c => rcar-isp/csisp.c} (89%)

-- 
2.48.1

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

[Niklas Söderlund]

Some R-Car ISP instances have in addition to the channel selector (CS)
an ISP core to perform operations on an image stream. The core function
is mapped to a different memory region and have a separate interrupt
that CS, extend the bindings to allow describing this.

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>
---
 .../bindings/media/renesas,isp.yaml           | 56 +++++++++++++++++--
 1 file changed, 50 insertions(+), 6 deletions(-)

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
 
   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. This property is not mandatory and not all ISP devices
+      have one attached.
 
   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";
             power-domains = <&sysc R8A779A0_PD_A3ISP01>;
             resets = <&cpg 613>;
+            reset-names = "cs";
 
             ports {
                     #address-cells = <1>;
-- 
2.48.1

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

[Krzysztof Kozlowski]

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

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

[Niklas Söderlund]

Hi Krzysztof,

Thanks for your feedback.

On 2025-03-17 12:31:51 +0100, Krzysztof Kozlowski wrote:
> 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.

I will expand the commit message. In short this can't be restricted per 
compatible, different instances of the IP on the same board can and can 
not have a core part.

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

It's the external IP that controls the DMA of data to the ISP. I will 
expand this description.

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

I can't do that as all variants can possibly have one attached to it.  
All instances of the ISP that have a core part have a VSPX. And on the 
same SoC different instances of the IP can have and can not have a core 
part.

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

Indeed this is wrong, there should be a core clock added here too, 
thanks for catching this.

> 
> Best regards,
> Krzysztof
> 

-- 
Kind Regards,
Niklas Söderlund

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

[Krzysztof Kozlowski]

On 17/03/2025 12:49, Niklas Söderlund wrote:
> Hi Krzysztof,
> 
> Thanks for your feedback.
> 
> On 2025-03-17 12:31:51 +0100, Krzysztof Kozlowski wrote:
>> 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.
> 
> I will expand the commit message. In short this can't be restricted per 
> compatible, different instances of the IP on the same board can and can 
> not have a core part.

s/Same board/same SoC/? Or you really meant that same SoC on different
boards will have or have not that ISP core?

Both are odd, first even weirder.

I wonder if some other difference is not the documented. E.g. same IP
blocks are not exactly the same, but have different programming model.

What is this ISP core responsible for inside Renesas ISP? How many ISPs
are inside of SoC?

And how would it work? You have two exactly the same IP blocks in the
SoC, but one you program differently than other. How do you know which one?

Best regards,
Krzysztof

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

[Niklas Söderlund]

On 2025-03-17 16:02:34 +0100, Krzysztof Kozlowski wrote:
> On 17/03/2025 12:49, Niklas Söderlund wrote:
> > Hi Krzysztof,
> > 
> > Thanks for your feedback.
> > 
> > On 2025-03-17 12:31:51 +0100, Krzysztof Kozlowski wrote:
> >> 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.
> > 
> > I will expand the commit message. In short this can't be restricted per 
> > compatible, different instances of the IP on the same board can and can 
> > not have a core part.
> 
> s/Same board/same SoC/? Or you really meant that same SoC on different
> boards will have or have not that ISP core?
> 
> Both are odd, first even weirder.
> 
> I wonder if some other difference is not the documented. E.g. same IP
> blocks are not exactly the same, but have different programming model.

The IP block named "ISP" is in fact two different things on R-Car Gen4 
SoCs. I know it's confusing and not logical but that's how they are 
made.

- One part is the ISP Channel Selector, this is a function that sits on 
  the CIS-2 receiver data bus. It is responsible for selecting which 
  CSI-2 Virtual Channel is routed to which DMA capture engine.

  This part is what the rcar-isp.ko driver have always supported and 
  every instance of the ISP that is described in tree deals with just 
  this one function as this is the one we always had documentation for.

  This block is the one the reg-names and clock-names labels "cs".

- One part that we now wish to add is the ISP Core. This is a 
  traditional ISP that act on image data in different ways. This is what 
  I try to model with the reg-name and clock-name labeled "core".

  This is new and we have not had documentation for this until recently.  
  Unfortunately the "core" and "cs" functions is implemented in the same 
  IP block. And to operate the "core" one needs to also deal with "cs".  

To make it more interesting all ISP Channel Selectors (cs) do not have a 
companion ISP Core, but most do. The lack of a ISP core is OK, it just 
means that video capture path can't manipulate the image as much as 
paths that do.

It's not ideal but to support the ISP Core and ISP Channel Selecotr the 
rcar-isp.ko module needs both "core" and "cs" clocks and regs. And to 
support just the Channel Selector it only needs the "cs" resources.


Sorry if I have been confusing. A good example of this is patch 4/7 in 
this series. It shows the V4M board that have 2 ISP instances, one that 
have both cs and core functions, and one that only have cs function.

> 
> What is this ISP core responsible for inside Renesas ISP? How many ISPs
> are inside of SoC?

The ISP core is responsible for image manipulation. ISP Channel Selector 
for CSI-2 channel routing. The number of ISP varies between SoCs:


V3U: Have 4 ISP instances, all 4 have cs and core.
V4H: Have 2 ISP instances, both have cs and core.
V4M: Have 2 ISP instances, one have cs and core, one have only cs.

> 
> And how would it work? You have two exactly the same IP blocks in the
> SoC, but one you program differently than other. How do you know which one?

All cs blocks are the same on all SoCs. The core block is the same on 
V4H and V4M, and different on V3U.

> 
> Best regards,
> Krzysztof

-- 
Kind Regards,
Niklas Söderlund

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

[Krzysztof Kozlowski]

On 17/03/2025 16:34, Niklas Söderlund wrote:
> SoCs. I know it's confusing and not logical but that's how they are 
> made.
> 
> - One part is the ISP Channel Selector, this is a function that sits on 
>   the CIS-2 receiver data bus. It is responsible for selecting which 
>   CSI-2 Virtual Channel is routed to which DMA capture engine.
> 
>   This part is what the rcar-isp.ko driver have always supported and 
>   every instance of the ISP that is described in tree deals with just 
>   this one function as this is the one we always had documentation for.
> 
>   This block is the one the reg-names and clock-names labels "cs".
> 
> - One part that we now wish to add is the ISP Core. This is a 
>   traditional ISP that act on image data in different ways. This is what 
>   I try to model with the reg-name and clock-name labeled "core".
> 
>   This is new and we have not had documentation for this until recently.  
>   Unfortunately the "core" and "cs" functions is implemented in the same 
>   IP block. And to operate the "core" one needs to also deal with "cs".  
> 
> To make it more interesting all ISP Channel Selectors (cs) do not have a 
> companion ISP Core, but most do. The lack of a ISP core is OK, it just 
> means that video capture path can't manipulate the image as much as 
> paths that do.
> 
> It's not ideal but to support the ISP Core and ISP Channel Selecotr the 
> rcar-isp.ko module needs both "core" and "cs" clocks and regs. And to 
> support just the Channel Selector it only needs the "cs" resources.
> 
> 
> Sorry if I have been confusing. A good example of this is patch 4/7 in 
> this series. It shows the V4M board that have 2 ISP instances, one that 
> have both cs and core functions, and one that only have cs function.

Based on this I think the instances with ISP core are not the same
hardware as instances without. You have there different (new)
programming model for entirely new part of hardware not present in "old"
instances.

Different device means different compatible.

And judging by the address:
reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
1. 0xfec0 < 0xfed0
2. Huge address range

that's not "renesas,r8a779h0-isp" at all, but your old "ISP" device is
actually a part of that 0xfec0_0000.

Probably the channel selector should have never been called "ISP"
because it does not process images. :/

> 
>>
>> What is this ISP core responsible for inside Renesas ISP? How many ISPs
>> are inside of SoC?
> 
> The ISP core is responsible for image manipulation. ISP Channel Selector 
> for CSI-2 channel routing. The number of ISP varies between SoCs:
> 
> 
> V3U: Have 4 ISP instances, all 4 have cs and core.
> V4H: Have 2 ISP instances, both have cs and core.
> V4M: Have 2 ISP instances, one have cs and core, one have only cs.


Best regards,
Krzysztof

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

[Geert Uytterhoeven]

Hi Krzysztof,

On Tue, 18 Mar 2025 at 08:27, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 17/03/2025 16:34, Niklas Söderlund wrote:
> > SoCs. I know it's confusing and not logical but that's how they are
> > made.
> >
> > - One part is the ISP Channel Selector, this is a function that sits on
> >   the CIS-2 receiver data bus. It is responsible for selecting which
> >   CSI-2 Virtual Channel is routed to which DMA capture engine.
> >
> >   This part is what the rcar-isp.ko driver have always supported and
> >   every instance of the ISP that is described in tree deals with just
> >   this one function as this is the one we always had documentation for.
> >
> >   This block is the one the reg-names and clock-names labels "cs".
> >
> > - One part that we now wish to add is the ISP Core. This is a
> >   traditional ISP that act on image data in different ways. This is what
> >   I try to model with the reg-name and clock-name labeled "core".
> >
> >   This is new and we have not had documentation for this until recently.
> >   Unfortunately the "core" and "cs" functions is implemented in the same
> >   IP block. And to operate the "core" one needs to also deal with "cs".
> >
> > To make it more interesting all ISP Channel Selectors (cs) do not have a
> > companion ISP Core, but most do. The lack of a ISP core is OK, it just
> > means that video capture path can't manipulate the image as much as
> > paths that do.
> >
> > It's not ideal but to support the ISP Core and ISP Channel Selecotr the
> > rcar-isp.ko module needs both "core" and "cs" clocks and regs. And to
> > support just the Channel Selector it only needs the "cs" resources.
> >
> >
> > Sorry if I have been confusing. A good example of this is patch 4/7 in
> > this series. It shows the V4M board that have 2 ISP instances, one that
> > have both cs and core functions, and one that only have cs function.
>
> Based on this I think the instances with ISP core are not the same
> hardware as instances without. You have there different (new)
> programming model for entirely new part of hardware not present in "old"
> instances.
>
> Different device means different compatible.

I think the intention has always been to represent the "full" ISP,
but we started with limited bindings, due to the lack of documentation.
Note that at the time the bindings were written, all SoCs we were
aware of only had the "full" ISP.

> And judging by the address:
> reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
> 1. 0xfec0 < 0xfed0

Relative addresses don't mean anything.

> 2. Huge address range
>
> that's not "renesas,r8a779h0-isp" at all, but your old "ISP" device is
> actually a part of that 0xfec0_0000.
>
> Probably the channel selector should have never been called "ISP"
> because it does not process images. :/

The documentation has just a single chapter for the combined Image
Signal Processor with Channel Selector, and considers it a single block.
From my point of view, the ISP processing core is just an optional feature,
which is not that dissimilar from e.g. the optional stream buffer in
Documentation/devicetree/bindings/net/renesas,etheravb.yaml.

> >> What is this ISP core responsible for inside Renesas ISP? How many ISPs
> >> are inside of SoC?
> >
> > The ISP core is responsible for image manipulation. ISP Channel Selector
> > for CSI-2 channel routing. The number of ISP varies between SoCs:
> >
> > V3U: Have 4 ISP instances, all 4 have cs and core.
> > V4H: Have 2 ISP instances, both have cs and core.
> > V4M: Have 2 ISP instances, one have cs and core, one have only cs.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Niklas Söderlund]

On 2025-03-18 08:27:36 +0100, Krzysztof Kozlowski wrote:
> On 17/03/2025 16:34, Niklas Söderlund wrote:
> > SoCs. I know it's confusing and not logical but that's how they are 
> > made.
> > 
> > - One part is the ISP Channel Selector, this is a function that sits on 
> >   the CIS-2 receiver data bus. It is responsible for selecting which 
> >   CSI-2 Virtual Channel is routed to which DMA capture engine.
> > 
> >   This part is what the rcar-isp.ko driver have always supported and 
> >   every instance of the ISP that is described in tree deals with just 
> >   this one function as this is the one we always had documentation for.
> > 
> >   This block is the one the reg-names and clock-names labels "cs".
> > 
> > - One part that we now wish to add is the ISP Core. This is a 
> >   traditional ISP that act on image data in different ways. This is what 
> >   I try to model with the reg-name and clock-name labeled "core".
> > 
> >   This is new and we have not had documentation for this until recently.  
> >   Unfortunately the "core" and "cs" functions is implemented in the same 
> >   IP block. And to operate the "core" one needs to also deal with "cs".  
> > 
> > To make it more interesting all ISP Channel Selectors (cs) do not have a 
> > companion ISP Core, but most do. The lack of a ISP core is OK, it just 
> > means that video capture path can't manipulate the image as much as 
> > paths that do.
> > 
> > It's not ideal but to support the ISP Core and ISP Channel Selecotr the 
> > rcar-isp.ko module needs both "core" and "cs" clocks and regs. And to 
> > support just the Channel Selector it only needs the "cs" resources.
> > 
> > 
> > Sorry if I have been confusing. A good example of this is patch 4/7 in 
> > this series. It shows the V4M board that have 2 ISP instances, one that 
> > have both cs and core functions, and one that only have cs function.
> 
> Based on this I think the instances with ISP core are not the same
> hardware as instances without. You have there different (new)
> programming model for entirely new part of hardware not present in "old"
> instances.
> 
> Different device means different compatible.
> 
> And judging by the address:
> reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
> 1. 0xfec0 < 0xfed0
> 2. Huge address range
> 
> that's not "renesas,r8a779h0-isp" at all, but your old "ISP" device is
> actually a part of that 0xfec0_0000.
> 
> Probably the channel selector should have never been called "ISP"
> because it does not process images. :/

There are always room for improvement, but we can only try and create 
bindings that describe the hardware as it is documented.

In the block diagrams the channel selector and the core isp are in the 
same block.  And for better or worse to operate the ISP to process 
images the driver need to poke at the channel selector, even tho I fail 
to see why some of the core registers where put in the cs block.

On Gen3 an equally interesting hardware design can be found, the CSI-2 
channel selector was there placed together with the IP block for image 
capture DMA engines... Luckily that only created a mess in the driver 
and not in the bindings.

Thanks again for your feedback, I really learn a lot!

> 
> > 
> >>
> >> What is this ISP core responsible for inside Renesas ISP? How many ISPs
> >> are inside of SoC?
> > 
> > The ISP core is responsible for image manipulation. ISP Channel Selector 
> > for CSI-2 channel routing. The number of ISP varies between SoCs:
> > 
> > 
> > V3U: Have 4 ISP instances, all 4 have cs and core.
> > V4H: Have 2 ISP instances, both have cs and core.
> > V4M: Have 2 ISP instances, one have cs and core, one have only cs.
> 
> 
> Best regards,
> Krzysztof

-- 
Kind Regards,
Niklas Söderlund

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

[Krzysztof Kozlowski]

On Sat, Mar 15, 2025 at 04:27:02PM +0100, Niklas Söderlund wrote:
>    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

Another point, this will spawn bunch of warnings for no real reason.
Just drop all the xxx-names from properties and from here.

Best regards,
Krzysztof

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

[Niklas Söderlund]

On 2025-03-17 12:33:07 +0100, Krzysztof Kozlowski wrote:
> On Sat, Mar 15, 2025 at 04:27:02PM +0100, Niklas Söderlund wrote:
> >    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
> 
> Another point, this will spawn bunch of warnings for no real reason.
> Just drop all the xxx-names from properties and from here.

I'm sorry maybe I'm missing something, but if I drop them from 
properties how can I add checks to makesure the names are either "cs" or 
"core"?

> 
> Best regards,
> Krzysztof
> 

-- 
Kind Regards,
Niklas Söderlund

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

[Krzysztof Kozlowski]

On 17/03/2025 12:50, Niklas Söderlund wrote:
> On 2025-03-17 12:33:07 +0100, Krzysztof Kozlowski wrote:
>> On Sat, Mar 15, 2025 at 04:27:02PM +0100, Niklas Söderlund wrote:
>>>    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
>>
>> Another point, this will spawn bunch of warnings for no real reason.
>> Just drop all the xxx-names from properties and from here.
> 
> I'm sorry maybe I'm missing something, but if I drop them from 
> properties how can I add checks to makesure the names are either "cs" or 

Why do you need to check for the names? There will be no names, so
nothing to check for.

> "core"?

Best regards,
Krzysztof

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

[Niklas Söderlund]

On 2025-03-17 15:57:31 +0100, Krzysztof Kozlowski wrote:
> On 17/03/2025 12:50, Niklas Söderlund wrote:
> > On 2025-03-17 12:33:07 +0100, Krzysztof Kozlowski wrote:
> >> On Sat, Mar 15, 2025 at 04:27:02PM +0100, Niklas Söderlund wrote:
> >>>    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
> >>
> >> Another point, this will spawn bunch of warnings for no real reason.
> >> Just drop all the xxx-names from properties and from here.
> > 
> > I'm sorry maybe I'm missing something, but if I drop them from 
> > properties how can I add checks to makesure the names are either "cs" or 
> 
> Why do you need to check for the names? There will be no names, so
> nothing to check for.

Ahh I see. But I would like to have names if possible.

The driver is backward compatible with the old bindings, and going 
forward we have better bindings with names. All users are updated in the 
next commits in this series so the warnings will go way rather quickly.

> > "core"?
> 
> Best regards,
> Krzysztof

-- 
Kind Regards,
Niklas Söderlund

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

[Geert Uytterhoeven]

Hi Niklas,

On Mon, 17 Mar 2025 at 16:37, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> On 2025-03-17 15:57:31 +0100, Krzysztof Kozlowski wrote:
> > On 17/03/2025 12:50, Niklas Söderlund wrote:
> > > On 2025-03-17 12:33:07 +0100, Krzysztof Kozlowski wrote:
> > >> On Sat, Mar 15, 2025 at 04:27:02PM +0100, Niklas Söderlund wrote:
> > >>>    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
> > >>
> > >> Another point, this will spawn bunch of warnings for no real reason.
> > >> Just drop all the xxx-names from properties and from here.
> > >
> > > I'm sorry maybe I'm missing something, but if I drop them from
> > > properties how can I add checks to makesure the names are either "cs" or
> >
> > Why do you need to check for the names? There will be no names, so
> > nothing to check for.
>
> Ahh I see. But I would like to have names if possible.
>
> The driver is backward compatible with the old bindings, and going
> forward we have better bindings with names. All users are updated in the
> next commits in this series so the warnings will go way rather quickly.

Note that the driver does not _have_ to obtain the "cs" clock by name,
as it will always be the first clock anyway ("make dtbs_check" will
sort-of enforce that).  So you can simplify the code by obtaining
the first clock without specifying a name, and the second (optional)
clock with a name.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Niklas Söderlund]

Hi Geert,

Thanks for your feedback.

On 2025-03-17 20:21:14 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Mon, 17 Mar 2025 at 16:37, Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > On 2025-03-17 15:57:31 +0100, Krzysztof Kozlowski wrote:
> > > On 17/03/2025 12:50, Niklas Söderlund wrote:
> > > > On 2025-03-17 12:33:07 +0100, Krzysztof Kozlowski wrote:
> > > >> On Sat, Mar 15, 2025 at 04:27:02PM +0100, Niklas Söderlund wrote:
> > > >>>    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
> > > >>
> > > >> Another point, this will spawn bunch of warnings for no real reason.
> > > >> Just drop all the xxx-names from properties and from here.
> > > >
> > > > I'm sorry maybe I'm missing something, but if I drop them from
> > > > properties how can I add checks to makesure the names are either "cs" or
> > >
> > > Why do you need to check for the names? There will be no names, so
> > > nothing to check for.
> >
> > Ahh I see. But I would like to have names if possible.
> >
> > The driver is backward compatible with the old bindings, and going
> > forward we have better bindings with names. All users are updated in the
> > next commits in this series so the warnings will go way rather quickly.
> 
> Note that the driver does not _have_ to obtain the "cs" clock by name,
> as it will always be the first clock anyway ("make dtbs_check" will
> sort-of enforce that).  So you can simplify the code by obtaining
> the first clock without specifying a name, and the second (optional)
> clock with a name.

I understand that, and for this fix this would be acceptable. I'm just 
trying to think a head, something I should have done when first writing 
these bindings...

I'm fearing a scenario where we will need to add a 3rd reg region or 
clock. I don't think we will need that for the compatible values we have 
here, but then I never though we get the documentation that now allows 
us to describe the second region...

If you and Krzysztof are happy without names I can move forward with 
that too, I'm just trying to learn from my mistakes ;-) I will give it a 
few days and then head down this road without names.

-- 
Kind Regards,
Niklas Söderlund

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

[Krzysztof Kozlowski]

On 17/03/2025 20:44, Niklas Söderlund wrote:
>>> Ahh I see. But I would like to have names if possible.
>>>
>>> The driver is backward compatible with the old bindings, and going
>>> forward we have better bindings with names. All users are updated in the
>>> next commits in this series so the warnings will go way rather quickly.
>>
>> Note that the driver does not _have_ to obtain the "cs" clock by name,
>> as it will always be the first clock anyway ("make dtbs_check" will
>> sort-of enforce that).  So you can simplify the code by obtaining
>> the first clock without specifying a name, and the second (optional)
>> clock with a name.
> 
> I understand that, and for this fix this would be acceptable. I'm just 
> trying to think a head, something I should have done when first writing 
> these bindings...
> 
> I'm fearing a scenario where we will need to add a 3rd reg region or 
> clock. I don't think we will need that for the compatible values we have 

Bindings should be complete, so add 3rd clock now.

If you need to add it later, what's the problem? The position or order
is strictly fixed, so at 3rd place you will always have new foo-clock.

> here, but then I never though we get the documentation that now allows 
> us to describe the second region...
Best regards,
Krzysztof

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

[Niklas Söderlund]

On 2025-03-18 08:29:17 +0100, Krzysztof Kozlowski wrote:
> On 17/03/2025 20:44, Niklas Söderlund wrote:
> >>> Ahh I see. But I would like to have names if possible.
> >>>
> >>> The driver is backward compatible with the old bindings, and going
> >>> forward we have better bindings with names. All users are updated in the
> >>> next commits in this series so the warnings will go way rather quickly.
> >>
> >> Note that the driver does not _have_ to obtain the "cs" clock by name,
> >> as it will always be the first clock anyway ("make dtbs_check" will
> >> sort-of enforce that).  So you can simplify the code by obtaining
> >> the first clock without specifying a name, and the second (optional)
> >> clock with a name.
> > 
> > I understand that, and for this fix this would be acceptable. I'm just 
> > trying to think a head, something I should have done when first writing 
> > these bindings...
> > 
> > I'm fearing a scenario where we will need to add a 3rd reg region or 
> > clock. I don't think we will need that for the compatible values we have 
> 
> Bindings should be complete, so add 3rd clock now.
> 
> If you need to add it later, what's the problem? The position or order
> is strictly fixed, so at 3rd place you will always have new foo-clock.

I agree, bindings should be complete. But it's hard to create complete 
bindings from incomplete documentation. There is no 3rd clock or memory 
region that can be added now, at lest not one in the documentation I 
have access too. I was only trying to make the point that I do want to 
add *-names properties now and not only depend on argument position.

Sorry if I have misunderstood you.

> 
> > here, but then I never though we get the documentation that now allows 
> > us to describe the second region...
> Best regards,
> Krzysztof

-- 
Kind Regards,
Niklas Söderlund

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

[Geert Uytterhoeven]

Hi Niklas,

On Mon, 17 Mar 2025 at 20:44, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> On 2025-03-17 20:21:14 +0100, Geert Uytterhoeven wrote:
> > On Mon, 17 Mar 2025 at 16:37, Niklas Söderlund
> > <niklas.soderlund+renesas@ragnatech.se> wrote:
> > > On 2025-03-17 15:57:31 +0100, Krzysztof Kozlowski wrote:
> > > > On 17/03/2025 12:50, Niklas Söderlund wrote:
> > > > > On 2025-03-17 12:33:07 +0100, Krzysztof Kozlowski wrote:
> > > > >> On Sat, Mar 15, 2025 at 04:27:02PM +0100, Niklas Söderlund wrote:
> > > > >>>    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
> > > > >>
> > > > >> Another point, this will spawn bunch of warnings for no real reason.
> > > > >> Just drop all the xxx-names from properties and from here.
> > > > >
> > > > > I'm sorry maybe I'm missing something, but if I drop them from
> > > > > properties how can I add checks to makesure the names are either "cs" or
> > > >
> > > > Why do you need to check for the names? There will be no names, so
> > > > nothing to check for.
> > >
> > > Ahh I see. But I would like to have names if possible.
> > >
> > > The driver is backward compatible with the old bindings, and going
> > > forward we have better bindings with names. All users are updated in the
> > > next commits in this series so the warnings will go way rather quickly.
> >
> > Note that the driver does not _have_ to obtain the "cs" clock by name,
> > as it will always be the first clock anyway ("make dtbs_check" will
> > sort-of enforce that).  So you can simplify the code by obtaining
> > the first clock without specifying a name, and the second (optional)
> > clock with a name.
>
> I understand that, and for this fix this would be acceptable. I'm just
> trying to think a head, something I should have done when first writing
> these bindings...
>
> I'm fearing a scenario where we will need to add a 3rd reg region or
> clock. I don't think we will need that for the compatible values we have
> here, but then I never though we get the documentation that now allows
> us to describe the second region...
>
> If you and Krzysztof are happy without names I can move forward with
> that too, I'm just trying to learn from my mistakes ;-) I will give it a
> few days and then head down this road without names.

I would still specify the names in the bindings, so full ISPs have
all names.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 2/7] arm64: dts: renesas: r8a779a0: Add ISP core function block

[Niklas Söderlund]

All ISP instances on V3U have both a channel select and core function
block, describe the core region in addition to the existing cs region.

The interrupt number already described intended to reflect the cs
function but did incorrectly describe the core block. This was not
noticed until now as the driver do not make use of the interrupt for the
cs block.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 arch/arm64/boot/dts/renesas/r8a779a0.dtsi | 60 +++++++++++++++++------
 1 file changed, 44 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
index f1613bfd1632..95ff69339991 100644
--- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
@@ -2588,13 +2588,20 @@ du_out_dsi1: endpoint {
 		isp0: isp@fed00000 {
 			compatible = "renesas,r8a779a0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed00000 0 0x10000>;
-			interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&cpg CPG_MOD 612>;
+			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779A0_PD_A3ISP01>;
-			resets = <&cpg 612>;
+			resets = <&cpg 612>, <&cpg 16>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx0>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -2672,13 +2679,20 @@ isp0vin07: endpoint {
 		isp1: isp@fed20000 {
 			compatible = "renesas,r8a779a0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed20000 0 0x10000>;
-			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&cpg CPG_MOD 613>;
+			reg = <0 0xfed20000 0 0x10000>, <0 0xfee00000 0 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>;
-			resets = <&cpg 613>;
+			resets = <&cpg 613>, <&cpg 17>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx1>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -2756,13 +2770,20 @@ isp1vin15: endpoint {
 		isp2: isp@fed30000 {
 			compatible = "renesas,r8a779a0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed30000 0 0x10000>;
-			interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&cpg CPG_MOD 614>;
+			reg = <0 0xfed30000 0 0x10000>, <0 0xfef00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 614>, <&cpg CPG_MOD 18>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779A0_PD_A3ISP23>;
-			resets = <&cpg 614>;
+			resets = <&cpg 614>, <&cpg 18>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx2>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -2840,13 +2861,20 @@ isp2vin23: endpoint {
 		isp3: isp@fed40000 {
 			compatible = "renesas,r8a779a0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed40000 0 0x10000>;
-			interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&cpg CPG_MOD 615>;
+			reg = <0 0xfed40000 0 0x10000>, <0 0xfe400000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 615>, <&cpg CPG_MOD 19>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779A0_PD_A3ISP23>;
-			resets = <&cpg 615>;
+			resets = <&cpg 615>, <&cpg 19>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx3>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
2.48.1

Re: [PATCH 2/7] arm64: dts: renesas: r8a779a0: Add ISP core function block

[Jacopo Mondi]

Hi Niklas

On Sat, Mar 15, 2025 at 04:27:03PM +0100, Niklas Söderlund wrote:
> All ISP instances on V3U have both a channel select and core function
> block, describe the core region in addition to the existing cs region.
>
> The interrupt number already described intended to reflect the cs
> function but did incorrectly describe the core block. This was not

I can't find the interrupt mapping table for V3U, so this is the only
thing I can't check

> noticed until now as the driver do not make use of the interrupt for the
> cs block.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

The rest looks good

> ---
>  arch/arm64/boot/dts/renesas/r8a779a0.dtsi | 60 +++++++++++++++++------
>  1 file changed, 44 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> index f1613bfd1632..95ff69339991 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> @@ -2588,13 +2588,20 @@ du_out_dsi1: endpoint {
>  		isp0: isp@fed00000 {
>  			compatible = "renesas,r8a779a0-isp",
>  				     "renesas,rcar-gen4-isp";
> -			reg = <0 0xfed00000 0 0x10000>;
> -			interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> -			clocks = <&cpg CPG_MOD 612>;
> +			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
> +			reg-names = "cs", "core";

However, won't the presence of a "core" part trigger the probing of
the forthcoming RPP core support, which should not support V3U as far
I understood ?

> +			interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "cs", "core";
> +			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
> +			clock-names = "cs", "core";
>  			power-domains = <&sysc R8A779A0_PD_A3ISP01>;
> -			resets = <&cpg 612>;
> +			resets = <&cpg 612>, <&cpg 16>;
> +			reset-names = "cs", "core";
>  			status = "disabled";
>
> +			renesas,vspx = <&vspx0>;
> +
>  			ports {
>  				#address-cells = <1>;
>  				#size-cells = <0>;
> @@ -2672,13 +2679,20 @@ isp0vin07: endpoint {
>  		isp1: isp@fed20000 {
>  			compatible = "renesas,r8a779a0-isp",
>  				     "renesas,rcar-gen4-isp";
> -			reg = <0 0xfed20000 0 0x10000>;
> -			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> -			clocks = <&cpg CPG_MOD 613>;
> +			reg = <0 0xfed20000 0 0x10000>, <0 0xfee00000 0 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>;
> -			resets = <&cpg 613>;
> +			resets = <&cpg 613>, <&cpg 17>;
> +			reset-names = "cs", "core";
>  			status = "disabled";
>
> +			renesas,vspx = <&vspx1>;
> +
>  			ports {
>  				#address-cells = <1>;
>  				#size-cells = <0>;
> @@ -2756,13 +2770,20 @@ isp1vin15: endpoint {
>  		isp2: isp@fed30000 {
>  			compatible = "renesas,r8a779a0-isp",
>  				     "renesas,rcar-gen4-isp";
> -			reg = <0 0xfed30000 0 0x10000>;
> -			interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
> -			clocks = <&cpg CPG_MOD 614>;
> +			reg = <0 0xfed30000 0 0x10000>, <0 0xfef00000 0 0x100000>;
> +			reg-names = "cs", "core";
> +			interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "cs", "core";
> +			clocks = <&cpg CPG_MOD 614>, <&cpg CPG_MOD 18>;
> +			clock-names = "cs", "core";
>  			power-domains = <&sysc R8A779A0_PD_A3ISP23>;
> -			resets = <&cpg 614>;
> +			resets = <&cpg 614>, <&cpg 18>;
> +			reset-names = "cs", "core";
>  			status = "disabled";
>
> +			renesas,vspx = <&vspx2>;
> +
>  			ports {
>  				#address-cells = <1>;
>  				#size-cells = <0>;
> @@ -2840,13 +2861,20 @@ isp2vin23: endpoint {
>  		isp3: isp@fed40000 {
>  			compatible = "renesas,r8a779a0-isp",
>  				     "renesas,rcar-gen4-isp";
> -			reg = <0 0xfed40000 0 0x10000>;
> -			interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
> -			clocks = <&cpg CPG_MOD 615>;
> +			reg = <0 0xfed40000 0 0x10000>, <0 0xfe400000 0 0x100000>;
> +			reg-names = "cs", "core";
> +			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "cs", "core";
> +			clocks = <&cpg CPG_MOD 615>, <&cpg CPG_MOD 19>;
> +			clock-names = "cs", "core";
>  			power-domains = <&sysc R8A779A0_PD_A3ISP23>;
> -			resets = <&cpg 615>;
> +			resets = <&cpg 615>, <&cpg 19>;
> +			reset-names = "cs", "core";
>  			status = "disabled";
>
> +			renesas,vspx = <&vspx3>;
> +
>  			ports {
>  				#address-cells = <1>;
>  				#size-cells = <0>;
> --
> 2.48.1
>

Re: [PATCH 2/7] arm64: dts: renesas: r8a779a0: Add ISP core function block

[Niklas Söderlund]

Hi Jacopo,

Thanks for your feedback.

On 2025-03-19 15:50:00 +0100, Jacopo Mondi wrote:
> Hi Niklas
> 
> On Sat, Mar 15, 2025 at 04:27:03PM +0100, Niklas Söderlund wrote:
> > All ISP instances on V3U have both a channel select and core function
> > block, describe the core region in addition to the existing cs region.
> >
> > The interrupt number already described intended to reflect the cs
> > function but did incorrectly describe the core block. This was not
> 
> I can't find the interrupt mapping table for V3U, so this is the only
> thing I can't check

Page number 820, or search for "SPI 152" (fist one).

> 
> > noticed until now as the driver do not make use of the interrupt for the
> > cs block.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> The rest looks good
> 
> > ---
> >  arch/arm64/boot/dts/renesas/r8a779a0.dtsi | 60 +++++++++++++++++------
> >  1 file changed, 44 insertions(+), 16 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> > index f1613bfd1632..95ff69339991 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> > @@ -2588,13 +2588,20 @@ du_out_dsi1: endpoint {
> >  		isp0: isp@fed00000 {
> >  			compatible = "renesas,r8a779a0-isp",
> >  				     "renesas,rcar-gen4-isp";
> > -			reg = <0 0xfed00000 0 0x10000>;
> > -			interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> > -			clocks = <&cpg CPG_MOD 612>;
> > +			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
> > +			reg-names = "cs", "core";
> 
> However, won't the presence of a "core" part trigger the probing of
> the forthcoming RPP core support, which should not support V3U as far
> I understood ?


Correct the RPPX1 library will be given the change to probe on V3U, it 
will detect it's not an RPPX1 gracefully not create an ISPCORE on V3U.  
This describes the hardware, and there is an ISP core mapped at this 
address, not just the same as on the others ;-) The driver is prepared 
for this.

> 
> > +			interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> > +			interrupt-names = "cs", "core";
> > +			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
> > +			clock-names = "cs", "core";
> >  			power-domains = <&sysc R8A779A0_PD_A3ISP01>;
> > -			resets = <&cpg 612>;
> > +			resets = <&cpg 612>, <&cpg 16>;
> > +			reset-names = "cs", "core";
> >  			status = "disabled";
> >
> > +			renesas,vspx = <&vspx0>;
> > +
> >  			ports {
> >  				#address-cells = <1>;
> >  				#size-cells = <0>;
> > @@ -2672,13 +2679,20 @@ isp0vin07: endpoint {
> >  		isp1: isp@fed20000 {
> >  			compatible = "renesas,r8a779a0-isp",
> >  				     "renesas,rcar-gen4-isp";
> > -			reg = <0 0xfed20000 0 0x10000>;
> > -			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> > -			clocks = <&cpg CPG_MOD 613>;
> > +			reg = <0 0xfed20000 0 0x10000>, <0 0xfee00000 0 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>;
> > -			resets = <&cpg 613>;
> > +			resets = <&cpg 613>, <&cpg 17>;
> > +			reset-names = "cs", "core";
> >  			status = "disabled";
> >
> > +			renesas,vspx = <&vspx1>;
> > +
> >  			ports {
> >  				#address-cells = <1>;
> >  				#size-cells = <0>;
> > @@ -2756,13 +2770,20 @@ isp1vin15: endpoint {
> >  		isp2: isp@fed30000 {
> >  			compatible = "renesas,r8a779a0-isp",
> >  				     "renesas,rcar-gen4-isp";
> > -			reg = <0 0xfed30000 0 0x10000>;
> > -			interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
> > -			clocks = <&cpg CPG_MOD 614>;
> > +			reg = <0 0xfed30000 0 0x10000>, <0 0xfef00000 0 0x100000>;
> > +			reg-names = "cs", "core";
> > +			interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
> > +			interrupt-names = "cs", "core";
> > +			clocks = <&cpg CPG_MOD 614>, <&cpg CPG_MOD 18>;
> > +			clock-names = "cs", "core";
> >  			power-domains = <&sysc R8A779A0_PD_A3ISP23>;
> > -			resets = <&cpg 614>;
> > +			resets = <&cpg 614>, <&cpg 18>;
> > +			reset-names = "cs", "core";
> >  			status = "disabled";
> >
> > +			renesas,vspx = <&vspx2>;
> > +
> >  			ports {
> >  				#address-cells = <1>;
> >  				#size-cells = <0>;
> > @@ -2840,13 +2861,20 @@ isp2vin23: endpoint {
> >  		isp3: isp@fed40000 {
> >  			compatible = "renesas,r8a779a0-isp",
> >  				     "renesas,rcar-gen4-isp";
> > -			reg = <0 0xfed40000 0 0x10000>;
> > -			interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
> > -			clocks = <&cpg CPG_MOD 615>;
> > +			reg = <0 0xfed40000 0 0x10000>, <0 0xfe400000 0 0x100000>;
> > +			reg-names = "cs", "core";
> > +			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
> > +			interrupt-names = "cs", "core";
> > +			clocks = <&cpg CPG_MOD 615>, <&cpg CPG_MOD 19>;
> > +			clock-names = "cs", "core";
> >  			power-domains = <&sysc R8A779A0_PD_A3ISP23>;
> > -			resets = <&cpg 615>;
> > +			resets = <&cpg 615>, <&cpg 19>;
> > +			reset-names = "cs", "core";
> >  			status = "disabled";
> >
> > +			renesas,vspx = <&vspx3>;
> > +
> >  			ports {
> >  				#address-cells = <1>;
> >  				#size-cells = <0>;
> > --
> > 2.48.1
> >

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH 2/7] arm64: dts: renesas: r8a779a0: Add ISP core function block

[Jacopo Mondi]

Hi Niklas

On Wed, Mar 19, 2025 at 04:07:45PM +0100, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your feedback.
>
> On 2025-03-19 15:50:00 +0100, Jacopo Mondi wrote:
> > Hi Niklas
> >
> > On Sat, Mar 15, 2025 at 04:27:03PM +0100, Niklas Söderlund wrote:
> > > All ISP instances on V3U have both a channel select and core function
> > > block, describe the core region in addition to the existing cs region.
> > >
> > > The interrupt number already described intended to reflect the cs
> > > function but did incorrectly describe the core block. This was not
> >
> > I can't find the interrupt mapping table for V3U, so this is the only
> > thing I can't check
>
> Page number 820, or search for "SPI 152" (fist one).
>

Uh, thanks

> >
> > > noticed until now as the driver do not make use of the interrupt for the
> > > cs block.
> > >
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >
> > The rest looks good
> >
> > > ---
> > >  arch/arm64/boot/dts/renesas/r8a779a0.dtsi | 60 +++++++++++++++++------
> > >  1 file changed, 44 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> > > index f1613bfd1632..95ff69339991 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> > > @@ -2588,13 +2588,20 @@ du_out_dsi1: endpoint {
> > >  		isp0: isp@fed00000 {
> > >  			compatible = "renesas,r8a779a0-isp",
> > >  				     "renesas,rcar-gen4-isp";
> > > -			reg = <0 0xfed00000 0 0x10000>;
> > > -			interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> > > -			clocks = <&cpg CPG_MOD 612>;
> > > +			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
> > > +			reg-names = "cs", "core";
> >
> > However, won't the presence of a "core" part trigger the probing of
> > the forthcoming RPP core support, which should not support V3U as far
> > I understood ?
>
>
> Correct the RPPX1 library will be given the change to probe on V3U, it
> will detect it's not an RPPX1 gracefully not create an ISPCORE on V3U.
> This describes the hardware, and there is an ISP core mapped at this
> address, not just the same as on the others ;-) The driver is prepared
> for this.
>

Ack, just wanted to validat that

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> >
> > > +			interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> > > +			interrupt-names = "cs", "core";
> > > +			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
> > > +			clock-names = "cs", "core";
> > >  			power-domains = <&sysc R8A779A0_PD_A3ISP01>;
> > > -			resets = <&cpg 612>;
> > > +			resets = <&cpg 612>, <&cpg 16>;
> > > +			reset-names = "cs", "core";
> > >  			status = "disabled";
> > >
> > > +			renesas,vspx = <&vspx0>;
> > > +
> > >  			ports {
> > >  				#address-cells = <1>;
> > >  				#size-cells = <0>;
> > > @@ -2672,13 +2679,20 @@ isp0vin07: endpoint {
> > >  		isp1: isp@fed20000 {
> > >  			compatible = "renesas,r8a779a0-isp",
> > >  				     "renesas,rcar-gen4-isp";
> > > -			reg = <0 0xfed20000 0 0x10000>;
> > > -			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> > > -			clocks = <&cpg CPG_MOD 613>;
> > > +			reg = <0 0xfed20000 0 0x10000>, <0 0xfee00000 0 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>;
> > > -			resets = <&cpg 613>;
> > > +			resets = <&cpg 613>, <&cpg 17>;
> > > +			reset-names = "cs", "core";
> > >  			status = "disabled";
> > >
> > > +			renesas,vspx = <&vspx1>;
> > > +
> > >  			ports {
> > >  				#address-cells = <1>;
> > >  				#size-cells = <0>;
> > > @@ -2756,13 +2770,20 @@ isp1vin15: endpoint {
> > >  		isp2: isp@fed30000 {
> > >  			compatible = "renesas,r8a779a0-isp",
> > >  				     "renesas,rcar-gen4-isp";
> > > -			reg = <0 0xfed30000 0 0x10000>;
> > > -			interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
> > > -			clocks = <&cpg CPG_MOD 614>;
> > > +			reg = <0 0xfed30000 0 0x10000>, <0 0xfef00000 0 0x100000>;
> > > +			reg-names = "cs", "core";
> > > +			interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
> > > +			interrupt-names = "cs", "core";
> > > +			clocks = <&cpg CPG_MOD 614>, <&cpg CPG_MOD 18>;
> > > +			clock-names = "cs", "core";
> > >  			power-domains = <&sysc R8A779A0_PD_A3ISP23>;
> > > -			resets = <&cpg 614>;
> > > +			resets = <&cpg 614>, <&cpg 18>;
> > > +			reset-names = "cs", "core";
> > >  			status = "disabled";
> > >
> > > +			renesas,vspx = <&vspx2>;
> > > +
> > >  			ports {
> > >  				#address-cells = <1>;
> > >  				#size-cells = <0>;
> > > @@ -2840,13 +2861,20 @@ isp2vin23: endpoint {
> > >  		isp3: isp@fed40000 {
> > >  			compatible = "renesas,r8a779a0-isp",
> > >  				     "renesas,rcar-gen4-isp";
> > > -			reg = <0 0xfed40000 0 0x10000>;
> > > -			interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
> > > -			clocks = <&cpg CPG_MOD 615>;
> > > +			reg = <0 0xfed40000 0 0x10000>, <0 0xfe400000 0 0x100000>;
> > > +			reg-names = "cs", "core";
> > > +			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
> > > +			interrupt-names = "cs", "core";
> > > +			clocks = <&cpg CPG_MOD 615>, <&cpg CPG_MOD 19>;
> > > +			clock-names = "cs", "core";
> > >  			power-domains = <&sysc R8A779A0_PD_A3ISP23>;
> > > -			resets = <&cpg 615>;
> > > +			resets = <&cpg 615>, <&cpg 19>;
> > > +			reset-names = "cs", "core";
> > >  			status = "disabled";
> > >
> > > +			renesas,vspx = <&vspx3>;
> > > +
> > >  			ports {
> > >  				#address-cells = <1>;
> > >  				#size-cells = <0>;
> > > --
> > > 2.48.1
> > >
>
> --
> Kind Regards,
> Niklas Söderlund

Re: [PATCH 2/7] arm64: dts: renesas: r8a779a0: Add ISP core function block

[Geert Uytterhoeven]

Hi Niklas,

On Sat, 15 Mar 2025 at 16:28, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> All ISP instances on V3U have both a channel select and core function
> block, describe the core region in addition to the existing cs region.
>
> The interrupt number already described intended to reflect the cs
> function but did incorrectly describe the core block. This was not
> noticed until now as the driver do not make use of the interrupt for the
> cs block.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> @@ -2588,13 +2588,20 @@ du_out_dsi1: endpoint {
>                 isp0: isp@fed00000 {
>                         compatible = "renesas,r8a779a0-isp",
>                                      "renesas,rcar-gen4-isp";
> -                       reg = <0 0xfed00000 0 0x10000>;
> -                       interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> -                       clocks = <&cpg CPG_MOD 612>;
> +                       reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
> +                       reg-names = "cs", "core";
> +                       interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;

So we used to describe the "wrong" interrupt before, but it didn't hurt,
as the driver doesn't use it anyway?

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Queuing in renesas-devel is postponed, pending acceptance of the DT
binding changes.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/7] arm64: dts: renesas: r8a779a0: Add ISP core function block

[Niklas Söderlund]

Hi Geert,

Thanks for your feedback.

On 2025-04-10 17:54:25 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Sat, 15 Mar 2025 at 16:28, Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > All ISP instances on V3U have both a channel select and core function
> > block, describe the core region in addition to the existing cs region.
> >
> > The interrupt number already described intended to reflect the cs
> > function but did incorrectly describe the core block. This was not
> > noticed until now as the driver do not make use of the interrupt for the
> > cs block.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> > @@ -2588,13 +2588,20 @@ du_out_dsi1: endpoint {
> >                 isp0: isp@fed00000 {
> >                         compatible = "renesas,r8a779a0-isp",
> >                                      "renesas,rcar-gen4-isp";
> > -                       reg = <0 0xfed00000 0 0x10000>;
> > -                       interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> > -                       clocks = <&cpg CPG_MOD 612>;
> > +                       reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
> > +                       reg-names = "cs", "core";
> > +                       interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> 
> So we used to describe the "wrong" interrupt before, but it didn't hurt,
> as the driver doesn't use it anyway?

Correct.

> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Queuing in renesas-devel is postponed, pending acceptance of the DT
> binding changes.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Kind Regards,
Niklas Söderlund

[PATCH 3/7] arm64: dts: renesas: r8a779g0: Add ISP core function block

[Niklas Söderlund]

All ISP instances on V4H have both a channel select and core function
block, describe the core region in addition to the existing cs region.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 30 +++++++++++++++++------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
index 1760720b7128..6dbf05a55935 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
@@ -2277,13 +2277,20 @@ du_out_dsi1: endpoint {
 		isp0: isp@fed00000 {
 			compatible = "renesas,r8a779g0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed00000 0 0x10000>;
-			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_LOW>;
-			clocks = <&cpg CPG_MOD 612>;
+			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779G0_PD_A3ISP0>;
-			resets = <&cpg 612>;
+			resets = <&cpg 612>, <&cpg 16>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx0>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -2361,13 +2368,20 @@ isp0vin07: endpoint {
 		isp1: isp@fed20000 {
 			compatible = "renesas,r8a779g0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed20000 0 0x10000>;
-			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_LOW>;
-			clocks = <&cpg CPG_MOD 613>;
+			reg = <0 0xfed20000 0 0x10000>, <0 0xfee00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 476 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 613>, <&cpg CPG_MOD 17>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779G0_PD_A3ISP1>;
-			resets = <&cpg 613>;
+			resets = <&cpg 613>, <&cpg 17>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx1>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
2.48.1

Re: [PATCH 3/7] arm64: dts: renesas: r8a779g0: Add ISP core function block

[Jacopo Mondi]

Hi Niklas

On Sat, Mar 15, 2025 at 04:27:04PM +0100, Niklas Söderlund wrote:
> All ISP instances on V4H have both a channel select and core function
> block, describe the core region in addition to the existing cs region.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 30 +++++++++++++++++------
>  1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> index 1760720b7128..6dbf05a55935 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> @@ -2277,13 +2277,20 @@ du_out_dsi1: endpoint {
>  		isp0: isp@fed00000 {
>  			compatible = "renesas,r8a779g0-isp",
>  				     "renesas,rcar-gen4-isp";
> -			reg = <0 0xfed00000 0 0x10000>;
> -			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_LOW>;
> -			clocks = <&cpg CPG_MOD 612>;
> +			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
> +			reg-names = "cs", "core";
> +			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "cs", "core";
> +			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
> +			clock-names = "cs", "core";
>  			power-domains = <&sysc R8A779G0_PD_A3ISP0>;
> -			resets = <&cpg 612>;
> +			resets = <&cpg 612>, <&cpg 16>;
> +			reset-names = "cs", "core";
>  			status = "disabled";
>
> +			renesas,vspx = <&vspx0>;
> +
>  			ports {
>  				#address-cells = <1>;
>  				#size-cells = <0>;
> @@ -2361,13 +2368,20 @@ isp0vin07: endpoint {
>  		isp1: isp@fed20000 {
>  			compatible = "renesas,r8a779g0-isp",
>  				     "renesas,rcar-gen4-isp";
> -			reg = <0 0xfed20000 0 0x10000>;
> -			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_LOW>;
> -			clocks = <&cpg CPG_MOD 613>;
> +			reg = <0 0xfed20000 0 0x10000>, <0 0xfee00000 0 0x100000>;
> +			reg-names = "cs", "core";
> +			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 476 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "cs", "core";
> +			clocks = <&cpg CPG_MOD 613>, <&cpg CPG_MOD 17>;
> +			clock-names = "cs", "core";
>  			power-domains = <&sysc R8A779G0_PD_A3ISP1>;
> -			resets = <&cpg 613>;
> +			resets = <&cpg 613>, <&cpg 17>;
> +			reset-names = "cs", "core";
>  			status = "disabled";
>
> +			renesas,vspx = <&vspx1>;
> +
>  			ports {
>  				#address-cells = <1>;
>  				#size-cells = <0>;
> --
> 2.48.1
>
>

Re: [PATCH 3/7] arm64: dts: renesas: r8a779g0: Add ISP core function block

[Geert Uytterhoeven]

On Sat, 15 Mar 2025 at 16:28, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> All ISP instances on V4H have both a channel select and core function
> block, describe the core region in addition to the existing cs region.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Queuing in renesas-devel is postponed, pending acceptance of the DT
binding changes.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 4/7] arm64: dts: renesas: r8a779h0: Add ISP core function block

[Niklas Söderlund]

The first ISP instances on V4M have both a channel select and core
function block, describe the core region in addition to the existing cs
region. While at it update the second ISP to match the new bindings and
add the reg-names and interrupt-names property.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 arch/arm64/boot/dts/renesas/r8a779h0.dtsi | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
index 8524a1e7205e..ed1eefa3515d 100644
--- a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
@@ -1968,13 +1968,20 @@ du_out_dsi0: endpoint {
 		isp0: isp@fed00000 {
 			compatible = "renesas,r8a779h0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed00000 0 0x10000>;
-			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_LOW>;
-			clocks = <&cpg CPG_MOD 612>;
+			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779H0_PD_A3ISP0>;
-			resets = <&cpg 612>;
+			resets = <&cpg 612>, <&cpg 16>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx0>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -2053,10 +2060,14 @@ isp1: isp@fed20000 {
 			compatible = "renesas,r8a779h0-isp",
 				     "renesas,rcar-gen4-isp";
 			reg = <0 0xfed20000 0 0x10000>;
-			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_LOW>;
+			reg-names = "cs";
+			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs";
 			clocks = <&cpg CPG_MOD 613>;
+			clock-names = "cs";
 			power-domains = <&sysc R8A779H0_PD_A3ISP0>;
 			resets = <&cpg 613>;
+			reset-names = "cs";
 			status = "disabled";
 
 			ports {
-- 
2.48.1

Re: [PATCH 4/7] arm64: dts: renesas: r8a779h0: Add ISP core function block

[Jacopo Mondi]

Hi Niklas

On Sat, Mar 15, 2025 at 04:27:05PM +0100, Niklas Söderlund wrote:
> The first ISP instances on V4M have both a channel select and core

instance -> has

> function block, describe the core region in addition to the existing cs
> region. While at it update the second ISP to match the new bindings and
> add the reg-names and interrupt-names property.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  arch/arm64/boot/dts/renesas/r8a779h0.dtsi | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> index 8524a1e7205e..ed1eefa3515d 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> @@ -1968,13 +1968,20 @@ du_out_dsi0: endpoint {
>  		isp0: isp@fed00000 {
>  			compatible = "renesas,r8a779h0-isp",
>  				     "renesas,rcar-gen4-isp";
> -			reg = <0 0xfed00000 0 0x10000>;
> -			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_LOW>;
> -			clocks = <&cpg CPG_MOD 612>;
> +			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
> +			reg-names = "cs", "core";
> +			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "cs", "core";
> +			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
> +			clock-names = "cs", "core";
>  			power-domains = <&sysc R8A779H0_PD_A3ISP0>;
> -			resets = <&cpg 612>;
> +			resets = <&cpg 612>, <&cpg 16>;
> +			reset-names = "cs", "core";
>  			status = "disabled";
>
> +			renesas,vspx = <&vspx0>;
> +
>  			ports {
>  				#address-cells = <1>;
>  				#size-cells = <0>;
> @@ -2053,10 +2060,14 @@ isp1: isp@fed20000 {
>  			compatible = "renesas,r8a779h0-isp",
>  				     "renesas,rcar-gen4-isp";
>  			reg = <0 0xfed20000 0 0x10000>;
> -			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_LOW>;
> +			reg-names = "cs";
> +			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "cs";
>  			clocks = <&cpg CPG_MOD 613>;
> +			clock-names = "cs";
>  			power-domains = <&sysc R8A779H0_PD_A3ISP0>;
>  			resets = <&cpg 613>;
> +			reset-names = "cs";
>  			status = "disabled";
>
>  			ports {
> --
> 2.48.1
>
>

Re: [PATCH 4/7] arm64: dts: renesas: r8a779h0: Add ISP core function block

[Geert Uytterhoeven]

On Sat, 15 Mar 2025 at 16:28, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> The first ISP instances on V4M have both a channel select and core
> function block, describe the core region in addition to the existing cs
> region. While at it update the second ISP to match the new bindings and
> add the reg-names and interrupt-names property.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Queuing in renesas-devel is postponed, pending acceptance of the DT
binding changes.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 5/7] media: rcar-isp: Move driver to own directory

[Niklas Söderlund]

Before extending the driver with functions from the R-Car ISP core that
will span multiple files move the existing driver to a separate
directory. While at it rename the single source file to allow future
files to be grouped by functions.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 MAINTAINERS                                    |  2 +-
 drivers/media/platform/renesas/Kconfig         | 18 +-----------------
 drivers/media/platform/renesas/Makefile        |  2 +-
 .../media/platform/renesas/rcar-isp/Kconfig    | 17 +++++++++++++++++
 .../media/platform/renesas/rcar-isp/Makefile   |  4 ++++
 .../renesas/{rcar-isp.c => rcar-isp/csisp.c}   |  0
 6 files changed, 24 insertions(+), 19 deletions(-)
 create mode 100644 drivers/media/platform/renesas/rcar-isp/Kconfig
 create mode 100644 drivers/media/platform/renesas/rcar-isp/Makefile
 rename drivers/media/platform/renesas/{rcar-isp.c => rcar-isp/csisp.c} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index f3658f16fa24..c2f36486f5f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14679,7 +14679,7 @@ F:	Documentation/devicetree/bindings/media/renesas,csi2.yaml
 F:	Documentation/devicetree/bindings/media/renesas,isp.yaml
 F:	Documentation/devicetree/bindings/media/renesas,vin.yaml
 F:	drivers/media/platform/renesas/rcar-csi2.c
-F:	drivers/media/platform/renesas/rcar-isp.c
+F:	drivers/media/platform/renesas/rcar-isp/
 F:	drivers/media/platform/renesas/rcar-vin/
 
 MEDIA DRIVERS FOR RENESAS - VSP1
diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
index c7fc718a30a5..27a54fa79083 100644
--- a/drivers/media/platform/renesas/Kconfig
+++ b/drivers/media/platform/renesas/Kconfig
@@ -30,23 +30,6 @@ config VIDEO_RCAR_CSI2
 	  To compile this driver as a module, choose M here: the
 	  module will be called rcar-csi2.
 
-config VIDEO_RCAR_ISP
-	tristate "R-Car Image Signal Processor (ISP)"
-	depends on V4L_PLATFORM_DRIVERS
-	depends on VIDEO_DEV && OF
-	depends on ARCH_RENESAS || COMPILE_TEST
-	select MEDIA_CONTROLLER
-	select VIDEO_V4L2_SUBDEV_API
-	select RESET_CONTROLLER
-	select V4L2_FWNODE
-	help
-	  Support for Renesas R-Car Image Signal Processor (ISP).
-	  Enable this to support the Renesas R-Car Image Signal
-	  Processor (ISP).
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called rcar-isp.
-
 config VIDEO_SH_VOU
 	tristate "SuperH VOU video output driver"
 	depends on V4L_PLATFORM_DRIVERS
@@ -56,6 +39,7 @@ config VIDEO_SH_VOU
 	help
 	  Support for the Video Output Unit (VOU) on SuperH SoCs.
 
+source "drivers/media/platform/renesas/rcar-isp/Kconfig"
 source "drivers/media/platform/renesas/rcar-vin/Kconfig"
 source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
 
diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
index 50774a20330c..1127259c09d6 100644
--- a/drivers/media/platform/renesas/Makefile
+++ b/drivers/media/platform/renesas/Makefile
@@ -3,13 +3,13 @@
 # Makefile for the Renesas capture/playback device drivers.
 #
 
+obj-y += rcar-isp/
 obj-y += rcar-vin/
 obj-y += rzg2l-cru/
 obj-y += vsp1/
 
 obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o
 obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
-obj-$(CONFIG_VIDEO_RCAR_ISP) += rcar-isp.o
 obj-$(CONFIG_VIDEO_RENESAS_CEU) += renesas-ceu.o
 obj-$(CONFIG_VIDEO_RENESAS_FCP) += rcar-fcp.o
 obj-$(CONFIG_VIDEO_RENESAS_FDP1) += rcar_fdp1.o
diff --git a/drivers/media/platform/renesas/rcar-isp/Kconfig b/drivers/media/platform/renesas/rcar-isp/Kconfig
new file mode 100644
index 000000000000..59e0d91862d1
--- /dev/null
+++ b/drivers/media/platform/renesas/rcar-isp/Kconfig
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+config VIDEO_RCAR_ISP
+	tristate "R-Car Image Signal Processor (ISP)"
+	depends on V4L_PLATFORM_DRIVERS
+	depends on VIDEO_DEV && OF
+	depends on ARCH_RENESAS || COMPILE_TEST
+	select MEDIA_CONTROLLER
+	select VIDEO_V4L2_SUBDEV_API
+	select RESET_CONTROLLER
+	select V4L2_FWNODE
+	help
+	  Support for Renesas R-Car Image Signal Processor (ISP).
+	  Enable this to support the Renesas R-Car Image Signal
+	  Processor (ISP).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rcar-isp.
diff --git a/drivers/media/platform/renesas/rcar-isp/Makefile b/drivers/media/platform/renesas/rcar-isp/Makefile
new file mode 100644
index 000000000000..b542118c831e
--- /dev/null
+++ b/drivers/media/platform/renesas/rcar-isp/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+rcar-isp-objs = csisp.o
+
+obj-$(CONFIG_VIDEO_RCAR_ISP) += rcar-isp.o
diff --git a/drivers/media/platform/renesas/rcar-isp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
similarity index 100%
rename from drivers/media/platform/renesas/rcar-isp.c
rename to drivers/media/platform/renesas/rcar-isp/csisp.c
-- 
2.48.1

Re: [PATCH 5/7] media: rcar-isp: Move driver to own directory

[Jacopo Mondi]

Hi Niklas

On Sat, Mar 15, 2025 at 04:27:06PM +0100, Niklas Söderlund wrote:
> Before extending the driver with functions from the R-Car ISP core that
> will span multiple files move the existing driver to a separate
> directory. While at it rename the single source file to allow future
> files to be grouped by functions.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  MAINTAINERS                                    |  2 +-
>  drivers/media/platform/renesas/Kconfig         | 18 +-----------------
>  drivers/media/platform/renesas/Makefile        |  2 +-
>  .../media/platform/renesas/rcar-isp/Kconfig    | 17 +++++++++++++++++
>  .../media/platform/renesas/rcar-isp/Makefile   |  4 ++++
>  .../renesas/{rcar-isp.c => rcar-isp/csisp.c}   |  0
>  6 files changed, 24 insertions(+), 19 deletions(-)
>  create mode 100644 drivers/media/platform/renesas/rcar-isp/Kconfig
>  create mode 100644 drivers/media/platform/renesas/rcar-isp/Makefile
>  rename drivers/media/platform/renesas/{rcar-isp.c => rcar-isp/csisp.c} (100%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f3658f16fa24..c2f36486f5f5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14679,7 +14679,7 @@ F:	Documentation/devicetree/bindings/media/renesas,csi2.yaml
>  F:	Documentation/devicetree/bindings/media/renesas,isp.yaml
>  F:	Documentation/devicetree/bindings/media/renesas,vin.yaml
>  F:	drivers/media/platform/renesas/rcar-csi2.c
> -F:	drivers/media/platform/renesas/rcar-isp.c
> +F:	drivers/media/platform/renesas/rcar-isp/
>  F:	drivers/media/platform/renesas/rcar-vin/
>
>  MEDIA DRIVERS FOR RENESAS - VSP1
> diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
> index c7fc718a30a5..27a54fa79083 100644
> --- a/drivers/media/platform/renesas/Kconfig
> +++ b/drivers/media/platform/renesas/Kconfig
> @@ -30,23 +30,6 @@ config VIDEO_RCAR_CSI2
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called rcar-csi2.
>
> -config VIDEO_RCAR_ISP
> -	tristate "R-Car Image Signal Processor (ISP)"
> -	depends on V4L_PLATFORM_DRIVERS
> -	depends on VIDEO_DEV && OF
> -	depends on ARCH_RENESAS || COMPILE_TEST
> -	select MEDIA_CONTROLLER
> -	select VIDEO_V4L2_SUBDEV_API
> -	select RESET_CONTROLLER
> -	select V4L2_FWNODE
> -	help
> -	  Support for Renesas R-Car Image Signal Processor (ISP).
> -	  Enable this to support the Renesas R-Car Image Signal
> -	  Processor (ISP).
> -
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called rcar-isp.
> -
>  config VIDEO_SH_VOU
>  	tristate "SuperH VOU video output driver"
>  	depends on V4L_PLATFORM_DRIVERS
> @@ -56,6 +39,7 @@ config VIDEO_SH_VOU
>  	help
>  	  Support for the Video Output Unit (VOU) on SuperH SoCs.
>
> +source "drivers/media/platform/renesas/rcar-isp/Kconfig"
>  source "drivers/media/platform/renesas/rcar-vin/Kconfig"
>  source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
>
> diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
> index 50774a20330c..1127259c09d6 100644
> --- a/drivers/media/platform/renesas/Makefile
> +++ b/drivers/media/platform/renesas/Makefile
> @@ -3,13 +3,13 @@
>  # Makefile for the Renesas capture/playback device drivers.
>  #
>
> +obj-y += rcar-isp/
>  obj-y += rcar-vin/
>  obj-y += rzg2l-cru/
>  obj-y += vsp1/
>
>  obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o
>  obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
> -obj-$(CONFIG_VIDEO_RCAR_ISP) += rcar-isp.o
>  obj-$(CONFIG_VIDEO_RENESAS_CEU) += renesas-ceu.o
>  obj-$(CONFIG_VIDEO_RENESAS_FCP) += rcar-fcp.o
>  obj-$(CONFIG_VIDEO_RENESAS_FDP1) += rcar_fdp1.o
> diff --git a/drivers/media/platform/renesas/rcar-isp/Kconfig b/drivers/media/platform/renesas/rcar-isp/Kconfig
> new file mode 100644
> index 000000000000..59e0d91862d1
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rcar-isp/Kconfig
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: GPL-2.0
> +config VIDEO_RCAR_ISP
> +	tristate "R-Car Image Signal Processor (ISP)"
> +	depends on V4L_PLATFORM_DRIVERS
> +	depends on VIDEO_DEV && OF
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	select MEDIA_CONTROLLER
> +	select VIDEO_V4L2_SUBDEV_API
> +	select RESET_CONTROLLER
> +	select V4L2_FWNODE
> +	help
> +	  Support for Renesas R-Car Image Signal Processor (ISP).
> +	  Enable this to support the Renesas R-Car Image Signal
> +	  Processor (ISP).
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called rcar-isp.
> diff --git a/drivers/media/platform/renesas/rcar-isp/Makefile b/drivers/media/platform/renesas/rcar-isp/Makefile
> new file mode 100644
> index 000000000000..b542118c831e
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rcar-isp/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +rcar-isp-objs = csisp.o
> +
> +obj-$(CONFIG_VIDEO_RCAR_ISP) += rcar-isp.o
> diff --git a/drivers/media/platform/renesas/rcar-isp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> similarity index 100%
> rename from drivers/media/platform/renesas/rcar-isp.c
> rename to drivers/media/platform/renesas/rcar-isp/csisp.c
> --
> 2.48.1
>
>

[PATCH 6/7] media: rcar-isp: Rename base register variable

[Niklas Söderlund]

Prepare for extending the driver to in addition to support the channel
selector (CS) also support the core ISP. The two different functions
have different base addresses so the driver needs to distinguish between
them.

Prepare for this by marking existing base address variable and
read/write functions to make it clear it operates on the CS portion of
the driver. There is no functional change.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 .../media/platform/renesas/rcar-isp/csisp.c   | 46 +++++++++----------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index c515278e3be5..a86d2a9a4915 100644
--- a/drivers/media/platform/renesas/rcar-isp/csisp.c
+++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
@@ -111,7 +111,7 @@ enum rcar_isp_pads {
 
 struct rcar_isp {
 	struct device *dev;
-	void __iomem *base;
+	void __iomem *csbase;
 	struct reset_control *rstc;
 
 	enum rcar_isp_input csi_input;
@@ -137,14 +137,14 @@ static inline struct rcar_isp *notifier_to_isp(struct v4l2_async_notifier *n)
 	return container_of(n, struct rcar_isp, notifier);
 }
 
-static void risp_write(struct rcar_isp *isp, u32 offset, u32 value)
+static void risp_write_cs(struct rcar_isp *isp, u32 offset, u32 value)
 {
-	iowrite32(value, isp->base + offset);
+	iowrite32(value, isp->csbase + offset);
 }
 
-static u32 risp_read(struct rcar_isp *isp, u32 offset)
+static u32 risp_read_cs(struct rcar_isp *isp, u32 offset)
 {
-	return ioread32(isp->base + offset);
+	return ioread32(isp->csbase + offset);
 }
 
 static int risp_power_on(struct rcar_isp *isp)
@@ -193,31 +193,31 @@ static int risp_start(struct rcar_isp *isp)
 	if (isp->csi_input == RISP_CSI_INPUT1)
 		sel_csi = ISPINPUTSEL0_SEL_CSI0;
 
-	risp_write(isp, ISPINPUTSEL0_REG,
-		   risp_read(isp, ISPINPUTSEL0_REG) | sel_csi);
+	risp_write_cs(isp, ISPINPUTSEL0_REG,
+		      risp_read_cs(isp, ISPINPUTSEL0_REG) | sel_csi);
 
 	/* Configure Channel Selector. */
 	for (vc = 0; vc < 4; vc++) {
 		u8 ch = vc + 4;
 		u8 dt = format->datatype;
 
-		risp_write(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
-		risp_write(isp, ISPCS_DT_CODE03_CH_REG(ch),
-			   ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
-			   ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
-			   ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
-			   ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
+		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
+		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
+			      ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
+			      ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
+			      ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
+			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
 	}
 
 	/* Setup processing method. */
-	risp_write(isp, ISPPROCMODE_DT_REG(format->datatype),
-		   ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
-		   ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
-		   ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
-		   ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
+	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
+		      ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
+		      ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
+		      ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
+		      ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
 
 	/* Start ISP. */
-	risp_write(isp, ISPSTART_REG, ISPSTART_START);
+	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);
 
 	ret = v4l2_subdev_call(isp->remote, video, s_stream, 1);
 	if (ret)
@@ -231,7 +231,7 @@ static void risp_stop(struct rcar_isp *isp)
 	v4l2_subdev_call(isp->remote, video, s_stream, 0);
 
 	/* Stop ISP. */
-	risp_write(isp, ISPSTART_REG, ISPSTART_STOP);
+	risp_write_cs(isp, ISPSTART_REG, ISPSTART_STOP);
 
 	risp_power_off(isp);
 }
@@ -419,9 +419,9 @@ static const struct media_entity_operations risp_entity_ops = {
 static int risp_probe_resources(struct rcar_isp *isp,
 				struct platform_device *pdev)
 {
-	isp->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
-	if (IS_ERR(isp->base))
-		return PTR_ERR(isp->base);
+	isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+	if (IS_ERR(isp->csbase))
+		return PTR_ERR(isp->csbase);
 
 	isp->rstc = devm_reset_control_get(&pdev->dev, NULL);
 
-- 
2.48.1

Re: [PATCH 6/7] media: rcar-isp: Rename base register variable

[Jacopo Mondi]

Hi Niklas

On Sat, Mar 15, 2025 at 04:27:07PM +0100, Niklas Söderlund wrote:
> Prepare for extending the driver to in addition to support the channel
> selector (CS) also support the core ISP. The two different functions
> have different base addresses so the driver needs to distinguish between
> them.
>
> Prepare for this by marking existing base address variable and
> read/write functions to make it clear it operates on the CS portion of
> the driver. There is no functional change.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  .../media/platform/renesas/rcar-isp/csisp.c   | 46 +++++++++----------
>  1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index c515278e3be5..a86d2a9a4915 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -111,7 +111,7 @@ enum rcar_isp_pads {
>
>  struct rcar_isp {
>  	struct device *dev;
> -	void __iomem *base;
> +	void __iomem *csbase;
>  	struct reset_control *rstc;
>
>  	enum rcar_isp_input csi_input;
> @@ -137,14 +137,14 @@ static inline struct rcar_isp *notifier_to_isp(struct v4l2_async_notifier *n)
>  	return container_of(n, struct rcar_isp, notifier);
>  }
>
> -static void risp_write(struct rcar_isp *isp, u32 offset, u32 value)
> +static void risp_write_cs(struct rcar_isp *isp, u32 offset, u32 value)
>  {
> -	iowrite32(value, isp->base + offset);
> +	iowrite32(value, isp->csbase + offset);
>  }
>
> -static u32 risp_read(struct rcar_isp *isp, u32 offset)
> +static u32 risp_read_cs(struct rcar_isp *isp, u32 offset)
>  {
> -	return ioread32(isp->base + offset);
> +	return ioread32(isp->csbase + offset);
>  }
>
>  static int risp_power_on(struct rcar_isp *isp)
> @@ -193,31 +193,31 @@ static int risp_start(struct rcar_isp *isp)
>  	if (isp->csi_input == RISP_CSI_INPUT1)
>  		sel_csi = ISPINPUTSEL0_SEL_CSI0;
>
> -	risp_write(isp, ISPINPUTSEL0_REG,
> -		   risp_read(isp, ISPINPUTSEL0_REG) | sel_csi);
> +	risp_write_cs(isp, ISPINPUTSEL0_REG,
> +		      risp_read_cs(isp, ISPINPUTSEL0_REG) | sel_csi);
>
>  	/* Configure Channel Selector. */
>  	for (vc = 0; vc < 4; vc++) {
>  		u8 ch = vc + 4;
>  		u8 dt = format->datatype;
>
> -		risp_write(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> -		risp_write(isp, ISPCS_DT_CODE03_CH_REG(ch),
> -			   ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
> -			   ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
> -			   ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
> -			   ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
> +		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> +		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
> +			      ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
> +			      ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
> +			      ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
> +			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
>  	}
>
>  	/* Setup processing method. */
> -	risp_write(isp, ISPPROCMODE_DT_REG(format->datatype),
> -		   ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
> -		   ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
> -		   ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
> -		   ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
> +	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
> +		      ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
>
>  	/* Start ISP. */
> -	risp_write(isp, ISPSTART_REG, ISPSTART_START);
> +	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);
>
>  	ret = v4l2_subdev_call(isp->remote, video, s_stream, 1);
>  	if (ret)
> @@ -231,7 +231,7 @@ static void risp_stop(struct rcar_isp *isp)
>  	v4l2_subdev_call(isp->remote, video, s_stream, 0);
>
>  	/* Stop ISP. */
> -	risp_write(isp, ISPSTART_REG, ISPSTART_STOP);
> +	risp_write_cs(isp, ISPSTART_REG, ISPSTART_STOP);
>
>  	risp_power_off(isp);
>  }
> @@ -419,9 +419,9 @@ static const struct media_entity_operations risp_entity_ops = {
>  static int risp_probe_resources(struct rcar_isp *isp,
>  				struct platform_device *pdev)
>  {
> -	isp->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> -	if (IS_ERR(isp->base))
> -		return PTR_ERR(isp->base);
> +	isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> +	if (IS_ERR(isp->csbase))
> +		return PTR_ERR(isp->csbase);
>
>  	isp->rstc = devm_reset_control_get(&pdev->dev, NULL);
>
> --
> 2.48.1
>
>

[PATCH 7/7] media: rcar-isp: Parse named cs memory region

[Niklas Söderlund]

Extend the device tree parsing to optionally parse the cs memory region
by name. The change is backward compatible with the device tree model
where a single unnamed region describing only the ISP channel select
function.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/renesas/rcar-isp/csisp.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index a86d2a9a4915..931c8e3a22be 100644
--- a/drivers/media/platform/renesas/rcar-isp/csisp.c
+++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
@@ -419,7 +419,17 @@ static const struct media_entity_operations risp_entity_ops = {
 static int risp_probe_resources(struct rcar_isp *isp,
 				struct platform_device *pdev)
 {
-	isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+	struct resource *res;
+
+	/* For backward compatibility allow cs base to be the only reg if no
+	 * reg-names are set in DT.
+	 */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cs");
+	if (!res)
+		isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+	else
+		isp->csbase = devm_ioremap_resource(&pdev->dev, res);
+
 	if (IS_ERR(isp->csbase))
 		return PTR_ERR(isp->csbase);
 
-- 
2.48.1

Re: [PATCH 7/7] media: rcar-isp: Parse named cs memory region

[Jacopo Mondi]

Hi Niklas

On Sat, Mar 15, 2025 at 04:27:08PM +0100, Niklas Söderlund wrote:
> Extend the device tree parsing to optionally parse the cs memory region
> by name. The change is backward compatible with the device tree model
> where a single unnamed region describing only the ISP channel select
> function.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index a86d2a9a4915..931c8e3a22be 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -419,7 +419,17 @@ static const struct media_entity_operations risp_entity_ops = {
>  static int risp_probe_resources(struct rcar_isp *isp,
>  				struct platform_device *pdev)
>  {
> -	isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> +	struct resource *res;
> +
> +	/* For backward compatibility allow cs base to be the only reg if no
> +	 * reg-names are set in DT.
> +	 */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cs");
> +	if (!res)
> +		isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> +	else
> +		isp->csbase = devm_ioremap_resource(&pdev->dev, res);
> +
>  	if (IS_ERR(isp->csbase))
>  		return PTR_ERR(isp->csbase);
>
> --
> 2.48.1
>
>