[PATCH 0/2] renesas: ceu: Add R-Mobile A1

[PATCH 0/2] renesas: ceu: Add R-Mobile A1

[Jacopo Mondi]

Hello,
   this small series add R-Mobile A1 R8A7740 to the list of CEU supported
SoCs, and adds the CEU node to r8a7740.dtsi.

All the information on CEU clocks, power domains and memory regions have been
deducted from the now-deleted board file:
arch/arm/mach-shmobile/board-armadillo800eva.c

Thanks
   j

Jacopo Mondi (2):
  dt-bindings: media: renesas-ceu: Add R-Mobile R8A7740
  ARM: dts: r8a7740: Enable CEU0

 Documentation/devicetree/bindings/media/renesas,ceu.txt |  7 ++++---
 arch/arm/boot/dts/r8a7740.dtsi                          | 10 ++++++++++
 drivers/media/platform/renesas-ceu.c                    |  1 +
 3 files changed, 15 insertions(+), 3 deletions(-)

--
2.7.4

[PATCH 1/2] dt-bindings: media: renesas-ceu: Add R-Mobile R8A7740

[Jacopo Mondi]

Add R-Mobile A1 R8A7740 SoC to the list of compatible values for the CEU
unit.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/renesas,ceu.txt | 7 ++++---
 drivers/media/platform/renesas-ceu.c                    | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt b/Documentation/devicetree/bindings/media/renesas,ceu.txt
index 3fc66df..8a7a616 100644
--- a/Documentation/devicetree/bindings/media/renesas,ceu.txt
+++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
@@ -2,14 +2,15 @@ Renesas Capture Engine Unit (CEU)
 ----------------------------------------------
 
 The Capture Engine Unit is the image capture interface found in the Renesas
-SH Mobile and RZ SoCs.
+SH Mobile, R-Mobile and RZ SoCs.
 
 The interface supports a single parallel input with data bus width of 8 or 16
 bits.
 
 Required properties:
-- compatible: Shall be "renesas,r7s72100-ceu" for CEU units found in RZ/A1H
-  and RZ/A1M SoCs.
+- compatible: Shall be one of the following values:
+	"renesas,r7s72100-ceu" for CEU units found in RZ/A1H and RZ/A1M SoCs
+	"renesas,r8a7740-ceu" for CEU units found in R-Mobile A1 R8A7740 SoCs
 - reg: Registers address base and size.
 - interrupts: The interrupt specifier.
 
diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
index 6599dba..c964a56 100644
--- a/drivers/media/platform/renesas-ceu.c
+++ b/drivers/media/platform/renesas-ceu.c
@@ -1545,6 +1545,7 @@ static const struct ceu_data ceu_data_sh4 = {
 #if IS_ENABLED(CONFIG_OF)
 static const struct of_device_id ceu_of_match[] = {
 	{ .compatible = "renesas,r7s72100-ceu", .data = &ceu_data_rz },
+	{ .compatible = "renesas,r8a7740-ceu", .data = &ceu_data_rz },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ceu_of_match);
-- 
2.7.4

[PATCH 2/2] ARM: dts: r8a7740: Enable CEU0

[Jacopo Mondi]

Enable CEU0 peripheral for Renesas R-Mobile A1 R8A7740.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm/boot/dts/r8a7740.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
index afd3bc5..05ec41e 100644
--- a/arch/arm/boot/dts/r8a7740.dtsi
+++ b/arch/arm/boot/dts/r8a7740.dtsi
@@ -67,6 +67,16 @@
 		power-domains = <&pd_d4>;
 	};
 
+	ceu0: ceu@fe910000 {
+		reg = <0xfe910000 0x100>;
+		compatible = "renesas,r8a7740-ceu";
+		interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp1_clks R8A7740_CLK_CEU20>;
+		clock-names = "ceu20";
+		power-domains = <&pd_a4mp>;
+		status = "disabled";
+	};
+
 	cmt1: timer@e6138000 {
 		compatible = "renesas,cmt-48-r8a7740", "renesas,cmt-48";
 		reg = <0xe6138000 0x170>;
-- 
2.7.4

Re: [PATCH 2/2] ARM: dts: r8a7740: Enable CEU0

[Simon Horman]

Thanks Jacopo,

I'm very pleased to see this series.

On Wed, Apr 25, 2018 at 01:15:20PM +0200, Jacopo Mondi wrote:
> Enable CEU0 peripheral for Renesas R-Mobile A1 R8A7740.

Given 'status = "disabled"' below I think you
are describing but not enabling CEU0. Also in the subject.

Should we also describe CEU1?

> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm/boot/dts/r8a7740.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
> index afd3bc5..05ec41e 100644
> --- a/arch/arm/boot/dts/r8a7740.dtsi
> +++ b/arch/arm/boot/dts/r8a7740.dtsi
> @@ -67,6 +67,16 @@
>  		power-domains = <&pd_d4>;
>  	};
>  
> +	ceu0: ceu@fe910000 {
> +		reg = <0xfe910000 0x100>;

Should the size of the range be 0x3000 ?
That would seem to match my reading of table 32.3
and also be consistent with r7s72100.dtsi.

> +		compatible = "renesas,r8a7740-ceu";
> +		interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&mstp1_clks R8A7740_CLK_CEU20>;
> +		clock-names = "ceu20";
> +		power-domains = <&pd_a4mp>;

My reading of table 1.7 is that the power domain should be A4R (&pd_a4r).

> +		status = "disabled";
> +	};
> +
>  	cmt1: timer@e6138000 {
>  		compatible = "renesas,cmt-48-r8a7740", "renesas,cmt-48";
>  		reg = <0xe6138000 0x170>;
> -- 
> 2.7.4
> 

Re: [PATCH 2/2] ARM: dts: r8a7740: Enable CEU0

[jacopo mondi]

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

Hi Simon,

On Thu, Apr 26, 2018 at 08:11:30AM +0200, Simon Horman wrote:
> Thanks Jacopo,
>
> I'm very pleased to see this series.

Credits to Geert that pointed out to me R-Mobile A1 comes with a CEU.
I should mention him in next iteration actually, sorry about that.

>
> On Wed, Apr 25, 2018 at 01:15:20PM +0200, Jacopo Mondi wrote:
> > Enable CEU0 peripheral for Renesas R-Mobile A1 R8A7740.
>
> Given 'status = "disabled"' below I think you
> are describing but not enabling CEU0. Also in the subject.

Right.

>
> Should we also describe CEU1?

Armadillo board file only describe CEU0. If there are R-Mobile A1
board files where I can steal informations from I can do that. If
there's a public datasheet, that would be even better.
>
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm/boot/dts/r8a7740.dtsi | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
> > index afd3bc5..05ec41e 100644
> > --- a/arch/arm/boot/dts/r8a7740.dtsi
> > +++ b/arch/arm/boot/dts/r8a7740.dtsi
> > @@ -67,6 +67,16 @@
> >  		power-domains = <&pd_d4>;
> >  	};
> >
> > +	ceu0: ceu@fe910000 {
> > +		reg = <0xfe910000 0x100>;
>
> Should the size of the range be 0x3000 ?
> That would seem to match my reading of table 32.3
> and also be consistent with r7s72100.dtsi.

I got this from

static struct resource ceu0_resources[] = {
	[0] = {
		.name	= "CEU",
		.start	= 0xfe910000,
		.end	= 0xfe91009f,
		.flags	= IORESOURCE_MEM,
	},
but I also noticed the r7s72100 one was bigger.
I'm fine enlarging this, if that's what the manual reports too.

> > +		compatible = "renesas,r8a7740-ceu";
> > +		interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&mstp1_clks R8A7740_CLK_CEU20>;
> > +		clock-names = "ceu20";
> > +		power-domains = <&pd_a4mp>;
>
> My reading of table 1.7 is that the power domain should be A4R (&pd_a4r).

Ah yes, my bad.

The long time goal would be describe the camera module (mt9t112) which
is installed on armadillo. Unfortunately that would probably require
some more work on the CEU side.

Thanks
   j

>
> > +		status = "disabled";
> > +	};
> > +
> >  	cmt1: timer@e6138000 {
> >  		compatible = "renesas,cmt-48-r8a7740", "renesas,cmt-48";
> >  		reg = <0xe6138000 0x170>;
> > --
> > 2.7.4
> >

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

Re: [PATCH 2/2] ARM: dts: r8a7740: Enable CEU0

[Simon Horman]

On Thu, Apr 26, 2018 at 09:26:09AM +0200, jacopo mondi wrote:
> Hi Simon,
> 
> On Thu, Apr 26, 2018 at 08:11:30AM +0200, Simon Horman wrote:
> > Thanks Jacopo,
> >
> > I'm very pleased to see this series.
> 
> Credits to Geert that pointed out to me R-Mobile A1 comes with a CEU.
> I should mention him in next iteration actually, sorry about that.
> 
> >
> > On Wed, Apr 25, 2018 at 01:15:20PM +0200, Jacopo Mondi wrote:
> > > Enable CEU0 peripheral for Renesas R-Mobile A1 R8A7740.
> >
> > Given 'status = "disabled"' below I think you
> > are describing but not enabling CEU0. Also in the subject.
> 
> Right.
> 
> >
> > Should we also describe CEU1?
> 
> Armadillo board file only describe CEU0. If there are R-Mobile A1
> board files where I can steal informations from I can do that. If
> there's a public datasheet, that would be even better.

I have the datasheet, so perhaps I can add CEU1 after you have added CEU0.

> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  arch/arm/boot/dts/r8a7740.dtsi | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
> > > index afd3bc5..05ec41e 100644
> > > --- a/arch/arm/boot/dts/r8a7740.dtsi
> > > +++ b/arch/arm/boot/dts/r8a7740.dtsi
> > > @@ -67,6 +67,16 @@
> > >  		power-domains = <&pd_d4>;
> > >  	};
> > >
> > > +	ceu0: ceu@fe910000 {
> > > +		reg = <0xfe910000 0x100>;
> >
> > Should the size of the range be 0x3000 ?
> > That would seem to match my reading of table 32.3
> > and also be consistent with r7s72100.dtsi.
> 
> I got this from
> 
> static struct resource ceu0_resources[] = {
> 	[0] = {
> 		.name	= "CEU",
> 		.start	= 0xfe910000,
> 		.end	= 0xfe91009f,
> 		.flags	= IORESOURCE_MEM,
> 	},
> but I also noticed the r7s72100 one was bigger.
> I'm fine enlarging this, if that's what the manual reports too.

I think that would be best.

> > > +		compatible = "renesas,r8a7740-ceu";
> > > +		interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
> > > +		clocks = <&mstp1_clks R8A7740_CLK_CEU20>;
> > > +		clock-names = "ceu20";
> > > +		power-domains = <&pd_a4mp>;
> >
> > My reading of table 1.7 is that the power domain should be A4R (&pd_a4r).
> 
> Ah yes, my bad.
> 
> The long time goal would be describe the camera module (mt9t112) which
> is installed on armadillo. Unfortunately that would probably require
> some more work on the CEU side.

Thanks, understood.