[PATCH/RFC 0/4] ARM: shmobile: marzen: Initialise SCIF devices using DT

[PATCH/RFC 0/4] ARM: shmobile: marzen: Initialise SCIF devices using DT

[Simon Horman]

The aim of this series is to provide a working prototype for enabling
SCIF devices using DT on the marzen board when booting using multiplatform.

This series aims to be in keeping with the discussion both online
and offline relating to "[PATCH/RFC] ARM: shmobile: Koelsch DT serial
integration prototype" (Magnus Damm, 21st April 2014).

Other relevant work in this area includes:

* "[PATCH v4 0/2] [RESEND] Lager and Koelsch reference serial port support"
  (Laurent Pinchart, 11th February 2014).
  Deprecated by the above mentioned discussion.

* "[PATCH v4 00/11] CCF support for Renesas r7s72100"
  (Wolfram Sang, 28th April 2014).
  I believe the approach of Wolfram's r7s72100 work and
  my marzen work are quite similar if not the same.


This series depends on:
[PATCH v5 00/20] ARM: shmobile: r8a7779, marzen: CCF and multiplatform

This series was created and tested with the above mentioned series applied
on top of the renesas-next-v3.15-rc1-20140416 tag of my renesas tree.

This series works in conjunction with but does not have a run or compile
time dependency on:
[PATCH/RFC v2] serial: sh-sci: Add device tree support for r8a7779


Simon Horman (4):
  ARM: shmobile: r8a7779: Add scif nodes to dtsi
  ARM: shmobile: marzen: Remove early_printk from command line
  ARM: shmobile: marzen: Initialise SCIF devices using DT
  ARM: shmobile: marzen: Use disabled variant of clock workaround for
    scif devices

 arch/arm/boot/dts/r8a7779-marzen.dts            | 24 ++++++++--
 arch/arm/boot/dts/r8a7779.dtsi                  | 60 +++++++++++++++++++++++++
 arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++---
 arch/arm/mach-shmobile/setup-r8a7779.c          | 10 ++---
 4 files changed, 91 insertions(+), 15 deletions(-)

-- 
1.8.5.2

[PATCH 1/4] ARM: shmobile: r8a7779: Add scif nodes to dtsi

[Simon Horman]

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/boot/dts/r8a7779.dtsi | 60 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi
index e924f96..3e9cca4 100644
--- a/arch/arm/boot/dts/r8a7779.dtsi
+++ b/arch/arm/boot/dts/r8a7779.dtsi
@@ -203,6 +203,66 @@
 		status = "disabled";
 	};
 
+	scif0: serial at ffe40000 {
+		compatible = "renesas,scif", "renesas,scif-r8a7779";
+		reg = <0xffe40000 265>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 88 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clock-names = "sci_ick";
+		status = "disabled";
+	};
+
+	scif1: serial at ffe41000 {
+		compatible = "renesas,scif", "renesas,scif-r8a7779";
+		reg = <0xffe41000 265>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clock-names = "sci_ick";
+		status = "disabled";
+	};
+
+	scif2: serial at ffe42000 {
+		compatible = "renesas,scif", "renesas,scif-r8a7779";
+		reg = <0xffe42000 265>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clock-names = "sci_ick";
+		status = "disabled";
+	};
+
+	scif3: serial at ffe43000 {
+		compatible = "renesas,scif", "renesas,scif-r8a7779";
+		reg = <0xffe43000 265>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 91 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clock-names = "sci_ick";
+		status = "disabled";
+	};
+
+	scif4: serial at ffe44000 {
+		compatible = "renesas,scif", "renesas,scif-r8a7779";
+		reg = <0xffe44000 265>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 92 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clock-names = "sci_ick";
+		status = "disabled";
+	};
+
+	scif5: serial at ffe45000 {
+		compatible = "renesas,scif", "renesas,scif-r8a7779";
+		reg = <0xffe45000 265>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 93 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clock-names = "sci_ick";
+		status = "disabled";
+	};
+
 	pfc: pfc at fffc0000 {
 		compatible = "renesas,pfc-r8a7779";
 		reg = <0xfffc0000 0x23c>;
-- 
1.8.5.2

[PATCH 2/4] ARM: shmobile: marzen: Remove early_printk from command line

[Simon Horman]

As far as I understand things this does not have any affect.
So remove it.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/boot/dts/r8a7779-marzen.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/r8a7779-marzen.dts b/arch/arm/boot/dts/r8a7779-marzen.dts
index 2c727cd..769ae84 100644
--- a/arch/arm/boot/dts/r8a7779-marzen.dts
+++ b/arch/arm/boot/dts/r8a7779-marzen.dts
@@ -19,7 +19,7 @@
 	compatible = "renesas,marzen", "renesas,r8a7779";
 
 	chosen {
-		bootargs = "console=ttySC2,115200 earlyprintk=sh-sci.2,115200 ignore_loglevel root=/dev/nfs ip=on";
+		bootargs = "console=ttySC2,115200 ignore_loglevel root=/dev/nfs ip=on";
 	};
 
 	memory {
-- 
1.8.5.2

[PATCH 3/4] ARM: shmobile: marzen: Initialise SCIF devices using DT

[Simon Horman]

Initialise SCIF devices using DT when booting marzen
using multiplatform.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/boot/dts/r8a7779-marzen.dts   | 22 +++++++++++++++++++---
 arch/arm/mach-shmobile/setup-r8a7779.c | 10 +++++-----
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7779-marzen.dts b/arch/arm/boot/dts/r8a7779-marzen.dts
index 769ae84..6e603db 100644
--- a/arch/arm/boot/dts/r8a7779-marzen.dts
+++ b/arch/arm/boot/dts/r8a7779-marzen.dts
@@ -18,6 +18,11 @@
 	model = "marzen";
 	compatible = "renesas,marzen", "renesas,r8a7779";
 
+	aliases {
+		serial2 = &scif2;
+		serial4 = &scif4;
+	};
+
 	chosen {
 		bootargs = "console=ttySC2,115200 ignore_loglevel root=/dev/nfs ip=on";
 	};
@@ -73,9 +78,6 @@
 };
 
 &pfc {
-	pinctrl-0 = <&scif2_pins &scif4_pins>;
-	pinctrl-names = "default";
-
 	lan0_pins: lan0 {
 		intc {
 			renesas,groups = "intc_irq1_b";
@@ -108,6 +110,20 @@
 	};
 };
 
+&scif2 {
+       pinctrl-0 = <&scif2_pins>;
+       pinctrl-names = "default";
+
+       status = "okay";
+};
+
+&scif4 {
+       pinctrl-0 = <&scif4_pins>;
+       pinctrl-names = "default";
+
+       status = "okay";
+};
+
 &sdhi0 {
 	pinctrl-0 = <&sdhi0_pins>;
 	pinctrl-names = "default";
diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c b/arch/arm/mach-shmobile/setup-r8a7779.c
index 3471a9b..0d2910c 100644
--- a/arch/arm/mach-shmobile/setup-r8a7779.c
+++ b/arch/arm/mach-shmobile/setup-r8a7779.c
@@ -679,17 +679,17 @@ static void __init r8a7779_register_hpb_dmae(void)
 }
 
 static struct platform_device *r8a7779_devices_dt[] __initdata = {
+	&tmu00_device,
+	&tmu01_device,
+};
+
+static struct platform_device *r8a7779_standard_devices[] __initdata = {
 	&scif0_device,
 	&scif1_device,
 	&scif2_device,
 	&scif3_device,
 	&scif4_device,
 	&scif5_device,
-	&tmu00_device,
-	&tmu01_device,
-};
-
-static struct platform_device *r8a7779_standard_devices[] __initdata = {
 	&i2c0_device,
 	&i2c1_device,
 	&i2c2_device,
-- 
1.8.5.2

[PATCH 4/4] ARM: shmobile: marzen: Use disabled variant of clock workaround for scif devices

[Simon Horman]

Now that SCIF devices are initialised using DT it should
be sufficient to use the disabled variant of the clock workaround.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-marzen-reference.c b/arch/arm/mach-shmobile/board-marzen-reference.c
index 6d34baf..1f14d80 100644
--- a/arch/arm/mach-shmobile/board-marzen-reference.c
+++ b/arch/arm/mach-shmobile/board-marzen-reference.c
@@ -40,12 +40,6 @@ static void __init marzen_init_timer(void)
  * devices until they get moved to DT.
  */
 static const struct clk_name clk_names[] __initconst = {
-	{ "scif0", NULL, "sh-sci.0" },
-	{ "scif1", NULL, "sh-sci.1" },
-	{ "scif2", NULL, "sh-sci.2" },
-	{ "scif3", NULL, "sh-sci.3" },
-	{ "scif4", NULL, "sh-sci.4" },
-	{ "scif5", NULL, "sh-sci.5" },
 	{ "tmu0", NULL, "sh_tmu.0" },
 	{ "tmu1", NULL, "sh_tmu.1" },
 	{ "tmu2", NULL, "sh_tmu.2" },
@@ -55,6 +49,12 @@ static const struct clk_name clk_names[] __initconst = {
  * This is a really crude hack to work around core platform clock issues
  */
 static const struct clk_name clk_enables[] __initconst = {
+	{ "scif0", NULL, "sh-sci.0" },
+	{ "scif1", NULL, "sh-sci.1" },
+	{ "scif2", NULL, "sh-sci.2" },
+	{ "scif3", NULL, "sh-sci.3" },
+	{ "scif4", NULL, "sh-sci.4" },
+	{ "scif5", NULL, "sh-sci.5" },
 	{ "sdhi0", NULL, "ffe4c000.sd" },
 	{ "thermal", NULL, "ffc48000.thermal" },
 };
-- 
1.8.5.2

[PATCH 4/4] ARM: shmobile: marzen: Use disabled variant of clock workaround for scif devices

[Magnus Damm]

On Tue, Apr 29, 2014 at 7:43 AM, Simon Horman
<horms+renesas@verge.net.au> wrote:
> Now that SCIF devices are initialised using DT it should
> be sufficient to use the disabled variant of the clock workaround.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/board-marzen-reference.c b/arch/arm/mach-shmobile/board-marzen-reference.c
> index 6d34baf..1f14d80 100644
> --- a/arch/arm/mach-shmobile/board-marzen-reference.c
> +++ b/arch/arm/mach-shmobile/board-marzen-reference.c
> @@ -40,12 +40,6 @@ static void __init marzen_init_timer(void)
>   * devices until they get moved to DT.
>   */
>  static const struct clk_name clk_names[] __initconst = {
> -       { "scif0", NULL, "sh-sci.0" },
> -       { "scif1", NULL, "sh-sci.1" },
> -       { "scif2", NULL, "sh-sci.2" },
> -       { "scif3", NULL, "sh-sci.3" },
> -       { "scif4", NULL, "sh-sci.4" },
> -       { "scif5", NULL, "sh-sci.5" },
>         { "tmu0", NULL, "sh_tmu.0" },
>         { "tmu1", NULL, "sh_tmu.1" },
>         { "tmu2", NULL, "sh_tmu.2" },
> @@ -55,6 +49,12 @@ static const struct clk_name clk_names[] __initconst = {
>   * This is a really crude hack to work around core platform clock issues
>   */
>  static const struct clk_name clk_enables[] __initconst = {
> +       { "scif0", NULL, "sh-sci.0" },
> +       { "scif1", NULL, "sh-sci.1" },
> +       { "scif2", NULL, "sh-sci.2" },
> +       { "scif3", NULL, "sh-sci.3" },
> +       { "scif4", NULL, "sh-sci.4" },
> +       { "scif5", NULL, "sh-sci.5" },
>         { "sdhi0", NULL, "ffe4c000.sd" },
>         { "thermal", NULL, "ffc48000.thermal" },
>  };

Hi Simon,

Thanks for your SCIF DT patches for r8a7779. They look good and clean
in general I think.

Regarding this specific patch, I'm not sure why you need to perform
this kind of change. Is it following the same style as other SoCs?

In my mind the SCIF driver at least used to rely on both Runtime PM
and the clock framework for clock control so only relying on clock
framework should be enough for now. The driver may have been updated
though, but if so we should make sure we follow the same pattern on
other SoCs as well.

Thanks,

/ magnus

[PATCH 4/4] ARM: shmobile: marzen: Use disabled variant of clock workaround for scif devices

[Simon Horman]

On Tue, Apr 29, 2014 at 01:40:08PM +0000, Magnus Damm wrote:
> On Tue, Apr 29, 2014 at 7:43 AM, Simon Horman
> <horms+renesas@verge.net.au> wrote:
> > Now that SCIF devices are initialised using DT it should
> > be sufficient to use the disabled variant of the clock workaround.
> >
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> >  arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm/mach-shmobile/board-marzen-reference.c b/arch/arm/mach-shmobile/board-marzen-reference.c
> > index 6d34baf..1f14d80 100644
> > --- a/arch/arm/mach-shmobile/board-marzen-reference.c
> > +++ b/arch/arm/mach-shmobile/board-marzen-reference.c
> > @@ -40,12 +40,6 @@ static void __init marzen_init_timer(void)
> >   * devices until they get moved to DT.
> >   */
> >  static const struct clk_name clk_names[] __initconst = {
> > -       { "scif0", NULL, "sh-sci.0" },
> > -       { "scif1", NULL, "sh-sci.1" },
> > -       { "scif2", NULL, "sh-sci.2" },
> > -       { "scif3", NULL, "sh-sci.3" },
> > -       { "scif4", NULL, "sh-sci.4" },
> > -       { "scif5", NULL, "sh-sci.5" },
> >         { "tmu0", NULL, "sh_tmu.0" },
> >         { "tmu1", NULL, "sh_tmu.1" },
> >         { "tmu2", NULL, "sh_tmu.2" },
> > @@ -55,6 +49,12 @@ static const struct clk_name clk_names[] __initconst = {
> >   * This is a really crude hack to work around core platform clock issues
> >   */
> >  static const struct clk_name clk_enables[] __initconst = {
> > +       { "scif0", NULL, "sh-sci.0" },
> > +       { "scif1", NULL, "sh-sci.1" },
> > +       { "scif2", NULL, "sh-sci.2" },
> > +       { "scif3", NULL, "sh-sci.3" },
> > +       { "scif4", NULL, "sh-sci.4" },
> > +       { "scif5", NULL, "sh-sci.5" },
> >         { "sdhi0", NULL, "ffe4c000.sd" },
> >         { "thermal", NULL, "ffc48000.thermal" },
> >  };
> 
> Hi Simon,
> 
> Thanks for your SCIF DT patches for r8a7779. They look good and clean
> in general I think.
> 
> Regarding this specific patch, I'm not sure why you need to perform
> this kind of change. Is it following the same style as other SoCs?
> 
> In my mind the SCIF driver at least used to rely on both Runtime PM
> and the clock framework for clock control so only relying on clock
> framework should be enough for now. The driver may have been updated
> though, but if so we should make sure we follow the same pattern on
> other SoCs as well.

Thanks, I agree this is not needed.

[PATCH 1/4] ARM: shmobile: r8a7779: Add scif nodes to dtsi

[Laurent Pinchart]

Hi Simon,

Thank you for the patch.

On Tuesday 29 April 2014 16:43:23 Simon Horman wrote:
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm/boot/dts/r8a7779.dtsi | 60 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi
> index e924f96..3e9cca4 100644
> --- a/arch/arm/boot/dts/r8a7779.dtsi
> +++ b/arch/arm/boot/dts/r8a7779.dtsi
> @@ -203,6 +203,66 @@
>  		status = "disabled";
>  	};
> 
> +	scif0: serial at ffe40000 {
> +		compatible = "renesas,scif", "renesas,scif-r8a7779";

The most specific compatible string should come first, so this should read

		compatible = "renesas,scif-r8a7779", "renesas,scif";

> +		reg = <0xffe40000 265>;

265 ? Don't you mean 256 ? 0x100 might be a better option.

> +		interrupt-parent = <&gic>;
> +		interrupts = <0 88 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> +		clock-names = "sci_ick";

Could you try the SCI RFC patch set I've just sent that drop the interface 
clock in favour of the functional clock ?

> +		status = "disabled";
> +	};
> +
> +	scif1: serial at ffe41000 {
> +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> +		reg = <0xffe41000 265>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> +		clock-names = "sci_ick";
> +		status = "disabled";
> +	};
> +
> +	scif2: serial at ffe42000 {
> +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> +		reg = <0xffe42000 265>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> +		clock-names = "sci_ick";
> +		status = "disabled";
> +	};
> +
> +	scif3: serial at ffe43000 {
> +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> +		reg = <0xffe43000 265>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 91 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> +		clock-names = "sci_ick";
> +		status = "disabled";
> +	};
> +
> +	scif4: serial at ffe44000 {
> +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> +		reg = <0xffe44000 265>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 92 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> +		clock-names = "sci_ick";
> +		status = "disabled";
> +	};
> +
> +	scif5: serial at ffe45000 {
> +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> +		reg = <0xffe45000 265>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 93 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> +		clock-names = "sci_ick";
> +		status = "disabled";
> +	};
> +
>  	pfc: pfc at fffc0000 {
>  		compatible = "renesas,pfc-r8a7779";
>  		reg = <0xfffc0000 0x23c>;

-- 
Regards,

Laurent Pinchart

[PATCH 2/4] ARM: shmobile: marzen: Remove early_printk from command line

[Laurent Pinchart]

Hi Simon,

Thank you for the patch.

On Tuesday 29 April 2014 16:43:24 Simon Horman wrote:
> As far as I understand things this does not have any affect.
> So remove it.

The earlyprintk parameter actually makes a different when early printk support 
is enabled in the kernel. As that's not enabled by default, dropping it from 
the default command line is fine with me.

> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  arch/arm/boot/dts/r8a7779-marzen.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/r8a7779-marzen.dts
> b/arch/arm/boot/dts/r8a7779-marzen.dts index 2c727cd..769ae84 100644
> --- a/arch/arm/boot/dts/r8a7779-marzen.dts
> +++ b/arch/arm/boot/dts/r8a7779-marzen.dts
> @@ -19,7 +19,7 @@
>  	compatible = "renesas,marzen", "renesas,r8a7779";
> 
>  	chosen {
> -		bootargs = "console=ttySC2,115200 earlyprintk=sh-sci.2,115200
> ignore_loglevel root=/dev/nfs ip=on"; +		bootargs = "console=ttySC2,115200
> ignore_loglevel root=/dev/nfs ip=on"; };
> 
>  	memory {

-- 
Regards,

Laurent Pinchart

[PATCH 3/4] ARM: shmobile: marzen: Initialise SCIF devices using DT

[Laurent Pinchart]

Hi Simon,

Thank you for the patch.

On Tuesday 29 April 2014 16:43:25 Simon Horman wrote:
> Initialise SCIF devices using DT when booting marzen
> using multiplatform.
> 
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  arch/arm/boot/dts/r8a7779-marzen.dts   | 22 +++++++++++++++++++---
>  arch/arm/mach-shmobile/setup-r8a7779.c | 10 +++++-----
>  2 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/r8a7779-marzen.dts
> b/arch/arm/boot/dts/r8a7779-marzen.dts index 769ae84..6e603db 100644
> --- a/arch/arm/boot/dts/r8a7779-marzen.dts
> +++ b/arch/arm/boot/dts/r8a7779-marzen.dts
> @@ -18,6 +18,11 @@
>  	model = "marzen";
>  	compatible = "renesas,marzen", "renesas,r8a7779";
> 
> +	aliases {
> +		serial2 = &scif2;
> +		serial4 = &scif4;
> +	};
> +
>  	chosen {
>  		bootargs = "console=ttySC2,115200 ignore_loglevel root=/dev/nfs 
ip=on";
>  	};
> @@ -73,9 +78,6 @@
>  };
> 
>  &pfc {
> -	pinctrl-0 = <&scif2_pins &scif4_pins>;
> -	pinctrl-names = "default";
> -
>  	lan0_pins: lan0 {
>  		intc {
>  			renesas,groups = "intc_irq1_b";
> @@ -108,6 +110,20 @@
>  	};
>  };
> 
> +&scif2 {
> +       pinctrl-0 = <&scif2_pins>;
> +       pinctrl-names = "default";
> +
> +       status = "okay";
> +};
> +
> +&scif4 {
> +       pinctrl-0 = <&scif4_pins>;
> +       pinctrl-names = "default";
> +
> +       status = "okay";
> +};
> +
>  &sdhi0 {
>  	pinctrl-0 = <&sdhi0_pins>;
>  	pinctrl-names = "default";
> diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c
> b/arch/arm/mach-shmobile/setup-r8a7779.c index 3471a9b..0d2910c 100644
> --- a/arch/arm/mach-shmobile/setup-r8a7779.c
> +++ b/arch/arm/mach-shmobile/setup-r8a7779.c
> @@ -679,17 +679,17 @@ static void __init r8a7779_register_hpb_dmae(void)
>  }
> 
>  static struct platform_device *r8a7779_devices_dt[] __initdata = {
> +	&tmu00_device,
> +	&tmu01_device,
> +};
> +
> +static struct platform_device *r8a7779_standard_devices[] __initdata = {
>  	&scif0_device,
>  	&scif1_device,
>  	&scif2_device,
>  	&scif3_device,
>  	&scif4_device,
>  	&scif5_device,
> -	&tmu00_device,
> -	&tmu01_device,
> -};
> -
> -static struct platform_device *r8a7779_standard_devices[] __initdata = {
>  	&i2c0_device,
>  	&i2c1_device,
>  	&i2c2_device,

-- 
Regards,

Laurent Pinchart

[PATCH 4/4] ARM: shmobile: marzen: Use disabled variant of clock workaround for scif devices

[Laurent Pinchart]

Hi Simon,

On Wednesday 30 April 2014 08:51:11 Simon Horman wrote:
> On Tue, Apr 29, 2014 at 01:40:08PM +0000, Magnus Damm wrote:
> > On Tue, Apr 29, 2014 at 7:43 AM, Simon Horman wrote:
> > > Now that SCIF devices are initialised using DT it should
> > > be sufficient to use the disabled variant of the clock workaround.
> > > 
> > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > > ---
> > > 
> > >  arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-shmobile/board-marzen-reference.c
> > > b/arch/arm/mach-shmobile/board-marzen-reference.c index
> > > 6d34baf..1f14d80 100644
> > > --- a/arch/arm/mach-shmobile/board-marzen-reference.c
> > > +++ b/arch/arm/mach-shmobile/board-marzen-reference.c
> > > @@ -40,12 +40,6 @@ static void __init marzen_init_timer(void)
> > > 
> > >   * devices until they get moved to DT.
> > >   */
> > >  
> > >  static const struct clk_name clk_names[] __initconst = {
> > > 
> > > -       { "scif0", NULL, "sh-sci.0" },
> > > -       { "scif1", NULL, "sh-sci.1" },
> > > -       { "scif2", NULL, "sh-sci.2" },
> > > -       { "scif3", NULL, "sh-sci.3" },
> > > -       { "scif4", NULL, "sh-sci.4" },
> > > -       { "scif5", NULL, "sh-sci.5" },
> > > 
> > >         { "tmu0", NULL, "sh_tmu.0" },
> > >         { "tmu1", NULL, "sh_tmu.1" },
> > >         { "tmu2", NULL, "sh_tmu.2" },
> > > 
> > > @@ -55,6 +49,12 @@ static const struct clk_name clk_names[] __initconst
> > > = {
> > > 
> > >   * This is a really crude hack to work around core platform clock
> > >   issues
> > >   */
> > >  
> > >  static const struct clk_name clk_enables[] __initconst = {
> > > 
> > > +       { "scif0", NULL, "sh-sci.0" },
> > > +       { "scif1", NULL, "sh-sci.1" },
> > > +       { "scif2", NULL, "sh-sci.2" },
> > > +       { "scif3", NULL, "sh-sci.3" },
> > > +       { "scif4", NULL, "sh-sci.4" },
> > > +       { "scif5", NULL, "sh-sci.5" },
> > > 
> > >         { "sdhi0", NULL, "ffe4c000.sd" },
> > >         { "thermal", NULL, "ffc48000.thermal" },
> > >  
> > >  };
> > 
> > Hi Simon,
> > 
> > Thanks for your SCIF DT patches for r8a7779. They look good and clean
> > in general I think.
> > 
> > Regarding this specific patch, I'm not sure why you need to perform
> > this kind of change. Is it following the same style as other SoCs?
> > 
> > In my mind the SCIF driver at least used to rely on both Runtime PM
> > and the clock framework for clock control so only relying on clock
> > framework should be enough for now. The driver may have been updated
> > though, but if so we should make sure we follow the same pattern on
> > other SoCs as well.
> 
> Thanks, I agree this is not needed.

Shouldn't we remove the scif clock entries from the clk_names array completely 
?

-- 
Regards,

Laurent Pinchart

[PATCH 4/4] ARM: shmobile: marzen: Use disabled variant of clock workaround for scif devices

[Simon Horman]

On Wed, Apr 30, 2014 at 02:47:30AM +0200, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Wednesday 30 April 2014 08:51:11 Simon Horman wrote:
> > On Tue, Apr 29, 2014 at 01:40:08PM +0000, Magnus Damm wrote:
> > > On Tue, Apr 29, 2014 at 7:43 AM, Simon Horman wrote:
> > > > Now that SCIF devices are initialised using DT it should
> > > > be sufficient to use the disabled variant of the clock workaround.
> > > > 
> > > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > > > ---
> > > > 
> > > >  arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
> > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-shmobile/board-marzen-reference.c
> > > > b/arch/arm/mach-shmobile/board-marzen-reference.c index
> > > > 6d34baf..1f14d80 100644
> > > > --- a/arch/arm/mach-shmobile/board-marzen-reference.c
> > > > +++ b/arch/arm/mach-shmobile/board-marzen-reference.c
> > > > @@ -40,12 +40,6 @@ static void __init marzen_init_timer(void)
> > > > 
> > > >   * devices until they get moved to DT.
> > > >   */
> > > >  
> > > >  static const struct clk_name clk_names[] __initconst = {
> > > > 
> > > > -       { "scif0", NULL, "sh-sci.0" },
> > > > -       { "scif1", NULL, "sh-sci.1" },
> > > > -       { "scif2", NULL, "sh-sci.2" },
> > > > -       { "scif3", NULL, "sh-sci.3" },
> > > > -       { "scif4", NULL, "sh-sci.4" },
> > > > -       { "scif5", NULL, "sh-sci.5" },
> > > > 
> > > >         { "tmu0", NULL, "sh_tmu.0" },
> > > >         { "tmu1", NULL, "sh_tmu.1" },
> > > >         { "tmu2", NULL, "sh_tmu.2" },
> > > > 
> > > > @@ -55,6 +49,12 @@ static const struct clk_name clk_names[] __initconst
> > > > = {
> > > > 
> > > >   * This is a really crude hack to work around core platform clock
> > > >   issues
> > > >   */
> > > >  
> > > >  static const struct clk_name clk_enables[] __initconst = {
> > > > 
> > > > +       { "scif0", NULL, "sh-sci.0" },
> > > > +       { "scif1", NULL, "sh-sci.1" },
> > > > +       { "scif2", NULL, "sh-sci.2" },
> > > > +       { "scif3", NULL, "sh-sci.3" },
> > > > +       { "scif4", NULL, "sh-sci.4" },
> > > > +       { "scif5", NULL, "sh-sci.5" },
> > > > 
> > > >         { "sdhi0", NULL, "ffe4c000.sd" },
> > > >         { "thermal", NULL, "ffc48000.thermal" },
> > > >  
> > > >  };
> > > 
> > > Hi Simon,
> > > 
> > > Thanks for your SCIF DT patches for r8a7779. They look good and clean
> > > in general I think.
> > > 
> > > Regarding this specific patch, I'm not sure why you need to perform
> > > this kind of change. Is it following the same style as other SoCs?
> > > 
> > > In my mind the SCIF driver at least used to rely on both Runtime PM
> > > and the clock framework for clock control so only relying on clock
> > > framework should be enough for now. The driver may have been updated
> > > though, but if so we should make sure we follow the same pattern on
> > > other SoCs as well.
> > 
> > Thanks, I agree this is not needed.
> 
> Shouldn't we remove the scif clock entries from the clk_names array completely 
> ?

To be honest I am somewhat confused.

My understanding was that clk_enables were used for devices
that are initialised using DT but whose clocks need to be forcibly
enabled because the interaction with runtime PM isn't handled properly
yet.

And my reading of Magnus's email was that SCI devices do not need
to be enabled as the driver interacts with runtime PM correctly.

So I tend to agree with you that we ought to be able to simply remove
the SCI devices from clk_names now they are initialised via DT
(unlike TMU which is still initialised as a platform device).

[PATCH 1/4] ARM: shmobile: r8a7779: Add scif nodes to dtsi

[Simon Horman]

On Wed, Apr 30, 2014 at 02:42:20AM +0200, Laurent Pinchart wrote:
> Hi Simon,
> 
> Thank you for the patch.
> 
> On Tuesday 29 April 2014 16:43:23 Simon Horman wrote:
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> >  arch/arm/boot/dts/r8a7779.dtsi | 60 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi
> > index e924f96..3e9cca4 100644
> > --- a/arch/arm/boot/dts/r8a7779.dtsi
> > +++ b/arch/arm/boot/dts/r8a7779.dtsi
> > @@ -203,6 +203,66 @@
> >  		status = "disabled";
> >  	};
> > 
> > +	scif0: serial at ffe40000 {
> > +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> 
> The most specific compatible string should come first, so this should read
> 
> 		compatible = "renesas,scif-r8a7779", "renesas,scif";

Thanks.

> 
> > +		reg = <0xffe40000 265>;
> 
> 265 ? Don't you mean 256 ? 0x100 might be a better option.

Thanks.

> 
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 88 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> > +		clock-names = "sci_ick";
> 
> Could you try the SCI RFC patch set I've just sent that drop the interface 
> clock in favour of the functional clock ?

Yes, I will test this patch against your SCI patch.

> > +		status = "disabled";
> > +	};
> > +
> > +	scif1: serial at ffe41000 {
> > +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> > +		reg = <0xffe41000 265>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> > +		clock-names = "sci_ick";
> > +		status = "disabled";
> > +	};
> > +
> > +	scif2: serial at ffe42000 {
> > +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> > +		reg = <0xffe42000 265>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> > +		clock-names = "sci_ick";
> > +		status = "disabled";
> > +	};
> > +
> > +	scif3: serial at ffe43000 {
> > +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> > +		reg = <0xffe43000 265>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 91 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> > +		clock-names = "sci_ick";
> > +		status = "disabled";
> > +	};
> > +
> > +	scif4: serial at ffe44000 {
> > +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> > +		reg = <0xffe44000 265>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 92 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> > +		clock-names = "sci_ick";
> > +		status = "disabled";
> > +	};
> > +
> > +	scif5: serial at ffe45000 {
> > +		compatible = "renesas,scif", "renesas,scif-r8a7779";
> > +		reg = <0xffe45000 265>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 93 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg_clocks R8A7779_CLK_P>;
> > +		clock-names = "sci_ick";
> > +		status = "disabled";
> > +	};
> > +
> >  	pfc: pfc at fffc0000 {
> >  		compatible = "renesas,pfc-r8a7779";
> >  		reg = <0xfffc0000 0x23c>;
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

[PATCH 4/4] ARM: shmobile: marzen: Use disabled variant of clock workaround for scif devices

[Magnus Damm]

On Wed, Apr 30, 2014 at 1:50 AM, Simon Horman <horms@verge.net.au> wrote:
> On Wed, Apr 30, 2014 at 02:47:30AM +0200, Laurent Pinchart wrote:
>> Hi Simon,
>>
>> On Wednesday 30 April 2014 08:51:11 Simon Horman wrote:
>> > On Tue, Apr 29, 2014 at 01:40:08PM +0000, Magnus Damm wrote:
>> > > On Tue, Apr 29, 2014 at 7:43 AM, Simon Horman wrote:
>> > > > Now that SCIF devices are initialised using DT it should
>> > > > be sufficient to use the disabled variant of the clock workaround.
>> > > >
>> > > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
>> > > > ---
>> > > >
>> > > >  arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
>> > > >  1 file changed, 6 insertions(+), 6 deletions(-)
>> > > >
>> > > > diff --git a/arch/arm/mach-shmobile/board-marzen-reference.c
>> > > > b/arch/arm/mach-shmobile/board-marzen-reference.c index
>> > > > 6d34baf..1f14d80 100644
>> > > > --- a/arch/arm/mach-shmobile/board-marzen-reference.c
>> > > > +++ b/arch/arm/mach-shmobile/board-marzen-reference.c
>> > > > @@ -40,12 +40,6 @@ static void __init marzen_init_timer(void)
>> > > >
>> > > >   * devices until they get moved to DT.
>> > > >   */
>> > > >
>> > > >  static const struct clk_name clk_names[] __initconst = {
>> > > >
>> > > > -       { "scif0", NULL, "sh-sci.0" },
>> > > > -       { "scif1", NULL, "sh-sci.1" },
>> > > > -       { "scif2", NULL, "sh-sci.2" },
>> > > > -       { "scif3", NULL, "sh-sci.3" },
>> > > > -       { "scif4", NULL, "sh-sci.4" },
>> > > > -       { "scif5", NULL, "sh-sci.5" },
>> > > >
>> > > >         { "tmu0", NULL, "sh_tmu.0" },
>> > > >         { "tmu1", NULL, "sh_tmu.1" },
>> > > >         { "tmu2", NULL, "sh_tmu.2" },
>> > > >
>> > > > @@ -55,6 +49,12 @@ static const struct clk_name clk_names[] __initconst
>> > > > = {
>> > > >
>> > > >   * This is a really crude hack to work around core platform clock
>> > > >   issues
>> > > >   */
>> > > >
>> > > >  static const struct clk_name clk_enables[] __initconst = {
>> > > >
>> > > > +       { "scif0", NULL, "sh-sci.0" },
>> > > > +       { "scif1", NULL, "sh-sci.1" },
>> > > > +       { "scif2", NULL, "sh-sci.2" },
>> > > > +       { "scif3", NULL, "sh-sci.3" },
>> > > > +       { "scif4", NULL, "sh-sci.4" },
>> > > > +       { "scif5", NULL, "sh-sci.5" },
>> > > >
>> > > >         { "sdhi0", NULL, "ffe4c000.sd" },
>> > > >         { "thermal", NULL, "ffc48000.thermal" },
>> > > >
>> > > >  };
>> > >
>> > > Hi Simon,
>> > >
>> > > Thanks for your SCIF DT patches for r8a7779. They look good and clean
>> > > in general I think.
>> > >
>> > > Regarding this specific patch, I'm not sure why you need to perform
>> > > this kind of change. Is it following the same style as other SoCs?
>> > >
>> > > In my mind the SCIF driver at least used to rely on both Runtime PM
>> > > and the clock framework for clock control so only relying on clock
>> > > framework should be enough for now. The driver may have been updated
>> > > though, but if so we should make sure we follow the same pattern on
>> > > other SoCs as well.
>> >
>> > Thanks, I agree this is not needed.
>>
>> Shouldn't we remove the scif clock entries from the clk_names array completely
>> ?
>
> To be honest I am somewhat confused.
>
> My understanding was that clk_enables were used for devices
> that are initialised using DT but whose clocks need to be forcibly
> enabled because the interaction with runtime PM isn't handled properly
> yet.
>
> And my reading of Magnus's email was that SCI devices do not need
> to be enabled as the driver interacts with runtime PM correctly.
>
> So I tend to agree with you that we ought to be able to simply remove
> the SCI devices from clk_names now they are initialised via DT
> (unlike TMU which is still initialised as a platform device).

I suspect that simply removing the SCIF bits from clk_names makes sense.

Thanks,

/ magnus

[PATCH 4/4] ARM: shmobile: marzen: Use disabled variant of clock workaround for scif devices

[Simon Horman]

On Wed, Apr 30, 2014 at 04:49:57AM +0000, Magnus Damm wrote:
> On Wed, Apr 30, 2014 at 1:50 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Apr 30, 2014 at 02:47:30AM +0200, Laurent Pinchart wrote:
> >> Hi Simon,
> >>
> >> On Wednesday 30 April 2014 08:51:11 Simon Horman wrote:
> >> > On Tue, Apr 29, 2014 at 01:40:08PM +0000, Magnus Damm wrote:
> >> > > On Tue, Apr 29, 2014 at 7:43 AM, Simon Horman wrote:
> >> > > > Now that SCIF devices are initialised using DT it should
> >> > > > be sufficient to use the disabled variant of the clock workaround.
> >> > > >
> >> > > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >> > > > ---
> >> > > >
> >> > > >  arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
> >> > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> >> > > >
> >> > > > diff --git a/arch/arm/mach-shmobile/board-marzen-reference.c
> >> > > > b/arch/arm/mach-shmobile/board-marzen-reference.c index
> >> > > > 6d34baf..1f14d80 100644
> >> > > > --- a/arch/arm/mach-shmobile/board-marzen-reference.c
> >> > > > +++ b/arch/arm/mach-shmobile/board-marzen-reference.c
> >> > > > @@ -40,12 +40,6 @@ static void __init marzen_init_timer(void)
> >> > > >
> >> > > >   * devices until they get moved to DT.
> >> > > >   */
> >> > > >
> >> > > >  static const struct clk_name clk_names[] __initconst = {
> >> > > >
> >> > > > -       { "scif0", NULL, "sh-sci.0" },
> >> > > > -       { "scif1", NULL, "sh-sci.1" },
> >> > > > -       { "scif2", NULL, "sh-sci.2" },
> >> > > > -       { "scif3", NULL, "sh-sci.3" },
> >> > > > -       { "scif4", NULL, "sh-sci.4" },
> >> > > > -       { "scif5", NULL, "sh-sci.5" },
> >> > > >
> >> > > >         { "tmu0", NULL, "sh_tmu.0" },
> >> > > >         { "tmu1", NULL, "sh_tmu.1" },
> >> > > >         { "tmu2", NULL, "sh_tmu.2" },
> >> > > >
> >> > > > @@ -55,6 +49,12 @@ static const struct clk_name clk_names[] __initconst
> >> > > > = {
> >> > > >
> >> > > >   * This is a really crude hack to work around core platform clock
> >> > > >   issues
> >> > > >   */
> >> > > >
> >> > > >  static const struct clk_name clk_enables[] __initconst = {
> >> > > >
> >> > > > +       { "scif0", NULL, "sh-sci.0" },
> >> > > > +       { "scif1", NULL, "sh-sci.1" },
> >> > > > +       { "scif2", NULL, "sh-sci.2" },
> >> > > > +       { "scif3", NULL, "sh-sci.3" },
> >> > > > +       { "scif4", NULL, "sh-sci.4" },
> >> > > > +       { "scif5", NULL, "sh-sci.5" },
> >> > > >
> >> > > >         { "sdhi0", NULL, "ffe4c000.sd" },
> >> > > >         { "thermal", NULL, "ffc48000.thermal" },
> >> > > >
> >> > > >  };
> >> > >
> >> > > Hi Simon,
> >> > >
> >> > > Thanks for your SCIF DT patches for r8a7779. They look good and clean
> >> > > in general I think.
> >> > >
> >> > > Regarding this specific patch, I'm not sure why you need to perform
> >> > > this kind of change. Is it following the same style as other SoCs?
> >> > >
> >> > > In my mind the SCIF driver at least used to rely on both Runtime PM
> >> > > and the clock framework for clock control so only relying on clock
> >> > > framework should be enough for now. The driver may have been updated
> >> > > though, but if so we should make sure we follow the same pattern on
> >> > > other SoCs as well.
> >> >
> >> > Thanks, I agree this is not needed.
> >>
> >> Shouldn't we remove the scif clock entries from the clk_names array completely
> >> ?
> >
> > To be honest I am somewhat confused.
> >
> > My understanding was that clk_enables were used for devices
> > that are initialised using DT but whose clocks need to be forcibly
> > enabled because the interaction with runtime PM isn't handled properly
> > yet.
> >
> > And my reading of Magnus's email was that SCI devices do not need
> > to be enabled as the driver interacts with runtime PM correctly.
> >
> > So I tend to agree with you that we ought to be able to simply remove
> > the SCI devices from clk_names now they are initialised via DT
> > (unlike TMU which is still initialised as a platform device).
> 
> I suspect that simply removing the SCIF bits from clk_names makes sense.

Thanks, I now suspect so too.

I'll update this patch accordingly.