ARM: EXYNOS: Add secure firmware support

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

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

Signed-off-by Tomasz Figa <[email protected]>

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

arch/arm/common/Makefile +2 -0

index e8a4e58..55d4182 100644
@@ -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

arch/arm/common/firmware.c new file +18 -0

new file mode 100644
index 0000000..27ddccb
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park <[email protected]>
+ * Tomasz Figa <[email protected]>
+ *
+ * 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;

arch/arm/include/asm/firmware.h new file +13 -0

new file mode 100644
index 0000000..ed51b02
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park <[email protected]>
+ * Tomasz Figa <[email protected]>
+ *
+ * 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
+

Quoted 118 lines · Tomasz Figa

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

Signed-off-by Tomasz Figa <[email protected]>

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

arch/arm/common/Makefile +2 -0

index e8a4e58..55d4182 100644
@@ -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

arch/arm/common/firmware.c new file +18 -0

new file mode 100644
index 0000000..27ddccb
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park <[email protected]>
+ * Tomasz Figa <[email protected]>
+ *
+ * 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;

arch/arm/include/asm/firmware.h new file +13 -0

new file mode 100644
index 0000000..ed51b02
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park <[email protected]>
+ * Tomasz Figa <[email protected]>
+ *
+ * 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
+

arch/arm/include/asm/firmware.h line 23

Quoted 10 lines

arch/arm/include/asm/firmware.h new file +10 -0

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

Quoted 134 lines · Olof Johansson

Quoted 118 lines · Tomasz Figa

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

Signed-off-by Tomasz Figa <[email protected]>

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

arch/arm/common/Makefile +2 -0

index e8a4e58..55d4182 100644
@@ -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

arch/arm/common/firmware.c new file +18 -0

new file mode 100644
index 0000000..27ddccb
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park <[email protected]>
+ * Tomasz Figa <[email protected]>
+ *
+ * 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;

arch/arm/include/asm/firmware.h new file +13 -0

new file mode 100644
index 0000000..ed51b02
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park <[email protected]>
+ * Tomasz Figa <[email protected]>
+ *
+ * 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
+

arch/arm/include/asm/firmware.h line 23

Quoted 10 lines

arch/arm/include/asm/firmware.h new file +10 -0

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

Quoted 2 lines

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.

Quoted 15 lines · Kyungmin Park

Quoted 11 lines · Olof Johansson

arch/arm/include/asm/firmware.h line 23

Quoted 2 lines · Tomasz Figa

arch/arm/include/asm/firmware.h new file +2 -0

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

Quoted 8 lines

#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,
}

static void __iomem *default_cpu_boot_reg(int cpu)

{
	return (void __iomem *)0;
}

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

Signed-off-by Tomasz Figa <[email protected]>

---
 .../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

Documentation/devicetree/bindings/arm/samsung-boards.txt +8 -0

index 0bf68be..f447059 100644
@@ -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";
+	};

arch/arm/mach-exynos/Makefile +1 -0

index 997864b..9451637 100644
@@ -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)

arch/arm/mach-exynos/common.h +2 -0

index d7f28ca..358f6a5 100644
@@ -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

arch/arm/mach-exynos/firmware.c new file +25 -0

new file mode 100644
index 0000000..3f3641d
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park <[email protected]>
+ * Tomasz Figa <[email protected]>
+ *
+ * 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;
+}

arch/arm/mach-exynos/firmware.c line 35

Quoted 4 lines · Tomasz Figa

arch/arm/mach-exynos/firmware.c new file +4 -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.

Hi Olof,

Quoted 11 lines · On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:

arch/arm/mach-exynos/firmware.c line 35

Quoted 4 lines · Tomasz Figa

arch/arm/mach-exynos/firmware.c new file +4 -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.

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?

Quoted 15 lines · Tomasz Figa

Hi Olof,

Quoted 11 lines · On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:

arch/arm/mach-exynos/firmware.c line 35

Quoted 4 lines · Tomasz Figa

arch/arm/mach-exynos/firmware.c new file +4 -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.

Quoted 3 lines

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?

Quoted 5 lines

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

Quoted 20 lines · Olof Johansson

Quoted 15 lines · Tomasz Figa

Hi Olof,

Quoted 11 lines · On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:

arch/arm/mach-exynos/firmware.c line 35

Quoted 4 lines · Tomasz Figa

arch/arm/mach-exynos/firmware.c new file +4 -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.

Quoted 3 lines

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.

Quoted 6 lines

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?

Quoted 30 lines · Kyungmin Park

Quoted 23 lines · Olof Johansson

Quoted 17 lines · Tomasz Figa

Hi Olof,

Quoted 13 lines · On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:

arch/arm/mach-exynos/firmware.c line 35

Quoted 4 lines · Tomasz Figa

arch/arm/mach-exynos/firmware.c new file +4 -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.

Quoted 4 lines

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.

Quoted 6 lines

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?

Quoted 4 lines

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?

Quoted 6 lines

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.

arch/arm/mach-exynos/firmware.c line 35

Quoted 4 lines · Tomasz Figa

arch/arm/mach-exynos/firmware.c new file +4 -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.

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

Hi,

Quoted 36 lines · 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 ++++

Quoted 8 lines

 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?

Quoted 48 lines · Tomasz Figa

Hi,

Quoted 44 lines · 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

Quoted 2 lines

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?