* [PATCH 0/5] ARM: EXYNOS: Add secure firmware support
@ 2012-09-13 8:13 Tomasz Figa
2012-09-13 8:13 ` [PATCH 1/5] ARM: EXYNOS: Add IO mapping for non-secure SYSRAM Tomasz Figa
` (5 more replies)
0 siblings, 6 replies; 22+ messages in thread
From: Tomasz Figa @ 2012-09-13 8:13 UTC (permalink / raw)
To: linux-arm-kernel
Some Exynos-based boards are running with secure firmware running in
TrustZone secure world, which changes the way some things have to be
initialized.
This series adds support for specifying firmware operations, implements
some firmware operations for Exynos secure firmware and adds a method of
enabling secure firmware operations on Exynos-based boards through board
file and device tree.
This is a continuation of the patch series by Kyungmin Park:
[PATCH v5 1/2] ARM: Make a compile firmware conditionally
http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607
[PATCH v5 2/2] ARM: EXYNOS: SMC instruction (aka firmware) support
http://thread.gmane.org/gmane.linux.ports.arm.kernel/183608/focus=184109
Tomasz Figa (5):
ARM: EXYNOS: Add IO mapping for non-secure SYSRAM.
ARM: Add interface for registering and calling firmware-specific
operations
ARM: EXYNOS: Add support for secure monitor calls
ARM: EXYNOS: Add support for Exynos secure firmware
ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up
.../devicetree/bindings/arm/samsung-boards.txt | 8 ++++
arch/arm/common/Makefile | 2 +
arch/arm/common/firmware.c | 18 ++++++++
arch/arm/include/asm/firmware.h | 30 +++++++++++++
arch/arm/mach-exynos/Makefile | 6 +++
arch/arm/mach-exynos/common.c | 34 ++++++++++++++
arch/arm/mach-exynos/common.h | 2 +
arch/arm/mach-exynos/exynos-smc.S | 22 +++++++++
arch/arm/mach-exynos/firmware.c | 52 ++++++++++++++++++++++
arch/arm/mach-exynos/include/mach/map.h | 3 ++
arch/arm/mach-exynos/mach-exynos4-dt.c | 1 +
arch/arm/mach-exynos/platsmp.c | 8 ++++
arch/arm/mach-exynos/smc.h | 31 +++++++++++++
arch/arm/plat-samsung/include/plat/map-s5p.h | 1 +
14 files changed, 218 insertions(+)
create mode 100644 arch/arm/common/firmware.c
create mode 100644 arch/arm/include/asm/firmware.h
create mode 100644 arch/arm/mach-exynos/exynos-smc.S
create mode 100644 arch/arm/mach-exynos/firmware.c
create mode 100644 arch/arm/mach-exynos/smc.h
--
1.7.12
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/5] ARM: EXYNOS: Add IO mapping for non-secure SYSRAM.
2012-09-13 8:13 [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
@ 2012-09-13 8:13 ` Tomasz Figa
2012-09-13 8:13 ` [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations Tomasz Figa
` (4 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2012-09-13 8:13 UTC (permalink / raw)
To: linux-arm-kernel
On TrustZone-enabled boards the non-secure SYSRAM is used for secondary
CPU bring-up, so add a mapping for it.
Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
arch/arm/mach-exynos/common.c | 34 ++++++++++++++++++++++++++++
arch/arm/mach-exynos/include/mach/map.h | 3 +++
arch/arm/plat-samsung/include/plat/map-s5p.h | 1 +
3 files changed, 38 insertions(+)
diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 715b690..b87feac 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -214,6 +214,33 @@ static struct map_desc exynos4_iodesc1[] __initdata = {
},
};
+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,
@@ -321,6 +348,13 @@ static void __init exynos4_map_io(void)
else
iotable_init(exynos4_iodesc1, ARRAY_SIZE(exynos4_iodesc1));
+ if (soc_is_exynos4210())
+ 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));
+
/* initialize device information early */
exynos4_default_sdhci0();
exynos4_default_sdhci1();
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 51943f2..869d8a0 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -26,6 +26,9 @@
#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 EXYNOS4_PA_FIMC0 0x11800000
#define EXYNOS4_PA_FIMC1 0x11810000
diff --git a/arch/arm/plat-samsung/include/plat/map-s5p.h b/arch/arm/plat-samsung/include/plat/map-s5p.h
index c2d7bda..c186786 100644
--- a/arch/arm/plat-samsung/include/plat/map-s5p.h
+++ b/arch/arm/plat-samsung/include/plat/map-s5p.h
@@ -22,6 +22,7 @@
#define S5P_VA_GPIO3 S3C_ADDR(0x02280000)
#define S5P_VA_SYSRAM S3C_ADDR(0x02400000)
+#define S5P_VA_SYSRAM_NS S3C_ADDR(0x02410000)
#define S5P_VA_DMC0 S3C_ADDR(0x02440000)
#define S5P_VA_DMC1 S3C_ADDR(0x02480000)
#define S5P_VA_SROMC S3C_ADDR(0x024C0000)
--
1.7.12
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations
2012-09-13 8:13 [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
2012-09-13 8:13 ` [PATCH 1/5] ARM: EXYNOS: Add IO mapping for non-secure SYSRAM Tomasz Figa
@ 2012-09-13 8:13 ` Tomasz Figa
2012-09-13 8:25 ` Tomasz Figa
2012-09-22 5:50 ` Olof Johansson
2012-09-13 8:13 ` [PATCH 3/5] ARM: EXYNOS: Add support for secure monitor calls Tomasz Figa
` (3 subsequent siblings)
5 siblings, 2 replies; 22+ messages in thread
From: Tomasz Figa @ 2012-09-13 8:13 UTC (permalink / raw)
To: linux-arm-kernel
Some boards are running with secure firmware running in TrustZone secure
world, which changes the way some things have to be initialized.
This patch adds an interface for platforms to specify available firmware
operations and call them.
A wrapper macro, call_firmware_op(), checks if the operation is provided
and calls it if so, otherwise returns 0.
By default no operations are provided.
This is a follow-up on the patch by Kyungmin Park:
[PATCH v5 1/2] ARM: Make a compile firmware conditionally
http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607/focus=183988
Example of use:
In code using firmware ops:
__raw_writel(virt_to_phys(exynos4_secondary_startup),
CPU1_BOOT_REG);
/* Call Exynos specific smc call */
do_firmware_op(cpu_boot, cpu);
gic_raise_softirq(cpumask_of(cpu), 1);
In board-/platform-specific code:
static int platformX_do_idle(void)
{
/* tell platformX firmware to enter idle */
return 0;
}
static void platformX_cpu_boot(int i)
{
/* tell platformX firmware to boot CPU i */
}
static const struct firmware_ops platformX_firmware_ops __initdata = {
.do_idle = exynos_do_idle,
.cpu_boot = exynos_cpu_boot,
/* cpu_boot_reg not available on platformX */
};
static void __init board_init_early(void)
{
????????register_firmware_ops(&platformX_firmware_ops);
}
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
arch/arm/common/Makefile | 2 ++
arch/arm/common/firmware.c | 18 ++++++++++++++++++
arch/arm/include/asm/firmware.h | 30 ++++++++++++++++++++++++++++++
3 files changed, 50 insertions(+)
create mode 100644 arch/arm/common/firmware.c
create mode 100644 arch/arm/include/asm/firmware.h
diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index e8a4e58..55d4182 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -2,6 +2,8 @@
# Makefile for the linux kernel.
#
+obj-y += firmware.o
+
obj-$(CONFIG_ARM_GIC) += gic.o
obj-$(CONFIG_ARM_VIC) += vic.o
obj-$(CONFIG_ICST) += icst.o
diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
new file mode 100644
index 0000000..27ddccb
--- /dev/null
+++ b/arch/arm/common/firmware.c
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park <kyungmin.park@samsung.com>
+ * Tomasz Figa <t.figa@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/suspend.h>
+
+#include <asm/firmware.h>
+
+static const struct firmware_ops default_firmware_ops;
+
+const struct firmware_ops *firmware_ops = &default_firmware_ops;
diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
new file mode 100644
index 0000000..ed51b02
--- /dev/null
+++ b/arch/arm/include/asm/firmware.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park <kyungmin.park@samsung.com>
+ * Tomasz Figa <t.figa@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARM_FIRMWARE_H
+#define __ASM_ARM_FIRMWARE_H
+
+struct firmware_ops {
+ int (*do_idle)(void);
+ void (*cpu_boot)(int cpu);
+ void __iomem *(*cpu_boot_reg)(int cpu);
+};
+
+extern const struct firmware_ops *firmware_ops;
+
+#define call_firmware_op(op, ...) \
+ ((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : 0)
+
+static inline void register_firmware_ops(const struct firmware_ops *ops)
+{
+ firmware_ops = ops;
+}
+
+#endif
--
1.7.12
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/5] ARM: EXYNOS: Add support for secure monitor calls
2012-09-13 8:13 [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
2012-09-13 8:13 ` [PATCH 1/5] ARM: EXYNOS: Add IO mapping for non-secure SYSRAM Tomasz Figa
2012-09-13 8:13 ` [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations Tomasz Figa
@ 2012-09-13 8:13 ` Tomasz Figa
2012-09-13 8:13 ` [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware Tomasz Figa
` (2 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2012-09-13 8:13 UTC (permalink / raw)
To: linux-arm-kernel
Some boards use secure monitor calls to communicate with secure
firmware.
This patch adds exynos_smc function which uses smc assembly instruction
to do secure monitor calls.
This is a follow-up on the patch by Kyungmin Park:
[PATCH v5 2/2] ARM: EXYNOS: SMC instruction (aka firmware) support
http://thread.gmane.org/gmane.linux.ports.arm.kernel/183608/focus=184109
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
arch/arm/mach-exynos/Makefile | 5 +++++
arch/arm/mach-exynos/exynos-smc.S | 22 ++++++++++++++++++++++
arch/arm/mach-exynos/smc.h | 31 +++++++++++++++++++++++++++++++
3 files changed, 58 insertions(+)
create mode 100644 arch/arm/mach-exynos/exynos-smc.S
create mode 100644 arch/arm/mach-exynos/smc.h
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index ee7fde9..997864b 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -30,6 +30,11 @@ obj-$(CONFIG_EXYNOS4_MCT) += mct.o
obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
+obj-$(CONFIG_ARCH_EXYNOS) += exynos-smc.o
+
+plus_sec := $(call as-instr,.arch_extension sec,+sec)
+AFLAGS_exynos-smc.o :=-Wa,-march=armv7-a$(plus_sec)
+
# machine support
obj-$(CONFIG_MACH_SMDKC210) += mach-smdkv310.o
diff --git a/arch/arm/mach-exynos/exynos-smc.S b/arch/arm/mach-exynos/exynos-smc.S
new file mode 100644
index 0000000..2e27aa3
--- /dev/null
+++ b/arch/arm/mach-exynos/exynos-smc.S
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ *
+ * Copied from omap-smc.S Copyright (C) 2010 Texas Instruments, Inc.
+ *
+ * This program is free software,you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+
+/*
+ * Function signature: void exynos_smc(u32 cmd, u32 arg1, u32 arg2, u32 arg3)
+ */
+
+ENTRY(exynos_smc)
+ stmfd sp!, {r4-r11, lr}
+ dsb
+ smc #0
+ ldmfd sp!, {r4-r11, pc}
+ENDPROC(exynos_smc)
diff --git a/arch/arm/mach-exynos/smc.h b/arch/arm/mach-exynos/smc.h
new file mode 100644
index 0000000..e972390
--- /dev/null
+++ b/arch/arm/mach-exynos/smc.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2012 Samsung Electronics.
+ *
+ * EXYNOS - SMC Call
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARCH_EXYNOS_SMC_H
+#define __ASM_ARCH_EXYNOS_SMC_H
+
+#define SMC_CMD_INIT (-1)
+#define SMC_CMD_INFO (-2)
+/* For Power Management */
+#define SMC_CMD_SLEEP (-3)
+#define SMC_CMD_CPU1BOOT (-4)
+#define SMC_CMD_CPU0AFTR (-5)
+/* For CP15 Access */
+#define SMC_CMD_C15RESUME (-11)
+/* For L2 Cache Access */
+#define SMC_CMD_L2X0CTRL (-21)
+#define SMC_CMD_L2X0SETUP1 (-22)
+#define SMC_CMD_L2X0SETUP2 (-23)
+#define SMC_CMD_L2X0INVALL (-24)
+#define SMC_CMD_L2X0DEBUG (-25)
+
+extern void exynos_smc(u32 cmd, u32 arg1, u32 arg2, u32 arg3);
+
+#endif
--
1.7.12
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware
2012-09-13 8:13 [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
` (2 preceding siblings ...)
2012-09-13 8:13 ` [PATCH 3/5] ARM: EXYNOS: Add support for secure monitor calls Tomasz Figa
@ 2012-09-13 8:13 ` Tomasz Figa
2012-09-16 0:44 ` Olof Johansson
2012-09-13 8:13 ` [PATCH 5/5] ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up Tomasz Figa
2012-09-19 13:29 ` [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
5 siblings, 1 reply; 22+ messages in thread
From: Tomasz Figa @ 2012-09-13 8:13 UTC (permalink / raw)
To: linux-arm-kernel
Some Exynos-based boards contain secure firmware and must use firmware
operations to set up some hardware.
This patch adds firmware operations for Exynos secure firmware and a way
for board code and device tree to specify that they must be used.
Example of use:
In board code:
...MACHINE_START(...)
/* ... */
.init_early = exynos_firmware_init,
/* ... */
MACHINE_END
In device tree:
/ {
/* ... */
firmware {
compatible = "samsung,secure-firmware";
};
/* ... */
};
This is a follow-up on the patch by Kyungmin Park:
[PATCH v5 2/2] ARM: EXYNOS: SMC instruction (aka firmware) support
http://thread.gmane.org/gmane.linux.ports.arm.kernel/183608/focus=184109
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
.../devicetree/bindings/arm/samsung-boards.txt | 8 ++++
arch/arm/mach-exynos/Makefile | 1 +
arch/arm/mach-exynos/common.h | 2 +
arch/arm/mach-exynos/firmware.c | 52 ++++++++++++++++++++++
arch/arm/mach-exynos/mach-exynos4-dt.c | 1 +
5 files changed, 64 insertions(+)
create mode 100644 arch/arm/mach-exynos/firmware.c
diff --git a/Documentation/devicetree/bindings/arm/samsung-boards.txt b/Documentation/devicetree/bindings/arm/samsung-boards.txt
index 0bf68be..f447059 100644
--- a/Documentation/devicetree/bindings/arm/samsung-boards.txt
+++ b/Documentation/devicetree/bindings/arm/samsung-boards.txt
@@ -6,3 +6,11 @@ Required root node properties:
- compatible = should be one or more of the following.
(a) "samsung,smdkv310" - for Samsung's SMDKV310 eval board.
(b) "samsung,exynos4210" - for boards based on Exynos4210 SoC.
+
+Optional:
+ - firmware node, specifying presence and type of secure firmware, currently
+ supported value of compatible property is "samsung,secure-firmware":
+
+ firmware {
+ compatible = "samsung,secure-firmware";
+ };
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 997864b..9451637 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_EXYNOS4_MCT) += mct.o
obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
obj-$(CONFIG_ARCH_EXYNOS) += exynos-smc.o
+obj-$(CONFIG_ARCH_EXYNOS) += firmware.o
plus_sec := $(call as-instr,.arch_extension sec,+sec)
AFLAGS_exynos-smc.o :=-Wa,-march=armv7-a$(plus_sec)
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index d7f28ca..358f6a5 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -21,6 +21,8 @@ void exynos4_restart(char mode, const char *cmd);
void exynos5_restart(char mode, const char *cmd);
void exynos_init_late(void);
+void exynos_firmware_init(void);
+
#ifdef CONFIG_PM_GENERIC_DOMAINS
int exynos_pm_late_initcall(void);
#else
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
new file mode 100644
index 0000000..3f3641d
--- /dev/null
+++ b/arch/arm/mach-exynos/firmware.c
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park <kyungmin.park@samsung.com>
+ * Tomasz Figa <t.figa@samsung.com>
+ *
+ * This program is free software,you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/of.h>
+
+#include <asm/firmware.h>
+
+#include <mach/map.h>
+
+#include "smc.h"
+
+static int exynos_do_idle(void)
+{
+ exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
+ return 0;
+}
+
+static void exynos_cpu_boot(int cpu)
+{
+ exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
+}
+
+static void __iomem *exynos_cpu_boot_reg(int cpu)
+{
+ return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
+}
+
+static const struct firmware_ops exynos_firmware_ops __initdata = {
+ .do_idle = exynos_do_idle,
+ .cpu_boot = exynos_cpu_boot,
+ .cpu_boot_reg = exynos_cpu_boot_reg,
+};
+
+void __init exynos_firmware_init(void)
+{
+ if (of_have_populated_dt() &&
+ !of_find_compatible_node(NULL, NULL, "samsung,secure-firmware"))
+ return;
+
+ pr_info("Running under secure firmware.\n");
+
+ register_firmware_ops(&exynos_firmware_ops);
+}
diff --git a/arch/arm/mach-exynos/mach-exynos4-dt.c b/arch/arm/mach-exynos/mach-exynos4-dt.c
index 5c48c82..e0827b3 100644
--- a/arch/arm/mach-exynos/mach-exynos4-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos4-dt.c
@@ -116,6 +116,7 @@ DT_MACHINE_START(EXYNOS4210_DT, "Samsung Exynos4 (Flattened Device Tree)")
.init_irq = exynos4_init_irq,
.map_io = exynos4_dt_map_io,
.handle_irq = gic_handle_irq,
+ .init_early = exynos_firmware_init,
.init_machine = exynos4_dt_machine_init,
.init_late = exynos_init_late,
.timer = &exynos4_timer,
--
1.7.12
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 5/5] ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up
2012-09-13 8:13 [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
` (3 preceding siblings ...)
2012-09-13 8:13 ` [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware Tomasz Figa
@ 2012-09-13 8:13 ` Tomasz Figa
2012-09-19 13:29 ` [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
5 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2012-09-13 8:13 UTC (permalink / raw)
To: linux-arm-kernel
Boards using secure firmware must use different CPU boot registers and
call secure firmware to boot the CPU.
This is a follow-up on the patch by Kyungmin Park:
[PATCH v5 2/2] ARM: EXYNOS: SMC instruction (aka firmware) support
http://thread.gmane.org/gmane.linux.ports.arm.kernel/183608/focus=184109
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
arch/arm/mach-exynos/platsmp.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 816a27d..9d65b1b 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -25,6 +25,7 @@
#include <asm/hardware/gic.h>
#include <asm/smp_plat.h>
#include <asm/smp_scu.h>
+#include <asm/firmware.h>
#include <mach/hardware.h>
#include <mach/regs-clock.h>
@@ -43,6 +44,9 @@ static inline void __iomem *cpu_boot_reg_base(void)
static inline void __iomem *cpu_boot_reg(int cpu)
{
+ void __iomem *fw_boot_reg = call_firmware_op(cpu_boot_reg, cpu);
+ if (fw_boot_reg)
+ return fw_boot_reg;
if (soc_is_exynos4412())
return cpu_boot_reg_base() + 4*cpu;
return cpu_boot_reg_base();
@@ -151,6 +155,10 @@ int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle)
__raw_writel(virt_to_phys(exynos4_secondary_startup),
cpu_boot_reg(phys_cpu));
+
+ /* Call Exynos specific smc call */
+ call_firmware_op(cpu_boot, phys_cpu);
+
gic_raise_softirq(cpumask_of(cpu), 1);
if (pen_release == -1)
--
1.7.12
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations
2012-09-13 8:13 ` [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations Tomasz Figa
@ 2012-09-13 8:25 ` Tomasz Figa
2012-09-22 5:50 ` Olof Johansson
1 sibling, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2012-09-13 8:25 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 13 of September 2012 10:13:35 Tomasz Figa wrote:
> In code using firmware ops:
>
> __raw_writel(virt_to_phys(exynos4_secondary_startup),
> CPU1_BOOT_REG);
>
> /* Call Exynos specific smc call */
> do_firmware_op(cpu_boot, cpu);
Typo, s/do_/call_/ .
Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware
2012-09-13 8:13 ` [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware Tomasz Figa
@ 2012-09-16 0:44 ` Olof Johansson
2012-09-19 10:10 ` Tomasz Figa
2012-09-24 14:39 ` Tomasz Figa
0 siblings, 2 replies; 22+ messages in thread
From: Olof Johansson @ 2012-09-16 0:44 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
> Some Exynos-based boards contain secure firmware and must use firmware
> operations to set up some hardware.
>
> This patch adds firmware operations for Exynos secure firmware and a way
> for board code and device tree to specify that they must be used.
>
> Example of use:
>
> In board code:
>
> ...MACHINE_START(...)
> /* ... */
> .init_early = exynos_firmware_init,
> /* ... */
> MACHINE_END
>
> In device tree:
>
> / {
> /* ... */
>
> firmware {
> compatible = "samsung,secure-firmware";
> };
>
> /* ... */
> };
>
> This is a follow-up on the patch by Kyungmin Park:
> [PATCH v5 2/2] ARM: EXYNOS: SMC instruction (aka firmware) support
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183608/focus=184109
>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
> .../devicetree/bindings/arm/samsung-boards.txt | 8 ++++
> arch/arm/mach-exynos/Makefile | 1 +
> arch/arm/mach-exynos/common.h | 2 +
> arch/arm/mach-exynos/firmware.c | 52 ++++++++++++++++++++++
> arch/arm/mach-exynos/mach-exynos4-dt.c | 1 +
> 5 files changed, 64 insertions(+)
> create mode 100644 arch/arm/mach-exynos/firmware.c
>
> diff --git a/Documentation/devicetree/bindings/arm/samsung-boards.txt b/Documentation/devicetree/bindings/arm/samsung-boards.txt
> index 0bf68be..f447059 100644
> --- a/Documentation/devicetree/bindings/arm/samsung-boards.txt
> +++ b/Documentation/devicetree/bindings/arm/samsung-boards.txt
> @@ -6,3 +6,11 @@ Required root node properties:
> - compatible = should be one or more of the following.
> (a) "samsung,smdkv310" - for Samsung's SMDKV310 eval board.
> (b) "samsung,exynos4210" - for boards based on Exynos4210 SoC.
> +
> +Optional:
> + - firmware node, specifying presence and type of secure firmware, currently
> + supported value of compatible property is "samsung,secure-firmware":
> +
> + firmware {
> + compatible = "samsung,secure-firmware";
> + };
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 997864b..9451637 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_EXYNOS4_MCT) += mct.o
> obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
>
> obj-$(CONFIG_ARCH_EXYNOS) += exynos-smc.o
> +obj-$(CONFIG_ARCH_EXYNOS) += firmware.o
>
> plus_sec := $(call as-instr,.arch_extension sec,+sec)
> AFLAGS_exynos-smc.o :=-Wa,-march=armv7-a$(plus_sec)
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index d7f28ca..358f6a5 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -21,6 +21,8 @@ void exynos4_restart(char mode, const char *cmd);
> void exynos5_restart(char mode, const char *cmd);
> void exynos_init_late(void);
>
> +void exynos_firmware_init(void);
> +
> #ifdef CONFIG_PM_GENERIC_DOMAINS
> int exynos_pm_late_initcall(void);
> #else
> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> new file mode 100644
> index 0000000..3f3641d
> --- /dev/null
> +++ b/arch/arm/mach-exynos/firmware.c
> @@ -0,0 +1,52 @@
> +/*
> + * Copyright (C) 2012 Samsung Electronics.
> + * Kyungmin Park <kyungmin.park@samsung.com>
> + * Tomasz Figa <t.figa@samsung.com>
> + *
> + * This program is free software,you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/of.h>
> +
> +#include <asm/firmware.h>
> +
> +#include <mach/map.h>
> +
> +#include "smc.h"
> +
> +static int exynos_do_idle(void)
> +{
> + exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
> + return 0;
> +}
> +
> +static void exynos_cpu_boot(int cpu)
> +{
> + exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
> +}
> +
> +static void __iomem *exynos_cpu_boot_reg(int cpu)
> +{
> + return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
> +}
This communication area in sysram should probably be seen as a part of
the firmware interface. It should thus be defined as part of the binding
instead, i.e. through a reg property or similar there. That also would
make it easy to convert to using ioremap() instead of iodesc tables,
which always a nice thing.
-Olof
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware
2012-09-16 0:44 ` Olof Johansson
@ 2012-09-19 10:10 ` Tomasz Figa
2012-09-22 5:39 ` Olof Johansson
2012-09-24 14:39 ` Tomasz Figa
1 sibling, 1 reply; 22+ messages in thread
From: Tomasz Figa @ 2012-09-19 10:10 UTC (permalink / raw)
To: linux-arm-kernel
Hi Olof,
On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:
> On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
> > +static void __iomem *exynos_cpu_boot_reg(int cpu)
> > +{
> > + return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
> > +}
>
> This communication area in sysram should probably be seen as a part of
> the firmware interface. It should thus be defined as part of the binding
> instead, i.e. through a reg property or similar there. That also would
> make it easy to convert to using ioremap() instead of iodesc tables,
> which always a nice thing.
The problem with SYSRAM_NS is that it might be also used in other code, not
related to firmware only. I don't know exactly all the use cases for it.
Is it really a big problem or we could let it be for now, merge the patches
for firmware and then convert SYSRAM_NS to dynamic mapping when its
situation clarifies?
Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 0/5] ARM: EXYNOS: Add secure firmware support
2012-09-13 8:13 [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
` (4 preceding siblings ...)
2012-09-13 8:13 ` [PATCH 5/5] ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up Tomasz Figa
@ 2012-09-19 13:29 ` Tomasz Figa
2012-09-21 8:39 ` Kyungmin Park
5 siblings, 1 reply; 22+ messages in thread
From: Tomasz Figa @ 2012-09-19 13:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Thursday 13 of September 2012 10:13:33 Tomasz Figa wrote:
> Some Exynos-based boards are running with secure firmware running in
> TrustZone secure world, which changes the way some things have to be
> initialized.
>
> This series adds support for specifying firmware operations, implements
> some firmware operations for Exynos secure firmware and adds a method of
> enabling secure firmware operations on Exynos-based boards through board
> file and device tree.
>
> This is a continuation of the patch series by Kyungmin Park:
> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607
> [PATCH v5 2/2] ARM: EXYNOS: SMC instruction (aka firmware) support
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183608/focus=184109
>
> Tomasz Figa (5):
> ARM: EXYNOS: Add IO mapping for non-secure SYSRAM.
> ARM: Add interface for registering and calling firmware-specific
> operations
> ARM: EXYNOS: Add support for secure monitor calls
> ARM: EXYNOS: Add support for Exynos secure firmware
> ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up
>
> .../devicetree/bindings/arm/samsung-boards.txt | 8 ++++
> arch/arm/common/Makefile | 2 +
> arch/arm/common/firmware.c | 18 ++++++++
> arch/arm/include/asm/firmware.h | 30 +++++++++++++
> arch/arm/mach-exynos/Makefile | 6 +++
> arch/arm/mach-exynos/common.c | 34 ++++++++++++++
> arch/arm/mach-exynos/common.h | 2 +
> arch/arm/mach-exynos/exynos-smc.S | 22 +++++++++
> arch/arm/mach-exynos/firmware.c | 52
> ++++++++++++++++++++++ arch/arm/mach-exynos/include/mach/map.h
> | 3 ++
> arch/arm/mach-exynos/mach-exynos4-dt.c | 1 +
> arch/arm/mach-exynos/platsmp.c | 8 ++++
> arch/arm/mach-exynos/smc.h | 31 +++++++++++++
> arch/arm/plat-samsung/include/plat/map-s5p.h | 1 +
> 14 files changed, 218 insertions(+)
> create mode 100644 arch/arm/common/firmware.c
> create mode 100644 arch/arm/include/asm/firmware.h
> create mode 100644 arch/arm/mach-exynos/exynos-smc.S
> create mode 100644 arch/arm/mach-exynos/firmware.c
> create mode 100644 arch/arm/mach-exynos/smc.h
Any further comments for this series? Maybe we could merge it for 3.7?
Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 0/5] ARM: EXYNOS: Add secure firmware support
2012-09-19 13:29 ` [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
@ 2012-09-21 8:39 ` Kyungmin Park
2012-09-22 5:52 ` Olof Johansson
0 siblings, 1 reply; 22+ messages in thread
From: Kyungmin Park @ 2012-09-21 8:39 UTC (permalink / raw)
To: linux-arm-kernel
On 9/19/12, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi,
>
> On Thursday 13 of September 2012 10:13:33 Tomasz Figa wrote:
>> Some Exynos-based boards are running with secure firmware running in
>> TrustZone secure world, which changes the way some things have to be
>> initialized.
>>
>> This series adds support for specifying firmware operations, implements
>> some firmware operations for Exynos secure firmware and adds a method of
>> enabling secure firmware operations on Exynos-based boards through board
>> file and device tree.
>>
>> This is a continuation of the patch series by Kyungmin Park:
>> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607
>> [PATCH v5 2/2] ARM: EXYNOS: SMC instruction (aka firmware) support
>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183608/focus=184109
>>
>> Tomasz Figa (5):
>> ARM: EXYNOS: Add IO mapping for non-secure SYSRAM.
>> ARM: Add interface for registering and calling firmware-specific
>> operations
>> ARM: EXYNOS: Add support for secure monitor calls
>> ARM: EXYNOS: Add support for Exynos secure firmware
>> ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up
>>
>> .../devicetree/bindings/arm/samsung-boards.txt | 8 ++++
>> arch/arm/common/Makefile | 2 +
>> arch/arm/common/firmware.c | 18 ++++++++
>> arch/arm/include/asm/firmware.h | 30 +++++++++++++
>> arch/arm/mach-exynos/Makefile | 6 +++
>> arch/arm/mach-exynos/common.c | 34 ++++++++++++++
>> arch/arm/mach-exynos/common.h | 2 +
>> arch/arm/mach-exynos/exynos-smc.S | 22 +++++++++
>> arch/arm/mach-exynos/firmware.c | 52
>> ++++++++++++++++++++++ arch/arm/mach-exynos/include/mach/map.h
>> | 3 ++
>> arch/arm/mach-exynos/mach-exynos4-dt.c | 1 +
>> arch/arm/mach-exynos/platsmp.c | 8 ++++
>> arch/arm/mach-exynos/smc.h | 31 +++++++++++++
>> arch/arm/plat-samsung/include/plat/map-s5p.h | 1 +
>> 14 files changed, 218 insertions(+)
>> create mode 100644 arch/arm/common/firmware.c
>> create mode 100644 arch/arm/include/asm/firmware.h
>> create mode 100644 arch/arm/mach-exynos/exynos-smc.S
>> create mode 100644 arch/arm/mach-exynos/firmware.c
>> create mode 100644 arch/arm/mach-exynos/smc.h
>
> Any further comments for this series? Maybe we could merge it for 3.7?
>
Hi Arnd, Olof,
Can you merge it for 3.7? If exynos implementation still has issues,
it's okay only merge firmware implementation.
Can you give your opinions?
Thank you,
Kyungmin Park
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware
2012-09-19 10:10 ` Tomasz Figa
@ 2012-09-22 5:39 ` Olof Johansson
2012-09-22 5:57 ` Kyungmin Park
0 siblings, 1 reply; 22+ messages in thread
From: Olof Johansson @ 2012-09-22 5:39 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 19, 2012 at 3:10 AM, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Olof,
>
> On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:
>> On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
>> > +static void __iomem *exynos_cpu_boot_reg(int cpu)
>> > +{
>> > + return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>> > +}
>>
>> This communication area in sysram should probably be seen as a part of
>> the firmware interface. It should thus be defined as part of the binding
>> instead, i.e. through a reg property or similar there. That also would
>> make it easy to convert to using ioremap() instead of iodesc tables,
>> which always a nice thing.
>
> The problem with SYSRAM_NS is that it might be also used in other code, not
> related to firmware only. I don't know exactly all the use cases for it.
If you don't know the use cases, and the use cases are not in the
kernel tree that we care about here (upstream), then there's really
nothing to worry about. It's after all just a define that's moved to
an ioremap, if there's some out of tree code that needs the old legacy
define then it can be added in whatever out-of-tree code that uses it.
Right?
> Is it really a big problem or we could let it be for now, merge the patches
> for firmware and then convert SYSRAM_NS to dynamic mapping when its
> situation clarifies?
What do you expect is required to clarify the situation, and when do
you expect that to happen?
-Olof
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations
2012-09-13 8:13 ` [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations Tomasz Figa
2012-09-13 8:25 ` Tomasz Figa
@ 2012-09-22 5:50 ` Olof Johansson
2012-09-22 6:01 ` Kyungmin Park
1 sibling, 1 reply; 22+ messages in thread
From: Olof Johansson @ 2012-09-22 5:50 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 13, 2012 at 10:13:35AM +0200, Tomasz Figa wrote:
> Some boards are running with secure firmware running in TrustZone secure
> world, which changes the way some things have to be initialized.
>
> This patch adds an interface for platforms to specify available firmware
> operations and call them.
>
> A wrapper macro, call_firmware_op(), checks if the operation is provided
> and calls it if so, otherwise returns 0.
>
> By default no operations are provided.
>
> This is a follow-up on the patch by Kyungmin Park:
> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607/focus=183988
>
> Example of use:
>
> In code using firmware ops:
>
> __raw_writel(virt_to_phys(exynos4_secondary_startup),
> CPU1_BOOT_REG);
>
> /* Call Exynos specific smc call */
> do_firmware_op(cpu_boot, cpu);
>
> gic_raise_softirq(cpumask_of(cpu), 1);
>
> In board-/platform-specific code:
>
> static int platformX_do_idle(void)
> {
> /* tell platformX firmware to enter idle */
> return 0;
> }
>
> static void platformX_cpu_boot(int i)
> {
> /* tell platformX firmware to boot CPU i */
> }
>
> static const struct firmware_ops platformX_firmware_ops __initdata = {
> .do_idle = exynos_do_idle,
> .cpu_boot = exynos_cpu_boot,
> /* cpu_boot_reg not available on platformX */
> };
>
> static void __init board_init_early(void)
> {
> ????????register_firmware_ops(&platformX_firmware_ops);
> }
>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
> arch/arm/common/Makefile | 2 ++
> arch/arm/common/firmware.c | 18 ++++++++++++++++++
> arch/arm/include/asm/firmware.h | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 50 insertions(+)
> create mode 100644 arch/arm/common/firmware.c
> create mode 100644 arch/arm/include/asm/firmware.h
>
> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> index e8a4e58..55d4182 100644
> --- a/arch/arm/common/Makefile
> +++ b/arch/arm/common/Makefile
> @@ -2,6 +2,8 @@
> # Makefile for the linux kernel.
> #
>
> +obj-y += firmware.o
> +
> obj-$(CONFIG_ARM_GIC) += gic.o
> obj-$(CONFIG_ARM_VIC) += vic.o
> obj-$(CONFIG_ICST) += icst.o
> diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
> new file mode 100644
> index 0000000..27ddccb
> --- /dev/null
> +++ b/arch/arm/common/firmware.c
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2012 Samsung Electronics.
> + * Kyungmin Park <kyungmin.park@samsung.com>
> + * Tomasz Figa <t.figa@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/suspend.h>
> +
> +#include <asm/firmware.h>
> +
> +static const struct firmware_ops default_firmware_ops;
> +
> +const struct firmware_ops *firmware_ops = &default_firmware_ops;
> diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
> new file mode 100644
> index 0000000..ed51b02
> --- /dev/null
> +++ b/arch/arm/include/asm/firmware.h
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright (C) 2012 Samsung Electronics.
> + * Kyungmin Park <kyungmin.park@samsung.com>
> + * Tomasz Figa <t.figa@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __ASM_ARM_FIRMWARE_H
> +#define __ASM_ARM_FIRMWARE_H
> +
> +struct firmware_ops {
> + int (*do_idle)(void);
> + void (*cpu_boot)(int cpu);
> + void __iomem *(*cpu_boot_reg)(int cpu);
> +};
> +
> +extern const struct firmware_ops *firmware_ops;
> +
> +#define call_firmware_op(op, ...) \
> + ((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : 0)
I think this will cause sparse warnings for call_firmware_op(cpu_boot_reg)
if there are no ops defined, since the '0' isn't annotated as __iomem. And
you can't annotate it since the other function pointers don't need it.
I think you might be better off with stub functions as fallbacks instead of
allowing and checking for NULL here.
-Olof
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 0/5] ARM: EXYNOS: Add secure firmware support
2012-09-21 8:39 ` Kyungmin Park
@ 2012-09-22 5:52 ` Olof Johansson
0 siblings, 0 replies; 22+ messages in thread
From: Olof Johansson @ 2012-09-22 5:52 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 21, 2012 at 05:39:41PM +0900, Kyungmin Park wrote:
> On 9/19/12, Tomasz Figa <t.figa@samsung.com> wrote:
> > Hi,
> >
> > On Thursday 13 of September 2012 10:13:33 Tomasz Figa wrote:
> >> Some Exynos-based boards are running with secure firmware running in
> >> TrustZone secure world, which changes the way some things have to be
> >> initialized.
> >>
> >> This series adds support for specifying firmware operations, implements
> >> some firmware operations for Exynos secure firmware and adds a method of
> >> enabling secure firmware operations on Exynos-based boards through board
> >> file and device tree.
> >>
> >> This is a continuation of the patch series by Kyungmin Park:
> >> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
> >> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607
> >> [PATCH v5 2/2] ARM: EXYNOS: SMC instruction (aka firmware) support
> >> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183608/focus=184109
> >>
> >> Tomasz Figa (5):
> >> ARM: EXYNOS: Add IO mapping for non-secure SYSRAM.
> >> ARM: Add interface for registering and calling firmware-specific
> >> operations
> >> ARM: EXYNOS: Add support for secure monitor calls
> >> ARM: EXYNOS: Add support for Exynos secure firmware
> >> ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up
> >>
> >> .../devicetree/bindings/arm/samsung-boards.txt | 8 ++++
> >> arch/arm/common/Makefile | 2 +
> >> arch/arm/common/firmware.c | 18 ++++++++
> >> arch/arm/include/asm/firmware.h | 30 +++++++++++++
> >> arch/arm/mach-exynos/Makefile | 6 +++
> >> arch/arm/mach-exynos/common.c | 34 ++++++++++++++
> >> arch/arm/mach-exynos/common.h | 2 +
> >> arch/arm/mach-exynos/exynos-smc.S | 22 +++++++++
> >> arch/arm/mach-exynos/firmware.c | 52
> >> ++++++++++++++++++++++ arch/arm/mach-exynos/include/mach/map.h
> >> | 3 ++
> >> arch/arm/mach-exynos/mach-exynos4-dt.c | 1 +
> >> arch/arm/mach-exynos/platsmp.c | 8 ++++
> >> arch/arm/mach-exynos/smc.h | 31 +++++++++++++
> >> arch/arm/plat-samsung/include/plat/map-s5p.h | 1 +
> >> 14 files changed, 218 insertions(+)
> >> create mode 100644 arch/arm/common/firmware.c
> >> create mode 100644 arch/arm/include/asm/firmware.h
> >> create mode 100644 arch/arm/mach-exynos/exynos-smc.S
> >> create mode 100644 arch/arm/mach-exynos/firmware.c
> >> create mode 100644 arch/arm/mach-exynos/smc.h
> >
> > Any further comments for this series? Maybe we could merge it for 3.7?
> >
> Hi Arnd, Olof,
> Can you merge it for 3.7? If exynos implementation still has issues,
> it's okay only merge firmware implementation.
>
> Can you give your opinions?
Hi,
There are still a few open questions to resolve on this before I think it
can be merged. And it doesn't make much sense to merge the infrastructure
without at least the first user of it.
-Olof
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware
2012-09-22 5:39 ` Olof Johansson
@ 2012-09-22 5:57 ` Kyungmin Park
2012-09-22 6:36 ` Olof Johansson
0 siblings, 1 reply; 22+ messages in thread
From: Kyungmin Park @ 2012-09-22 5:57 UTC (permalink / raw)
To: linux-arm-kernel
On 9/22/12, Olof Johansson <olof@lixom.net> wrote:
> On Wed, Sep 19, 2012 at 3:10 AM, Tomasz Figa <t.figa@samsung.com> wrote:
>> Hi Olof,
>>
>> On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:
>>> On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
>>> > +static void __iomem *exynos_cpu_boot_reg(int cpu)
>>> > +{
>>> > + return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>>> > +}
>>>
>>> This communication area in sysram should probably be seen as a part of
>>> the firmware interface. It should thus be defined as part of the binding
>>> instead, i.e. through a reg property or similar there. That also would
>>> make it easy to convert to using ioremap() instead of iodesc tables,
>>> which always a nice thing.
>>
>> The problem with SYSRAM_NS is that it might be also used in other code,
>> not
>> related to firmware only. I don't know exactly all the use cases for it.
>
> If you don't know the use cases, and the use cases are not in the
> kernel tree that we care about here (upstream), then there's really
> nothing to worry about. It's after all just a define that's moved to
> an ioremap, if there's some out of tree code that needs the old legacy
> define then it can be added in whatever out-of-tree code that uses it.
> Right?
Now this address is used at cpu boot, cpuidle, inform at this time.
As it touched at several places, it's defined at iodesc. if it changed
with ioremap, it has to export remaped address and each codes should
use it.
As I wrote at cover letter, if you want to use ioremap, it can be
modified. however can you merge firmware codes itself? since ioremap
is not related with trustzone or firmware issues and it's exynos
specific implementation issues. Right?
Thank you,
Kyungmin Park
>
>> Is it really a big problem or we could let it be for now, merge the
>> patches
>> for firmware and then convert SYSRAM_NS to dynamic mapping when its
>> situation clarifies?
>
> What do you expect is required to clarify the situation, and when do
> you expect that to happen?
>
>
> -Olof
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations
2012-09-22 5:50 ` Olof Johansson
@ 2012-09-22 6:01 ` Kyungmin Park
2012-09-22 6:23 ` Olof Johansson
0 siblings, 1 reply; 22+ messages in thread
From: Kyungmin Park @ 2012-09-22 6:01 UTC (permalink / raw)
To: linux-arm-kernel
On 9/22/12, Olof Johansson <olof@lixom.net> wrote:
> On Thu, Sep 13, 2012 at 10:13:35AM +0200, Tomasz Figa wrote:
>> Some boards are running with secure firmware running in TrustZone secure
>> world, which changes the way some things have to be initialized.
>>
>> This patch adds an interface for platforms to specify available firmware
>> operations and call them.
>>
>> A wrapper macro, call_firmware_op(), checks if the operation is provided
>> and calls it if so, otherwise returns 0.
>>
>> By default no operations are provided.
>>
>> This is a follow-up on the patch by Kyungmin Park:
>> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607/focus=183988
>>
>> Example of use:
>>
>> In code using firmware ops:
>>
>> __raw_writel(virt_to_phys(exynos4_secondary_startup),
>> CPU1_BOOT_REG);
>>
>> /* Call Exynos specific smc call */
>> do_firmware_op(cpu_boot, cpu);
>>
>> gic_raise_softirq(cpumask_of(cpu), 1);
>>
>> In board-/platform-specific code:
>>
>> static int platformX_do_idle(void)
>> {
>> /* tell platformX firmware to enter idle */
>> return 0;
>> }
>>
>> static void platformX_cpu_boot(int i)
>> {
>> /* tell platformX firmware to boot CPU i */
>> }
>>
>> static const struct firmware_ops platformX_firmware_ops __initdata = {
>> .do_idle = exynos_do_idle,
>> .cpu_boot = exynos_cpu_boot,
>> /* cpu_boot_reg not available on platformX */
>> };
>>
>> static void __init board_init_early(void)
>> {
>> register_firmware_ops(&platformX_firmware_ops);
>> }
>>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
>> ---
>> arch/arm/common/Makefile | 2 ++
>> arch/arm/common/firmware.c | 18 ++++++++++++++++++
>> arch/arm/include/asm/firmware.h | 30 ++++++++++++++++++++++++++++++
>> 3 files changed, 50 insertions(+)
>> create mode 100644 arch/arm/common/firmware.c
>> create mode 100644 arch/arm/include/asm/firmware.h
>>
>> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
>> index e8a4e58..55d4182 100644
>> --- a/arch/arm/common/Makefile
>> +++ b/arch/arm/common/Makefile
>> @@ -2,6 +2,8 @@
>> # Makefile for the linux kernel.
>> #
>>
>> +obj-y += firmware.o
>> +
>> obj-$(CONFIG_ARM_GIC) += gic.o
>> obj-$(CONFIG_ARM_VIC) += vic.o
>> obj-$(CONFIG_ICST) += icst.o
>> diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
>> new file mode 100644
>> index 0000000..27ddccb
>> --- /dev/null
>> +++ b/arch/arm/common/firmware.c
>> @@ -0,0 +1,18 @@
>> +/*
>> + * Copyright (C) 2012 Samsung Electronics.
>> + * Kyungmin Park <kyungmin.park@samsung.com>
>> + * Tomasz Figa <t.figa@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/suspend.h>
>> +
>> +#include <asm/firmware.h>
>> +
>> +static const struct firmware_ops default_firmware_ops;
>> +
>> +const struct firmware_ops *firmware_ops = &default_firmware_ops;
>> diff --git a/arch/arm/include/asm/firmware.h
>> b/arch/arm/include/asm/firmware.h
>> new file mode 100644
>> index 0000000..ed51b02
>> --- /dev/null
>> +++ b/arch/arm/include/asm/firmware.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Copyright (C) 2012 Samsung Electronics.
>> + * Kyungmin Park <kyungmin.park@samsung.com>
>> + * Tomasz Figa <t.figa@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef __ASM_ARM_FIRMWARE_H
>> +#define __ASM_ARM_FIRMWARE_H
>> +
>> +struct firmware_ops {
>> + int (*do_idle)(void);
>> + void (*cpu_boot)(int cpu);
>> + void __iomem *(*cpu_boot_reg)(int cpu);
>> +};
>> +
>> +extern const struct firmware_ops *firmware_ops;
>> +
>> +#define call_firmware_op(op, ...) \
>> + ((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : 0)
>
> I think this will cause sparse warnings for call_firmware_op(cpu_boot_reg)
> if there are no ops defined, since the '0' isn't annotated as __iomem. And
> you can't annotate it since the other function pointers don't need it.
>
> I think you might be better off with stub functions as fallbacks instead of
> allowing and checking for NULL here.
do you mean like this?
#Ifdef CONFIG_ARM_FIRMWARE
#define call_firmware_op(op, ...) ((firmware_ops->op) ?
firmware_ops->op(__VA_ARGS__) : 0)
#else
#define call_firmware_op(op, ...) do { } while (0)
#endif
No problem to modify it.
Thank you,
Kyungmin Park
>
>
> -Olof
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations
2012-09-22 6:01 ` Kyungmin Park
@ 2012-09-22 6:23 ` Olof Johansson
2012-09-22 13:17 ` Tomasz Figa
2012-09-22 13:22 ` Russell King - ARM Linux
0 siblings, 2 replies; 22+ messages in thread
From: Olof Johansson @ 2012-09-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Sep 22, 2012 at 03:01:56PM +0900, Kyungmin Park wrote:
> On 9/22/12, Olof Johansson <olof@lixom.net> wrote:
> > On Thu, Sep 13, 2012 at 10:13:35AM +0200, Tomasz Figa wrote:
> >> Some boards are running with secure firmware running in TrustZone secure
> >> world, which changes the way some things have to be initialized.
> >>
> >> This patch adds an interface for platforms to specify available firmware
> >> operations and call them.
> >>
> >> A wrapper macro, call_firmware_op(), checks if the operation is provided
> >> and calls it if so, otherwise returns 0.
> >>
> >> By default no operations are provided.
> >>
> >> This is a follow-up on the patch by Kyungmin Park:
> >> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
> >> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607/focus=183988
> >>
> >> Example of use:
> >>
> >> In code using firmware ops:
> >>
> >> __raw_writel(virt_to_phys(exynos4_secondary_startup),
> >> CPU1_BOOT_REG);
> >>
> >> /* Call Exynos specific smc call */
> >> do_firmware_op(cpu_boot, cpu);
> >>
> >> gic_raise_softirq(cpumask_of(cpu), 1);
> >>
> >> In board-/platform-specific code:
> >>
> >> static int platformX_do_idle(void)
> >> {
> >> /* tell platformX firmware to enter idle */
> >> return 0;
> >> }
> >>
> >> static void platformX_cpu_boot(int i)
> >> {
> >> /* tell platformX firmware to boot CPU i */
> >> }
> >>
> >> static const struct firmware_ops platformX_firmware_ops __initdata = {
> >> .do_idle = exynos_do_idle,
> >> .cpu_boot = exynos_cpu_boot,
> >> /* cpu_boot_reg not available on platformX */
> >> };
> >>
> >> static void __init board_init_early(void)
> >> {
> >> register_firmware_ops(&platformX_firmware_ops);
> >> }
> >>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> >> ---
> >> arch/arm/common/Makefile | 2 ++
> >> arch/arm/common/firmware.c | 18 ++++++++++++++++++
> >> arch/arm/include/asm/firmware.h | 30 ++++++++++++++++++++++++++++++
> >> 3 files changed, 50 insertions(+)
> >> create mode 100644 arch/arm/common/firmware.c
> >> create mode 100644 arch/arm/include/asm/firmware.h
> >>
> >> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> >> index e8a4e58..55d4182 100644
> >> --- a/arch/arm/common/Makefile
> >> +++ b/arch/arm/common/Makefile
> >> @@ -2,6 +2,8 @@
> >> # Makefile for the linux kernel.
> >> #
> >>
> >> +obj-y += firmware.o
> >> +
> >> obj-$(CONFIG_ARM_GIC) += gic.o
> >> obj-$(CONFIG_ARM_VIC) += vic.o
> >> obj-$(CONFIG_ICST) += icst.o
> >> diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
> >> new file mode 100644
> >> index 0000000..27ddccb
> >> --- /dev/null
> >> +++ b/arch/arm/common/firmware.c
> >> @@ -0,0 +1,18 @@
> >> +/*
> >> + * Copyright (C) 2012 Samsung Electronics.
> >> + * Kyungmin Park <kyungmin.park@samsung.com>
> >> + * Tomasz Figa <t.figa@samsung.com>
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + */
> >> +
> >> +#include <linux/kernel.h>
> >> +#include <linux/suspend.h>
> >> +
> >> +#include <asm/firmware.h>
> >> +
> >> +static const struct firmware_ops default_firmware_ops;
> >> +
> >> +const struct firmware_ops *firmware_ops = &default_firmware_ops;
> >> diff --git a/arch/arm/include/asm/firmware.h
> >> b/arch/arm/include/asm/firmware.h
> >> new file mode 100644
> >> index 0000000..ed51b02
> >> --- /dev/null
> >> +++ b/arch/arm/include/asm/firmware.h
> >> @@ -0,0 +1,30 @@
> >> +/*
> >> + * Copyright (C) 2012 Samsung Electronics.
> >> + * Kyungmin Park <kyungmin.park@samsung.com>
> >> + * Tomasz Figa <t.figa@samsung.com>
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + */
> >> +
> >> +#ifndef __ASM_ARM_FIRMWARE_H
> >> +#define __ASM_ARM_FIRMWARE_H
> >> +
> >> +struct firmware_ops {
> >> + int (*do_idle)(void);
> >> + void (*cpu_boot)(int cpu);
> >> + void __iomem *(*cpu_boot_reg)(int cpu);
> >> +};
> >> +
> >> +extern const struct firmware_ops *firmware_ops;
> >> +
> >> +#define call_firmware_op(op, ...) \
> >> + ((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : 0)
> >
> > I think this will cause sparse warnings for call_firmware_op(cpu_boot_reg)
> > if there are no ops defined, since the '0' isn't annotated as __iomem. And
> > you can't annotate it since the other function pointers don't need it.
> >
> > I think you might be better off with stub functions as fallbacks instead of
> > allowing and checking for NULL here.
> do you mean like this?
>
> #Ifdef CONFIG_ARM_FIRMWARE
> #define call_firmware_op(op, ...) ((firmware_ops->op) ?
> firmware_ops->op(__VA_ARGS__) : 0)
> #else
> #define call_firmware_op(op, ...) do { } while (0)
> #endif
>
> No problem to modify it.
To get the types and return values right you still need to do something
like:
#define call_firmware_op(op, ...) (firmware_ops->op(__VA_ARGS))
And then, in firmware.c:
static int default_do_idle(void)
{
return 0;
}
static default_cpu_boot(int cpu)
{
return;
}
static void __iomem *default_cpu_boot_reg(int cpu)
{
return (void __iomem *)0;
}
static const struct firmware_ops default_firmware_ops = {
.do_idle = default_do_idle,
.cpu_boot = default_cpu_boot,
.cpu_boot_reg = default_cpu_boot_reg,
}
const struct firmware_ops *firmware_ops = &default_firmware_ops;
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware
2012-09-22 5:57 ` Kyungmin Park
@ 2012-09-22 6:36 ` Olof Johansson
2012-09-22 6:39 ` Kyungmin Park
0 siblings, 1 reply; 22+ messages in thread
From: Olof Johansson @ 2012-09-22 6:36 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 21, 2012 at 10:57 PM, Kyungmin Park
<kyungmin.park@samsung.com> wrote:
> On 9/22/12, Olof Johansson <olof@lixom.net> wrote:
>> On Wed, Sep 19, 2012 at 3:10 AM, Tomasz Figa <t.figa@samsung.com> wrote:
>>> Hi Olof,
>>>
>>> On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:
>>>> On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
>>>> > +static void __iomem *exynos_cpu_boot_reg(int cpu)
>>>> > +{
>>>> > + return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>>>> > +}
>>>>
>>>> This communication area in sysram should probably be seen as a part of
>>>> the firmware interface. It should thus be defined as part of the binding
>>>> instead, i.e. through a reg property or similar there. That also would
>>>> make it easy to convert to using ioremap() instead of iodesc tables,
>>>> which always a nice thing.
>>>
>>> The problem with SYSRAM_NS is that it might be also used in other code,
>>> not
>>> related to firmware only. I don't know exactly all the use cases for it.
>>
>> If you don't know the use cases, and the use cases are not in the
>> kernel tree that we care about here (upstream), then there's really
>> nothing to worry about. It's after all just a define that's moved to
>> an ioremap, if there's some out of tree code that needs the old legacy
>> define then it can be added in whatever out-of-tree code that uses it.
>> Right?
> Now this address is used at cpu boot, cpuidle, inform at this time.
> As it touched at several places, it's defined at iodesc. if it changed
> with ioremap, it has to export remaped address and each codes should
> use it.
Won't you have to push down all the references to VA_SYSRAM vs
VA_SYSRAM_NS into the firmware interface anyway, since you will need
to use different addresses for whether you have firmware enabled or
not? I.e. you'll have a "firmware call" at the appropriate level for
the non-trustzone cases that uses the equivalent of VA_SYSRAM, and for
the trustzone firmware op you'll use VA_SYSRAM_NS?
> As I wrote at cover letter, if you want to use ioremap, it can be
> modified. however can you merge firmware codes itself? since ioremap
> is not related with trustzone or firmware issues and it's exynos
> specific implementation issues. Right?
I'm not quite sure which part you are asking me to merge, if it's the
infrastructure pieces or the exynos-specific pieces.
Either way, one isn't really usable without the other, and it doesn't
make sense to merge code that can't be used. Infrastructure is best
merged together with the first user of it.
-Olof
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware
2012-09-22 6:36 ` Olof Johansson
@ 2012-09-22 6:39 ` Kyungmin Park
0 siblings, 0 replies; 22+ messages in thread
From: Kyungmin Park @ 2012-09-22 6:39 UTC (permalink / raw)
To: linux-arm-kernel
On 9/22/12, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 21, 2012 at 10:57 PM, Kyungmin Park
> <kyungmin.park@samsung.com> wrote:
>> On 9/22/12, Olof Johansson <olof@lixom.net> wrote:
>>> On Wed, Sep 19, 2012 at 3:10 AM, Tomasz Figa <t.figa@samsung.com> wrote:
>>>> Hi Olof,
>>>>
>>>> On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:
>>>>> On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
>>>>> > +static void __iomem *exynos_cpu_boot_reg(int cpu)
>>>>> > +{
>>>>> > + return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>>>>> > +}
>>>>>
>>>>> This communication area in sysram should probably be seen as a part of
>>>>> the firmware interface. It should thus be defined as part of the
>>>>> binding
>>>>> instead, i.e. through a reg property or similar there. That also
>>>>> would
>>>>> make it easy to convert to using ioremap() instead of iodesc tables,
>>>>> which always a nice thing.
>>>>
>>>> The problem with SYSRAM_NS is that it might be also used in other code,
>>>> not
>>>> related to firmware only. I don't know exactly all the use cases for
>>>> it.
>>>
>>> If you don't know the use cases, and the use cases are not in the
>>> kernel tree that we care about here (upstream), then there's really
>>> nothing to worry about. It's after all just a define that's moved to
>>> an ioremap, if there's some out of tree code that needs the old legacy
>>> define then it can be added in whatever out-of-tree code that uses it.
>>> Right?
>> Now this address is used at cpu boot, cpuidle, inform at this time.
>> As it touched at several places, it's defined at iodesc. if it changed
>> with ioremap, it has to export remaped address and each codes should
>> use it.
>
> Won't you have to push down all the references to VA_SYSRAM vs
> VA_SYSRAM_NS into the firmware interface anyway, since you will need
> to use different addresses for whether you have firmware enabled or
> not? I.e. you'll have a "firmware call" at the appropriate level for
> the non-trustzone cases that uses the equivalent of VA_SYSRAM, and for
> the trustzone firmware op you'll use VA_SYSRAM_NS?
Right, in case of no firmware, it uses VA_SYSRAM, but VA_SYSRAM_NS is
used at firmware case.
>
>
>> As I wrote at cover letter, if you want to use ioremap, it can be
>> modified. however can you merge firmware codes itself? since ioremap
>> is not related with trustzone or firmware issues and it's exynos
>> specific implementation issues. Right?
>
> I'm not quite sure which part you are asking me to merge, if it's the
> infrastructure pieces or the exynos-specific pieces.
>
> Either way, one isn't really usable without the other, and it doesn't
> make sense to merge code that can't be used. Infrastructure is best
> merged together with the first user of it.
Okay, we will fix it.
Thank you,
Kyungmin Park
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations
2012-09-22 6:23 ` Olof Johansson
@ 2012-09-22 13:17 ` Tomasz Figa
2012-09-22 13:22 ` Russell King - ARM Linux
1 sibling, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2012-09-22 13:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Olof, Kyungmin,
On Friday 21 of September 2012 23:23:22 Olof Johansson wrote:
> On Sat, Sep 22, 2012 at 03:01:56PM +0900, Kyungmin Park wrote:
> > On 9/22/12, Olof Johansson <olof@lixom.net> wrote:
> > > On Thu, Sep 13, 2012 at 10:13:35AM +0200, Tomasz Figa wrote:
> > >> +#define call_firmware_op(op, ...) \
> > >> + ((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : 0)
> > >
> > > I think this will cause sparse warnings for
> > > call_firmware_op(cpu_boot_reg) if there are no ops defined, since
> > > the '0' isn't annotated as __iomem. And you can't annotate it since
> > > the other function pointers don't need it.
> > >
> > > I think you might be better off with stub functions as fallbacks
> > > instead of allowing and checking for NULL here.
> >
> > do you mean like this?
> >
> > #Ifdef CONFIG_ARM_FIRMWARE
> > #define call_firmware_op(op, ...) ((firmware_ops->op) ?
> > firmware_ops->op(__VA_ARGS__) : 0)
> > #else
> > #define call_firmware_op(op, ...) do { } while (0)
> > #endif
> >
> > No problem to modify it.
>
> To get the types and return values right you still need to do something
> like:
>
> #define call_firmware_op(op, ...) (firmware_ops->op(__VA_ARGS))
>
> And then, in firmware.c:
>
> static int default_do_idle(void)
> {
> return 0;
> }
>
> static default_cpu_boot(int cpu)
> {
> return;
> }
>
> static void __iomem *default_cpu_boot_reg(int cpu)
> {
> return (void __iomem *)0;
> }
>
> static const struct firmware_ops default_firmware_ops = {
> .do_idle = default_do_idle,
> .cpu_boot = default_cpu_boot,
> .cpu_boot_reg = default_cpu_boot_reg,
> }
Thanks for pointing this out. I will address it in next version of the
series.
Also it might be useful to be able to check if an operation was provided,
so maybe an unimplemented operation should return an error value (which one
would be the most appropriate? -ENOSYS?)? What do you think?
Best regards,
Tomasz Figa
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations
2012-09-22 6:23 ` Olof Johansson
2012-09-22 13:17 ` Tomasz Figa
@ 2012-09-22 13:22 ` Russell King - ARM Linux
1 sibling, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2012-09-22 13:22 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 21, 2012 at 11:23:22PM -0700, Olof Johansson wrote:
> static void __iomem *default_cpu_boot_reg(int cpu)
> {
> return (void __iomem *)0;
> }
NULL is perfectly acceptable here.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware
2012-09-16 0:44 ` Olof Johansson
2012-09-19 10:10 ` Tomasz Figa
@ 2012-09-24 14:39 ` Tomasz Figa
1 sibling, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2012-09-24 14:39 UTC (permalink / raw)
To: linux-arm-kernel
Hi Olof,
On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:
> On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
> > +static void __iomem *exynos_cpu_boot_reg(int cpu)
> > +{
> > + return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
> > +}
>
> This communication area in sysram should probably be seen as a part of
> the firmware interface. It should thus be defined as part of the binding
> instead, i.e. through a reg property or similar there. That also would
> make it easy to convert to using ioremap() instead of iodesc tables,
> which always a nice thing.
I have tried to get around the need of statical mapping for SYSRAM, but the
firmware has to be initialized very early, before low level L2x0 cache
initialization (which is an early initcall, so it has to be in init_early
machine callback) and at that time ioremap is not available yet.
I think we should just allow this additional static mapping, I don't see
any sane way of mapping it dynamically, at least at the moment.
Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2012-09-24 14:39 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-13 8:13 [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
2012-09-13 8:13 ` [PATCH 1/5] ARM: EXYNOS: Add IO mapping for non-secure SYSRAM Tomasz Figa
2012-09-13 8:13 ` [PATCH 2/5] ARM: Add interface for registering and calling firmware-specific operations Tomasz Figa
2012-09-13 8:25 ` Tomasz Figa
2012-09-22 5:50 ` Olof Johansson
2012-09-22 6:01 ` Kyungmin Park
2012-09-22 6:23 ` Olof Johansson
2012-09-22 13:17 ` Tomasz Figa
2012-09-22 13:22 ` Russell King - ARM Linux
2012-09-13 8:13 ` [PATCH 3/5] ARM: EXYNOS: Add support for secure monitor calls Tomasz Figa
2012-09-13 8:13 ` [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware Tomasz Figa
2012-09-16 0:44 ` Olof Johansson
2012-09-19 10:10 ` Tomasz Figa
2012-09-22 5:39 ` Olof Johansson
2012-09-22 5:57 ` Kyungmin Park
2012-09-22 6:36 ` Olof Johansson
2012-09-22 6:39 ` Kyungmin Park
2012-09-24 14:39 ` Tomasz Figa
2012-09-13 8:13 ` [PATCH 5/5] ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up Tomasz Figa
2012-09-19 13:29 ` [PATCH 0/5] ARM: EXYNOS: Add secure firmware support Tomasz Figa
2012-09-21 8:39 ` Kyungmin Park
2012-09-22 5:52 ` Olof Johansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).