Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sachin,

The whole series looks quite good, but I have one concern about support for Universal C210 board. Please see my comment inline.

On 02.05.2014 07:06, Sachin Kamat wrote:
Instead of hardcoding the SYSRAM details for each SoC,
pass this information through device tree (DT) and make
the code SoC agnostic. Generic SRAM bindings are used
for achieving this.

Signed-off-by: Sachin Kamat <sachin.kamat@xxxxxxxxxx>
Acked-by: Arnd Bergmann <arnd@xxxxxxxx>
Acked-by: Heiko Stuebner <heiko@xxxxxxxxx>
---
This patch is based on linux next (next-20140501) on top of
my Kconfig consolidation patch
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/28642

Changes since v1:
Type and presence of sram nodes is SoC/board dependent. V1 mandated the
presence of both the nodes and used to return an error if one of the
nodes was absent and thus fail the boot altogether. Removed this dependency.

Tested on 4210/4412 Origen, 5250/5420 Arndale and SMDK5420 boards.
---
  arch/arm/Kconfig                                |    1 +
  arch/arm/boot/dts/exynos4210-universal_c210.dts |   17 ++++++
  arch/arm/boot/dts/exynos4210.dtsi               |   18 +++++++
  arch/arm/boot/dts/exynos4x12.dtsi               |   18 +++++++
  arch/arm/boot/dts/exynos5250.dtsi               |   18 +++++++
  arch/arm/boot/dts/exynos5420.dtsi               |   18 +++++++
  arch/arm/mach-exynos/common.h                   |    1 +
  arch/arm/mach-exynos/exynos.c                   |   64 -----------------------
  arch/arm/mach-exynos/firmware.c                 |    8 ++-
  arch/arm/mach-exynos/include/mach/map.h         |    7 ---
  arch/arm/mach-exynos/platsmp.c                  |   33 ++++++++++--
  11 files changed, 128 insertions(+), 75 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a6aaaad19b1a..f66ea9453df9 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -855,6 +855,7 @@ config ARCH_EXYNOS
  	select S5P_DEV_MFC
  	select SAMSUNG_DMADEV
  	select SPARSE_IRQ
+	select SRAM
  	select USE_OF
  	help
  	  Support for SAMSUNG's EXYNOS SoCs (EXYNOS4/5)
diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts
index 63e34b24b04f..8d4de5c0d0c7 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -28,6 +28,23 @@
  		bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw rootwait earlyprintk panic=5 maxcpus=1";
  	};

+	sram@02020000 {
+		status = "disabled";

Here you just disable just the top level node of non-secure SYSRAM, but the sub-nodes are still present and enabled.

+	};
+
+	sram@02025000 {
+		compatible = "mmio-sram";
+		reg = <0x02025000 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x02025000 0x1000>;
+
+		smp-sram@0 {
+			compatible = "samsung,exynos4210-sram";
+			reg = <0x0 0x1000>;
+		};
+	};
+
  	mct@10050000 {
  		compatible = "none";
  	};

[snip]

diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 03e5e9f94705..0aac03204f9f 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -20,6 +20,7 @@
  #include <linux/jiffies.h>
  #include <linux/smp.h>
  #include <linux/io.h>
+#include <linux/of_address.h>

  #include <asm/cacheflush.h>
  #include <asm/smp_plat.h>
@@ -33,11 +34,33 @@

  extern void exynos4_secondary_startup(void);

+static void __iomem *sram_base_addr;
+void __iomem *sram_ns_base_addr;
+
+static void __init exynos_smp_prepare_sram(void)
+{
+	struct device_node *node;
+
+	node = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-sram");

Now here you don't check whether the node is "okay", so on Universal C210 it will pick just the first node with this compatible string,

I think you should be using for_each_compatible_node() here, then check if the node is "okay" using of_devicE_is_available() and only then use this node to map the SYSRAM.

+	if (node) {
+		sram_base_addr = of_iomap(node, 0);
+		if (!sram_base_addr)
+			pr_err("Secondary CPU boot address not found\n");
+	}
+
+	node = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-sram-ns");

Same here.

+	if (node) {
+		sram_ns_base_addr = of_iomap(node, 0);
+		if (!sram_ns_base_addr)
+			pr_err("Secondary CPU boot address not found\n");
+	}
+}
+
  static inline void __iomem *cpu_boot_reg_base(void)
  {
  	if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
  		return S5P_INFORM5;
-	return S5P_VA_SYSRAM;
+	return sram_base_addr;
  }

  static inline void __iomem *cpu_boot_reg(int cpu)
@@ -147,7 +170,8 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
  		 * and fall back to boot register if it fails.
  		 */
  		if (call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr))
-			__raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
+			if (cpu_boot_reg_base())

When can this condition be not met?

+				__raw_writel(boot_addr, cpu_boot_reg(phys_cpu));

  		call_firmware_op(cpu_boot, phys_cpu);

@@ -205,6 +229,8 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
  	if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
  		scu_enable(scu_base_addr());

+	exynos_smp_prepare_sram();
+
  	/*
  	 * Write the address of secondary startup into the
  	 * system-wide flags register. The boot monitor waits
@@ -222,7 +248,8 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
  		boot_addr = virt_to_phys(exynos4_secondary_startup);

  		if (call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr))
-			__raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
+			if (cpu_boot_reg_base())

Ditto.

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




[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux