[PATCH v2 0/6] rcar-isp: Add support for R-Car V4M

[PATCH v2 0/6] rcar-isp: Add support for R-Car V4M

[Niklas Söderlund]

Hi,

This series adds bindings and support to rcar-isp for R-Car V4M by the
means of adding a Gen4 family fallback compatible.

Previous versions of this work did not recognise that the CSISP on found 
on R-Car Gen4 is very similar and a family compatible to cover them all 
would have been beneficial. The same is true for the VIN module found on 
Gen4, and a similar series adds a family compatible for that modules 
[1]. This series mirrors that work for the CSISP module. Hopefully this 
pain now will pay off later.

This change requires updating existing DTS files to add this new family
fallback. This is done in a backward compatible way and the driver
retains the compatible values.

See individual patches for changes since previous versions.

1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
   https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/

Niklas Söderlund (6):
  dt-bindings: media: renesas,isp: Add Gen4 family fallback
  arm64: dts: renesas: r8a779g0: Add family fallback for CSISP IP
  arm64: dts: renesas: r8a779a0: Add family fallback for CSISP IP
  media: rcar-isp: Add family compatible for R-Car Gen4 family
  dt-bindings: media: renesas,isp: Add binding for V4M
  arm64: dts: renesas: r8a779h0: Add family fallback for CSISP IP

 .../devicetree/bindings/media/renesas,isp.yaml       |  4 +++-
 arch/arm64/boot/dts/renesas/r8a779a0.dtsi            | 12 ++++++++----
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi            |  6 ++++--
 arch/arm64/boot/dts/renesas/r8a779h0.dtsi            |  6 ++++--
 drivers/media/platform/renesas/rcar-isp.c            |  3 +++
 5 files changed, 22 insertions(+), 9 deletions(-)

-- 
2.45.2

[PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Niklas Söderlund]

The ISP Channel Selector IP is the same for all current Gen4 devices.
This was not known when adding support for V3U and V4H and a single SoC
specific compatible was used.

Before adding more SoC specific bindings for V4M add a family compatible
fallback for Gen4. That way the driver only needs to be updated once for
Gen4, and we still have the option to fix any problems in the driver if
any testable differences between the SoCs are found.

There are already DTS files using the V3U and V4H compatibles which
needs to be updated to not produce a warning for DTS checks. The driver
also needs to kept the compatible values to be backward compatible , but
for new Gen4 SoCs such as V4M we can avoid this.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v1
- New in v2.
---
 Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
index 33650a1ea034..730c86f2d7b1 100644
--- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
@@ -22,6 +22,7 @@ properties:
       - enum:
           - renesas,r8a779a0-isp # V3U
           - renesas,r8a779g0-isp # V4H
+      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
   reg:
     maxItems: 1
 
@@ -116,7 +117,7 @@ examples:
     #include <dt-bindings/power/r8a779a0-sysc.h>
 
     isp1: isp@fed20000 {
-            compatible = "renesas,r8a779a0-isp";
+            compatible = "renesas,r8a779a0-isp", "renesas,rcar-gen4-isp";
             reg = <0xfed20000 0x10000>;
             interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
             clocks = <&cpg CPG_MOD 613>;
-- 
2.45.2

[PATCH v2 2/6] arm64: dts: renesas: r8a779g0: Add family fallback for CSISP IP

[Niklas Söderlund]

To make it easier to support new Gen4 SoCs add a family fallback
compatible similar to what was done for VIN on Gen4.

There is no functional change, but the addition of the family fallback
in the bindings produces warnings for V4H for DTS checks if they are not
added.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v1
- New in v2.
---
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
index ac3d4062c2fa..ded805f0e633 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
@@ -2075,7 +2075,8 @@ du_out_dsi1: endpoint {
 		};
 
 		isp0: isp@fed00000 {
-			compatible = "renesas,r8a779g0-isp";
+			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>;
@@ -2158,7 +2159,8 @@ isp0vin07: endpoint {
 		};
 
 		isp1: isp@fed20000 {
-			compatible = "renesas,r8a779g0-isp";
+			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>;
-- 
2.45.2

[PATCH v2 3/6] arm64: dts: renesas: r8a779a0: Add family fallback for CSISP IP

[Niklas Söderlund]

To make it easier to support new Gen4 SoCs add a family fallback
compatible similar to what was done for VIN on Gen4.

There is no functional change, but the addition of the family fallback
in the bindings produces warnings for V3U for DTS checks if they are not
added.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v1
- New in v2.
---
 arch/arm64/boot/dts/renesas/r8a779a0.dtsi | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
index bf23cc1dc7be..1f4ab27acc33 100644
--- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
@@ -2501,7 +2501,8 @@ du_out_dsi1: endpoint {
 		};
 
 		isp0: isp@fed00000 {
-			compatible = "renesas,r8a779a0-isp";
+			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>;
@@ -2584,7 +2585,8 @@ isp0vin07: endpoint {
 		};
 
 		isp1: isp@fed20000 {
-			compatible = "renesas,r8a779a0-isp";
+			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>;
@@ -2667,7 +2669,8 @@ isp1vin15: endpoint {
 		};
 
 		isp2: isp@fed30000 {
-			compatible = "renesas,r8a779a0-isp";
+			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>;
@@ -2750,7 +2753,8 @@ isp2vin23: endpoint {
 		};
 
 		isp3: isp@fed40000 {
-			compatible = "renesas,r8a779a0-isp";
+			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>;
-- 
2.45.2

[PATCH v2 4/6] media: rcar-isp: Add family compatible for R-Car Gen4 family

[Niklas Söderlund]

Add the Gen4 family compatible. This will be used instead of a SoC
specific compatible for the new Gen4 SoC V4M. Two Gen4 boards (V3U and
V4H) have already been added prior and their bindings need to be kept
for backward compatibility.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v1
- Reworked to add a family compatible instead of V4M specific one.
---
 drivers/media/platform/renesas/rcar-isp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/renesas/rcar-isp.c b/drivers/media/platform/renesas/rcar-isp.c
index 4512ac338ca5..6504c7025a40 100644
--- a/drivers/media/platform/renesas/rcar-isp.c
+++ b/drivers/media/platform/renesas/rcar-isp.c
@@ -429,8 +429,11 @@ static int risp_probe_resources(struct rcar_isp *isp,
 }
 
 static const struct of_device_id risp_of_id_table[] = {
+	/* Keep to be compatible with old DTS files. */
 	{ .compatible = "renesas,r8a779a0-isp" },
+	/* Keep to be compatible with old DTS files. */
 	{ .compatible = "renesas,r8a779g0-isp" },
+	{ .compatible = "renesas,rcar-gen4-isp" },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, risp_of_id_table);
-- 
2.45.2

[PATCH v2 5/6] dt-bindings: media: renesas,isp: Add binding for V4M

[Niklas Söderlund]

Document support for the ISP module in the Renesas V4M (r8a779h0) SoC.
This device is compatible with the CSISP module on the other Gen4 SoCs.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v1
- Add to commit message that V4M is compatible with other Gen4 SoCs.
---
 Documentation/devicetree/bindings/media/renesas,isp.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
index 730c86f2d7b1..c4de4555b753 100644
--- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
@@ -22,6 +22,7 @@ properties:
       - enum:
           - renesas,r8a779a0-isp # V3U
           - renesas,r8a779g0-isp # V4H
+          - renesas,r8a779h0-isp # V4M
       - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
   reg:
     maxItems: 1
-- 
2.45.2

[PATCH v2 6/6] arm64: dts: renesas: r8a779h0: Add family fallback for CSISP IP

[Niklas Söderlund]

The usage of the V4M CSISP bindings where merged before the bindings
where approved. At that time the family fallback compatible where not
part of the bindings, add them.

Fixes: 2bb78d9fb7c9 ("arm64: dts: renesas: r8a779h0: Add video capture nodes")
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v1
- New in v2.
---
 arch/arm64/boot/dts/renesas/r8a779h0.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
index 28ca59e735e5..12d8be3fd579 100644
--- a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
@@ -1829,7 +1829,8 @@ csi41isp1: endpoint {
 		};
 
 		isp0: isp@fed00000 {
-			compatible = "renesas,r8a779h0-isp";
+			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>;
@@ -1912,7 +1913,8 @@ isp0vin07: endpoint {
 		};
 
 		isp1: isp@fed20000 {
-			compatible = "renesas,r8a779h0-isp";
+			compatible = "renesas,r8a779h0-isp",
+				     "renesas,rcar-gen4-isp";
 			reg = <0 0xfed20000 0 0x10000>;
 			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_LOW>;
 			clocks = <&cpg CPG_MOD 613>;
-- 
2.45.2

RE: [PATCH v2 4/6] media: rcar-isp: Add family compatible for R-Car Gen4 family

[Biju Das]

Hi Niklas Söderlund,

Thanks for the patch.

> -----Original Message-----
> From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Sent: Monday, August 26, 2024 3:44 PM
> Subject: [PATCH v2 4/6] media: rcar-isp: Add family compatible for R-Car Gen4 family
> 
> Add the Gen4 family compatible. This will be used instead of a SoC specific compatible for the new
> Gen4 SoC V4M. Two Gen4 boards (V3U and
> V4H) have already been added prior and their bindings need to be kept for backward compatibility.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - Reworked to add a family compatible instead of V4M specific one.
> ---
>  drivers/media/platform/renesas/rcar-isp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp.c b/drivers/media/platform/renesas/rcar-isp.c
> index 4512ac338ca5..6504c7025a40 100644
> --- a/drivers/media/platform/renesas/rcar-isp.c
> +++ b/drivers/media/platform/renesas/rcar-isp.c
> @@ -429,8 +429,11 @@ static int risp_probe_resources(struct rcar_isp *isp,  }
> 
>  static const struct of_device_id risp_of_id_table[] = {
> +	/* Keep to be compatible with old DTS files. */
>  	{ .compatible = "renesas,r8a779a0-isp" },
> +	/* Keep to be compatible with old DTS files. */
>  	{ .compatible = "renesas,r8a779g0-isp" },
> +	{ .compatible = "renesas,rcar-gen4-isp" },
>  	{ /* sentinel */ },

Nit pick:
    Comma can be dropped from last entry.

Cheers,
Biju

>  };
>  MODULE_DEVICE_TABLE(of, risp_of_id_table);
> --
> 2.45.2
> 

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Krzysztof Kozlowski]

On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
> The ISP Channel Selector IP is the same for all current Gen4 devices.
> This was not known when adding support for V3U and V4H and a single SoC
> specific compatible was used.
> 
> Before adding more SoC specific bindings for V4M add a family compatible
> fallback for Gen4. That way the driver only needs to be updated once for
> Gen4, and we still have the option to fix any problems in the driver if
> any testable differences between the SoCs are found.
> 
> There are already DTS files using the V3U and V4H compatibles which
> needs to be updated to not produce a warning for DTS checks. The driver
> also needs to kept the compatible values to be backward compatible , but
> for new Gen4 SoCs such as V4M we can avoid this.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - New in v2.
> ---
>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> index 33650a1ea034..730c86f2d7b1 100644
> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> @@ -22,6 +22,7 @@ properties:
>        - enum:
>            - renesas,r8a779a0-isp # V3U
>            - renesas,r8a779g0-isp # V4H
> +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4

Adding generic fallback post-factum is odd, does not feel reliable.
Instead use specific compatibles as fallbacks.

Best regards,
Krzysztof

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Niklas Söderlund]

On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
> On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
> > The ISP Channel Selector IP is the same for all current Gen4 devices.
> > This was not known when adding support for V3U and V4H and a single SoC
> > specific compatible was used.
> > 
> > Before adding more SoC specific bindings for V4M add a family compatible
> > fallback for Gen4. That way the driver only needs to be updated once for
> > Gen4, and we still have the option to fix any problems in the driver if
> > any testable differences between the SoCs are found.
> > 
> > There are already DTS files using the V3U and V4H compatibles which
> > needs to be updated to not produce a warning for DTS checks. The driver
> > also needs to kept the compatible values to be backward compatible , but
> > for new Gen4 SoCs such as V4M we can avoid this.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> > * Changes since v1
> > - New in v2.
> > ---
> >  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > index 33650a1ea034..730c86f2d7b1 100644
> > --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > @@ -22,6 +22,7 @@ properties:
> >        - enum:
> >            - renesas,r8a779a0-isp # V3U
> >            - renesas,r8a779g0-isp # V4H
> > +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
> 
> Adding generic fallback post-factum is odd, does not feel reliable.
> Instead use specific compatibles as fallbacks.

I agree, it feels a bit odd. But this was the road we hammered out at 
great pain for how to be able to move forward with this issue for the 
other IP block involved in video capture for R-Car Gen4, VIN [1]. This 
just mirrors that long discussion decision for the R-Car CSISP.

I would hate to have different solutions for the two.

1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
   https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v2 4/6] media: rcar-isp: Add family compatible for R-Car Gen4 family

[Niklas Söderlund]

Hello Biju,

Thanks for your feedback.

On 2024-08-26 14:48:07 +0000, Biju Das wrote:
> Hi Niklas Söderlund,
> 
> Thanks for the patch.
> 
> > -----Original Message-----
> > From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Sent: Monday, August 26, 2024 3:44 PM
> > Subject: [PATCH v2 4/6] media: rcar-isp: Add family compatible for R-Car Gen4 family
> > 
> > Add the Gen4 family compatible. This will be used instead of a SoC specific compatible for the new
> > Gen4 SoC V4M. Two Gen4 boards (V3U and
> > V4H) have already been added prior and their bindings need to be kept for backward compatibility.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> > * Changes since v1
> > - Reworked to add a family compatible instead of V4M specific one.
> > ---
> >  drivers/media/platform/renesas/rcar-isp.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/media/platform/renesas/rcar-isp.c b/drivers/media/platform/renesas/rcar-isp.c
> > index 4512ac338ca5..6504c7025a40 100644
> > --- a/drivers/media/platform/renesas/rcar-isp.c
> > +++ b/drivers/media/platform/renesas/rcar-isp.c
> > @@ -429,8 +429,11 @@ static int risp_probe_resources(struct rcar_isp *isp,  }
> > 
> >  static const struct of_device_id risp_of_id_table[] = {
> > +	/* Keep to be compatible with old DTS files. */
> >  	{ .compatible = "renesas,r8a779a0-isp" },
> > +	/* Keep to be compatible with old DTS files. */
> >  	{ .compatible = "renesas,r8a779g0-isp" },
> > +	{ .compatible = "renesas,rcar-gen4-isp" },
> >  	{ /* sentinel */ },
> 
> Nit pick:
>     Comma can be dropped from last entry.

Indeed it could be dropped from the "sentinel" entry, but that would be 
a different patch.

> 
> Cheers,
> Biju
> 
> >  };
> >  MODULE_DEVICE_TABLE(of, risp_of_id_table);
> > --
> > 2.45.2
> > 
> 

-- 
Kind Regards,
Niklas Söderlund

RE: [PATCH v2 4/6] media: rcar-isp: Add family compatible for R-Car Gen4 family

[Biju Das]

Hi Niklas,

> -----Original Message-----
> From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Sent: Tuesday, August 27, 2024 9:14 AM
> Subject: Re: [PATCH v2 4/6] media: rcar-isp: Add family compatible for R-Car Gen4 family
> 
> Hello Biju,
> 
> Thanks for your feedback.
> 
> On 2024-08-26 14:48:07 +0000, Biju Das wrote:
> > Hi Niklas Söderlund,
> >
> > Thanks for the patch.
> >
> > > -----Original Message-----
> > > From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > Sent: Monday, August 26, 2024 3:44 PM
> > > Subject: [PATCH v2 4/6] media: rcar-isp: Add family compatible for
> > > R-Car Gen4 family
> > >
> > > Add the Gen4 family compatible. This will be used instead of a SoC
> > > specific compatible for the new
> > > Gen4 SoC V4M. Two Gen4 boards (V3U and
> > > V4H) have already been added prior and their bindings need to be kept for backward compatibility.
> > >
> > > Signed-off-by: Niklas Söderlund
> > > <niklas.soderlund+renesas@ragnatech.se>
> > > ---
> > > * Changes since v1
> > > - Reworked to add a family compatible instead of V4M specific one.
> > > ---
> > >  drivers/media/platform/renesas/rcar-isp.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/media/platform/renesas/rcar-isp.c
> > > b/drivers/media/platform/renesas/rcar-isp.c
> > > index 4512ac338ca5..6504c7025a40 100644
> > > --- a/drivers/media/platform/renesas/rcar-isp.c
> > > +++ b/drivers/media/platform/renesas/rcar-isp.c
> > > @@ -429,8 +429,11 @@ static int risp_probe_resources(struct rcar_isp
> > > *isp,  }
> > >
> > >  static const struct of_device_id risp_of_id_table[] = {
> > > +	/* Keep to be compatible with old DTS files. */
> > >  	{ .compatible = "renesas,r8a779a0-isp" },
> > > +	/* Keep to be compatible with old DTS files. */
> > >  	{ .compatible = "renesas,r8a779g0-isp" },
> > > +	{ .compatible = "renesas,rcar-gen4-isp" },
> > >  	{ /* sentinel */ },
> >
> > Nit pick:
> >     Comma can be dropped from last entry.
> 
> Indeed it could be dropped from the "sentinel" entry, but that would be a different patch.

Agreed, It is a separate patch. since you are touching this part,I got similar comment 
in one of patches as "removing the trailing comma in the terminator entry for the ID
table making code robust against (theoretical) misrebases or other
similar things where the new entry goes _after_ the termination without
the compiler noticing."

Cheers,
Biju

> 
> >
> > Cheers,
> > Biju
> >
> > >  };
> > >  MODULE_DEVICE_TABLE(of, risp_of_id_table);
> > > --
> > > 2.45.2
> > >
> >
> 
> --
> Kind Regards,
> Niklas Söderlund

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Laurent Pinchart]

On Tue, Aug 27, 2024 at 10:12:33AM +0200, Niklas Söderlund wrote:
> On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
> > On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
> > > The ISP Channel Selector IP is the same for all current Gen4 devices.
> > > This was not known when adding support for V3U and V4H and a single SoC
> > > specific compatible was used.
> > > 
> > > Before adding more SoC specific bindings for V4M add a family compatible
> > > fallback for Gen4. That way the driver only needs to be updated once for
> > > Gen4, and we still have the option to fix any problems in the driver if
> > > any testable differences between the SoCs are found.
> > > 
> > > There are already DTS files using the V3U and V4H compatibles which
> > > needs to be updated to not produce a warning for DTS checks. The driver
> > > also needs to kept the compatible values to be backward compatible , but
> > > for new Gen4 SoCs such as V4M we can avoid this.
> > > 
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > ---
> > > * Changes since v1
> > > - New in v2.
> > > ---
> > >  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > > index 33650a1ea034..730c86f2d7b1 100644
> > > --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > > +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > > @@ -22,6 +22,7 @@ properties:
> > >        - enum:
> > >            - renesas,r8a779a0-isp # V3U
> > >            - renesas,r8a779g0-isp # V4H
> > > +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
> > 
> > Adding generic fallback post-factum is odd, does not feel reliable.
> > Instead use specific compatibles as fallbacks.
> 
> I agree, it feels a bit odd. But this was the road we hammered out at 
> great pain for how to be able to move forward with this issue for the 
> other IP block involved in video capture for R-Car Gen4, VIN [1]. This 
> just mirrors that long discussion decision for the R-Car CSISP.
> 
> I would hate to have different solutions for the two.
> 
> 1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
>    https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/

The compatible fallback for VIN has been added following a request from
Conor and Rob, so it would be nice if the three of you could agree to
achieve consistency in the bindings :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/6] arm64: dts: renesas: r8a779g0: Add family fallback for CSISP IP

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Aug 26, 2024 at 04:43:48PM +0200, Niklas Söderlund wrote:
> To make it easier to support new Gen4 SoCs add a family fallback
> compatible similar to what was done for VIN on Gen4.
> 
> There is no functional change, but the addition of the family fallback
> in the bindings produces warnings for V4H for DTS checks if they are not
> added.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Assuming 1/6 gets accepted,

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

> ---
> * Changes since v1
> - New in v2.
> ---
>  arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> index ac3d4062c2fa..ded805f0e633 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> @@ -2075,7 +2075,8 @@ du_out_dsi1: endpoint {
>  		};
>  
>  		isp0: isp@fed00000 {
> -			compatible = "renesas,r8a779g0-isp";
> +			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>;
> @@ -2158,7 +2159,8 @@ isp0vin07: endpoint {
>  		};
>  
>  		isp1: isp@fed20000 {
> -			compatible = "renesas,r8a779g0-isp";
> +			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>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/6] arm64: dts: renesas: r8a779a0: Add family fallback for CSISP IP

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Aug 26, 2024 at 04:43:49PM +0200, Niklas Söderlund wrote:
> To make it easier to support new Gen4 SoCs add a family fallback
> compatible similar to what was done for VIN on Gen4.
> 
> There is no functional change, but the addition of the family fallback
> in the bindings produces warnings for V3U for DTS checks if they are not
> added.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Assuming 1/6 gets accepted,

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

> ---
> * Changes since v1
> - New in v2.
> ---
>  arch/arm64/boot/dts/renesas/r8a779a0.dtsi | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> index bf23cc1dc7be..1f4ab27acc33 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> @@ -2501,7 +2501,8 @@ du_out_dsi1: endpoint {
>  		};
>  
>  		isp0: isp@fed00000 {
> -			compatible = "renesas,r8a779a0-isp";
> +			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>;
> @@ -2584,7 +2585,8 @@ isp0vin07: endpoint {
>  		};
>  
>  		isp1: isp@fed20000 {
> -			compatible = "renesas,r8a779a0-isp";
> +			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>;
> @@ -2667,7 +2669,8 @@ isp1vin15: endpoint {
>  		};
>  
>  		isp2: isp@fed30000 {
> -			compatible = "renesas,r8a779a0-isp";
> +			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>;
> @@ -2750,7 +2753,8 @@ isp2vin23: endpoint {
>  		};
>  
>  		isp3: isp@fed40000 {
> -			compatible = "renesas,r8a779a0-isp";
> +			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>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 4/6] media: rcar-isp: Add family compatible for R-Car Gen4 family

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Aug 26, 2024 at 04:43:50PM +0200, Niklas Söderlund wrote:
> Add the Gen4 family compatible. This will be used instead of a SoC
> specific compatible for the new Gen4 SoC V4M. Two Gen4 boards (V3U and
> V4H) have already been added prior and their bindings need to be kept
> for backward compatibility.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Assuming 1/6 gets accepted,

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

> ---
> * Changes since v1
> - Reworked to add a family compatible instead of V4M specific one.
> ---
>  drivers/media/platform/renesas/rcar-isp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp.c b/drivers/media/platform/renesas/rcar-isp.c
> index 4512ac338ca5..6504c7025a40 100644
> --- a/drivers/media/platform/renesas/rcar-isp.c
> +++ b/drivers/media/platform/renesas/rcar-isp.c
> @@ -429,8 +429,11 @@ static int risp_probe_resources(struct rcar_isp *isp,
>  }
>  
>  static const struct of_device_id risp_of_id_table[] = {
> +	/* Keep to be compatible with old DTS files. */
>  	{ .compatible = "renesas,r8a779a0-isp" },
> +	/* Keep to be compatible with old DTS files. */
>  	{ .compatible = "renesas,r8a779g0-isp" },
> +	{ .compatible = "renesas,rcar-gen4-isp" },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, risp_of_id_table);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 5/6] dt-bindings: media: renesas,isp: Add binding for V4M

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Aug 26, 2024 at 04:43:51PM +0200, Niklas Söderlund wrote:
> Document support for the ISP module in the Renesas V4M (r8a779h0) SoC.
> This device is compatible with the CSISP module on the other Gen4 SoCs.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

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

> ---
> * Changes since v1
> - Add to commit message that V4M is compatible with other Gen4 SoCs.
> ---
>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> index 730c86f2d7b1..c4de4555b753 100644
> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> @@ -22,6 +22,7 @@ properties:
>        - enum:
>            - renesas,r8a779a0-isp # V3U
>            - renesas,r8a779g0-isp # V4H
> +          - renesas,r8a779h0-isp # V4M
>        - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
>    reg:
>      maxItems: 1

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/6] arm64: dts: renesas: r8a779h0: Add family fallback for CSISP IP

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Aug 26, 2024 at 04:43:52PM +0200, Niklas Söderlund wrote:
> The usage of the V4M CSISP bindings where merged before the bindings
> where approved. At that time the family fallback compatible where not
> part of the bindings, add them.
> 
> Fixes: 2bb78d9fb7c9 ("arm64: dts: renesas: r8a779h0: Add video capture nodes")
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

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

> ---
> * Changes since v1
> - New in v2.
> ---
>  arch/arm64/boot/dts/renesas/r8a779h0.dtsi | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> index 28ca59e735e5..12d8be3fd579 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
> @@ -1829,7 +1829,8 @@ csi41isp1: endpoint {
>  		};
>  
>  		isp0: isp@fed00000 {
> -			compatible = "renesas,r8a779h0-isp";
> +			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>;
> @@ -1912,7 +1913,8 @@ isp0vin07: endpoint {
>  		};
>  
>  		isp1: isp@fed20000 {
> -			compatible = "renesas,r8a779h0-isp";
> +			compatible = "renesas,r8a779h0-isp",
> +				     "renesas,rcar-gen4-isp";
>  			reg = <0 0xfed20000 0 0x10000>;
>  			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_LOW>;
>  			clocks = <&cpg CPG_MOD 613>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Krzysztof Kozlowski]

On 27/08/2024 23:34, Laurent Pinchart wrote:
> On Tue, Aug 27, 2024 at 10:12:33AM +0200, Niklas Söderlund wrote:
>> On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
>>> On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
>>>> The ISP Channel Selector IP is the same for all current Gen4 devices.
>>>> This was not known when adding support for V3U and V4H and a single SoC
>>>> specific compatible was used.
>>>>
>>>> Before adding more SoC specific bindings for V4M add a family compatible
>>>> fallback for Gen4. That way the driver only needs to be updated once for
>>>> Gen4, and we still have the option to fix any problems in the driver if
>>>> any testable differences between the SoCs are found.
>>>>
>>>> There are already DTS files using the V3U and V4H compatibles which
>>>> needs to be updated to not produce a warning for DTS checks. The driver
>>>> also needs to kept the compatible values to be backward compatible , but
>>>> for new Gen4 SoCs such as V4M we can avoid this.
>>>>
>>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>>>> ---
>>>> * Changes since v1
>>>> - New in v2.
>>>> ---
>>>>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
>>>> index 33650a1ea034..730c86f2d7b1 100644
>>>> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
>>>> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
>>>> @@ -22,6 +22,7 @@ properties:
>>>>        - enum:
>>>>            - renesas,r8a779a0-isp # V3U
>>>>            - renesas,r8a779g0-isp # V4H
>>>> +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
>>>
>>> Adding generic fallback post-factum is odd, does not feel reliable.
>>> Instead use specific compatibles as fallbacks.
>>
>> I agree, it feels a bit odd. But this was the road we hammered out at 
>> great pain for how to be able to move forward with this issue for the 
>> other IP block involved in video capture for R-Car Gen4, VIN [1]. This 
>> just mirrors that long discussion decision for the R-Car CSISP.
>>
>> I would hate to have different solutions for the two.
>>
>> 1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
>>    https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/
> 
> The compatible fallback for VIN has been added following a request from
> Conor and Rob, so it would be nice if the three of you could agree to
> achieve consistency in the bindings :-)

Don't twist our answers. You need fallback, but specific, not family.
There was a countless number of answers from Rob that specific
compatibles are preferred.

Look, Conor's reply:

https://lore.kernel.org/all/20240620-gating-coherent-af984389b2d7@spud/
Do you see family fallback? I think "r8a779g0" is SoC.

Look here:
https://lore.kernel.org/all/20240610-screen-wolverine-78370c66d40f@spud/

Or here
https://lore.kernel.org/all/20240624-rented-danger-300652ab8eeb@wendy/
where Conor agrees against!

So let me actually NAK it - you got multiple comments on VIN to use
specific compatible.

Best regards,
Krzysztof

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Geert Uytterhoeven]

On Mon, Aug 26, 2024 at 4:44 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> The ISP Channel Selector IP is the same for all current Gen4 devices.
> This was not known when adding support for V3U and V4H and a single SoC
> specific compatible was used.
>
> Before adding more SoC specific bindings for V4M add a family compatible
> fallback for Gen4. That way the driver only needs to be updated once for
> Gen4, and we still have the option to fix any problems in the driver if
> any testable differences between the SoCs are found.
>
> There are already DTS files using the V3U and V4H compatibles which
> needs to be updated to not produce a warning for DTS checks. The driver
> also needs to kept the compatible values to be backward compatible , but
> for new Gen4 SoCs such as V4M we can avoid this.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - New in v2.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

This is in line with Section 2.3.1 ("Standard Properties / Compatible")
of the Devicetree Specification, Release v0.4, as it is used "to express
compatibility with a family of similar devices".  Note that this does
deviate slightly from the recommended format in the specification
("manufacturer,model"), as the fallback "model" specifies a family,
not a model number.

As such I think this is fine, and consistent with lots of other existing
family-specific compatible values.

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 v2 2/6] arm64: dts: renesas: r8a779g0: Add family fallback for CSISP IP

[Geert Uytterhoeven]

On Mon, Aug 26, 2024 at 4:44 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> To make it easier to support new Gen4 SoCs add a family fallback
> compatible similar to what was done for VIN on Gen4.
>
> There is no functional change, but the addition of the family fallback
> in the bindings produces warnings for V4H for DTS checks if they are not
> added.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - New in v2.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v2 3/6] arm64: dts: renesas: r8a779a0: Add family fallback for CSISP IP

[Geert Uytterhoeven]

On Mon, Aug 26, 2024 at 4:44 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> To make it easier to support new Gen4 SoCs add a family fallback
> compatible similar to what was done for VIN on Gen4.
>
> There is no functional change, but the addition of the family fallback
> in the bindings produces warnings for V3U for DTS checks if they are not
> added.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - New in v2.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v2 4/6] media: rcar-isp: Add family compatible for R-Car Gen4 family

[Geert Uytterhoeven]

Hi Niklas,

On Mon, Aug 26, 2024 at 4:44 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Add the Gen4 family compatible. This will be used instead of a SoC
> specific compatible for the new Gen4 SoC V4M. Two Gen4 boards (V3U and
> V4H) have already been added prior and their bindings need to be kept
> for backward compatibility.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - Reworked to add a family compatible instead of V4M specific one.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/drivers/media/platform/renesas/rcar-isp.c
> +++ b/drivers/media/platform/renesas/rcar-isp.c
> @@ -429,8 +429,11 @@ static int risp_probe_resources(struct rcar_isp *isp,
>  }
>
>  static const struct of_device_id risp_of_id_table[] = {
> +       /* Keep to be compatible with old DTS files. */
>         { .compatible = "renesas,r8a779a0-isp" },
> +       /* Keep to be compatible with old DTS files. */
>         { .compatible = "renesas,r8a779g0-isp" },

Nit: perhaps a single comment "/* Keep above for compatibility with
old DTB files */" here?

> +       { .compatible = "renesas,rcar-gen4-isp" },
>         { /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, risp_of_id_table);

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 v2 5/6] dt-bindings: media: renesas,isp: Add binding for V4M

[Geert Uytterhoeven]

On Mon, Aug 26, 2024 at 4:44 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Document support for the ISP module in the Renesas V4M (r8a779h0) SoC.
> This device is compatible with the CSISP module on the other Gen4 SoCs.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - Add to commit message that V4M is compatible with other Gen4 SoCs.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v2 6/6] arm64: dts: renesas: r8a779h0: Add family fallback for CSISP IP

[Geert Uytterhoeven]

On Mon, Aug 26, 2024 at 4:44 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> The usage of the V4M CSISP bindings where merged before the bindings
> where approved. At that time the family fallback compatible where not
> part of the bindings, add them.
>
> Fixes: 2bb78d9fb7c9 ("arm64: dts: renesas: r8a779h0: Add video capture nodes")
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - New in v2.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Laurent Pinchart]

On Wed, Aug 28, 2024 at 07:36:35AM +0200, Krzysztof Kozlowski wrote:
> On 27/08/2024 23:34, Laurent Pinchart wrote:
> > On Tue, Aug 27, 2024 at 10:12:33AM +0200, Niklas Söderlund wrote:
> >> On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
> >>> On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
> >>>> The ISP Channel Selector IP is the same for all current Gen4 devices.
> >>>> This was not known when adding support for V3U and V4H and a single SoC
> >>>> specific compatible was used.
> >>>>
> >>>> Before adding more SoC specific bindings for V4M add a family compatible
> >>>> fallback for Gen4. That way the driver only needs to be updated once for
> >>>> Gen4, and we still have the option to fix any problems in the driver if
> >>>> any testable differences between the SoCs are found.
> >>>>
> >>>> There are already DTS files using the V3U and V4H compatibles which
> >>>> needs to be updated to not produce a warning for DTS checks. The driver
> >>>> also needs to kept the compatible values to be backward compatible , but
> >>>> for new Gen4 SoCs such as V4M we can avoid this.
> >>>>
> >>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >>>> ---
> >>>> * Changes since v1
> >>>> - New in v2.
> >>>> ---
> >>>>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
> >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>>> index 33650a1ea034..730c86f2d7b1 100644
> >>>> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>>> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>>> @@ -22,6 +22,7 @@ properties:
> >>>>        - enum:
> >>>>            - renesas,r8a779a0-isp # V3U
> >>>>            - renesas,r8a779g0-isp # V4H
> >>>> +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
> >>>
> >>> Adding generic fallback post-factum is odd, does not feel reliable.
> >>> Instead use specific compatibles as fallbacks.
> >>
> >> I agree, it feels a bit odd. But this was the road we hammered out at 
> >> great pain for how to be able to move forward with this issue for the 
> >> other IP block involved in video capture for R-Car Gen4, VIN [1]. This 
> >> just mirrors that long discussion decision for the R-Car CSISP.
> >>
> >> I would hate to have different solutions for the two.
> >>
> >> 1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
> >>    https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/
> > 
> > The compatible fallback for VIN has been added following a request from
> > Conor and Rob, so it would be nice if the three of you could agree to
> > achieve consistency in the bindings :-)
> 
> Don't twist our answers. You need fallback, but specific, not family.
> There was a countless number of answers from Rob that specific
> compatibles are preferred.
> 
> Look, Conor's reply:
> 
> https://lore.kernel.org/all/20240620-gating-coherent-af984389b2d7@spud/
> Do you see family fallback? I think "r8a779g0" is SoC.
> 
> Look here:
> https://lore.kernel.org/all/20240610-screen-wolverine-78370c66d40f@spud/
> 
> Or here
> https://lore.kernel.org/all/20240624-rented-danger-300652ab8eeb@wendy/
> where Conor agrees against!
> 
> So let me actually NAK it - you got multiple comments on VIN to use
> specific compatible.

Krzysztof, this tone is not acceptable, regardless of the technical
argument. Period.

Rob, could you please mediate this ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Krzysztof Kozlowski]

On 28/08/2024 12:50, Laurent Pinchart wrote:
> On Wed, Aug 28, 2024 at 07:36:35AM +0200, Krzysztof Kozlowski wrote:
>> On 27/08/2024 23:34, Laurent Pinchart wrote:
>>> On Tue, Aug 27, 2024 at 10:12:33AM +0200, Niklas Söderlund wrote:
>>>> On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
>>>>> On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
>>>>>> The ISP Channel Selector IP is the same for all current Gen4 devices.
>>>>>> This was not known when adding support for V3U and V4H and a single SoC
>>>>>> specific compatible was used.
>>>>>>
>>>>>> Before adding more SoC specific bindings for V4M add a family compatible
>>>>>> fallback for Gen4. That way the driver only needs to be updated once for
>>>>>> Gen4, and we still have the option to fix any problems in the driver if
>>>>>> any testable differences between the SoCs are found.
>>>>>>
>>>>>> There are already DTS files using the V3U and V4H compatibles which
>>>>>> needs to be updated to not produce a warning for DTS checks. The driver
>>>>>> also needs to kept the compatible values to be backward compatible , but
>>>>>> for new Gen4 SoCs such as V4M we can avoid this.
>>>>>>
>>>>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>>>>>> ---
>>>>>> * Changes since v1
>>>>>> - New in v2.
>>>>>> ---
>>>>>>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
>>>>>> index 33650a1ea034..730c86f2d7b1 100644
>>>>>> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
>>>>>> @@ -22,6 +22,7 @@ properties:
>>>>>>        - enum:
>>>>>>            - renesas,r8a779a0-isp # V3U
>>>>>>            - renesas,r8a779g0-isp # V4H
>>>>>> +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
>>>>>
>>>>> Adding generic fallback post-factum is odd, does not feel reliable.
>>>>> Instead use specific compatibles as fallbacks.
>>>>
>>>> I agree, it feels a bit odd. But this was the road we hammered out at 
>>>> great pain for how to be able to move forward with this issue for the 
>>>> other IP block involved in video capture for R-Car Gen4, VIN [1]. This 
>>>> just mirrors that long discussion decision for the R-Car CSISP.
>>>>
>>>> I would hate to have different solutions for the two.
>>>>
>>>> 1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
>>>>    https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/
>>>
>>> The compatible fallback for VIN has been added following a request from
>>> Conor and Rob, so it would be nice if the three of you could agree to
>>> achieve consistency in the bindings :-)
>>
>> Don't twist our answers. You need fallback, but specific, not family.
>> There was a countless number of answers from Rob that specific
>> compatibles are preferred.
>>
>> Look, Conor's reply:
>>
>> https://lore.kernel.org/all/20240620-gating-coherent-af984389b2d7@spud/
>> Do you see family fallback? I think "r8a779g0" is SoC.
>>
>> Look here:
>> https://lore.kernel.org/all/20240610-screen-wolverine-78370c66d40f@spud/
>>
>> Or here
>> https://lore.kernel.org/all/20240624-rented-danger-300652ab8eeb@wendy/
>> where Conor agrees against!
>>
>> So let me actually NAK it - you got multiple comments on VIN to use
>> specific compatible.
> 
> Krzysztof, this tone is not acceptable, regardless of the technical
> argument. Period.

Except elevated arguments I don't think the tone is not acceptable.

Anyway, please provide references supporting your statement that Conor
and Rob encouraged using generic (not specific) fallback compatible.

I provided what I found, so I keep the discussion based on facts. I
expect the same from you.


Best regards,
Krzysztof

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Krzysztof Kozlowski]

On 27/08/2024 23:34, Laurent Pinchart wrote:
> On Tue, Aug 27, 2024 at 10:12:33AM +0200, Niklas Söderlund wrote:
>> On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
>>> On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
>>>> The ISP Channel Selector IP is the same for all current Gen4 devices.
>>>> This was not known when adding support for V3U and V4H and a single SoC
>>>> specific compatible was used.
>>>>
>>>> Before adding more SoC specific bindings for V4M add a family compatible
>>>> fallback for Gen4. That way the driver only needs to be updated once for
>>>> Gen4, and we still have the option to fix any problems in the driver if
>>>> any testable differences between the SoCs are found.
>>>>
>>>> There are already DTS files using the V3U and V4H compatibles which
>>>> needs to be updated to not produce a warning for DTS checks. The driver
>>>> also needs to kept the compatible values to be backward compatible , but
>>>> for new Gen4 SoCs such as V4M we can avoid this.
>>>>
>>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>>>> ---
>>>> * Changes since v1
>>>> - New in v2.
>>>> ---
>>>>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
>>>> index 33650a1ea034..730c86f2d7b1 100644
>>>> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
>>>> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
>>>> @@ -22,6 +22,7 @@ properties:
>>>>        - enum:
>>>>            - renesas,r8a779a0-isp # V3U
>>>>            - renesas,r8a779g0-isp # V4H
>>>> +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
>>>
>>> Adding generic fallback post-factum is odd, does not feel reliable.
>>> Instead use specific compatibles as fallbacks.
>>
>> I agree, it feels a bit odd. But this was the road we hammered out at 
>> great pain for how to be able to move forward with this issue for the 
>> other IP block involved in video capture for R-Car Gen4, VIN [1]. This 
>> just mirrors that long discussion decision for the R-Car CSISP.
>>
>> I would hate to have different solutions for the two.
>>
>> 1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
>>    https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/
> 
> The compatible fallback for VIN has been added following a request from
> Conor and Rob, so it would be nice if the three of you could agree to
> achieve consistency in the bindings :-)

We are consistent. Above snarky comment is not helping, so please keep
it civilized.

Best regards,
Krzysztof

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Rob Herring]

On Wed, Aug 28, 2024 at 07:36:35AM +0200, Krzysztof Kozlowski wrote:
> On 27/08/2024 23:34, Laurent Pinchart wrote:
> > On Tue, Aug 27, 2024 at 10:12:33AM +0200, Niklas Söderlund wrote:
> >> On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
> >>> On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
> >>>> The ISP Channel Selector IP is the same for all current Gen4 devices.
> >>>> This was not known when adding support for V3U and V4H and a single SoC
> >>>> specific compatible was used.
> >>>>
> >>>> Before adding more SoC specific bindings for V4M add a family compatible
> >>>> fallback for Gen4. That way the driver only needs to be updated once for
> >>>> Gen4, and we still have the option to fix any problems in the driver if
> >>>> any testable differences between the SoCs are found.
> >>>>
> >>>> There are already DTS files using the V3U and V4H compatibles which
> >>>> needs to be updated to not produce a warning for DTS checks. The driver
> >>>> also needs to kept the compatible values to be backward compatible , but
> >>>> for new Gen4 SoCs such as V4M we can avoid this.
> >>>>
> >>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >>>> ---
> >>>> * Changes since v1
> >>>> - New in v2.
> >>>> ---
> >>>>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
> >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>>> index 33650a1ea034..730c86f2d7b1 100644
> >>>> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>>> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>>> @@ -22,6 +22,7 @@ properties:
> >>>>        - enum:
> >>>>            - renesas,r8a779a0-isp # V3U
> >>>>            - renesas,r8a779g0-isp # V4H
> >>>> +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
> >>>
> >>> Adding generic fallback post-factum is odd, does not feel reliable.
> >>> Instead use specific compatibles as fallbacks.
> >>
> >> I agree, it feels a bit odd. But this was the road we hammered out at 
> >> great pain for how to be able to move forward with this issue for the 
> >> other IP block involved in video capture for R-Car Gen4, VIN [1]. This 
> >> just mirrors that long discussion decision for the R-Car CSISP.
> >>
> >> I would hate to have different solutions for the two.
> >>
> >> 1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
> >>    https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/
> > 
> > The compatible fallback for VIN has been added following a request from
> > Conor and Rob, so it would be nice if the three of you could agree to
> > achieve consistency in the bindings :-)
> 
> Don't twist our answers. You need fallback, but specific, not family.
> There was a countless number of answers from Rob that specific
> compatibles are preferred.

Preferred, definitely. But preferred is not absolute. The Renesas 
bindings have consistently followed the above style for some time. For 
the most part that has worked out it seems (based on Geert's slides 
linked in one of the threads). If you want to continue that here, it's 
not something I care to argue about.

However, I have to agree that adding the fallback after the fact is not 
ideal. Why design it where you have to carry renesas,r8a779g0-isp and 
renesas,rcar-gen4-isp in the driver forever when you could have 0 driver 
changes instead? The problem with genericish fallbacks is you have to 
know the future. Am I going to have a family of chips with the same 
block? It's much easier to just say "oh, this new chip is compatible 
with this old chip".


> Look, Conor's reply:
> 
> https://lore.kernel.org/all/20240620-gating-coherent-af984389b2d7@spud/
> Do you see family fallback? I think "r8a779g0" is SoC.
> 
> Look here:
> https://lore.kernel.org/all/20240610-screen-wolverine-78370c66d40f@spud/
> 
> Or here
> https://lore.kernel.org/all/20240624-rented-danger-300652ab8eeb@wendy/
> where Conor agrees against!

But he ultimately Acked it.

> 
> So let me actually NAK it - you got multiple comments on VIN to use
> specific compatible.

This doesn't help anything. You've made your point without it.

Rob

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Rob Herring]

On Wed, Aug 28, 2024 at 01:06:37PM +0200, Krzysztof Kozlowski wrote:
> On 28/08/2024 12:50, Laurent Pinchart wrote:
> > On Wed, Aug 28, 2024 at 07:36:35AM +0200, Krzysztof Kozlowski wrote:
> >> On 27/08/2024 23:34, Laurent Pinchart wrote:
> >>> On Tue, Aug 27, 2024 at 10:12:33AM +0200, Niklas Söderlund wrote:
> >>>> On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
> >>>>> On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
> >>>>>> The ISP Channel Selector IP is the same for all current Gen4 devices.
> >>>>>> This was not known when adding support for V3U and V4H and a single SoC
> >>>>>> specific compatible was used.
> >>>>>>
> >>>>>> Before adding more SoC specific bindings for V4M add a family compatible
> >>>>>> fallback for Gen4. That way the driver only needs to be updated once for
> >>>>>> Gen4, and we still have the option to fix any problems in the driver if
> >>>>>> any testable differences between the SoCs are found.
> >>>>>>
> >>>>>> There are already DTS files using the V3U and V4H compatibles which
> >>>>>> needs to be updated to not produce a warning for DTS checks. The driver
> >>>>>> also needs to kept the compatible values to be backward compatible , but
> >>>>>> for new Gen4 SoCs such as V4M we can avoid this.
> >>>>>>
> >>>>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >>>>>> ---
> >>>>>> * Changes since v1
> >>>>>> - New in v2.
> >>>>>> ---
> >>>>>>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
> >>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>>>>> index 33650a1ea034..730c86f2d7b1 100644
> >>>>>> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>>>>> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>>>>> @@ -22,6 +22,7 @@ properties:
> >>>>>>        - enum:
> >>>>>>            - renesas,r8a779a0-isp # V3U
> >>>>>>            - renesas,r8a779g0-isp # V4H
> >>>>>> +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
> >>>>>
> >>>>> Adding generic fallback post-factum is odd, does not feel reliable.
> >>>>> Instead use specific compatibles as fallbacks.
> >>>>
> >>>> I agree, it feels a bit odd. But this was the road we hammered out at 
> >>>> great pain for how to be able to move forward with this issue for the 
> >>>> other IP block involved in video capture for R-Car Gen4, VIN [1]. This 
> >>>> just mirrors that long discussion decision for the R-Car CSISP.
> >>>>
> >>>> I would hate to have different solutions for the two.
> >>>>
> >>>> 1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
> >>>>    https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/
> >>>
> >>> The compatible fallback for VIN has been added following a request from
> >>> Conor and Rob, so it would be nice if the three of you could agree to
> >>> achieve consistency in the bindings :-)
> >>
> >> Don't twist our answers. You need fallback, but specific, not family.
> >> There was a countless number of answers from Rob that specific
> >> compatibles are preferred.
> >>
> >> Look, Conor's reply:
> >>
> >> https://lore.kernel.org/all/20240620-gating-coherent-af984389b2d7@spud/
> >> Do you see family fallback? I think "r8a779g0" is SoC.
> >>
> >> Look here:
> >> https://lore.kernel.org/all/20240610-screen-wolverine-78370c66d40f@spud/
> >>
> >> Or here
> >> https://lore.kernel.org/all/20240624-rented-danger-300652ab8eeb@wendy/
> >> where Conor agrees against!
> >>
> >> So let me actually NAK it - you got multiple comments on VIN to use
> >> specific compatible.
> > 
> > Krzysztof, this tone is not acceptable, regardless of the technical
> > argument. Period.
> 
> Except elevated arguments I don't think the tone is not acceptable.

You cannot control nor change how someone interprets your tone, so there 
is little point in arguing about it. But it would be worthwhile to 
reflect on the comment.

> Anyway, please provide references supporting your statement that Conor
> and Rob encouraged using generic (not specific) fallback compatible.

Encouraged? Certainly not, but tolerated or allowed, yes. Every other 
Renesas binding reflects that.

Rob

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4951 bytes --]

On Wed, Aug 28, 2024 at 09:46:44AM -0500, Rob Herring wrote:
> On Wed, Aug 28, 2024 at 07:36:35AM +0200, Krzysztof Kozlowski wrote:
> > On 27/08/2024 23:34, Laurent Pinchart wrote:
> > > On Tue, Aug 27, 2024 at 10:12:33AM +0200, Niklas Söderlund wrote:
> > >> On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
> > >>> On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
> > >>>> The ISP Channel Selector IP is the same for all current Gen4 devices.
> > >>>> This was not known when adding support for V3U and V4H and a single SoC
> > >>>> specific compatible was used.
> > >>>>
> > >>>> Before adding more SoC specific bindings for V4M add a family compatible
> > >>>> fallback for Gen4. That way the driver only needs to be updated once for
> > >>>> Gen4, and we still have the option to fix any problems in the driver if
> > >>>> any testable differences between the SoCs are found.
> > >>>>
> > >>>> There are already DTS files using the V3U and V4H compatibles which
> > >>>> needs to be updated to not produce a warning for DTS checks. The driver
> > >>>> also needs to kept the compatible values to be backward compatible , but
> > >>>> for new Gen4 SoCs such as V4M we can avoid this.
> > >>>>
> > >>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > >>>> ---
> > >>>> * Changes since v1
> > >>>> - New in v2.
> > >>>> ---
> > >>>>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
> > >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > >>>> index 33650a1ea034..730c86f2d7b1 100644
> > >>>> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > >>>> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > >>>> @@ -22,6 +22,7 @@ properties:
> > >>>>        - enum:
> > >>>>            - renesas,r8a779a0-isp # V3U
> > >>>>            - renesas,r8a779g0-isp # V4H
> > >>>> +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
> > >>>
> > >>> Adding generic fallback post-factum is odd, does not feel reliable.
> > >>> Instead use specific compatibles as fallbacks.
> > >>
> > >> I agree, it feels a bit odd. But this was the road we hammered out at 
> > >> great pain for how to be able to move forward with this issue for the 
> > >> other IP block involved in video capture for R-Car Gen4, VIN [1]. This 
> > >> just mirrors that long discussion decision for the R-Car CSISP.
> > >>
> > >> I would hate to have different solutions for the two.
> > >>
> > >> 1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
> > >>    https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/
> > > 
> > > The compatible fallback for VIN has been added following a request from
> > > Conor and Rob, so it would be nice if the three of you could agree to
> > > achieve consistency in the bindings :-)
> > 
> > Don't twist our answers. You need fallback, but specific, not family.
> > There was a countless number of answers from Rob that specific
> > compatibles are preferred.
> 
> Preferred, definitely. But preferred is not absolute. The Renesas 
> bindings have consistently followed the above style for some time. For 
> the most part that has worked out it seems (based on Geert's slides 
> linked in one of the threads). If you want to continue that here, it's 
> not something I care to argue about.
> 
> However, I have to agree that adding the fallback after the fact is not 
> ideal. Why design it where you have to carry renesas,r8a779g0-isp and 
> renesas,rcar-gen4-isp in the driver forever when you could have 0 driver 
> changes instead? The problem with genericish fallbacks is you have to 
> know the future. Am I going to have a family of chips with the same 
> block? It's much easier to just say "oh, this new chip is compatible 
> with this old chip".

Yep, that's what I said pretty much. When I acked it I did so with the
comment:
| Same caveat here. Using the g model as a fallback is, as we already
| discussed, an option too and would be less disruptive.
| (at https://lore.kernel.org/all/20240626-unnatural-ember-26ae8895c008@spud/)

But...

> > Look, Conor's reply:
> > 
> > https://lore.kernel.org/all/20240620-gating-coherent-af984389b2d7@spud/
> > Do you see family fallback? I think "r8a779g0" is SoC.
> > 
> > Look here:
> > https://lore.kernel.org/all/20240610-screen-wolverine-78370c66d40f@spud/
> > 
> > Or here
> > https://lore.kernel.org/all/20240624-rented-danger-300652ab8eeb@wendy/
> > where Conor agrees against!
> 
> But he ultimately Acked it.

...since Geert was happy enough with taking the modifications to existing
devicetrees, and well aware of the pros/cons of each approach, I figured
I had argued it enough and let it be.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

[Geert Uytterhoeven]

On Wed, Aug 28, 2024 at 4:46 PM Rob Herring <robh@kernel.org> wrote:
> On Wed, Aug 28, 2024 at 07:36:35AM +0200, Krzysztof Kozlowski wrote:
> > On 27/08/2024 23:34, Laurent Pinchart wrote:
> > > On Tue, Aug 27, 2024 at 10:12:33AM +0200, Niklas Söderlund wrote:
> > >> On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
> > >>> On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
> > >>>> The ISP Channel Selector IP is the same for all current Gen4 devices.
> > >>>> This was not known when adding support for V3U and V4H and a single SoC
> > >>>> specific compatible was used.
> > >>>>
> > >>>> Before adding more SoC specific bindings for V4M add a family compatible
> > >>>> fallback for Gen4. That way the driver only needs to be updated once for
> > >>>> Gen4, and we still have the option to fix any problems in the driver if
> > >>>> any testable differences between the SoCs are found.
> > >>>>
> > >>>> There are already DTS files using the V3U and V4H compatibles which
> > >>>> needs to be updated to not produce a warning for DTS checks. The driver
> > >>>> also needs to kept the compatible values to be backward compatible , but
> > >>>> for new Gen4 SoCs such as V4M we can avoid this.
> > >>>>
> > >>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > >>>> ---
> > >>>> * Changes since v1
> > >>>> - New in v2.
> > >>>> ---
> > >>>>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
> > >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > >>>> index 33650a1ea034..730c86f2d7b1 100644
> > >>>> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > >>>> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > >>>> @@ -22,6 +22,7 @@ properties:
> > >>>>        - enum:
> > >>>>            - renesas,r8a779a0-isp # V3U
> > >>>>            - renesas,r8a779g0-isp # V4H
> > >>>> +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
> > >>>
> > >>> Adding generic fallback post-factum is odd, does not feel reliable.
> > >>> Instead use specific compatibles as fallbacks.
> > >>
> > >> I agree, it feels a bit odd. But this was the road we hammered out at
> > >> great pain for how to be able to move forward with this issue for the
> > >> other IP block involved in video capture for R-Car Gen4, VIN [1]. This
> > >> just mirrors that long discussion decision for the R-Car CSISP.
> > >>
> > >> I would hate to have different solutions for the two.
> > >>
> > >> 1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
> > >>    https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@ragnatech.se/
> > >
> > > The compatible fallback for VIN has been added following a request from
> > > Conor and Rob, so it would be nice if the three of you could agree to
> > > achieve consistency in the bindings :-)
> >
> > Don't twist our answers. You need fallback, but specific, not family.
> > There was a countless number of answers from Rob that specific
> > compatibles are preferred.
>
> Preferred, definitely. But preferred is not absolute. The Renesas
> bindings have consistently followed the above style for some time. For
> the most part that has worked out it seems (based on Geert's slides
> linked in one of the threads). If you want to continue that here, it's
> not something I care to argue about.
>
> However, I have to agree that adding the fallback after the fact is not
> ideal. Why design it where you have to carry renesas,r8a779g0-isp and
> renesas,rcar-gen4-isp in the driver forever when you could have 0 driver
> changes instead? The problem with genericish fallbacks is you have to
> know the future. Am I going to have a family of chips with the same
> block? It's much easier to just say "oh, this new chip is compatible
> with this old chip".

I agree it is not ideal, but it worked well for us.

R-Car Gen4 was a bit special, as its first member was named R-Car V3U,
and at that time it wasn't clear whether R-Car V3U was some sort
of one-off intermediate between Gen3 and Gen4, or the real thing.
In addition, the second Gen4 SoC (R-Car S4-8) is network-oriented,
and doesn't have any media support at all.  So we had to wait for
R-Car V4H and V4M to get a better picture of media commonalities.

Note that unlike on R-Car Gen2, R-Car Gen3 Video-In uses only
SoC-specific compatible values, as the VIN module on Gen3 had
SoC-specific routings, and a generic compatible value thus couldn't
work.  Hence for R-Car Gen4, we also started with only SoC-specific
compatible values, which turned out not needed (so far ;-) due to the
introduction of a new player in the pipeline (the ISP).

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 v2 5/6] dt-bindings: media: renesas,isp: Add binding for V4M

[Krzysztof Kozlowski]

On Mon, Aug 26, 2024 at 04:43:51PM +0200, Niklas Söderlund wrote:
> Document support for the ISP module in the Renesas V4M (r8a779h0) SoC.
> This device is compatible with the CSISP module on the other Gen4 SoCs.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - Add to commit message that V4M is compatible with other Gen4 SoCs.
> ---
>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof