* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-02 5:06 ` Sachin Kamat
0 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-02 5:06 UTC (permalink / raw)
To: linux-samsung-soc
Cc: linux-arm-kernel, devicetree, heiko, arnd, robh+dt, kgene.kim,
sachin.kamat
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@linaro.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
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";
+ };
+
+ 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";
};
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index cacf6140dd2f..d3d727b0c263 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -31,6 +31,24 @@
pinctrl2 = &pinctrl_2;
};
+ sram@02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x20000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x20000>;
+
+ smp-sram@0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram@1f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x1f000 0x1000>;
+ };
+ };
+
pd_lcd1: lcd1-power-domain@10023CA0 {
compatible = "samsung,exynos4210-pd";
reg = <0x10023CA0 0x20>;
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index c4a9306f8529..75fb3e7e3999 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -37,6 +37,24 @@
interrupts = <2 2>, <3 2>, <18 2>, <19 2>;
};
+ sram@02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x40000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x40000>;
+
+ smp-sram@0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram@2f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x2f000 0x1000>;
+ };
+ };
+
pd_isp: isp-power-domain@10023CA0 {
compatible = "samsung,exynos4210-pd";
reg = <0x10023CA0 0x20>;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 37423314a028..8d724d56a5c6 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -72,6 +72,24 @@
};
};
+ sram@02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x30000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x30000>;
+
+ smp-sram@0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram@2f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x2f000 0x1000>;
+ };
+ };
+
pd_gsc: gsc-power-domain@10044000 {
compatible = "samsung,exynos4210-pd";
reg = <0x10044000 0x20>;
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index c3a9a66c5767..ff496adfabde 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -110,6 +110,24 @@
};
};
+ sram@02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x54000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x54000>;
+
+ smp-sram@0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram@53000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x53000 0x1000>;
+ };
+ };
+
clock: clock-controller@10010000 {
compatible = "samsung,exynos5420-clock";
reg = <0x10010000 0x30000>;
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9ef3f83efaff..47cbab0f008e 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -18,6 +18,7 @@
void mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1);
struct map_desc;
+extern void __iomem *sram_ns_base_addr;
void exynos_init_io(void);
void exynos_restart(enum reboot_mode mode, const char *cmd);
void exynos_cpuidle_init(void);
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 77293d39dfc9..556d148e6413 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -114,51 +114,6 @@ static struct map_desc exynos4_iodesc[] __initdata = {
},
};
-static struct map_desc exynos4_iodesc0[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS4_PA_SYSRAM0),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4_iodesc1[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS4_PA_SYSRAM1),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4210_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS4210_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4x12_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS4x12_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos5250_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS5250_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
static struct map_desc exynos5_iodesc[] __initdata = {
{
.virtual = (unsigned long)S3C_VA_SYS,
@@ -181,11 +136,6 @@ static struct map_desc exynos5_iodesc[] __initdata = {
.length = SZ_4K,
.type = MT_DEVICE,
}, {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS5_PA_SYSRAM),
- .length = SZ_4K,
- .type = MT_DEVICE,
- }, {
.virtual = (unsigned long)S5P_VA_CMU,
.pfn = __phys_to_pfn(EXYNOS5_PA_CMU),
.length = 144 * SZ_1K,
@@ -280,20 +230,6 @@ static void __init exynos_map_io(void)
if (soc_is_exynos5())
iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos5_iodesc));
-
- if (soc_is_exynos4210()) {
- if (samsung_rev() == EXYNOS4210_REV_0)
- iotable_init(exynos4_iodesc0,
- ARRAY_SIZE(exynos4_iodesc0));
- else
- iotable_init(exynos4_iodesc1,
- ARRAY_SIZE(exynos4_iodesc1));
- iotable_init(exynos4210_iodesc, ARRAY_SIZE(exynos4210_iodesc));
- }
- if (soc_is_exynos4212() || soc_is_exynos4412())
- iotable_init(exynos4x12_iodesc, ARRAY_SIZE(exynos4x12_iodesc));
- if (soc_is_exynos5250())
- iotable_init(exynos5250_iodesc, ARRAY_SIZE(exynos5250_iodesc));
}
void __init exynos_init_io(void)
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index 932129ef26c6..7d583cb73850 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -18,6 +18,7 @@
#include <mach/map.h>
+#include "common.h"
#include "smc.h"
static int exynos_do_idle(void)
@@ -34,7 +35,12 @@ static int exynos_cpu_boot(int cpu)
static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
{
- void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
+ void __iomem *boot_reg;
+
+ if (!sram_ns_base_addr)
+ return 0;
+
+ boot_reg = sram_ns_base_addr + 0x1c + 4*cpu;
__raw_writel(boot_addr, boot_reg);
return 0;
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 7b046b59d9ec..548269a60634 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -23,13 +23,6 @@
#include <plat/map-s5p.h>
-#define EXYNOS4_PA_SYSRAM0 0x02025000
-#define EXYNOS4_PA_SYSRAM1 0x02020000
-#define EXYNOS5_PA_SYSRAM 0x02020000
-#define EXYNOS4210_PA_SYSRAM_NS 0x0203F000
-#define EXYNOS4x12_PA_SYSRAM_NS 0x0204F000
-#define EXYNOS5250_PA_SYSRAM_NS 0x0204F000
-
#define EXYNOS_PA_CHIPID 0x10000000
#define EXYNOS4_PA_SYSCON 0x10010000
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");
+ 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");
+ 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())
+ __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())
+ __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
}
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-02 5:06 ` Sachin Kamat
0 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-02 5:06 UTC (permalink / raw)
To: linux-arm-kernel
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@linaro.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
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 at 02020000 {
+ status = "disabled";
+ };
+
+ sram at 02025000 {
+ compatible = "mmio-sram";
+ reg = <0x02025000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02025000 0x1000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+ };
+
mct at 10050000 {
compatible = "none";
};
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index cacf6140dd2f..d3d727b0c263 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -31,6 +31,24 @@
pinctrl2 = &pinctrl_2;
};
+ sram at 02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x20000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x20000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram at 1f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x1f000 0x1000>;
+ };
+ };
+
pd_lcd1: lcd1-power-domain at 10023CA0 {
compatible = "samsung,exynos4210-pd";
reg = <0x10023CA0 0x20>;
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index c4a9306f8529..75fb3e7e3999 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -37,6 +37,24 @@
interrupts = <2 2>, <3 2>, <18 2>, <19 2>;
};
+ sram at 02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x40000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x40000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram at 2f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x2f000 0x1000>;
+ };
+ };
+
pd_isp: isp-power-domain at 10023CA0 {
compatible = "samsung,exynos4210-pd";
reg = <0x10023CA0 0x20>;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 37423314a028..8d724d56a5c6 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -72,6 +72,24 @@
};
};
+ sram at 02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x30000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x30000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram at 2f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x2f000 0x1000>;
+ };
+ };
+
pd_gsc: gsc-power-domain at 10044000 {
compatible = "samsung,exynos4210-pd";
reg = <0x10044000 0x20>;
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index c3a9a66c5767..ff496adfabde 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -110,6 +110,24 @@
};
};
+ sram at 02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x54000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x54000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram at 53000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x53000 0x1000>;
+ };
+ };
+
clock: clock-controller at 10010000 {
compatible = "samsung,exynos5420-clock";
reg = <0x10010000 0x30000>;
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9ef3f83efaff..47cbab0f008e 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -18,6 +18,7 @@
void mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1);
struct map_desc;
+extern void __iomem *sram_ns_base_addr;
void exynos_init_io(void);
void exynos_restart(enum reboot_mode mode, const char *cmd);
void exynos_cpuidle_init(void);
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 77293d39dfc9..556d148e6413 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -114,51 +114,6 @@ static struct map_desc exynos4_iodesc[] __initdata = {
},
};
-static struct map_desc exynos4_iodesc0[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS4_PA_SYSRAM0),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4_iodesc1[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS4_PA_SYSRAM1),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4210_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS4210_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4x12_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS4x12_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos5250_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS5250_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
static struct map_desc exynos5_iodesc[] __initdata = {
{
.virtual = (unsigned long)S3C_VA_SYS,
@@ -181,11 +136,6 @@ static struct map_desc exynos5_iodesc[] __initdata = {
.length = SZ_4K,
.type = MT_DEVICE,
}, {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS5_PA_SYSRAM),
- .length = SZ_4K,
- .type = MT_DEVICE,
- }, {
.virtual = (unsigned long)S5P_VA_CMU,
.pfn = __phys_to_pfn(EXYNOS5_PA_CMU),
.length = 144 * SZ_1K,
@@ -280,20 +230,6 @@ static void __init exynos_map_io(void)
if (soc_is_exynos5())
iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos5_iodesc));
-
- if (soc_is_exynos4210()) {
- if (samsung_rev() == EXYNOS4210_REV_0)
- iotable_init(exynos4_iodesc0,
- ARRAY_SIZE(exynos4_iodesc0));
- else
- iotable_init(exynos4_iodesc1,
- ARRAY_SIZE(exynos4_iodesc1));
- iotable_init(exynos4210_iodesc, ARRAY_SIZE(exynos4210_iodesc));
- }
- if (soc_is_exynos4212() || soc_is_exynos4412())
- iotable_init(exynos4x12_iodesc, ARRAY_SIZE(exynos4x12_iodesc));
- if (soc_is_exynos5250())
- iotable_init(exynos5250_iodesc, ARRAY_SIZE(exynos5250_iodesc));
}
void __init exynos_init_io(void)
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index 932129ef26c6..7d583cb73850 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -18,6 +18,7 @@
#include <mach/map.h>
+#include "common.h"
#include "smc.h"
static int exynos_do_idle(void)
@@ -34,7 +35,12 @@ static int exynos_cpu_boot(int cpu)
static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
{
- void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
+ void __iomem *boot_reg;
+
+ if (!sram_ns_base_addr)
+ return 0;
+
+ boot_reg = sram_ns_base_addr + 0x1c + 4*cpu;
__raw_writel(boot_addr, boot_reg);
return 0;
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 7b046b59d9ec..548269a60634 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -23,13 +23,6 @@
#include <plat/map-s5p.h>
-#define EXYNOS4_PA_SYSRAM0 0x02025000
-#define EXYNOS4_PA_SYSRAM1 0x02020000
-#define EXYNOS5_PA_SYSRAM 0x02020000
-#define EXYNOS4210_PA_SYSRAM_NS 0x0203F000
-#define EXYNOS4x12_PA_SYSRAM_NS 0x0204F000
-#define EXYNOS5250_PA_SYSRAM_NS 0x0204F000
-
#define EXYNOS_PA_CHIPID 0x10000000
#define EXYNOS4_PA_SYSCON 0x10010000
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");
+ 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");
+ 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())
+ __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())
+ __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
}
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 2/2] Documentation: DT: Exynos: Bind SRAM though DT
2014-05-02 5:06 ` Sachin Kamat
@ 2014-05-02 5:06 ` Sachin Kamat
-1 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-02 5:06 UTC (permalink / raw)
To: linux-samsung-soc
Cc: linux-arm-kernel, devicetree, heiko, arnd, robh+dt, kgene.kim,
sachin.kamat
Add SRAM binding documentation.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
Changes since v1:
Minor re-wording for better clarity.
---
.../devicetree/bindings/arm/exynos/smp-sram.txt | 38 ++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/exynos/smp-sram.txt
diff --git a/Documentation/devicetree/bindings/arm/exynos/smp-sram.txt b/Documentation/devicetree/bindings/arm/exynos/smp-sram.txt
new file mode 100644
index 000000000000..c9ff2f58f9b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/exynos/smp-sram.txt
@@ -0,0 +1,38 @@
+Samsung Exynos SRAM for SMP bringup:
+------------------------------------
+
+Samsung SMP-capable Exynos SoCs use part of the SRAM for the bringup
+of the secondary cores. Once the core gets powered up it executes the
+code that is residing at some specific location of the SRAM.
+
+Therefore reserved section sub-nodes have to be added to the mmio-sram
+declaration. These nodes are of two types depending upon secure or
+non-secure execution environment.
+
+Required sub-node properties:
+- compatible : depending upon boot mode, should be
+ "samsung,exynos4210-sram" : for Secure SYSRAM
+ "samsung,exynos4210-sram-ns" : for Non-secure SYSRAM
+
+The rest of the properties should follow the generic mmio-sram discription
+found in ../../misc/sram.txt
+
+Example:
+
+ sram@02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x54000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x54000>;
+
+ smp-sram@0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram@53000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x53000 0x1000>;
+ };
+ };
--
1.7.9.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 2/2] Documentation: DT: Exynos: Bind SRAM though DT
@ 2014-05-02 5:06 ` Sachin Kamat
0 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-02 5:06 UTC (permalink / raw)
To: linux-arm-kernel
Add SRAM binding documentation.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
Changes since v1:
Minor re-wording for better clarity.
---
.../devicetree/bindings/arm/exynos/smp-sram.txt | 38 ++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/exynos/smp-sram.txt
diff --git a/Documentation/devicetree/bindings/arm/exynos/smp-sram.txt b/Documentation/devicetree/bindings/arm/exynos/smp-sram.txt
new file mode 100644
index 000000000000..c9ff2f58f9b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/exynos/smp-sram.txt
@@ -0,0 +1,38 @@
+Samsung Exynos SRAM for SMP bringup:
+------------------------------------
+
+Samsung SMP-capable Exynos SoCs use part of the SRAM for the bringup
+of the secondary cores. Once the core gets powered up it executes the
+code that is residing at some specific location of the SRAM.
+
+Therefore reserved section sub-nodes have to be added to the mmio-sram
+declaration. These nodes are of two types depending upon secure or
+non-secure execution environment.
+
+Required sub-node properties:
+- compatible : depending upon boot mode, should be
+ "samsung,exynos4210-sram" : for Secure SYSRAM
+ "samsung,exynos4210-sram-ns" : for Non-secure SYSRAM
+
+The rest of the properties should follow the generic mmio-sram discription
+found in ../../misc/sram.txt
+
+Example:
+
+ sram at 02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x54000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x54000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram at 53000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x53000 0x1000>;
+ };
+ };
--
1.7.9.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/2] Documentation: DT: Exynos: Bind SRAM though DT
2014-05-02 5:06 ` Sachin Kamat
@ 2014-05-02 8:53 ` Arnd Bergmann
-1 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2014-05-02 8:53 UTC (permalink / raw)
To: Sachin Kamat
Cc: linux-samsung-soc, linux-arm-kernel, devicetree, heiko, robh+dt,
kgene.kim
On Friday 02 May 2014 10:36:20 Sachin Kamat wrote:
> + sram@02020000 {
> + compatible = "mmio-sram";
> + reg = <0x02020000 0x54000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x02020000 0x54000>;
> +
That is actually quite a lot of unused SRAM. Since it came up this
morning in another thread, there may be value in using this for
coherent DMA allocations for some devices. Not sure about how
to best hook this up, but there could be some serious performance
improvements. A typical case would be DMA descriptors for a
gigabit ethernet adapter, which are a pain to maintain on platforms
without cache-coherent DMA.
You could check what drivers you have that call dma_alloc_coherent,
and see if any of them are performance-critical, then hack them
up to use this memory instead as an experiment.
Arnd
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 2/2] Documentation: DT: Exynos: Bind SRAM though DT
@ 2014-05-02 8:53 ` Arnd Bergmann
0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2014-05-02 8:53 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 02 May 2014 10:36:20 Sachin Kamat wrote:
> + sram at 02020000 {
> + compatible = "mmio-sram";
> + reg = <0x02020000 0x54000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x02020000 0x54000>;
> +
That is actually quite a lot of unused SRAM. Since it came up this
morning in another thread, there may be value in using this for
coherent DMA allocations for some devices. Not sure about how
to best hook this up, but there could be some serious performance
improvements. A typical case would be DMA descriptors for a
gigabit ethernet adapter, which are a pain to maintain on platforms
without cache-coherent DMA.
You could check what drivers you have that call dma_alloc_coherent,
and see if any of them are performance-critical, then hack them
up to use this memory instead as an experiment.
Arnd
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
2014-05-02 5:06 ` Sachin Kamat
@ 2014-05-02 17:54 ` Tomasz Figa
-1 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2014-05-02 17:54 UTC (permalink / raw)
To: Sachin Kamat, linux-samsung-soc
Cc: linux-arm-kernel, devicetree, heiko, arnd, robh+dt, kgene.kim
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@linaro.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> 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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-02 17:54 ` Tomasz Figa
0 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2014-05-02 17:54 UTC (permalink / raw)
To: linux-arm-kernel
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@linaro.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> 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 at 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 at 02025000 {
> + compatible = "mmio-sram";
> + reg = <0x02025000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x02025000 0x1000>;
> +
> + smp-sram at 0 {
> + compatible = "samsung,exynos4210-sram";
> + reg = <0x0 0x1000>;
> + };
> + };
> +
> mct at 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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
2014-05-02 17:54 ` Tomasz Figa
@ 2014-05-04 15:17 ` Sachin Kamat
-1 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-04 15:17 UTC (permalink / raw)
To: Tomasz Figa
Cc: linux-samsung-soc, linux-arm-kernel, devicetree,
Heiko Stübner, Arnd Bergmann, robh+dt, Kukjin Kim
Hi Tomasz,
On 2 May 2014 23:24, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Sachin,
>
> The whole series looks quite good,
Thanks :)
>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@linaro.org>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>> Acked-by: Heiko Stuebner <heiko@sntech.de>
>> ---
>> 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.
I was under the impression that disabling parent node would also
disable the sub-nodes.
I will disable all of them in this case.
>
>
>> + };
>> +
>> + 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,
Right. Missed that one.
>
> 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.
OK.
>
>
>> + 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.
OK.
>
>
>> + 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?
I experimented with various combinations of node presence/absence in dts files
and in one such case if we do not have the sram-ns node present (on
arndale-octa),
the system just hung at boot time (as base address was null) and this
check became
necessary.
>
>
>> + __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.
ditto.
Thanks for your review. Will update the same in v3.
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-04 15:17 ` Sachin Kamat
0 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-04 15:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On 2 May 2014 23:24, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Sachin,
>
> The whole series looks quite good,
Thanks :)
>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@linaro.org>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>> Acked-by: Heiko Stuebner <heiko@sntech.de>
>> ---
>> 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 at 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.
I was under the impression that disabling parent node would also
disable the sub-nodes.
I will disable all of them in this case.
>
>
>> + };
>> +
>> + sram at 02025000 {
>> + compatible = "mmio-sram";
>> + reg = <0x02025000 0x1000>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges = <0 0x02025000 0x1000>;
>> +
>> + smp-sram at 0 {
>> + compatible = "samsung,exynos4210-sram";
>> + reg = <0x0 0x1000>;
>> + };
>> + };
>> +
>> mct at 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,
Right. Missed that one.
>
> 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.
OK.
>
>
>> + 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.
OK.
>
>
>> + 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?
I experimented with various combinations of node presence/absence in dts files
and in one such case if we do not have the sram-ns node present (on
arndale-octa),
the system just hung at boot time (as base address was null) and this
check became
necessary.
>
>
>> + __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.
ditto.
Thanks for your review. Will update the same in v3.
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
2014-05-07 17:03 ` Tomasz Figa
@ 2014-05-07 18:26 ` Sachin Kamat
-1 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-07 18:26 UTC (permalink / raw)
To: Tomasz Figa
Cc: Tomasz Figa, linux-samsung-soc, linux-arm-kernel, devicetree,
robh+dt, Kukjin Kim, Arnd Bergmann, Heiko Stübner
Hi Tomasz,
On 7 May 2014 22:33, Tomasz Figa <t.figa@samsung.com> wrote:
> On 06.05.2014 20:09, Sachin Kamat wrote:
> [snip]
>> On 6 May 2014 22:14, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> [snip]
>>> On 06.05.2014 10:10, Sachin Kamat wrote:
> [snip]
>>>> diff --git a/arch/arm/mach-exynos/platsmp.c
>>>> b/arch/arm/mach-exynos/platsmp.c
>>>> index 03e5e9f94705..ccbcdd7b8a86 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,35 @@
>>>>
>>>> extern void exynos4_secondary_startup(void);
>>>>
>>>> +static void __iomem *sram_base_addr;
>>>> +void __iomem *sram_ns_base_addr;
>>>
>>>
>>> This is probably just a matter of preference, but I'd make this static and
>>> provide a getter function, like exynos_get_sram_ns_base();
>>>
>>> Also this address seems to be used by the firmware code exclusively. If we
>>> want to make the firmware code more self-contained, maybe the mapping of
>>> firmware-specific SRAM region could be handled there, instead? This would
>>> also eliminate the need for having an exported variable or getter function.
>>> What do you think?
>>
>> I thought of the same. However 2 reasons prevented me from implementing this.
>>
>> 1. Code duplication
>> 2. This code should be executed only once. I put it in exynos_firmware_init.
>> However it gave a crash while doing of_iomap. So moved it back to the current
>> location.
>>
>
> Right, exynos_firmware_init() is called too early, before ioremap() is
> available, so there is no good place to put this in firmware code. So,
> let's keep this as is for now and eventually move it to firmware if
> needed and/or a proper method of initialization is available.
OK.
>
>>>
>>>
>>>> +
>>>> +static void __init exynos_smp_prepare_sram(void)
>>>> +{
>>>> + struct device_node *node;
>>>> +
>>>> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
>>>> + if (of_device_is_available(node)) {
>>>> + sram_base_addr = of_iomap(node, 0);
>>>> + if (!sram_base_addr)
>>>> + pr_err("Secondary CPU boot address not
>>>> found\n");
>>>
>>>
>>> I don't think this is an error at this stage. Some platforms might not have
>>> either of these SRAM reserved regions (such as those using INFORM registers
>>> instead). Instead, the base address should be checked whenever it is needed
>>> and errors should be handled then, like in exynos_set_cpu_boot_addr().
>>
>> Yes. This is more from an information perspective. Probably pr_warn or
>> pr_info would
>> be better?
>>
>
> IMHO this shouldn't produce any messages at this stage. Just whenever
> either of addresses is actually needed and is not initialized a message
> should be printed.
OK.
>>>> +}
>>>> +
>>>> 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 +172,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())
>>>> + __raw_writel(boot_addr,
>>>> cpu_boot_reg(phys_cpu));
>>>
>>>
>>> I'd rework this for proper error handling, e.g.
>>>
>>> int ret;
>>>
>>> /* ... */
>>>
>>> ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
>>> if (ret && ret != -ENOSYS)
>>> goto fail;
>>> if (ret == -ENOSYS) {
>>> /* Fall back to firmware-less way. */
>>> void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
>>>
>>> if (IS_ERR(boot_reg)) {
>>> ret = PTR_ERR(boot_reg);
>>> goto fail;
>>> }
>>> }
>>>
>>> /* ... */
>>>
>>> fail:
>>> /* Clean-up after error */
>>>
>>> Of course, cpu_boot_reg() will need to be converted to follow the ERR_PTR()
>>> model, but IMHO proper error handling is a good reason to do so.
>>
>> How about handling this separately outside this patch?
>>
>
> This patch is already changing surroundings of this code, so I'd say it
> should do it the right way from the start.
Yes, I have incorporated it in my patch. Will send the same tomorrow.
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-07 18:26 ` Sachin Kamat
0 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-07 18:26 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On 7 May 2014 22:33, Tomasz Figa <t.figa@samsung.com> wrote:
> On 06.05.2014 20:09, Sachin Kamat wrote:
> [snip]
>> On 6 May 2014 22:14, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> [snip]
>>> On 06.05.2014 10:10, Sachin Kamat wrote:
> [snip]
>>>> diff --git a/arch/arm/mach-exynos/platsmp.c
>>>> b/arch/arm/mach-exynos/platsmp.c
>>>> index 03e5e9f94705..ccbcdd7b8a86 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,35 @@
>>>>
>>>> extern void exynos4_secondary_startup(void);
>>>>
>>>> +static void __iomem *sram_base_addr;
>>>> +void __iomem *sram_ns_base_addr;
>>>
>>>
>>> This is probably just a matter of preference, but I'd make this static and
>>> provide a getter function, like exynos_get_sram_ns_base();
>>>
>>> Also this address seems to be used by the firmware code exclusively. If we
>>> want to make the firmware code more self-contained, maybe the mapping of
>>> firmware-specific SRAM region could be handled there, instead? This would
>>> also eliminate the need for having an exported variable or getter function.
>>> What do you think?
>>
>> I thought of the same. However 2 reasons prevented me from implementing this.
>>
>> 1. Code duplication
>> 2. This code should be executed only once. I put it in exynos_firmware_init.
>> However it gave a crash while doing of_iomap. So moved it back to the current
>> location.
>>
>
> Right, exynos_firmware_init() is called too early, before ioremap() is
> available, so there is no good place to put this in firmware code. So,
> let's keep this as is for now and eventually move it to firmware if
> needed and/or a proper method of initialization is available.
OK.
>
>>>
>>>
>>>> +
>>>> +static void __init exynos_smp_prepare_sram(void)
>>>> +{
>>>> + struct device_node *node;
>>>> +
>>>> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
>>>> + if (of_device_is_available(node)) {
>>>> + sram_base_addr = of_iomap(node, 0);
>>>> + if (!sram_base_addr)
>>>> + pr_err("Secondary CPU boot address not
>>>> found\n");
>>>
>>>
>>> I don't think this is an error at this stage. Some platforms might not have
>>> either of these SRAM reserved regions (such as those using INFORM registers
>>> instead). Instead, the base address should be checked whenever it is needed
>>> and errors should be handled then, like in exynos_set_cpu_boot_addr().
>>
>> Yes. This is more from an information perspective. Probably pr_warn or
>> pr_info would
>> be better?
>>
>
> IMHO this shouldn't produce any messages at this stage. Just whenever
> either of addresses is actually needed and is not initialized a message
> should be printed.
OK.
>>>> +}
>>>> +
>>>> 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 +172,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())
>>>> + __raw_writel(boot_addr,
>>>> cpu_boot_reg(phys_cpu));
>>>
>>>
>>> I'd rework this for proper error handling, e.g.
>>>
>>> int ret;
>>>
>>> /* ... */
>>>
>>> ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
>>> if (ret && ret != -ENOSYS)
>>> goto fail;
>>> if (ret == -ENOSYS) {
>>> /* Fall back to firmware-less way. */
>>> void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
>>>
>>> if (IS_ERR(boot_reg)) {
>>> ret = PTR_ERR(boot_reg);
>>> goto fail;
>>> }
>>> }
>>>
>>> /* ... */
>>>
>>> fail:
>>> /* Clean-up after error */
>>>
>>> Of course, cpu_boot_reg() will need to be converted to follow the ERR_PTR()
>>> model, but IMHO proper error handling is a good reason to do so.
>>
>> How about handling this separately outside this patch?
>>
>
> This patch is already changing surroundings of this code, so I'd say it
> should do it the right way from the start.
Yes, I have incorporated it in my patch. Will send the same tomorrow.
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
2014-05-06 18:09 ` Sachin Kamat
@ 2014-05-07 17:03 ` Tomasz Figa
-1 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2014-05-07 17:03 UTC (permalink / raw)
To: Sachin Kamat, Tomasz Figa
Cc: linux-samsung-soc, linux-arm-kernel, devicetree, robh+dt,
Kukjin Kim, Arnd Bergmann, Heiko Stübner
On 06.05.2014 20:09, Sachin Kamat wrote:
[snip]
> On 6 May 2014 22:14, Tomasz Figa <tomasz.figa@gmail.com> wrote:
[snip]
>> On 06.05.2014 10:10, Sachin Kamat wrote:
[snip]
>>> diff --git a/arch/arm/mach-exynos/platsmp.c
>>> b/arch/arm/mach-exynos/platsmp.c
>>> index 03e5e9f94705..ccbcdd7b8a86 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,35 @@
>>>
>>> extern void exynos4_secondary_startup(void);
>>>
>>> +static void __iomem *sram_base_addr;
>>> +void __iomem *sram_ns_base_addr;
>>
>>
>> This is probably just a matter of preference, but I'd make this static and
>> provide a getter function, like exynos_get_sram_ns_base();
>>
>> Also this address seems to be used by the firmware code exclusively. If we
>> want to make the firmware code more self-contained, maybe the mapping of
>> firmware-specific SRAM region could be handled there, instead? This would
>> also eliminate the need for having an exported variable or getter function.
>> What do you think?
>
> I thought of the same. However 2 reasons prevented me from implementing this.
>
> 1. Code duplication
> 2. This code should be executed only once. I put it in exynos_firmware_init.
> However it gave a crash while doing of_iomap. So moved it back to the current
> location.
>
Right, exynos_firmware_init() is called too early, before ioremap() is
available, so there is no good place to put this in firmware code. So,
let's keep this as is for now and eventually move it to firmware if
needed and/or a proper method of initialization is available.
>>
>>
>>> +
>>> +static void __init exynos_smp_prepare_sram(void)
>>> +{
>>> + struct device_node *node;
>>> +
>>> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
>>> + if (of_device_is_available(node)) {
>>> + sram_base_addr = of_iomap(node, 0);
>>> + if (!sram_base_addr)
>>> + pr_err("Secondary CPU boot address not
>>> found\n");
>>
>>
>> I don't think this is an error at this stage. Some platforms might not have
>> either of these SRAM reserved regions (such as those using INFORM registers
>> instead). Instead, the base address should be checked whenever it is needed
>> and errors should be handled then, like in exynos_set_cpu_boot_addr().
>
> Yes. This is more from an information perspective. Probably pr_warn or
> pr_info would
> be better?
>
IMHO this shouldn't produce any messages at this stage. Just whenever
either of addresses is actually needed and is not initialized a message
should be printed.
>>> +}
>>> +
>>> 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 +172,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())
>>> + __raw_writel(boot_addr,
>>> cpu_boot_reg(phys_cpu));
>>
>>
>> I'd rework this for proper error handling, e.g.
>>
>> int ret;
>>
>> /* ... */
>>
>> ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
>> if (ret && ret != -ENOSYS)
>> goto fail;
>> if (ret == -ENOSYS) {
>> /* Fall back to firmware-less way. */
>> void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
>>
>> if (IS_ERR(boot_reg)) {
>> ret = PTR_ERR(boot_reg);
>> goto fail;
>> }
>> }
>>
>> /* ... */
>>
>> fail:
>> /* Clean-up after error */
>>
>> Of course, cpu_boot_reg() will need to be converted to follow the ERR_PTR()
>> model, but IMHO proper error handling is a good reason to do so.
>
> How about handling this separately outside this patch?
>
This patch is already changing surroundings of this code, so I'd say it
should do it the right way from the start.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-07 17:03 ` Tomasz Figa
0 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2014-05-07 17:03 UTC (permalink / raw)
To: linux-arm-kernel
On 06.05.2014 20:09, Sachin Kamat wrote:
[snip]
> On 6 May 2014 22:14, Tomasz Figa <tomasz.figa@gmail.com> wrote:
[snip]
>> On 06.05.2014 10:10, Sachin Kamat wrote:
[snip]
>>> diff --git a/arch/arm/mach-exynos/platsmp.c
>>> b/arch/arm/mach-exynos/platsmp.c
>>> index 03e5e9f94705..ccbcdd7b8a86 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,35 @@
>>>
>>> extern void exynos4_secondary_startup(void);
>>>
>>> +static void __iomem *sram_base_addr;
>>> +void __iomem *sram_ns_base_addr;
>>
>>
>> This is probably just a matter of preference, but I'd make this static and
>> provide a getter function, like exynos_get_sram_ns_base();
>>
>> Also this address seems to be used by the firmware code exclusively. If we
>> want to make the firmware code more self-contained, maybe the mapping of
>> firmware-specific SRAM region could be handled there, instead? This would
>> also eliminate the need for having an exported variable or getter function.
>> What do you think?
>
> I thought of the same. However 2 reasons prevented me from implementing this.
>
> 1. Code duplication
> 2. This code should be executed only once. I put it in exynos_firmware_init.
> However it gave a crash while doing of_iomap. So moved it back to the current
> location.
>
Right, exynos_firmware_init() is called too early, before ioremap() is
available, so there is no good place to put this in firmware code. So,
let's keep this as is for now and eventually move it to firmware if
needed and/or a proper method of initialization is available.
>>
>>
>>> +
>>> +static void __init exynos_smp_prepare_sram(void)
>>> +{
>>> + struct device_node *node;
>>> +
>>> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
>>> + if (of_device_is_available(node)) {
>>> + sram_base_addr = of_iomap(node, 0);
>>> + if (!sram_base_addr)
>>> + pr_err("Secondary CPU boot address not
>>> found\n");
>>
>>
>> I don't think this is an error at this stage. Some platforms might not have
>> either of these SRAM reserved regions (such as those using INFORM registers
>> instead). Instead, the base address should be checked whenever it is needed
>> and errors should be handled then, like in exynos_set_cpu_boot_addr().
>
> Yes. This is more from an information perspective. Probably pr_warn or
> pr_info would
> be better?
>
IMHO this shouldn't produce any messages at this stage. Just whenever
either of addresses is actually needed and is not initialized a message
should be printed.
>>> +}
>>> +
>>> 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 +172,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())
>>> + __raw_writel(boot_addr,
>>> cpu_boot_reg(phys_cpu));
>>
>>
>> I'd rework this for proper error handling, e.g.
>>
>> int ret;
>>
>> /* ... */
>>
>> ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
>> if (ret && ret != -ENOSYS)
>> goto fail;
>> if (ret == -ENOSYS) {
>> /* Fall back to firmware-less way. */
>> void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
>>
>> if (IS_ERR(boot_reg)) {
>> ret = PTR_ERR(boot_reg);
>> goto fail;
>> }
>> }
>>
>> /* ... */
>>
>> fail:
>> /* Clean-up after error */
>>
>> Of course, cpu_boot_reg() will need to be converted to follow the ERR_PTR()
>> model, but IMHO proper error handling is a good reason to do so.
>
> How about handling this separately outside this patch?
>
This patch is already changing surroundings of this code, so I'd say it
should do it the right way from the start.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
2014-05-06 18:09 ` Sachin Kamat
@ 2014-05-07 4:05 ` Sachin Kamat
-1 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-07 4:05 UTC (permalink / raw)
To: Tomasz Figa
Cc: linux-samsung-soc, linux-arm-kernel, devicetree, robh+dt,
Tomasz Figa, Kukjin Kim, Arnd Bergmann, Heiko Stübner
Hi Tomasz,
On 6 May 2014 23:39, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> Hi Tomasz,
>
> On 6 May 2014 22:14, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> Hi Sachin,
>>
>> Thanks for addressing the comments. I need to verify few things on Universal
>> C210 board first, before I could give my Reviewed-by tag or further
>> comments.
>>
>> I also have some general comments that I missed before, due to limited time
>> for review. Please see inline.
>
> Thanks for your review.
>
>>
>>
>> On 06.05.2014 10:10, 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@linaro.org>
>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>> Acked-by: Heiko Stuebner <heiko@sntech.de>
>>> ---
[snip]
>>>
>>> static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>>> {
>>> - void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>>> + void __iomem *boot_reg;
>>> +
>>> + if (!sram_ns_base_addr)
>>> + return 0;
>>
>>
>> Shouldn't this return an error instead? I'm not sure which one would be
>> appropriate, though, probably one of -ENODEV, -ENXIO or -EFAULT.
>
> IIRC, returning error here causes the system to hang and even primary
> cpu does not boot.
> Since any error or absence of sram nodes should atleast boot the
> primary CPU, I thought
> this is better.
Small correction. The above explanation was for the absence of the check.
Sorry about the confusion. Will add -ENODEV here.
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-07 4:05 ` Sachin Kamat
0 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-07 4:05 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On 6 May 2014 23:39, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> Hi Tomasz,
>
> On 6 May 2014 22:14, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> Hi Sachin,
>>
>> Thanks for addressing the comments. I need to verify few things on Universal
>> C210 board first, before I could give my Reviewed-by tag or further
>> comments.
>>
>> I also have some general comments that I missed before, due to limited time
>> for review. Please see inline.
>
> Thanks for your review.
>
>>
>>
>> On 06.05.2014 10:10, 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@linaro.org>
>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>> Acked-by: Heiko Stuebner <heiko@sntech.de>
>>> ---
[snip]
>>>
>>> static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>>> {
>>> - void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>>> + void __iomem *boot_reg;
>>> +
>>> + if (!sram_ns_base_addr)
>>> + return 0;
>>
>>
>> Shouldn't this return an error instead? I'm not sure which one would be
>> appropriate, though, probably one of -ENODEV, -ENXIO or -EFAULT.
>
> IIRC, returning error here causes the system to hang and even primary
> cpu does not boot.
> Since any error or absence of sram nodes should atleast boot the
> primary CPU, I thought
> this is better.
Small correction. The above explanation was for the absence of the check.
Sorry about the confusion. Will add -ENODEV here.
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
2014-05-06 16:44 ` Tomasz Figa
@ 2014-05-06 18:09 ` Sachin Kamat
-1 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-06 18:09 UTC (permalink / raw)
To: Tomasz Figa
Cc: linux-samsung-soc, linux-arm-kernel, devicetree, robh+dt,
Tomasz Figa, Kukjin Kim, Arnd Bergmann, Heiko Stübner
Hi Tomasz,
On 6 May 2014 22:14, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Sachin,
>
> Thanks for addressing the comments. I need to verify few things on Universal
> C210 board first, before I could give my Reviewed-by tag or further
> comments.
>
> I also have some general comments that I missed before, due to limited time
> for review. Please see inline.
Thanks for your review.
>
>
> On 06.05.2014 10:10, 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@linaro.org>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>> Acked-by: Heiko Stuebner <heiko@sntech.de>
>> ---
>> Changes since v1:
>> * Addressed Tomasz Figa's comments
>> - Fixed sram node for universal_c210
>> - Check the node status before mapping the address
>>
>> 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
>
>
> [snip]
>
>
>> diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts
>> b/arch/arm/boot/dts/exynos4210-universal_c210.dts
>> index 63e34b24b04f..9813b068cfd8 100644
>> --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
>> +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
>> @@ -28,6 +28,30 @@
>> bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5
>> rw rootwait earlyprintk panic=5 maxcpus=1";
>> };
>>
>> + sram@02020000 {
>> + status = "disabled";
>> + smp-sram@0 {
>> + status = "disabled";
>> + };
>> +
>> + smp-sram@1f000 {
>> + status = "disabled";
>> + };
>> + };
>> +
>> + 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>;
>> + };
>> + };
>
>
> I wonder if this really should be defined like this. 0x2025000 really looks
> just like an offset from the normal SRAM address. This is the thing I need
> to check in documentation and by experiment when I'll return back to work
> tomorrow, but maybe it could be possible to normally use the sram@02020000
> and just disable smp-sram@0 and smp-sram@1f000, replacing them with
> smp-sram@5000 on Universal C210.
I do not have any info about universal C210 board and need your inputs
for the same :)
>
>
>> +
>> mct@10050000 {
>> compatible = "none";
>> };
>
>
> [snip]
>
>
>> diff --git a/arch/arm/mach-exynos/firmware.c
>> b/arch/arm/mach-exynos/firmware.c
>> index 932129ef26c6..7d583cb73850 100644
>> --- a/arch/arm/mach-exynos/firmware.c
>> +++ b/arch/arm/mach-exynos/firmware.c
>> @@ -18,6 +18,7 @@
>>
>> #include <mach/map.h>
>>
>> +#include "common.h"
>> #include "smc.h"
>>
>> static int exynos_do_idle(void)
>> @@ -34,7 +35,12 @@ static int exynos_cpu_boot(int cpu)
>>
>> static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> {
>> - void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>> + void __iomem *boot_reg;
>> +
>> + if (!sram_ns_base_addr)
>> + return 0;
>
>
> Shouldn't this return an error instead? I'm not sure which one would be
> appropriate, though, probably one of -ENODEV, -ENXIO or -EFAULT.
IIRC, returning error here causes the system to hang and even primary
cpu does not boot.
Since any error or absence of sram nodes should atleast boot the
primary CPU, I thought
this is better.
>
>
>> +
>> + boot_reg = sram_ns_base_addr + 0x1c + 4*cpu;
>>
>> __raw_writel(boot_addr, boot_reg);
>> return 0;
>> diff --git a/arch/arm/mach-exynos/include/mach/map.h
>> b/arch/arm/mach-exynos/include/mach/map.h
>> index 7b046b59d9ec..548269a60634 100644
>> --- a/arch/arm/mach-exynos/include/mach/map.h
>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>> @@ -23,13 +23,6 @@
>>
>> #include <plat/map-s5p.h>
>>
>> -#define EXYNOS4_PA_SYSRAM0 0x02025000
>> -#define EXYNOS4_PA_SYSRAM1 0x02020000
>> -#define EXYNOS5_PA_SYSRAM 0x02020000
>> -#define EXYNOS4210_PA_SYSRAM_NS 0x0203F000
>> -#define EXYNOS4x12_PA_SYSRAM_NS 0x0204F000
>> -#define EXYNOS5250_PA_SYSRAM_NS 0x0204F000
>> -
>> #define EXYNOS_PA_CHIPID 0x10000000
>>
>> #define EXYNOS4_PA_SYSCON 0x10010000
>> diff --git a/arch/arm/mach-exynos/platsmp.c
>> b/arch/arm/mach-exynos/platsmp.c
>> index 03e5e9f94705..ccbcdd7b8a86 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,35 @@
>>
>> extern void exynos4_secondary_startup(void);
>>
>> +static void __iomem *sram_base_addr;
>> +void __iomem *sram_ns_base_addr;
>
>
> This is probably just a matter of preference, but I'd make this static and
> provide a getter function, like exynos_get_sram_ns_base();
>
> Also this address seems to be used by the firmware code exclusively. If we
> want to make the firmware code more self-contained, maybe the mapping of
> firmware-specific SRAM region could be handled there, instead? This would
> also eliminate the need for having an exported variable or getter function.
> What do you think?
I thought of the same. However 2 reasons prevented me from implementing this.
1. Code duplication
2. This code should be executed only once. I put it in exynos_firmware_init.
However it gave a crash while doing of_iomap. So moved it back to the current
location.
>
>
>> +
>> +static void __init exynos_smp_prepare_sram(void)
>> +{
>> + struct device_node *node;
>> +
>> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
>> + if (of_device_is_available(node)) {
>> + sram_base_addr = of_iomap(node, 0);
>> + if (!sram_base_addr)
>> + pr_err("Secondary CPU boot address not
>> found\n");
>
>
> I don't think this is an error at this stage. Some platforms might not have
> either of these SRAM reserved regions (such as those using INFORM registers
> instead). Instead, the base address should be checked whenever it is needed
> and errors should be handled then, like in exynos_set_cpu_boot_addr().
Yes. This is more from an information perspective. Probably pr_warn or
pr_info would
be better?
>
>> + }
>> + }
>
>
> Also we don't need to look further in DT after we find a matching node
> already. So combining both comments the resulting code would be:
>
> for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
> if (!of_device_is_available(node))
> continue;
> sram_base_addr = of_iomap(node, 0);
> break;
>
> }
OK.
>
>> +
>> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram-ns")
>> {
>> + if (of_device_is_available(node)) {
>> + sram_ns_base_addr = of_iomap(node, 0);
>> + if (!sram_ns_base_addr)
>> + pr_err("Secondary CPU boot address not
>> found\n");
>> + }
>> + }
>
>
> Same comments here.
OK.
>
>
>> +}
>> +
>> 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 +172,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())
>> + __raw_writel(boot_addr,
>> cpu_boot_reg(phys_cpu));
>
>
> I'd rework this for proper error handling, e.g.
>
> int ret;
>
> /* ... */
>
> ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
> if (ret && ret != -ENOSYS)
> goto fail;
> if (ret == -ENOSYS) {
> /* Fall back to firmware-less way. */
> void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
>
> if (IS_ERR(boot_reg)) {
> ret = PTR_ERR(boot_reg);
> goto fail;
> }
> }
>
> /* ... */
>
> fail:
> /* Clean-up after error */
>
> Of course, cpu_boot_reg() will need to be converted to follow the ERR_PTR()
> model, but IMHO proper error handling is a good reason to do so.
How about handling this separately outside this patch?
>
>
>>
>> call_firmware_op(cpu_boot, phys_cpu);
>>
>> @@ -205,6 +231,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 +250,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())
>> + __raw_writel(boot_addr,
>> cpu_boot_reg(phys_cpu));
>
>
> I wonder if setting the addresses at this stage is really needed. IMHO doing
> it once in exynos_boot_secondary() should be enough. But this is probably a
> material for further patch.
Sure.
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-06 18:09 ` Sachin Kamat
0 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-06 18:09 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
On 6 May 2014 22:14, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Sachin,
>
> Thanks for addressing the comments. I need to verify few things on Universal
> C210 board first, before I could give my Reviewed-by tag or further
> comments.
>
> I also have some general comments that I missed before, due to limited time
> for review. Please see inline.
Thanks for your review.
>
>
> On 06.05.2014 10:10, 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@linaro.org>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>> Acked-by: Heiko Stuebner <heiko@sntech.de>
>> ---
>> Changes since v1:
>> * Addressed Tomasz Figa's comments
>> - Fixed sram node for universal_c210
>> - Check the node status before mapping the address
>>
>> 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
>
>
> [snip]
>
>
>> diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts
>> b/arch/arm/boot/dts/exynos4210-universal_c210.dts
>> index 63e34b24b04f..9813b068cfd8 100644
>> --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
>> +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
>> @@ -28,6 +28,30 @@
>> bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5
>> rw rootwait earlyprintk panic=5 maxcpus=1";
>> };
>>
>> + sram at 02020000 {
>> + status = "disabled";
>> + smp-sram at 0 {
>> + status = "disabled";
>> + };
>> +
>> + smp-sram at 1f000 {
>> + status = "disabled";
>> + };
>> + };
>> +
>> + sram at 02025000 {
>> + compatible = "mmio-sram";
>> + reg = <0x02025000 0x1000>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges = <0 0x02025000 0x1000>;
>> +
>> + smp-sram at 0 {
>> + compatible = "samsung,exynos4210-sram";
>> + reg = <0x0 0x1000>;
>> + };
>> + };
>
>
> I wonder if this really should be defined like this. 0x2025000 really looks
> just like an offset from the normal SRAM address. This is the thing I need
> to check in documentation and by experiment when I'll return back to work
> tomorrow, but maybe it could be possible to normally use the sram at 02020000
> and just disable smp-sram at 0 and smp-sram at 1f000, replacing them with
> smp-sram at 5000 on Universal C210.
I do not have any info about universal C210 board and need your inputs
for the same :)
>
>
>> +
>> mct at 10050000 {
>> compatible = "none";
>> };
>
>
> [snip]
>
>
>> diff --git a/arch/arm/mach-exynos/firmware.c
>> b/arch/arm/mach-exynos/firmware.c
>> index 932129ef26c6..7d583cb73850 100644
>> --- a/arch/arm/mach-exynos/firmware.c
>> +++ b/arch/arm/mach-exynos/firmware.c
>> @@ -18,6 +18,7 @@
>>
>> #include <mach/map.h>
>>
>> +#include "common.h"
>> #include "smc.h"
>>
>> static int exynos_do_idle(void)
>> @@ -34,7 +35,12 @@ static int exynos_cpu_boot(int cpu)
>>
>> static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> {
>> - void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>> + void __iomem *boot_reg;
>> +
>> + if (!sram_ns_base_addr)
>> + return 0;
>
>
> Shouldn't this return an error instead? I'm not sure which one would be
> appropriate, though, probably one of -ENODEV, -ENXIO or -EFAULT.
IIRC, returning error here causes the system to hang and even primary
cpu does not boot.
Since any error or absence of sram nodes should atleast boot the
primary CPU, I thought
this is better.
>
>
>> +
>> + boot_reg = sram_ns_base_addr + 0x1c + 4*cpu;
>>
>> __raw_writel(boot_addr, boot_reg);
>> return 0;
>> diff --git a/arch/arm/mach-exynos/include/mach/map.h
>> b/arch/arm/mach-exynos/include/mach/map.h
>> index 7b046b59d9ec..548269a60634 100644
>> --- a/arch/arm/mach-exynos/include/mach/map.h
>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>> @@ -23,13 +23,6 @@
>>
>> #include <plat/map-s5p.h>
>>
>> -#define EXYNOS4_PA_SYSRAM0 0x02025000
>> -#define EXYNOS4_PA_SYSRAM1 0x02020000
>> -#define EXYNOS5_PA_SYSRAM 0x02020000
>> -#define EXYNOS4210_PA_SYSRAM_NS 0x0203F000
>> -#define EXYNOS4x12_PA_SYSRAM_NS 0x0204F000
>> -#define EXYNOS5250_PA_SYSRAM_NS 0x0204F000
>> -
>> #define EXYNOS_PA_CHIPID 0x10000000
>>
>> #define EXYNOS4_PA_SYSCON 0x10010000
>> diff --git a/arch/arm/mach-exynos/platsmp.c
>> b/arch/arm/mach-exynos/platsmp.c
>> index 03e5e9f94705..ccbcdd7b8a86 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,35 @@
>>
>> extern void exynos4_secondary_startup(void);
>>
>> +static void __iomem *sram_base_addr;
>> +void __iomem *sram_ns_base_addr;
>
>
> This is probably just a matter of preference, but I'd make this static and
> provide a getter function, like exynos_get_sram_ns_base();
>
> Also this address seems to be used by the firmware code exclusively. If we
> want to make the firmware code more self-contained, maybe the mapping of
> firmware-specific SRAM region could be handled there, instead? This would
> also eliminate the need for having an exported variable or getter function.
> What do you think?
I thought of the same. However 2 reasons prevented me from implementing this.
1. Code duplication
2. This code should be executed only once. I put it in exynos_firmware_init.
However it gave a crash while doing of_iomap. So moved it back to the current
location.
>
>
>> +
>> +static void __init exynos_smp_prepare_sram(void)
>> +{
>> + struct device_node *node;
>> +
>> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
>> + if (of_device_is_available(node)) {
>> + sram_base_addr = of_iomap(node, 0);
>> + if (!sram_base_addr)
>> + pr_err("Secondary CPU boot address not
>> found\n");
>
>
> I don't think this is an error at this stage. Some platforms might not have
> either of these SRAM reserved regions (such as those using INFORM registers
> instead). Instead, the base address should be checked whenever it is needed
> and errors should be handled then, like in exynos_set_cpu_boot_addr().
Yes. This is more from an information perspective. Probably pr_warn or
pr_info would
be better?
>
>> + }
>> + }
>
>
> Also we don't need to look further in DT after we find a matching node
> already. So combining both comments the resulting code would be:
>
> for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
> if (!of_device_is_available(node))
> continue;
> sram_base_addr = of_iomap(node, 0);
> break;
>
> }
OK.
>
>> +
>> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram-ns")
>> {
>> + if (of_device_is_available(node)) {
>> + sram_ns_base_addr = of_iomap(node, 0);
>> + if (!sram_ns_base_addr)
>> + pr_err("Secondary CPU boot address not
>> found\n");
>> + }
>> + }
>
>
> Same comments here.
OK.
>
>
>> +}
>> +
>> 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 +172,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())
>> + __raw_writel(boot_addr,
>> cpu_boot_reg(phys_cpu));
>
>
> I'd rework this for proper error handling, e.g.
>
> int ret;
>
> /* ... */
>
> ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
> if (ret && ret != -ENOSYS)
> goto fail;
> if (ret == -ENOSYS) {
> /* Fall back to firmware-less way. */
> void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
>
> if (IS_ERR(boot_reg)) {
> ret = PTR_ERR(boot_reg);
> goto fail;
> }
> }
>
> /* ... */
>
> fail:
> /* Clean-up after error */
>
> Of course, cpu_boot_reg() will need to be converted to follow the ERR_PTR()
> model, but IMHO proper error handling is a good reason to do so.
How about handling this separately outside this patch?
>
>
>>
>> call_firmware_op(cpu_boot, phys_cpu);
>>
>> @@ -205,6 +231,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 +250,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())
>> + __raw_writel(boot_addr,
>> cpu_boot_reg(phys_cpu));
>
>
> I wonder if setting the addresses at this stage is really needed. IMHO doing
> it once in exynos_boot_secondary() should be enough. But this is probably a
> material for further patch.
Sure.
--
With warm regards,
Sachin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
2014-05-06 8:10 ` Sachin Kamat
@ 2014-05-06 16:44 ` Tomasz Figa
-1 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2014-05-06 16:44 UTC (permalink / raw)
To: Sachin Kamat, linux-samsung-soc
Cc: devicetree, kgene.kim, heiko, arnd, t.figa, robh+dt, linux-arm-kernel
Hi Sachin,
Thanks for addressing the comments. I need to verify few things on
Universal C210 board first, before I could give my Reviewed-by tag or
further comments.
I also have some general comments that I missed before, due to limited
time for review. Please see inline.
On 06.05.2014 10:10, 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@linaro.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> Changes since v1:
> * Addressed Tomasz Figa's comments
> - Fixed sram node for universal_c210
> - Check the node status before mapping the address
>
> 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
[snip]
> diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts
> index 63e34b24b04f..9813b068cfd8 100644
> --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
> +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
> @@ -28,6 +28,30 @@
> bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw rootwait earlyprintk panic=5 maxcpus=1";
> };
>
> + sram@02020000 {
> + status = "disabled";
> + smp-sram@0 {
> + status = "disabled";
> + };
> +
> + smp-sram@1f000 {
> + status = "disabled";
> + };
> + };
> +
> + 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>;
> + };
> + };
I wonder if this really should be defined like this. 0x2025000 really
looks just like an offset from the normal SRAM address. This is the
thing I need to check in documentation and by experiment when I'll
return back to work tomorrow, but maybe it could be possible to normally
use the sram@02020000 and just disable smp-sram@0 and smp-sram@1f000,
replacing them with smp-sram@5000 on Universal C210.
> +
> mct@10050000 {
> compatible = "none";
> };
[snip]
> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> index 932129ef26c6..7d583cb73850 100644
> --- a/arch/arm/mach-exynos/firmware.c
> +++ b/arch/arm/mach-exynos/firmware.c
> @@ -18,6 +18,7 @@
>
> #include <mach/map.h>
>
> +#include "common.h"
> #include "smc.h"
>
> static int exynos_do_idle(void)
> @@ -34,7 +35,12 @@ static int exynos_cpu_boot(int cpu)
>
> static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> {
> - void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
> + void __iomem *boot_reg;
> +
> + if (!sram_ns_base_addr)
> + return 0;
Shouldn't this return an error instead? I'm not sure which one would be
appropriate, though, probably one of -ENODEV, -ENXIO or -EFAULT.
> +
> + boot_reg = sram_ns_base_addr + 0x1c + 4*cpu;
>
> __raw_writel(boot_addr, boot_reg);
> return 0;
> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> index 7b046b59d9ec..548269a60634 100644
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ b/arch/arm/mach-exynos/include/mach/map.h
> @@ -23,13 +23,6 @@
>
> #include <plat/map-s5p.h>
>
> -#define EXYNOS4_PA_SYSRAM0 0x02025000
> -#define EXYNOS4_PA_SYSRAM1 0x02020000
> -#define EXYNOS5_PA_SYSRAM 0x02020000
> -#define EXYNOS4210_PA_SYSRAM_NS 0x0203F000
> -#define EXYNOS4x12_PA_SYSRAM_NS 0x0204F000
> -#define EXYNOS5250_PA_SYSRAM_NS 0x0204F000
> -
> #define EXYNOS_PA_CHIPID 0x10000000
>
> #define EXYNOS4_PA_SYSCON 0x10010000
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 03e5e9f94705..ccbcdd7b8a86 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,35 @@
>
> extern void exynos4_secondary_startup(void);
>
> +static void __iomem *sram_base_addr;
> +void __iomem *sram_ns_base_addr;
This is probably just a matter of preference, but I'd make this static
and provide a getter function, like exynos_get_sram_ns_base();
Also this address seems to be used by the firmware code exclusively. If
we want to make the firmware code more self-contained, maybe the mapping
of firmware-specific SRAM region could be handled there, instead? This
would also eliminate the need for having an exported variable or getter
function. What do you think?
> +
> +static void __init exynos_smp_prepare_sram(void)
> +{
> + struct device_node *node;
> +
> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
> + if (of_device_is_available(node)) {
> + sram_base_addr = of_iomap(node, 0);
> + if (!sram_base_addr)
> + pr_err("Secondary CPU boot address not found\n");
I don't think this is an error at this stage. Some platforms might not
have either of these SRAM reserved regions (such as those using INFORM
registers instead). Instead, the base address should be checked whenever
it is needed and errors should be handled then, like in
exynos_set_cpu_boot_addr().
> + }
> + }
Also we don't need to look further in DT after we find a matching node
already. So combining both comments the resulting code would be:
for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
if (!of_device_is_available(node))
continue;
sram_base_addr = of_iomap(node, 0);
break;
}
> +
> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram-ns") {
> + if (of_device_is_available(node)) {
> + sram_ns_base_addr = of_iomap(node, 0);
> + if (!sram_ns_base_addr)
> + pr_err("Secondary CPU boot address not found\n");
> + }
> + }
Same comments here.
> +}
> +
> 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 +172,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())
> + __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
I'd rework this for proper error handling, e.g.
int ret;
/* ... */
ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
if (ret && ret != -ENOSYS)
goto fail;
if (ret == -ENOSYS) {
/* Fall back to firmware-less way. */
void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
if (IS_ERR(boot_reg)) {
ret = PTR_ERR(boot_reg);
goto fail;
}
}
/* ... */
fail:
/* Clean-up after error */
Of course, cpu_boot_reg() will need to be converted to follow the
ERR_PTR() model, but IMHO proper error handling is a good reason to do so.
>
> call_firmware_op(cpu_boot, phys_cpu);
>
> @@ -205,6 +231,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 +250,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())
> + __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
I wonder if setting the addresses at this stage is really needed. IMHO
doing it once in exynos_boot_secondary() should be enough. But this is
probably a material for further patch.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-06 16:44 ` Tomasz Figa
0 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2014-05-06 16:44 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sachin,
Thanks for addressing the comments. I need to verify few things on
Universal C210 board first, before I could give my Reviewed-by tag or
further comments.
I also have some general comments that I missed before, due to limited
time for review. Please see inline.
On 06.05.2014 10:10, 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@linaro.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> Changes since v1:
> * Addressed Tomasz Figa's comments
> - Fixed sram node for universal_c210
> - Check the node status before mapping the address
>
> 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
[snip]
> diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts
> index 63e34b24b04f..9813b068cfd8 100644
> --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
> +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
> @@ -28,6 +28,30 @@
> bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw rootwait earlyprintk panic=5 maxcpus=1";
> };
>
> + sram at 02020000 {
> + status = "disabled";
> + smp-sram at 0 {
> + status = "disabled";
> + };
> +
> + smp-sram at 1f000 {
> + status = "disabled";
> + };
> + };
> +
> + sram at 02025000 {
> + compatible = "mmio-sram";
> + reg = <0x02025000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x02025000 0x1000>;
> +
> + smp-sram at 0 {
> + compatible = "samsung,exynos4210-sram";
> + reg = <0x0 0x1000>;
> + };
> + };
I wonder if this really should be defined like this. 0x2025000 really
looks just like an offset from the normal SRAM address. This is the
thing I need to check in documentation and by experiment when I'll
return back to work tomorrow, but maybe it could be possible to normally
use the sram at 02020000 and just disable smp-sram at 0 and smp-sram at 1f000,
replacing them with smp-sram at 5000 on Universal C210.
> +
> mct at 10050000 {
> compatible = "none";
> };
[snip]
> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> index 932129ef26c6..7d583cb73850 100644
> --- a/arch/arm/mach-exynos/firmware.c
> +++ b/arch/arm/mach-exynos/firmware.c
> @@ -18,6 +18,7 @@
>
> #include <mach/map.h>
>
> +#include "common.h"
> #include "smc.h"
>
> static int exynos_do_idle(void)
> @@ -34,7 +35,12 @@ static int exynos_cpu_boot(int cpu)
>
> static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> {
> - void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
> + void __iomem *boot_reg;
> +
> + if (!sram_ns_base_addr)
> + return 0;
Shouldn't this return an error instead? I'm not sure which one would be
appropriate, though, probably one of -ENODEV, -ENXIO or -EFAULT.
> +
> + boot_reg = sram_ns_base_addr + 0x1c + 4*cpu;
>
> __raw_writel(boot_addr, boot_reg);
> return 0;
> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> index 7b046b59d9ec..548269a60634 100644
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ b/arch/arm/mach-exynos/include/mach/map.h
> @@ -23,13 +23,6 @@
>
> #include <plat/map-s5p.h>
>
> -#define EXYNOS4_PA_SYSRAM0 0x02025000
> -#define EXYNOS4_PA_SYSRAM1 0x02020000
> -#define EXYNOS5_PA_SYSRAM 0x02020000
> -#define EXYNOS4210_PA_SYSRAM_NS 0x0203F000
> -#define EXYNOS4x12_PA_SYSRAM_NS 0x0204F000
> -#define EXYNOS5250_PA_SYSRAM_NS 0x0204F000
> -
> #define EXYNOS_PA_CHIPID 0x10000000
>
> #define EXYNOS4_PA_SYSCON 0x10010000
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 03e5e9f94705..ccbcdd7b8a86 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,35 @@
>
> extern void exynos4_secondary_startup(void);
>
> +static void __iomem *sram_base_addr;
> +void __iomem *sram_ns_base_addr;
This is probably just a matter of preference, but I'd make this static
and provide a getter function, like exynos_get_sram_ns_base();
Also this address seems to be used by the firmware code exclusively. If
we want to make the firmware code more self-contained, maybe the mapping
of firmware-specific SRAM region could be handled there, instead? This
would also eliminate the need for having an exported variable or getter
function. What do you think?
> +
> +static void __init exynos_smp_prepare_sram(void)
> +{
> + struct device_node *node;
> +
> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
> + if (of_device_is_available(node)) {
> + sram_base_addr = of_iomap(node, 0);
> + if (!sram_base_addr)
> + pr_err("Secondary CPU boot address not found\n");
I don't think this is an error at this stage. Some platforms might not
have either of these SRAM reserved regions (such as those using INFORM
registers instead). Instead, the base address should be checked whenever
it is needed and errors should be handled then, like in
exynos_set_cpu_boot_addr().
> + }
> + }
Also we don't need to look further in DT after we find a matching node
already. So combining both comments the resulting code would be:
for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
if (!of_device_is_available(node))
continue;
sram_base_addr = of_iomap(node, 0);
break;
}
> +
> + for_each_compatible_node(node, NULL, "samsung,exynos4210-sram-ns") {
> + if (of_device_is_available(node)) {
> + sram_ns_base_addr = of_iomap(node, 0);
> + if (!sram_ns_base_addr)
> + pr_err("Secondary CPU boot address not found\n");
> + }
> + }
Same comments here.
> +}
> +
> 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 +172,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())
> + __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
I'd rework this for proper error handling, e.g.
int ret;
/* ... */
ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
if (ret && ret != -ENOSYS)
goto fail;
if (ret == -ENOSYS) {
/* Fall back to firmware-less way. */
void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
if (IS_ERR(boot_reg)) {
ret = PTR_ERR(boot_reg);
goto fail;
}
}
/* ... */
fail:
/* Clean-up after error */
Of course, cpu_boot_reg() will need to be converted to follow the
ERR_PTR() model, but IMHO proper error handling is a good reason to do so.
>
> call_firmware_op(cpu_boot, phys_cpu);
>
> @@ -205,6 +231,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 +250,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())
> + __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
I wonder if setting the addresses at this stage is really needed. IMHO
doing it once in exynos_boot_secondary() should be enough. But this is
probably a material for further patch.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-06 8:10 ` Sachin Kamat
0 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-06 8:10 UTC (permalink / raw)
To: linux-samsung-soc
Cc: devicetree, kgene.kim, heiko, arnd, sachin.kamat, t.figa,
robh+dt, linux-arm-kernel
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@linaro.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
Changes since v1:
* Addressed Tomasz Figa's comments
- Fixed sram node for universal_c210
- Check the node status before mapping the address
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
Tested on 4210/4412 Origen, 5250/5420 Arndale and SMDK5420 boards.
---
arch/arm/Kconfig | 1 +
arch/arm/boot/dts/exynos4210-universal_c210.dts | 24 +++++++++
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 | 35 +++++++++++--
11 files changed, 137 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..9813b068cfd8 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -28,6 +28,30 @@
bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw rootwait earlyprintk panic=5 maxcpus=1";
};
+ sram@02020000 {
+ status = "disabled";
+ smp-sram@0 {
+ status = "disabled";
+ };
+
+ smp-sram@1f000 {
+ status = "disabled";
+ };
+ };
+
+ 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";
};
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index cacf6140dd2f..d3d727b0c263 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -31,6 +31,24 @@
pinctrl2 = &pinctrl_2;
};
+ sram@02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x20000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x20000>;
+
+ smp-sram@0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram@1f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x1f000 0x1000>;
+ };
+ };
+
pd_lcd1: lcd1-power-domain@10023CA0 {
compatible = "samsung,exynos4210-pd";
reg = <0x10023CA0 0x20>;
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index c4a9306f8529..75fb3e7e3999 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -37,6 +37,24 @@
interrupts = <2 2>, <3 2>, <18 2>, <19 2>;
};
+ sram@02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x40000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x40000>;
+
+ smp-sram@0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram@2f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x2f000 0x1000>;
+ };
+ };
+
pd_isp: isp-power-domain@10023CA0 {
compatible = "samsung,exynos4210-pd";
reg = <0x10023CA0 0x20>;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 37423314a028..8d724d56a5c6 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -72,6 +72,24 @@
};
};
+ sram@02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x30000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x30000>;
+
+ smp-sram@0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram@2f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x2f000 0x1000>;
+ };
+ };
+
pd_gsc: gsc-power-domain@10044000 {
compatible = "samsung,exynos4210-pd";
reg = <0x10044000 0x20>;
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index c3a9a66c5767..ff496adfabde 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -110,6 +110,24 @@
};
};
+ sram@02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x54000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x54000>;
+
+ smp-sram@0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram@53000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x53000 0x1000>;
+ };
+ };
+
clock: clock-controller@10010000 {
compatible = "samsung,exynos5420-clock";
reg = <0x10010000 0x30000>;
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9ef3f83efaff..47cbab0f008e 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -18,6 +18,7 @@
void mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1);
struct map_desc;
+extern void __iomem *sram_ns_base_addr;
void exynos_init_io(void);
void exynos_restart(enum reboot_mode mode, const char *cmd);
void exynos_cpuidle_init(void);
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 77293d39dfc9..556d148e6413 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -114,51 +114,6 @@ static struct map_desc exynos4_iodesc[] __initdata = {
},
};
-static struct map_desc exynos4_iodesc0[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS4_PA_SYSRAM0),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4_iodesc1[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS4_PA_SYSRAM1),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4210_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS4210_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4x12_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS4x12_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos5250_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS5250_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
static struct map_desc exynos5_iodesc[] __initdata = {
{
.virtual = (unsigned long)S3C_VA_SYS,
@@ -181,11 +136,6 @@ static struct map_desc exynos5_iodesc[] __initdata = {
.length = SZ_4K,
.type = MT_DEVICE,
}, {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS5_PA_SYSRAM),
- .length = SZ_4K,
- .type = MT_DEVICE,
- }, {
.virtual = (unsigned long)S5P_VA_CMU,
.pfn = __phys_to_pfn(EXYNOS5_PA_CMU),
.length = 144 * SZ_1K,
@@ -280,20 +230,6 @@ static void __init exynos_map_io(void)
if (soc_is_exynos5())
iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos5_iodesc));
-
- if (soc_is_exynos4210()) {
- if (samsung_rev() == EXYNOS4210_REV_0)
- iotable_init(exynos4_iodesc0,
- ARRAY_SIZE(exynos4_iodesc0));
- else
- iotable_init(exynos4_iodesc1,
- ARRAY_SIZE(exynos4_iodesc1));
- iotable_init(exynos4210_iodesc, ARRAY_SIZE(exynos4210_iodesc));
- }
- if (soc_is_exynos4212() || soc_is_exynos4412())
- iotable_init(exynos4x12_iodesc, ARRAY_SIZE(exynos4x12_iodesc));
- if (soc_is_exynos5250())
- iotable_init(exynos5250_iodesc, ARRAY_SIZE(exynos5250_iodesc));
}
void __init exynos_init_io(void)
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index 932129ef26c6..7d583cb73850 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -18,6 +18,7 @@
#include <mach/map.h>
+#include "common.h"
#include "smc.h"
static int exynos_do_idle(void)
@@ -34,7 +35,12 @@ static int exynos_cpu_boot(int cpu)
static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
{
- void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
+ void __iomem *boot_reg;
+
+ if (!sram_ns_base_addr)
+ return 0;
+
+ boot_reg = sram_ns_base_addr + 0x1c + 4*cpu;
__raw_writel(boot_addr, boot_reg);
return 0;
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 7b046b59d9ec..548269a60634 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -23,13 +23,6 @@
#include <plat/map-s5p.h>
-#define EXYNOS4_PA_SYSRAM0 0x02025000
-#define EXYNOS4_PA_SYSRAM1 0x02020000
-#define EXYNOS5_PA_SYSRAM 0x02020000
-#define EXYNOS4210_PA_SYSRAM_NS 0x0203F000
-#define EXYNOS4x12_PA_SYSRAM_NS 0x0204F000
-#define EXYNOS5250_PA_SYSRAM_NS 0x0204F000
-
#define EXYNOS_PA_CHIPID 0x10000000
#define EXYNOS4_PA_SYSCON 0x10010000
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 03e5e9f94705..ccbcdd7b8a86 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,35 @@
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;
+
+ for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
+ if (of_device_is_available(node)) {
+ sram_base_addr = of_iomap(node, 0);
+ if (!sram_base_addr)
+ pr_err("Secondary CPU boot address not found\n");
+ }
+ }
+
+ for_each_compatible_node(node, NULL, "samsung,exynos4210-sram-ns") {
+ if (of_device_is_available(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 +172,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())
+ __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
call_firmware_op(cpu_boot, phys_cpu);
@@ -205,6 +231,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 +250,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())
+ __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
}
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings
@ 2014-05-06 8:10 ` Sachin Kamat
0 siblings, 0 replies; 22+ messages in thread
From: Sachin Kamat @ 2014-05-06 8:10 UTC (permalink / raw)
To: linux-arm-kernel
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@linaro.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
Changes since v1:
* Addressed Tomasz Figa's comments
- Fixed sram node for universal_c210
- Check the node status before mapping the address
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
Tested on 4210/4412 Origen, 5250/5420 Arndale and SMDK5420 boards.
---
arch/arm/Kconfig | 1 +
arch/arm/boot/dts/exynos4210-universal_c210.dts | 24 +++++++++
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 | 35 +++++++++++--
11 files changed, 137 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..9813b068cfd8 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -28,6 +28,30 @@
bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw rootwait earlyprintk panic=5 maxcpus=1";
};
+ sram at 02020000 {
+ status = "disabled";
+ smp-sram at 0 {
+ status = "disabled";
+ };
+
+ smp-sram at 1f000 {
+ status = "disabled";
+ };
+ };
+
+ sram at 02025000 {
+ compatible = "mmio-sram";
+ reg = <0x02025000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02025000 0x1000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+ };
+
mct at 10050000 {
compatible = "none";
};
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index cacf6140dd2f..d3d727b0c263 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -31,6 +31,24 @@
pinctrl2 = &pinctrl_2;
};
+ sram at 02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x20000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x20000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram at 1f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x1f000 0x1000>;
+ };
+ };
+
pd_lcd1: lcd1-power-domain at 10023CA0 {
compatible = "samsung,exynos4210-pd";
reg = <0x10023CA0 0x20>;
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index c4a9306f8529..75fb3e7e3999 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -37,6 +37,24 @@
interrupts = <2 2>, <3 2>, <18 2>, <19 2>;
};
+ sram at 02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x40000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x40000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram at 2f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x2f000 0x1000>;
+ };
+ };
+
pd_isp: isp-power-domain at 10023CA0 {
compatible = "samsung,exynos4210-pd";
reg = <0x10023CA0 0x20>;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 37423314a028..8d724d56a5c6 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -72,6 +72,24 @@
};
};
+ sram at 02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x30000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x30000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram at 2f000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x2f000 0x1000>;
+ };
+ };
+
pd_gsc: gsc-power-domain at 10044000 {
compatible = "samsung,exynos4210-pd";
reg = <0x10044000 0x20>;
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index c3a9a66c5767..ff496adfabde 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -110,6 +110,24 @@
};
};
+ sram at 02020000 {
+ compatible = "mmio-sram";
+ reg = <0x02020000 0x54000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x02020000 0x54000>;
+
+ smp-sram at 0 {
+ compatible = "samsung,exynos4210-sram";
+ reg = <0x0 0x1000>;
+ };
+
+ smp-sram at 53000 {
+ compatible = "samsung,exynos4210-sram-ns";
+ reg = <0x53000 0x1000>;
+ };
+ };
+
clock: clock-controller at 10010000 {
compatible = "samsung,exynos5420-clock";
reg = <0x10010000 0x30000>;
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9ef3f83efaff..47cbab0f008e 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -18,6 +18,7 @@
void mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1);
struct map_desc;
+extern void __iomem *sram_ns_base_addr;
void exynos_init_io(void);
void exynos_restart(enum reboot_mode mode, const char *cmd);
void exynos_cpuidle_init(void);
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 77293d39dfc9..556d148e6413 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -114,51 +114,6 @@ static struct map_desc exynos4_iodesc[] __initdata = {
},
};
-static struct map_desc exynos4_iodesc0[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS4_PA_SYSRAM0),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4_iodesc1[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS4_PA_SYSRAM1),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4210_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS4210_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos4x12_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS4x12_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
-static struct map_desc exynos5250_iodesc[] __initdata = {
- {
- .virtual = (unsigned long)S5P_VA_SYSRAM_NS,
- .pfn = __phys_to_pfn(EXYNOS5250_PA_SYSRAM_NS),
- .length = SZ_4K,
- .type = MT_DEVICE,
- },
-};
-
static struct map_desc exynos5_iodesc[] __initdata = {
{
.virtual = (unsigned long)S3C_VA_SYS,
@@ -181,11 +136,6 @@ static struct map_desc exynos5_iodesc[] __initdata = {
.length = SZ_4K,
.type = MT_DEVICE,
}, {
- .virtual = (unsigned long)S5P_VA_SYSRAM,
- .pfn = __phys_to_pfn(EXYNOS5_PA_SYSRAM),
- .length = SZ_4K,
- .type = MT_DEVICE,
- }, {
.virtual = (unsigned long)S5P_VA_CMU,
.pfn = __phys_to_pfn(EXYNOS5_PA_CMU),
.length = 144 * SZ_1K,
@@ -280,20 +230,6 @@ static void __init exynos_map_io(void)
if (soc_is_exynos5())
iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos5_iodesc));
-
- if (soc_is_exynos4210()) {
- if (samsung_rev() == EXYNOS4210_REV_0)
- iotable_init(exynos4_iodesc0,
- ARRAY_SIZE(exynos4_iodesc0));
- else
- iotable_init(exynos4_iodesc1,
- ARRAY_SIZE(exynos4_iodesc1));
- iotable_init(exynos4210_iodesc, ARRAY_SIZE(exynos4210_iodesc));
- }
- if (soc_is_exynos4212() || soc_is_exynos4412())
- iotable_init(exynos4x12_iodesc, ARRAY_SIZE(exynos4x12_iodesc));
- if (soc_is_exynos5250())
- iotable_init(exynos5250_iodesc, ARRAY_SIZE(exynos5250_iodesc));
}
void __init exynos_init_io(void)
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index 932129ef26c6..7d583cb73850 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -18,6 +18,7 @@
#include <mach/map.h>
+#include "common.h"
#include "smc.h"
static int exynos_do_idle(void)
@@ -34,7 +35,12 @@ static int exynos_cpu_boot(int cpu)
static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
{
- void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
+ void __iomem *boot_reg;
+
+ if (!sram_ns_base_addr)
+ return 0;
+
+ boot_reg = sram_ns_base_addr + 0x1c + 4*cpu;
__raw_writel(boot_addr, boot_reg);
return 0;
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 7b046b59d9ec..548269a60634 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -23,13 +23,6 @@
#include <plat/map-s5p.h>
-#define EXYNOS4_PA_SYSRAM0 0x02025000
-#define EXYNOS4_PA_SYSRAM1 0x02020000
-#define EXYNOS5_PA_SYSRAM 0x02020000
-#define EXYNOS4210_PA_SYSRAM_NS 0x0203F000
-#define EXYNOS4x12_PA_SYSRAM_NS 0x0204F000
-#define EXYNOS5250_PA_SYSRAM_NS 0x0204F000
-
#define EXYNOS_PA_CHIPID 0x10000000
#define EXYNOS4_PA_SYSCON 0x10010000
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 03e5e9f94705..ccbcdd7b8a86 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,35 @@
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;
+
+ for_each_compatible_node(node, NULL, "samsung,exynos4210-sram") {
+ if (of_device_is_available(node)) {
+ sram_base_addr = of_iomap(node, 0);
+ if (!sram_base_addr)
+ pr_err("Secondary CPU boot address not found\n");
+ }
+ }
+
+ for_each_compatible_node(node, NULL, "samsung,exynos4210-sram-ns") {
+ if (of_device_is_available(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 +172,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())
+ __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
call_firmware_op(cpu_boot, phys_cpu);
@@ -205,6 +231,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 +250,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())
+ __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
}
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2014-05-07 18:26 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-02 5:06 [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings Sachin Kamat
2014-05-02 5:06 ` Sachin Kamat
2014-05-02 5:06 ` [PATCH v2 2/2] Documentation: DT: Exynos: Bind SRAM though DT Sachin Kamat
2014-05-02 5:06 ` Sachin Kamat
2014-05-02 8:53 ` Arnd Bergmann
2014-05-02 8:53 ` Arnd Bergmann
2014-05-02 17:54 ` [PATCH v2 1/2] ARM: EXYNOS: Map SYSRAM through generic SRAM bindings Tomasz Figa
2014-05-02 17:54 ` Tomasz Figa
2014-05-04 15:17 ` Sachin Kamat
2014-05-04 15:17 ` Sachin Kamat
2014-05-06 8:10 Sachin Kamat
2014-05-06 8:10 ` Sachin Kamat
2014-05-06 16:44 ` Tomasz Figa
2014-05-06 16:44 ` Tomasz Figa
2014-05-06 18:09 ` Sachin Kamat
2014-05-06 18:09 ` Sachin Kamat
2014-05-07 4:05 ` Sachin Kamat
2014-05-07 4:05 ` Sachin Kamat
2014-05-07 17:03 ` Tomasz Figa
2014-05-07 17:03 ` Tomasz Figa
2014-05-07 18:26 ` Sachin Kamat
2014-05-07 18:26 ` Sachin Kamat
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.