Skip to contentSkip to conversation

Add SCU device node support for Exynos4

PATCH New Pankaj Dubey 4 participants 20 messages Nov 4, 2016 – Nov 8, 2016

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover Add SCU device node support for Exynos4

This patch series does some cleanup for Exynos4 SoC based boards. We are currently statically mapping SCU SFRs in mach-exynos/exynos.c which can be avoided and related dependencies can be removed by introduction of SCU device node.

This patch series is tested on Exynos4210 based Origen board for SMP boot.

Pankaj Dubey (4):
ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c
ARM: dts: exynos: Add SCU device node to exynos4.dtsi
ARM: EXYNOS: Remove static mapping of SCU SFR
ARM: EXYNOS: Remove unused soc_is_exynos{4,5}

 arch/arm/boot/dts/exynos4.dtsi               |  5 +++
 arch/arm/mach-exynos/common.h                |  5 ---
 arch/arm/mach-exynos/exynos.c                | 22 -------------
 arch/arm/mach-exynos/include/mach/map.h      |  2 --
 arch/arm/mach-exynos/platsmp.c               | 49 +++++++---------------------
 arch/arm/mach-exynos/pm.c                    | 14 ++++++--
 arch/arm/mach-exynos/suspend.c               | 15 ++++++---
 arch/arm/plat-samsung/include/plat/map-s5p.h |  4 ---
 8 files changed, 38 insertions(+), 78 deletions(-)
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

We can safely remove exynos_smp_init_cpus() hook from mach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.

---
 arch/arm/mach-exynos/platsmp.c | 31 -------------------------------
 1 file changed, 31 deletions(-)
arch/arm/mach-exynos/platsmp.c +0 -31
index 98ffe1e..a5d6841 100644
@@ -385,36 +385,6 @@ fail:
 	return pen_release != -1 ? ret : 0;
 }
 
-/*
- * Initialise the CPU possible map early - this describes the CPUs
- * which may be present or become present in the system.
- */
-
-static void __init exynos_smp_init_cpus(void)
-{
-	void __iomem *scu_base = scu_base_addr();
-	unsigned int i, ncores;
-
-	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
-		ncores = scu_base ? scu_get_core_count(scu_base) : 1;
-	else
-		/*
-		 * CPU Nodes are passed thru DT and set_cpu_possible
-		 * is set by "arm_dt_init_cpu_maps".
-		 */
-		return;
-
-	/* sanity check */
-	if (ncores > nr_cpu_ids) {
-		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
-			ncores, nr_cpu_ids);
-		ncores = nr_cpu_ids;
-	}
-
-	for (i = 0; i < ncores; i++)
-		set_cpu_possible(i, true);
-}
-
 static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
 {
 	int i;
@@ -479,7 +449,6 @@ static void exynos_cpu_die(unsigned int cpu)
 #endif /* CONFIG_HOTPLUG_CPU */
 
 const struct smp_operations exynos_smp_ops __initconst = {
-	.smp_init_cpus		= exynos_smp_init_cpus,
 	.smp_prepare_cpus	= exynos_smp_prepare_cpus,
 	.smp_secondary_init	= exynos_secondary_init,
 	.smp_boot_secondary	= exynos_boot_secondary,
Signature · 1 line
2.7.4
reviewer
#2 lore raw

PATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

Hi Pankaj,

Quoted 6 lines · Pankaj Dubey

We can safely remove exynos_smp_init_cpus() hook from mach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.

---

Looks good.

Quoted 53 lines
  arch/arm/mach-exynos/platsmp.c | 31 -------------------------------
  1 file changed, 31 deletions(-)
arch/arm/mach-exynos/platsmp.c +0 -31
index 98ffe1e..a5d6841 100644
@@ -385,36 +385,6 @@ fail:
  	return pen_release != -1 ? ret : 0;
  }
 
-/*
- * Initialise the CPU possible map early - this describes the CPUs
- * which may be present or become present in the system.
- */
-
-static void __init exynos_smp_init_cpus(void)
-{
-	void __iomem *scu_base = scu_base_addr();
-	unsigned int i, ncores;
-
-	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
-		ncores = scu_base ? scu_get_core_count(scu_base) : 1;
-	else
-		/*
-		 * CPU Nodes are passed thru DT and set_cpu_possible
-		 * is set by "arm_dt_init_cpu_maps".
-		 */
-		return;
-
-	/* sanity check */
-	if (ncores > nr_cpu_ids) {
-		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
-			ncores, nr_cpu_ids);
-		ncores = nr_cpu_ids;
-	}
-
-	for (i = 0; i < ncores; i++)
-		set_cpu_possible(i, true);
-}
-
  static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
  {
  	int i;
@@ -479,7 +449,6 @@ static void exynos_cpu_die(unsigned int cpu)
  #endif /* CONFIG_HOTPLUG_CPU */
 
  const struct smp_operations exynos_smp_ops __initconst = {
-	.smp_init_cpus		= exynos_smp_init_cpus,
  	.smp_prepare_cpus	= exynos_smp_prepare_cpus,
  	.smp_secondary_init	= exynos_secondary_init,
  	.smp_boot_secondary	= exynos_boot_secondary,
maintainer Applied
#3 lore raw

PATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

In reply to Pankaj Dubey
Quoted 9 lines · Pankaj Dubey

We can safely remove exynos_smp_init_cpus() hook from mach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.

---
 arch/arm/mach-exynos/platsmp.c | 31 -------------------------------
 1 file changed, 31 deletions(-)

Thanks, applied. Somehow your patchset did not reach linux-samsung-soc's Parchwork. I don't have a clue why... It is on linux-arm-kernel's Patchwork.

Best regards,
Krzysztof

#4 lore raw

PATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

Hi Krzysztof,

Quoted 12 lines · Krzysztof Kozlowski
Quoted 9 lines · Pankaj Dubey

We can safely remove exynos_smp_init_cpus() hook from mach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.

---
 arch/arm/mach-exynos/platsmp.c | 31 -------------------------------
 1 file changed, 31 deletions(-)
Quoted 4 lines

Thanks, applied. Somehow your patchset did not reach linux-samsung-soc's Parchwork. I don't have a clue why... It is on linux-arm-kernel's Patchwork.

Thanks for looking into this series.

I am also not sure why its not reflecting for you on linux-samsung-soc's Patchwork, but I can see this series in linux-samsung-soc Patchwork at below links:

[1/4]: https://patchwork.kernel.org/patch/9411787/
[2/4]: https://patchwork.kernel.org/patch/9411789/
[3/4]: https://patchwork.kernel.org/patch/9411791/
[4/4]: https://patchwork.kernel.org/patch/9411793/

Will you please review other two patches (3/4 and 4/4) in this series?

Thanks,
Pankaj Dubey

Quoted 6 lines

Best regards,
Krzysztof

#5 lore raw

PATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

Quoted 20 lines · pankaj.dubey

Hi Krzysztof,

Quoted 12 lines · Krzysztof Kozlowski
Quoted 9 lines · Pankaj Dubey

We can safely remove exynos_smp_init_cpus() hook from mach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.

---
 arch/arm/mach-exynos/platsmp.c | 31 -------------------------------
 1 file changed, 31 deletions(-)
Quoted 4 lines

Thanks, applied. Somehow your patchset did not reach linux-samsung-soc's Parchwork. I don't have a clue why... It is on linux-arm-kernel's Patchwork.

Quoted 5 lines

Thanks for looking into this series.

I am also not sure why its not reflecting for you on linux-samsung-soc's Patchwork, but I can see this series in linux-samsung-soc Patchwork at below links:

Hmmm... it is weird. Why I didn't see them before? It seems that I even updated their status. Confusing...

Quoted 1 line

Will you please review other two patches (3/4 and 4/4) in this series?

4/4 is okay but it depends on 3/4 which already has a valid comment - what will happen when DT node is not present (which first of all will happen because DTS is applied on separate branch... and anyway the code must be prepared for different DTSes). Initially I thought there will be NULL pointer exception on of_iomap() but after looking at the code it might work, I mean fail in a reasonable way.

Best regards,
Krzysztof

#6 lore raw

PATCH 1/4 ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c

Hi Krzysztof,

Quoted 31 lines · Krzysztof Kozlowski
Quoted 20 lines · pankaj.dubey

Hi Krzysztof,

Quoted 12 lines · Krzysztof Kozlowski
Quoted 9 lines · Pankaj Dubey

We can safely remove exynos_smp_init_cpus() hook from mach-exynos/platsmp.c, as all SMP platforms in mach-exynos can rely on DT for CPU core description instead of determining number of cores from the SCU.

---
 arch/arm/mach-exynos/platsmp.c | 31 -------------------------------
 1 file changed, 31 deletions(-)
Quoted 4 lines

Thanks, applied. Somehow your patchset did not reach linux-samsung-soc's Parchwork. I don't have a clue why... It is on linux-arm-kernel's Patchwork.

Quoted 5 lines

Thanks for looking into this series.

I am also not sure why its not reflecting for you on linux-samsung-soc's Patchwork, but I can see this series in linux-samsung-soc Patchwork at below links:

Hmmm... it is weird. Why I didn't see them before? It seems that I even updated their status. Confusing...

Quoted 9 lines
Quoted 1 line

Will you please review other two patches (3/4 and 4/4) in this series?

4/4 is okay but it depends on 3/4 which already has a valid comment - what will happen when DT node is not present (which first of all will happen because DTS is applied on separate branch... and anyway the code must be prepared for different DTSes). Initially I thought there will be NULL pointer exception on of_iomap() but after looking at the code it might work, I mean fail in a reasonable way.

Yes, you are right, even we do not add check for NULL on "np" it won't cause NULL pointer exception on of_iomap(), and it handles it gracefully, I have given explanation about this on patch [4/4] as reply to Alim.

So we can safely avoid check for NULL on "np" without assuming anything, but at the same time I noticed that in case of_iomap returns NULL, we can't move ahead and configure secondary startup address in exynos_smp_prepare_cpus, so I will resubmit [3/4] and [4/4] with appropriate changes to take care of this case. If you see any other concern please let me know.

Thanks,
Pankaj Dubey

Quoted 6 lines

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw

PATCH 2/4 ARM: dts: exynos: Add SCU device node to exynos4.dtsi

In reply to Pankaj Dubey

Exynos4 like other Cortex-A9 SoC's has a Snoop Control Unit(SCU) and its SFR are used during SMP boot and S2R. Add SCU node to the device tree.

---
 arch/arm/boot/dts/exynos4.dtsi | 5 +++++
 1 file changed, 5 insertions(+)
arch/arm/boot/dts/exynos4.dtsi +5 -0
index 5f034eb..6865ca9 100644
@@ -78,6 +78,11 @@
 		reg = <0x10000000 0x100>;
 	};
 
+	scu: snoop-control-unit@10500000 {
+		compatible = "arm,cortex-a9-scu";
+		reg = <0x10500000 0x2000>;
+	};
+
 	memory-controller@12570000 {
 		compatible = "samsung,exynos4210-srom";
 		reg = <0x12570000 0x14>;
Signature · 1 line
2.7.4
reviewer
#8 lore raw

PATCH 2/4 ARM: dts: exynos: Add SCU device node to exynos4.dtsi

Hi Pankaj,

Quoted 5 lines · Pankaj Dubey

Exynos4 like other Cortex-A9 SoC's has a Snoop Control Unit(SCU) and its SFR are used during SMP boot and S2R. Add SCU node to the device tree.

---

Quoted 20 lines
  arch/arm/boot/dts/exynos4.dtsi | 5 +++++
  1 file changed, 5 insertions(+)
arch/arm/boot/dts/exynos4.dtsi +5 -0
index 5f034eb..6865ca9 100644
@@ -78,6 +78,11 @@
  		reg = <0x10000000 0x100>;
  	};
 
+	scu: snoop-control-unit at 10500000 {
+		compatible = "arm,cortex-a9-scu";
+		reg = <0x10500000 0x2000>;
+	};
+
  	memory-controller at 12570000 {
  		compatible = "samsung,exynos4210-srom";
  		reg = <0x12570000 0x14>;
maintainer Applied
#9 lore raw

PATCH 2/4 ARM: dts: exynos: Add SCU device node to exynos4.dtsi

In reply to Pankaj Dubey
Quoted 8 lines · Pankaj Dubey

Exynos4 like other Cortex-A9 SoC's has a Snoop Control Unit(SCU) and its SFR are used during SMP boot and S2R. Add SCU node to the device tree.

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

Thanks, applied.

Best regards,
Krzysztof

#10 lore raw

PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR

In reply to Pankaj Dubey

Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards. Instead use mapping from device tree node of SCU.

---
 arch/arm/mach-exynos/exynos.c                | 22 ----------------------
 arch/arm/mach-exynos/include/mach/map.h      |  2 --
 arch/arm/mach-exynos/platsmp.c               | 18 +++++++++++-------
 arch/arm/mach-exynos/pm.c                    | 14 +++++++++++---
 arch/arm/mach-exynos/suspend.c               | 15 +++++++++++----
 arch/arm/plat-samsung/include/plat/map-s5p.h |  4 ----
 6 files changed, 33 insertions(+), 42 deletions(-)
arch/arm/mach-exynos/exynos.c +0 -22
index 757fc11..fa08ef9 100644
@@ -28,15 +28,6 @@
 
 #include "common.h"
 
-static struct map_desc exynos4_iodesc[] __initdata = {
-	{
-		.virtual	= (unsigned long)S5P_VA_COREPERI_BASE,
-		.pfn		= __phys_to_pfn(EXYNOS4_PA_COREPERI),
-		.length		= SZ_8K,
-		.type		= MT_DEVICE,
-	},
-};
-
 static struct platform_device exynos_cpuidle = {
 	.name              = "exynos_cpuidle",
 #ifdef CONFIG_ARM_EXYNOS_CPUIDLE
@@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
 	return 1;
 }
 
-/*
- * exynos_map_io
- *
- * register the standard cpu IO areas
- */
-static void __init exynos_map_io(void)
-{
-	if (soc_is_exynos4())
-		iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
-}
-
 static void __init exynos_init_io(void)
 {
 	debug_ll_io_init();
@@ -118,8 +98,6 @@ static void __init exynos_init_io(void)
 
 	/* detect cpu id and rev. */
 	s5p_init_cpu(S5P_VA_CHIPID);
-
-	exynos_map_io();
 }
 
 /*
arch/arm/mach-exynos/include/mach/map.h +0 -2
index 5fb0040..0eef407 100644
@@ -18,6 +18,4 @@
 
 #define EXYNOS_PA_CHIPID		0x10000000
 
-#define EXYNOS4_PA_COREPERI		0x10500000
-
 #endif /* __ASM_ARCH_MAP_H */
arch/arm/mach-exynos/platsmp.c +11 -7
index a5d6841..553d0d9 100644
@@ -224,11 +224,6 @@ static void write_pen_release(int val)
 	sync_cache_w(&pen_release);
 }
 
-static void __iomem *scu_base_addr(void)
-{
-	return (void __iomem *)(S5P_VA_SCU);
-}
-
 static DEFINE_SPINLOCK(boot_lock);
 
 static void exynos_secondary_init(unsigned int cpu)
@@ -387,14 +382,23 @@ fail:
 
 static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
 {
+	struct device_node *np;
+	void __iomem *scu_base;
 	int i;
 
 	exynos_sysram_init();
 
 	exynos_set_delayed_reset_assertion(true);
 
-	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
-		scu_enable(scu_base_addr());
+	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
+		np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+		scu_base = of_iomap(np, 0);
+		if (scu_base) {
+			scu_enable(scu_base);
+			iounmap(scu_base);
+		}
+		of_node_put(np);
+	}
 
 	/*
 	 * Write the address of secondary startup into the
arch/arm/mach-exynos/pm.c +11 -3
index 487295f..60e6827 100644
@@ -18,6 +18,7 @@
 #include <linux/cpu_pm.h>
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/soc/samsung/exynos-regs-pmu.h>
 #include <linux/soc/samsung/exynos-pmu.h>
 
@@ -26,8 +27,6 @@
 #include <asm/suspend.h>
 #include <asm/cacheflush.h>
 
-#include <mach/map.h>
-
 #include "common.h"
 
 static inline void __iomem *exynos_boot_vector_addr(void)
@@ -158,6 +157,8 @@ static int exynos_aftr_finisher(unsigned long flags)
 
 void exynos_enter_aftr(void)
 {
+	struct device_node *np;
+	void __iomem *scu_base;
 	unsigned int cpuid = smp_processor_id();
 
 	cpu_pm_enter();
@@ -177,7 +178,14 @@ void exynos_enter_aftr(void)
 	cpu_suspend(0, exynos_aftr_finisher);
 
 	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
-		scu_enable(S5P_VA_SCU);
+		np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+		scu_base = of_iomap(np, 0);
+		if (scu_base) {
+			scu_enable(scu_base);
+			iounmap(scu_base);
+		}
+		of_node_put(np);
+
 		if (call_firmware_op(resume) == -ENOSYS)
 			exynos_cpu_restore_register();
 	}
arch/arm/mach-exynos/suspend.c +11 -4
index 06332f6..7ab7e67 100644
@@ -34,8 +34,6 @@
 #include <asm/smp_scu.h>
 #include <asm/suspend.h>
 
-#include <mach/map.h>
-
 #include <plat/pm-common.h>
 
 #include "common.h"
@@ -453,6 +451,8 @@ static void exynos_pm_release_retention(void)
 
 static void exynos_pm_resume(void)
 {
+	struct device_node *np;
+	void __iomem *scu_base;
 	u32 cpuid = read_cpuid_part();
 
 	if (exynos_pm_central_resume())
@@ -461,8 +461,15 @@ static void exynos_pm_resume(void)
 	/* For release retention */
 	exynos_pm_release_retention();
 
-	if (cpuid == ARM_CPU_PART_CORTEX_A9)
-		scu_enable(S5P_VA_SCU);
+	if (cpuid == ARM_CPU_PART_CORTEX_A9) {
+		np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+		scu_base = of_iomap(np, 0);
+		if (scu_base) {
+			scu_enable(scu_base);
+			iounmap(scu_base);
+		}
+		of_node_put(np);
+	}
 
 	if (call_firmware_op(resume) == -ENOSYS
 	    && cpuid == ARM_CPU_PART_CORTEX_A9)
arch/arm/plat-samsung/include/plat/map-s5p.h +0 -4
index 0fe2828..512ed1f 100644
@@ -15,10 +15,6 @@
 
 #define S5P_VA_CHIPID		S3C_ADDR(0x02000000)
 
-#define S5P_VA_COREPERI_BASE	S3C_ADDR(0x02800000)
-#define S5P_VA_COREPERI(x)	(S5P_VA_COREPERI_BASE + (x))
-#define S5P_VA_SCU		S5P_VA_COREPERI(0x0)
-
 #define VA_VIC(x)		(S3C_VA_IRQ + ((x) * 0x10000))
 #define VA_VIC0			VA_VIC(0)
 #define VA_VIC1			VA_VIC(1)
Signature · 1 line
2.7.4
reviewer
#11 lore raw

PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR

Hi Pankaj,

Quoted 93 lines · Pankaj Dubey

Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards. Instead use mapping from device tree node of SCU.

---
  arch/arm/mach-exynos/exynos.c                | 22 ----------------------
  arch/arm/mach-exynos/include/mach/map.h      |  2 --
  arch/arm/mach-exynos/platsmp.c               | 18 +++++++++++-------
  arch/arm/mach-exynos/pm.c                    | 14 +++++++++++---
  arch/arm/mach-exynos/suspend.c               | 15 +++++++++++----
  arch/arm/plat-samsung/include/plat/map-s5p.h |  4 ----
  6 files changed, 33 insertions(+), 42 deletions(-)
arch/arm/mach-exynos/exynos.c +0 -22
index 757fc11..fa08ef9 100644
@@ -28,15 +28,6 @@
 
  #include "common.h"
 
-static struct map_desc exynos4_iodesc[] __initdata = {
-	{
-		.virtual	= (unsigned long)S5P_VA_COREPERI_BASE,
-		.pfn		= __phys_to_pfn(EXYNOS4_PA_COREPERI),
-		.length		= SZ_8K,
-		.type		= MT_DEVICE,
-	},
-};
-
  static struct platform_device exynos_cpuidle = {
  	.name              = "exynos_cpuidle",
  #ifdef CONFIG_ARM_EXYNOS_CPUIDLE
@@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
  	return 1;
  }
 
-/*
- * exynos_map_io
- *
- * register the standard cpu IO areas
- */
-static void __init exynos_map_io(void)
-{
-	if (soc_is_exynos4())
-		iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
-}
-
  static void __init exynos_init_io(void)
  {
  	debug_ll_io_init();
@@ -118,8 +98,6 @@ static void __init exynos_init_io(void)
 
  	/* detect cpu id and rev. */
  	s5p_init_cpu(S5P_VA_CHIPID);
-
-	exynos_map_io();
  }
 
  /*
arch/arm/mach-exynos/include/mach/map.h +0 -2
index 5fb0040..0eef407 100644
@@ -18,6 +18,4 @@
 
  #define EXYNOS_PA_CHIPID		0x10000000
 
-#define EXYNOS4_PA_COREPERI		0x10500000
-
  #endif /* __ASM_ARCH_MAP_H */
arch/arm/mach-exynos/platsmp.c +2 -5
index a5d6841..553d0d9 100644
@@ -224,11 +224,6 @@ static void write_pen_release(int val)
  	sync_cache_w(&pen_release);
  }
 
-static void __iomem *scu_base_addr(void)
-{
-	return (void __iomem *)(S5P_VA_SCU);
-}
-
  static DEFINE_SPINLOCK(boot_lock);
 
  static void exynos_secondary_init(unsigned int cpu)
@@ -387,14 +382,23 @@ fail:
 
  static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
  {
+	struct device_node *np;
+	void __iomem *scu_base;

arch/arm/mach-exynos/platsmp.c fail: · line 394

Quoted 10 lines
arch/arm/mach-exynos/platsmp.c +2 -2
  	int i;
 
  	exynos_sysram_init();
 
  	exynos_set_delayed_reset_assertion(true);
 
-	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
-		scu_enable(scu_base_addr());
+	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
+		np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");

what if of_find_compatible_node() fails? May be add a error check for the same?

Quoted 41 lines
arch/arm/mach-exynos/platsmp.c +7 -0
+		scu_base = of_iomap(np, 0);
+		if (scu_base) {
+			scu_enable(scu_base);
+			iounmap(scu_base);
+		}
+		of_node_put(np);
+	}
 
  	/*
  	 * Write the address of secondary startup into the
arch/arm/mach-exynos/pm.c +3 -2
index 487295f..60e6827 100644
@@ -18,6 +18,7 @@
  #include <linux/cpu_pm.h>
  #include <linux/io.h>
  #include <linux/of.h>
+#include <linux/of_address.h>
  #include <linux/soc/samsung/exynos-regs-pmu.h>
  #include <linux/soc/samsung/exynos-pmu.h>
 
@@ -26,8 +27,6 @@
  #include <asm/suspend.h>
  #include <asm/cacheflush.h>
 
-#include <mach/map.h>
-
  #include "common.h"
 
  static inline void __iomem *exynos_boot_vector_addr(void)
@@ -158,6 +157,8 @@ static int exynos_aftr_finisher(unsigned long flags)
 
  void exynos_enter_aftr(void)
  {
+	struct device_node *np;
+	void __iomem *scu_base;
  	unsigned int cpuid = smp_processor_id();
 
  	cpu_pm_enter();

arch/arm/mach-exynos/pm.c void exynos_enter_aftr(void) · line 181

Quoted 5 lines
arch/arm/mach-exynos/pm.c +1 -1
@@ -177,7 +178,14 @@ void exynos_enter_aftr(void)
  	cpu_suspend(0, exynos_aftr_finisher);
 
  	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
-		scu_enable(S5P_VA_SCU);
+		np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");

same as above

Quoted 33 lines
arch/arm/mach-exynos/pm.c +7 -0
+		scu_base = of_iomap(np, 0);
+		if (scu_base) {
+			scu_enable(scu_base);
+			iounmap(scu_base);
+		}
+		of_node_put(np);
+
  		if (call_firmware_op(resume) == -ENOSYS)
  			exynos_cpu_restore_register();
  	}
arch/arm/mach-exynos/suspend.c +2 -2
index 06332f6..7ab7e67 100644
@@ -34,8 +34,6 @@
  #include <asm/smp_scu.h>
  #include <asm/suspend.h>
 
-#include <mach/map.h>
-
  #include <plat/pm-common.h>
 
  #include "common.h"
@@ -453,6 +451,8 @@ static void exynos_pm_release_retention(void)
 
  static void exynos_pm_resume(void)
  {
+	struct device_node *np;
+	void __iomem *scu_base;
  	u32 cpuid = read_cpuid_part();
 
  	if (exynos_pm_central_resume())

arch/arm/mach-exynos/suspend.c static void exynos_pm_resume(void) · line 465

Quoted 7 lines
arch/arm/mach-exynos/suspend.c +2 -2
@@ -461,8 +461,15 @@ static void exynos_pm_resume(void)
  	/* For release retention */
  	exynos_pm_release_retention();
 
-	if (cpuid == ARM_CPU_PART_CORTEX_A9)
-		scu_enable(S5P_VA_SCU);
+	if (cpuid == ARM_CPU_PART_CORTEX_A9) {
+		np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");

and here otherwise patch looks good.

Quoted 26 lines
arch/arm/mach-exynos/suspend.c +7 -0
+		scu_base = of_iomap(np, 0);
+		if (scu_base) {
+			scu_enable(scu_base);
+			iounmap(scu_base);
+		}
+		of_node_put(np);
+	}
 
  	if (call_firmware_op(resume) == -ENOSYS
  	    && cpuid == ARM_CPU_PART_CORTEX_A9)
arch/arm/plat-samsung/include/plat/map-s5p.h +0 -4
index 0fe2828..512ed1f 100644
@@ -15,10 +15,6 @@
 
  #define S5P_VA_CHIPID		S3C_ADDR(0x02000000)
 
-#define S5P_VA_COREPERI_BASE	S3C_ADDR(0x02800000)
-#define S5P_VA_COREPERI(x)	(S5P_VA_COREPERI_BASE + (x))
-#define S5P_VA_SCU		S5P_VA_COREPERI(0x0)
-
  #define VA_VIC(x)		(S3C_VA_IRQ + ((x) * 0x10000))
  #define VA_VIC0			VA_VIC(0)
  #define VA_VIC1			VA_VIC(1)
#12 lore raw

PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR

Hi Alim,

Quoted 108 lines · Alim Akhtar

Hi Pankaj,

Quoted 105 lines · Pankaj Dubey

Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards.
Instead use mapping from device tree node of SCU.

---
  arch/arm/mach-exynos/exynos.c                | 22
----------------------
  arch/arm/mach-exynos/include/mach/map.h      |  2 --
  arch/arm/mach-exynos/platsmp.c               | 18 +++++++++++-------
  arch/arm/mach-exynos/pm.c                    | 14 +++++++++++---
  arch/arm/mach-exynos/suspend.c               | 15 +++++++++++----
  arch/arm/plat-samsung/include/plat/map-s5p.h |  4 ----
  6 files changed, 33 insertions(+), 42 deletions(-)
arch/arm/mach-exynos/exynos.c +0 -20
index 757fc11..fa08ef9 100644
@@ -28,15 +28,6 @@
 
  #include "common.h"
 
-static struct map_desc exynos4_iodesc[] __initdata = {
-    {
-        .virtual    = (unsigned long)S5P_VA_COREPERI_BASE,
-        .pfn        = __phys_to_pfn(EXYNOS4_PA_COREPERI),
-        .length        = SZ_8K,
-        .type        = MT_DEVICE,
-    },
-};
-
  static struct platform_device exynos_cpuidle = {
      .name              = "exynos_cpuidle",
  #ifdef CONFIG_ARM_EXYNOS_CPUIDLE
@@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned
 long node, const char *uname,
      return 1;
  }
 
-/*
- * exynos_map_io
- *
- * register the standard cpu IO areas
- */
-static void __init exynos_map_io(void)
-{
-    if (soc_is_exynos4())
-        iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
-}
-
  static void __init exynos_init_io(void)
  {
      debug_ll_io_init();
arch/arm/mach-exynos/exynos.c +0 -2
@@ -118,8 +98,6 @@ static void __init exynos_init_io(void)
 
      /* detect cpu id and rev. */
      s5p_init_cpu(S5P_VA_CHIPID);
-
-    exynos_map_io();
  }
 
  /*
arch/arm/mach-exynos/include/mach/map.h +0 -2
index 5fb0040..0eef407 100644
@@ -18,6 +18,4 @@
 
  #define EXYNOS_PA_CHIPID        0x10000000
 
-#define EXYNOS4_PA_COREPERI        0x10500000
-
  #endif /* __ASM_ARCH_MAP_H */
arch/arm/mach-exynos/platsmp.c +2 -5
index a5d6841..553d0d9 100644
@@ -224,11 +224,6 @@ static void write_pen_release(int val)
      sync_cache_w(&pen_release);
  }
 
-static void __iomem *scu_base_addr(void)
-{
-    return (void __iomem *)(S5P_VA_SCU);
-}
-
  static DEFINE_SPINLOCK(boot_lock);
 
  static void exynos_secondary_init(unsigned int cpu)
@@ -387,14 +382,23 @@ fail:
 
  static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
  {
+    struct device_node *np;
+    void __iomem *scu_base;
      int i;
 
      exynos_sysram_init();
 
      exynos_set_delayed_reset_assertion(true);
 
Quoted 7 lines

arch/arm/mach-exynos/platsmp.c fail: · line 394

Quoted 4 lines
arch/arm/mach-exynos/platsmp.c +2 -2
-    if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
-        scu_enable(scu_base_addr());
+    if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
+        np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");

what if of_find_compatible_node() fails? May be add a error check for the same?

Thanks for review.

You are right of_find_compatible_node() is bound to fail, but only in case supplied compatible is missing in DT. In our case this piece of code will execute only for Cortex-A9 based SoC (which in case of Exynos SoC is applicable only for Exynos4 series) and we will for sure providing "arm,cortex-a9-scu" in DT, so there is no chance of failure. So I feel extra check on "np" for NULL will add no benefit here.

Thanks,
Pankaj Dubey

reviewer
#13 lore raw

PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR

Hi Pankaj,

Quoted 119 lines · pankaj.dubey

Hi Alim,

Quoted 108 lines · Alim Akhtar

Hi Pankaj,

Quoted 105 lines · Pankaj Dubey

Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards.
Instead use mapping from device tree node of SCU.

---
   arch/arm/mach-exynos/exynos.c                | 22
----------------------
   arch/arm/mach-exynos/include/mach/map.h      |  2 --
   arch/arm/mach-exynos/platsmp.c               | 18 +++++++++++-------
   arch/arm/mach-exynos/pm.c                    | 14 +++++++++++---
   arch/arm/mach-exynos/suspend.c               | 15 +++++++++++----
   arch/arm/plat-samsung/include/plat/map-s5p.h |  4 ----
   6 files changed, 33 insertions(+), 42 deletions(-)
arch/arm/mach-exynos/exynos.c +0 -20
index 757fc11..fa08ef9 100644
@@ -28,15 +28,6 @@
 
   #include "common.h"
 
-static struct map_desc exynos4_iodesc[] __initdata = {
-    {
-        .virtual    = (unsigned long)S5P_VA_COREPERI_BASE,
-        .pfn        = __phys_to_pfn(EXYNOS4_PA_COREPERI),
-        .length        = SZ_8K,
-        .type        = MT_DEVICE,
-    },
-};
-
   static struct platform_device exynos_cpuidle = {
       .name              = "exynos_cpuidle",
   #ifdef CONFIG_ARM_EXYNOS_CPUIDLE
@@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned
 long node, const char *uname,
       return 1;
   }
 
-/*
- * exynos_map_io
- *
- * register the standard cpu IO areas
- */
-static void __init exynos_map_io(void)
-{
-    if (soc_is_exynos4())
-        iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
-}
-
   static void __init exynos_init_io(void)
   {
       debug_ll_io_init();
arch/arm/mach-exynos/exynos.c +0 -2
@@ -118,8 +98,6 @@ static void __init exynos_init_io(void)
 
       /* detect cpu id and rev. */
       s5p_init_cpu(S5P_VA_CHIPID);
-
-    exynos_map_io();
   }
 
   /*
arch/arm/mach-exynos/include/mach/map.h +0 -2
index 5fb0040..0eef407 100644
@@ -18,6 +18,4 @@
 
   #define EXYNOS_PA_CHIPID        0x10000000
 
-#define EXYNOS4_PA_COREPERI        0x10500000
-
   #endif /* __ASM_ARCH_MAP_H */
arch/arm/mach-exynos/platsmp.c +2 -5
index a5d6841..553d0d9 100644
@@ -224,11 +224,6 @@ static void write_pen_release(int val)
       sync_cache_w(&pen_release);
   }
 
-static void __iomem *scu_base_addr(void)
-{
-    return (void __iomem *)(S5P_VA_SCU);
-}
-
   static DEFINE_SPINLOCK(boot_lock);
 
   static void exynos_secondary_init(unsigned int cpu)
@@ -387,14 +382,23 @@ fail:
 
   static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
   {
+    struct device_node *np;
+    void __iomem *scu_base;
       int i;
 
       exynos_sysram_init();
 
       exynos_set_delayed_reset_assertion(true);
 
Quoted 7 lines

arch/arm/mach-exynos/platsmp.c fail: · line 394

Quoted 4 lines
arch/arm/mach-exynos/platsmp.c +2 -2
-    if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
-        scu_enable(scu_base_addr());
+    if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
+        np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");

what if of_find_compatible_node() fails? May be add a error check for the same?

Quoted 9 lines

Thanks for review.

You are right of_find_compatible_node() is bound to fail, but only in case supplied compatible is missing in DT. In our case this piece of code will execute only for Cortex-A9 based SoC (which in case of Exynos SoC is applicable only for Exynos4 series) and we will for sure providing "arm,cortex-a9-scu" in DT, so there is no chance of failure. So I feel extra check on "np" for NULL will add no benefit here.

Well I am not entirely convenience here, I still feel it better to have those check, lets not assume anything about future, but when I see of_find_compatible_node() uses elsewhere in kernel, both kind of uses are there (with/without error check).
So, I leave it to you and maintainer to take a call on this, otherwise this patch looks good.

Quoted 5 lines

Thanks,
Pankaj Dubey

#14 lore raw

PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR

Hi Alim,

Quoted 138 lines · Alim Akhtar

Hi Pankaj,

Quoted 124 lines · pankaj.dubey

Hi Alim,

Quoted 110 lines · Alim Akhtar

Hi Pankaj,

Quoted 106 lines · Pankaj Dubey

Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards.
Instead use mapping from device tree node of SCU.

---
   arch/arm/mach-exynos/exynos.c                | 22
----------------------
   arch/arm/mach-exynos/include/mach/map.h      |  2 --
   arch/arm/mach-exynos/platsmp.c               | 18 +++++++++++-------
   arch/arm/mach-exynos/pm.c                    | 14 +++++++++++---
   arch/arm/mach-exynos/suspend.c               | 15 +++++++++++----
   arch/arm/plat-samsung/include/plat/map-s5p.h |  4 ----
   6 files changed, 33 insertions(+), 42 deletions(-)
arch/arm/mach-exynos/exynos.c +0 -20
index 757fc11..fa08ef9 100644
@@ -28,15 +28,6 @@
 
   #include "common.h"
 
-static struct map_desc exynos4_iodesc[] __initdata = {
-    {
-        .virtual    = (unsigned long)S5P_VA_COREPERI_BASE,
-        .pfn        = __phys_to_pfn(EXYNOS4_PA_COREPERI),
-        .length        = SZ_8K,
-        .type        = MT_DEVICE,
-    },
-};
-
   static struct platform_device exynos_cpuidle = {
       .name              = "exynos_cpuidle",
   #ifdef CONFIG_ARM_EXYNOS_CPUIDLE
@@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned
 long node, const char *uname,
       return 1;
   }
 
-/*
- * exynos_map_io
- *
- * register the standard cpu IO areas
- */
-static void __init exynos_map_io(void)
-{
-    if (soc_is_exynos4())
-        iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
-}
-
   static void __init exynos_init_io(void)
   {
       debug_ll_io_init();
arch/arm/mach-exynos/exynos.c +0 -2
@@ -118,8 +98,6 @@ static void __init exynos_init_io(void)
 
       /* detect cpu id and rev. */
       s5p_init_cpu(S5P_VA_CHIPID);
-
-    exynos_map_io();
   }
 
   /*
arch/arm/mach-exynos/include/mach/map.h +0 -2
index 5fb0040..0eef407 100644
@@ -18,6 +18,4 @@
 
   #define EXYNOS_PA_CHIPID        0x10000000
 
-#define EXYNOS4_PA_COREPERI        0x10500000
-
   #endif /* __ASM_ARCH_MAP_H */
arch/arm/mach-exynos/platsmp.c +2 -5
index a5d6841..553d0d9 100644
@@ -224,11 +224,6 @@ static void write_pen_release(int val)
       sync_cache_w(&pen_release);
   }
 
-static void __iomem *scu_base_addr(void)
-{
-    return (void __iomem *)(S5P_VA_SCU);
-}
-
   static DEFINE_SPINLOCK(boot_lock);
 
   static void exynos_secondary_init(unsigned int cpu)
@@ -387,14 +382,23 @@ fail:
 
   static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
   {
+    struct device_node *np;
+    void __iomem *scu_base;
       int i;
 
       exynos_sysram_init();
 
       exynos_set_delayed_reset_assertion(true);
 
Quoted 8 lines

arch/arm/mach-exynos/platsmp.c fail: · line 394

Quoted 4 lines
arch/arm/mach-exynos/platsmp.c +2 -2
-    if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
-        scu_enable(scu_base_addr());
+    if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
+        np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");

what if of_find_compatible_node() fails? May be add a error check for the same?

Quoted 9 lines

Thanks for review.

You are right of_find_compatible_node() is bound to fail, but only in case supplied compatible is missing in DT. In our case this piece of code will execute only for Cortex-A9 based SoC (which in case of Exynos SoC is applicable only for Exynos4 series) and we will for sure providing "arm,cortex-a9-scu" in DT, so there is no chance of failure. So I feel extra check on "np" for NULL will add no benefit here.

Quoted 5 lines

Well I am not entirely convenience here, I still feel it better to have those check, lets not assume anything about future, but when I see of_find_compatible_node() uses elsewhere in kernel, both kind of uses are there (with/without error check). So, I leave it to you and maintainer to take a call on this, otherwise this patch looks good.

Well, I further checked details of of_find_compatible_node() and of_iomap and below is function call chain

of_find_compatible_node //Returns vaild pointer for match, and NULL for no match
--> __of_device_is_compatible () //Returns 0 for no match, and a
positive integer on match

So in our case lets say for any reason "arm,cortex-a9-scu" compatible is not present, "np" will be set as NULL.

Next we have following line without check on "np" for NULL, scu_base = of_iomap(np, 0);

If we see function call sequence of of_iomap we have,

of_iomap(..) //Returns NULL is of_address_to_resource returns non-zero
--> of_address_to_resource(...) // Returns -EINVAL if "addrp" is NULL

         --> of_get_address(...)  //Returns NULL if "parent" is NULL
               --> of_get_parent(...)  //Returns np with refcount

incremented if np is NOT NULL else returns NULL

So in our case we will get scu_base as NULL if we pass "np" as NULL, and on very next line we have check on scu_base, which will prevent us from de-referencing a NULL pointer. So there won't be any CRASH or abnormal behavior, due to not checking "np" for NULL.

I think this is the reason in many places in kernel there is no check for NULL for of_find_compatible_node, surely some places there are checks, but as per current code flow if someone is calling of_iomap with np as NULL there won't be any crash and of_iomap will gracefully returns NULL in that case. Also of_node_put and of_node_get has check for NULL received device_node and handles it gracefully.

But one thing I missed is that in case scu_base is NULL we are not suppose to move ahead in function exynos_smp_prepare_cpus(..), This part I will update and post next version with appropriate change.

Thanks,
Pankaj Dubey

Quoted 10 lines
Quoted 6 lines

Thanks,
Pankaj Dubey

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

#15 lore raw

PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR

In reply to Pankaj Dubey
Quoted 99 lines · Pankaj Dubey

Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards. Instead use mapping from device tree node of SCU.

---
 arch/arm/mach-exynos/exynos.c                | 22 ----------------------
 arch/arm/mach-exynos/include/mach/map.h      |  2 --
 arch/arm/mach-exynos/platsmp.c               | 18 +++++++++++-------
 arch/arm/mach-exynos/pm.c                    | 14 +++++++++++---
 arch/arm/mach-exynos/suspend.c               | 15 +++++++++++----
 arch/arm/plat-samsung/include/plat/map-s5p.h |  4 ----
 6 files changed, 33 insertions(+), 42 deletions(-)
arch/arm/mach-exynos/exynos.c +0 -22
index 757fc11..fa08ef9 100644
@@ -28,15 +28,6 @@
 
 #include "common.h"
 
-static struct map_desc exynos4_iodesc[] __initdata = {
-	{
-		.virtual	= (unsigned long)S5P_VA_COREPERI_BASE,
-		.pfn		= __phys_to_pfn(EXYNOS4_PA_COREPERI),
-		.length		= SZ_8K,
-		.type		= MT_DEVICE,
-	},
-};
-
 static struct platform_device exynos_cpuidle = {
 	.name              = "exynos_cpuidle",
 #ifdef CONFIG_ARM_EXYNOS_CPUIDLE
@@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
 	return 1;
 }
 
-/*
- * exynos_map_io
- *
- * register the standard cpu IO areas
- */
-static void __init exynos_map_io(void)
-{
-	if (soc_is_exynos4())
-		iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
-}
-
 static void __init exynos_init_io(void)
 {
 	debug_ll_io_init();
@@ -118,8 +98,6 @@ static void __init exynos_init_io(void)
 
 	/* detect cpu id and rev. */
 	s5p_init_cpu(S5P_VA_CHIPID);
-
-	exynos_map_io();
 }
 
 /*
arch/arm/mach-exynos/include/mach/map.h +0 -2
index 5fb0040..0eef407 100644
@@ -18,6 +18,4 @@
 
 #define EXYNOS_PA_CHIPID		0x10000000
 
-#define EXYNOS4_PA_COREPERI		0x10500000
-
 #endif /* __ASM_ARCH_MAP_H */
arch/arm/mach-exynos/platsmp.c +2 -5
index a5d6841..553d0d9 100644
@@ -224,11 +224,6 @@ static void write_pen_release(int val)
 	sync_cache_w(&pen_release);
 }
 
-static void __iomem *scu_base_addr(void)
-{
-	return (void __iomem *)(S5P_VA_SCU);
-}
-
 static DEFINE_SPINLOCK(boot_lock);
 
 static void exynos_secondary_init(unsigned int cpu)
@@ -387,14 +382,23 @@ fail:
 
 static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
 {
+	struct device_node *np;
+	void __iomem *scu_base;
 	int i;
 
 	exynos_sysram_init();
 
 	exynos_set_delayed_reset_assertion(true);
 

arch/arm/mach-exynos/platsmp.c fail: · line 400

Quoted 10 lines
arch/arm/mach-exynos/platsmp.c +8 -2
-	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
-		scu_enable(scu_base_addr());
+	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
+		np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+		scu_base = of_iomap(np, 0);
+		if (scu_base) {
+			scu_enable(scu_base);
+			iounmap(scu_base);
+		}
+		of_node_put(np);

I do not like the duplication - this code appears in three places and they are the same. Could you split it into separate function?

As you mentioned to Alim, in case of lack of node, it would be nice to notify the user (pr_err() etc).

Best regards,
Krzysztof

#16 lore raw

PATCH 3/4 ARM: EXYNOS: Remove static mapping of SCU SFR

Hi Krzysztof,

Quoted 100 lines · Krzysztof Kozlowski
Quoted 99 lines · Pankaj Dubey

Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards. Instead use mapping from device tree node of SCU.

---
 arch/arm/mach-exynos/exynos.c                | 22 ----------------------
 arch/arm/mach-exynos/include/mach/map.h      |  2 --
 arch/arm/mach-exynos/platsmp.c               | 18 +++++++++++-------
 arch/arm/mach-exynos/pm.c                    | 14 +++++++++++---
 arch/arm/mach-exynos/suspend.c               | 15 +++++++++++----
 arch/arm/plat-samsung/include/plat/map-s5p.h |  4 ----
 6 files changed, 33 insertions(+), 42 deletions(-)
arch/arm/mach-exynos/exynos.c +0 -22
index 757fc11..fa08ef9 100644
@@ -28,15 +28,6 @@
 
 #include "common.h"
 
-static struct map_desc exynos4_iodesc[] __initdata = {
-	{
-		.virtual	= (unsigned long)S5P_VA_COREPERI_BASE,
-		.pfn		= __phys_to_pfn(EXYNOS4_PA_COREPERI),
-		.length		= SZ_8K,
-		.type		= MT_DEVICE,
-	},
-};
-
 static struct platform_device exynos_cpuidle = {
 	.name              = "exynos_cpuidle",
 #ifdef CONFIG_ARM_EXYNOS_CPUIDLE
@@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
 	return 1;
 }
 
-/*
- * exynos_map_io
- *
- * register the standard cpu IO areas
- */
-static void __init exynos_map_io(void)
-{
-	if (soc_is_exynos4())
-		iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
-}
-
 static void __init exynos_init_io(void)
 {
 	debug_ll_io_init();
@@ -118,8 +98,6 @@ static void __init exynos_init_io(void)
 
 	/* detect cpu id and rev. */
 	s5p_init_cpu(S5P_VA_CHIPID);
-
-	exynos_map_io();
 }
 
 /*
arch/arm/mach-exynos/include/mach/map.h +0 -2
index 5fb0040..0eef407 100644
@@ -18,6 +18,4 @@
 
 #define EXYNOS_PA_CHIPID		0x10000000
 
-#define EXYNOS4_PA_COREPERI		0x10500000
-
 #endif /* __ASM_ARCH_MAP_H */
arch/arm/mach-exynos/platsmp.c +2 -5
index a5d6841..553d0d9 100644
@@ -224,11 +224,6 @@ static void write_pen_release(int val)
 	sync_cache_w(&pen_release);
 }
 
-static void __iomem *scu_base_addr(void)
-{
-	return (void __iomem *)(S5P_VA_SCU);
-}
-
 static DEFINE_SPINLOCK(boot_lock);
 
 static void exynos_secondary_init(unsigned int cpu)
@@ -387,14 +382,23 @@ fail:
 
 static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
 {
+	struct device_node *np;
+	void __iomem *scu_base;
 	int i;
 
 	exynos_sysram_init();
 
 	exynos_set_delayed_reset_assertion(true);
 
Quoted 14 lines

arch/arm/mach-exynos/platsmp.c fail: · line 400

Quoted 10 lines
arch/arm/mach-exynos/platsmp.c +8 -2
-	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
-		scu_enable(scu_base_addr());
+	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
+		np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+		scu_base = of_iomap(np, 0);
+		if (scu_base) {
+			scu_enable(scu_base);
+			iounmap(scu_base);
+		}
+		of_node_put(np);

I do not like the duplication - this code appears in three places and they are the same. Could you split it into separate function?

OK, will do these changes in v2.

Only pm.c and suspend.c file's scu_enable can be moved to single place without introducing more dependencies between these mach files.

Quoted 3 lines

As you mentioned to Alim, in case of lack of node, it would be nice to notify the user (pr_err() etc).

OK, will do these changes in v2.

Thanks,
Pankaj Dubey

Quoted 6 lines

Best regards,
Krzysztof

#17 lore raw

PATCH 4/4 ARM: EXYNOS: Remove unused soc_is_exynos{4,5}

In reply to Pankaj Dubey

As no more user of soc_is_exynos{4,5} we can safely remove them.

---
 arch/arm/mach-exynos/common.h | 5 -----
 1 file changed, 5 deletions(-)
arch/arm/mach-exynos/common.h +0 -5
index 9424a8a..d19064b 100644
@@ -105,11 +105,6 @@ IS_SAMSUNG_CPU(exynos5800, EXYNOS5800_SOC_ID, EXYNOS5_SOC_MASK)
 # define soc_is_exynos5800()	0
 #endif
 
-#define soc_is_exynos4() (soc_is_exynos4210() || soc_is_exynos4212() || \
-			  soc_is_exynos4412())
-#define soc_is_exynos5() (soc_is_exynos5250() || soc_is_exynos5410() || \
-			  soc_is_exynos5420() || soc_is_exynos5800())
-
 extern u32 cp15_save_diag;
 extern u32 cp15_save_power;
 
Signature · 1 line
2.7.4
reviewer
#18 lore raw

PATCH 4/4 ARM: EXYNOS: Remove unused soc_is_exynos{4,5}

Hi Pankaj,

Quoted 4 lines · Pankaj Dubey

As no more user of soc_is_exynos{4,5} we can safely remove them.

---

Also I have complied tested this series, looks good.

Quoted 20 lines
  arch/arm/mach-exynos/common.h | 5 -----
  1 file changed, 5 deletions(-)
arch/arm/mach-exynos/common.h +0 -5
index 9424a8a..d19064b 100644
@@ -105,11 +105,6 @@ IS_SAMSUNG_CPU(exynos5800, EXYNOS5800_SOC_ID, EXYNOS5_SOC_MASK)
  # define soc_is_exynos5800()	0
  #endif
 
-#define soc_is_exynos4() (soc_is_exynos4210() || soc_is_exynos4212() || \
-			  soc_is_exynos4412())
-#define soc_is_exynos5() (soc_is_exynos5250() || soc_is_exynos5410() || \
-			  soc_is_exynos5420() || soc_is_exynos5800())
-
  extern u32 cp15_save_diag;
  extern u32 cp15_save_power;
 
#19 lore raw

cover Add SCU device node support for Exynos4

In reply to Pankaj Dubey

Hi Pankaj,

Quoted 13 lines · Pankaj Dubey

This patch series does some cleanup for Exynos4 SoC based boards. We are currently statically mapping SCU SFRs in mach-exynos/exynos.c which can be avoided and related dependencies can be removed by introduction of SCU device node.

This patch series is tested on Exynos4210 based Origen board for SMP boot.

Pankaj Dubey (4):
ARM: EXYNOS: Remove smp_init_cpus hook from platsmp.c
ARM: dts: exynos: Add SCU device node to exynos4.dtsi
ARM: EXYNOS: Remove static mapping of SCU SFR
ARM: EXYNOS: Remove unused soc_is_exynos{4,5}

Quoted 9 lines
  arch/arm/boot/dts/exynos4.dtsi               |  5 +++
  arch/arm/mach-exynos/common.h                |  5 ---
  arch/arm/mach-exynos/exynos.c                | 22 -------------
  arch/arm/mach-exynos/include/mach/map.h      |  2 --
  arch/arm/mach-exynos/platsmp.c               | 49 +++++++---------------------
  arch/arm/mach-exynos/pm.c                    | 14 ++++++--
  arch/arm/mach-exynos/suspend.c               | 15 ++++++---
  arch/arm/plat-samsung/include/plat/map-s5p.h |  4 ---
  8 files changed, 38 insertions(+), 78 deletions(-)

Nice cleanup! Works fine on Exynos4412-based Odroid U3 (SMP boot and S2R).

Best regards

Signature · 2 lines
Marek Szyprowski, PhD
Samsung R&D Institute Poland