ARM: shmobile: marzen: Initialise SCIF devices using DT

PATCH Superseded Simon Horman 4 participants 15 messages Apr 29, 2014 – Apr 30, 2014

Subsystems (5)

Device tree bindings Arm (32-bit) Arm sub-architectures SuperH +1 more subsystems

lore

Share & tools

lore

Simon Horman

author

2014-04-29 07:43 UTC #0 lore raw

cover ARM: shmobile: marzen: Initialise SCIF devices using DT

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(-)

Signature · 1 line

1.8.5.2

Simon Horman

author

2014-04-29 07:43 UTC #1 lore raw

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

Signed-off-by Simon Horman <[email protected]>

---
 arch/arm/boot/dts/r8a7779.dtsi | 60 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

arch/arm/boot/dts/r8a7779.dtsi +60 -0

index e924f96..3e9cca4 100644
@@ -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>;

Signature · 1 line

1.8.5.2

Laurent Pinchart

2014-04-30 00:42 UTC #2 lore raw

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

Hi Simon,

Thank you for the patch.

Quoted 10 lines · On Tuesday 29 April 2014 16:43:23 Simon Horman wrote:

Signed-off-by Simon Horman <[email protected]>
---
 arch/arm/boot/dts/r8a7779.dtsi | 60 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)
arch/arm/boot/dts/r8a7779.dtsi +0 -0
index e924f96..3e9cca4 100644

arch/arm/boot/dts/r8a7779.dtsi line 207

Quoted 5 lines

arch/arm/boot/dts/r8a7779.dtsi +2 -0

@@ -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";

arch/arm/boot/dts/r8a7779.dtsi line 208

Quoted 1 line

arch/arm/boot/dts/r8a7779.dtsi +1 -0
+		reg = <0xffe40000 265>;

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

arch/arm/boot/dts/r8a7779.dtsi line 212

Quoted 4 lines

arch/arm/boot/dts/r8a7779.dtsi +4 -0

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

Quoted 56 lines

arch/arm/boot/dts/r8a7779.dtsi +53 -0

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

Signature · 3 lines

Regards,

Laurent Pinchart

Simon Horman

2014-04-30 01:51 UTC #3 lore raw

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

Quoted 18 lines · Laurent Pinchart

Hi Simon,
Thank you for the patch.
Quoted 13 lines · On Tuesday 29 April 2014 16:43:23 Simon Horman wrote:
Signed-off-by Simon Horman <[email protected]>
---
 arch/arm/boot/dts/r8a7779.dtsi | 60 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)
arch/arm/boot/dts/r8a7779.dtsi +0 -0
index e924f96..3e9cca4 100644
@@ -203,6 +203,66 @@
 		status = "disabled";
 	};
 

Quoted 6 lines

arch/arm/boot/dts/r8a7779.dtsi line 207
Quoted 2 lines
arch/arm/boot/dts/r8a7779.dtsi +2 -0
+	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.

Quoted 4 lines

arch/arm/boot/dts/r8a7779.dtsi line 208
Quoted 2 lines
arch/arm/boot/dts/r8a7779.dtsi +1 -0
+		reg = <0xffe40000 265>;
265 ? Don't you mean 256 ? 0x100 might be a better option.

Thanks.

Quoted 8 lines

arch/arm/boot/dts/r8a7779.dtsi line 212
Quoted 5 lines
arch/arm/boot/dts/r8a7779.dtsi +4 -0
+		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.

Quoted 62 lines

Quoted 56 lines

arch/arm/boot/dts/r8a7779.dtsi +53 -0

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

Signature · 3 lines

Regards,

Laurent Pinchart

Simon Horman

author

2014-04-29 07:43 UTC #4 lore raw

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

In reply to Simon Horman

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

Signed-off-by Simon Horman <[email protected]>

---
 arch/arm/boot/dts/r8a7779-marzen.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/boot/dts/r8a7779-marzen.dts +1 -1

index 2c727cd..769ae84 100644
@@ -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 {

Signature · 1 line

1.8.5.2

Laurent Pinchart

2014-04-30 00:45 UTC #5 lore raw

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

Hi Simon,

Thank you for the patch.

Quoted 2 lines · 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.

Quoted 1 line

Signed-off-by Simon Horman <[email protected]>

Acked-by Laurent Pinchart <[email protected]>

Quoted 17 lines

---
 arch/arm/boot/dts/r8a7779-marzen.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0
b/arch/arm/boot/dts/r8a7779-marzen.dts index 2c727cd..769ae84 100644
arch/arm/boot/dts/r8a7779-marzen.dts +0 -1
@@ -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 {

Signature · 3 lines

Regards,

Laurent Pinchart

Simon Horman

author

2014-04-29 07:43 UTC #6 lore raw

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

In reply to Simon Horman

Initialise SCIF devices using DT when booting marzen using multiplatform.

Signed-off-by Simon Horman <[email protected]>

---
 arch/arm/boot/dts/r8a7779-marzen.dts   | 22 +++++++++++++++++++---
 arch/arm/mach-shmobile/setup-r8a7779.c | 10 +++++-----
 2 files changed, 24 insertions(+), 8 deletions(-)

arch/arm/boot/dts/r8a7779-marzen.dts +19 -3

index 769ae84..6e603db 100644
@@ -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";

arch/arm/mach-shmobile/setup-r8a7779.c +5 -5

index 3471a9b..0d2910c 100644
@@ -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,

Signature · 1 line

1.8.5.2

Laurent Pinchart

2014-04-30 00:45 UTC #7 lore raw

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

Hi Simon,

Thank you for the patch.

Quoted 4 lines · 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 <[email protected]>

Acked-by Laurent Pinchart <[email protected]>

Quoted 10 lines

---
 arch/arm/boot/dts/r8a7779-marzen.dts   | 22 +++++++++++++++++++---
 arch/arm/mach-shmobile/setup-r8a7779.c | 10 +++++-----
 2 files changed, 24 insertions(+), 8 deletions(-)
(quoted diff) +0 -0
b/arch/arm/boot/dts/r8a7779-marzen.dts index 769ae84..6e603db 100644

arch/arm/boot/dts/r8a7779-marzen.dts line 27

Quoted 10 lines

arch/arm/boot/dts/r8a7779-marzen.dts +5 -0

@@ -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";

Quoted 59 lines

arch/arm/boot/dts/r8a7779-marzen.dts +14 -3

};

@@ -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";

(quoted diff) +0 -0

b/arch/arm/mach-shmobile/setup-r8a7779.c index 3471a9b..0d2910c 100644

arch/arm/mach-shmobile/setup-r8a7779.c +5 -5

@@ -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,

Signature · 3 lines

Regards,

Laurent Pinchart

Simon Horman

author

2014-04-29 07:43 UTC #8 lore raw

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

In reply to 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 <[email protected]>

---
 arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/arm/mach-shmobile/board-marzen-reference.c +6 -6

index 6d34baf..1f14d80 100644
@@ -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" },
 };

Signature · 1 line

1.8.5.2

Magnus Damm

maintainer

2014-04-29 13:40 UTC #9 lore raw

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

Quoted 28 lines · 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 <[email protected]>

---
 arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/arm/mach-shmobile/board-marzen-reference.c +0 -6

index 6d34baf..1f14d80 100644
@@ -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
  */

arch/arm/mach-shmobile/board-marzen-reference.c static const struct clk_name clk_names[] __initconst = { · line 60

Quoted 10 lines

arch/arm/mach-shmobile/board-marzen-reference.c +6 -0

 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

Simon Horman

2014-04-29 23:51 UTC #10 lore raw

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

Quoted 46 lines · Magnus Damm

Quoted 28 lines · 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 <[email protected]>

---
 arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/arm/mach-shmobile/board-marzen-reference.c +0 -6

index 6d34baf..1f14d80 100644
@@ -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
  */

arch/arm/mach-shmobile/board-marzen-reference.c static const struct clk_name clk_names[] __initconst = { · line 60

Quoted 10 lines

arch/arm/mach-shmobile/board-marzen-reference.c +6 -0

 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.

Quoted 8 lines

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.

Laurent Pinchart

2014-04-30 00:47 UTC #11 lore raw

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

Hi Simon,

Quoted 63 lines · On Wednesday 30 April 2014 08:51:11 Simon Horman wrote:

Quoted 62 lines · Magnus Damm
Quoted 42 lines · 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 <[email protected]>
---
 arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/arch/arm/mach-shmobile/board-marzen-reference.c index 6d34baf..1f14d80 100644
arch/arm/mach-shmobile/board-marzen-reference.c +0 -6
@@ -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" },
arch/arm/mach-shmobile/board-marzen-reference.c +1 -0
@@ -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" },
arch/arm/mach-shmobile/board-marzen-reference.c static const struct clk_name clk_names[] __initconst · line 60
Quoted 10 lines
arch/arm/mach-shmobile/board-marzen-reference.c +5 -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?

Quoted 7 lines

Quoted 5 lines
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 ?

Signature · 3 lines

Regards,

Laurent Pinchart

Simon Horman

2014-04-30 01:50 UTC #12 lore raw

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

Quoted 72 lines · Laurent Pinchart

Hi Simon,
Quoted 69 lines · On Wednesday 30 April 2014 08:51:11 Simon Horman wrote:
Quoted 67 lines · Magnus Damm
Quoted 42 lines · 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 <[email protected]>
---
 arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/arch/arm/mach-shmobile/board-marzen-reference.c index 6d34baf..1f14d80 100644
arch/arm/mach-shmobile/board-marzen-reference.c +0 -6
@@ -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" },
arch/arm/mach-shmobile/board-marzen-reference.c +1 -0
@@ -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" },
arch/arm/mach-shmobile/board-marzen-reference.c static const struct clk_name clk_names[] __initconst · line 60
Quoted 10 lines
arch/arm/mach-shmobile/board-marzen-reference.c +5 -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.

Quoted 4 lines

Quoted 1 line
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).

Magnus Damm

maintainer

2014-04-30 04:49 UTC #13 lore raw

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

Quoted 85 lines · Simon Horman

Quoted 72 lines · Laurent Pinchart
Hi Simon,
Quoted 69 lines · On Wednesday 30 April 2014 08:51:11 Simon Horman wrote:
Quoted 67 lines · Magnus Damm
Quoted 42 lines · 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 <[email protected]>
---
 arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/arch/arm/mach-shmobile/board-marzen-reference.c index 6d34baf..1f14d80 100644
arch/arm/mach-shmobile/board-marzen-reference.c +0 -6
@@ -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" },
arch/arm/mach-shmobile/board-marzen-reference.c +1 -0
@@ -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" },
arch/arm/mach-shmobile/board-marzen-reference.c static const struct clk_name clk_names[] __initconst · line 60
Quoted 10 lines
arch/arm/mach-shmobile/board-marzen-reference.c +5 -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.
Quoted 4 lines
Quoted 1 line
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.

Quoted 6 lines

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

Simon Horman

2014-04-30 05:31 UTC #14 lore raw

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

Quoted 86 lines · Magnus Damm

Quoted 85 lines · Simon Horman
Quoted 72 lines · Laurent Pinchart
Hi Simon,
Quoted 69 lines · On Wednesday 30 April 2014 08:51:11 Simon Horman wrote:
Quoted 67 lines · Magnus Damm
Quoted 42 lines · 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 <[email protected]>
---
 arch/arm/mach-shmobile/board-marzen-reference.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/arch/arm/mach-shmobile/board-marzen-reference.c index 6d34baf..1f14d80 100644
arch/arm/mach-shmobile/board-marzen-reference.c +0 -6
@@ -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" },
arch/arm/mach-shmobile/board-marzen-reference.c +1 -0
@@ -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" },
arch/arm/mach-shmobile/board-marzen-reference.c static const struct clk_name clk_names[] __initconst · line 60
Quoted 10 lines
arch/arm/mach-shmobile/board-marzen-reference.c +5 -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.
Quoted 4 lines
Quoted 1 line
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.

Quoted 8 lines

Quoted 6 lines
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.