| Message ID | 1351178560-19188-5-git-send-email-t.figa@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 25, 2012 at 05:22:38PM +0200, Tomasz Figa wrote: > +static int exynos_do_idle(void) > +{ > + exynos_smc(SMC_CMD_SLEEP, 0, 0, 0); > + return 0; > +} This looks fine as an API - it has a defined purpose. > + > +static int exynos_cpu_boot(int cpu) > +{ > + exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0); > + return 0; > +} Same for this (though, what _exactly_ is 'cpu', is it the physical CPU number or the logical CPU number?) > + > +static int exynos_cpu_boot_reg(int cpu, void __iomem **ptr) > +{ > + *ptr = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu; > + return 0; > +} This is really bad. What's it trying to do? What is the significance of the 'ptr' returned? What if a platform doesn't have a boot register?
Hi Russel, On Monday 12 of November 2012 09:51:14 Russell King - ARM Linux wrote: > > + > > +static int exynos_cpu_boot(int cpu) > > +{ > > + exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0); > > + return 0; > > +} > > Same for this (though, what _exactly_ is 'cpu', is it the physical CPU > number or the logical CPU number?) Yes, it's the physical CPU number. > > + > > +static int exynos_cpu_boot_reg(int cpu, void __iomem **ptr) > > +{ > > + *ptr = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu; > > + return 0; > > +} > > This is really bad. What's it trying to do? What is the significance > of the 'ptr' returned? What if a platform doesn't have a boot register? It returns a pointer to the area where boot code (secondary startup) address must be stored. This callback (just as all the firmware callbacks) is optional, if it is not appropriate for given platform, it will not use it. However, now when I think of it, it may be better to just add a callback like set_boot_addr(cpu, addr), which would set boot address of given CPU without exporting address of its boot register outside firmware code. Are you OK with this kind of approach? Best regards,
2012/11/12, Tomasz Figa <t.figa@samsung.com>: > Hi Russel, > > On Monday 12 of November 2012 09:51:14 Russell King - ARM Linux wrote: >> > + >> > +static int exynos_cpu_boot(int cpu) >> > +{ >> > + exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0); >> > + return 0; >> > +} >> >> Same for this (though, what _exactly_ is 'cpu', is it the physical CPU >> number or the logical CPU number?) > > Yes, it's the physical CPU number. > >> > + >> > +static int exynos_cpu_boot_reg(int cpu, void __iomem **ptr) >> > +{ >> > + *ptr = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu; >> > + return 0; >> > +} >> >> This is really bad. What's it trying to do? What is the significance >> of the 'ptr' returned? What if a platform doesn't have a boot register? > > It returns a pointer to the area where boot code (secondary startup) > address must be stored. > > This callback (just as all the firmware callbacks) is optional, if it is > not appropriate for given platform, it will not use it. > > However, now when I think of it, it may be better to just add a callback > like set_boot_addr(cpu, addr), which would set boot address of given CPU > without exporting address of its boot register outside firmware code. Are > you OK with this kind of approach? it seems the firmware_ops you are providing is just for samsung EXYNOS SMC, esepecially for the boot of secondary physical cpu. then it really should be namespaced. more callbacks for a common secure monitor APIs might need to cover 1. boot of 2nd CPU 2. do_idle(might let the other RTOS running on secure mode to be schedued in) 3. call SMC, send param and get feedback from firmware 4. might need to handle the steal time of firmware, firmware is also taking the CPU time, but linux doesn't know. it will affect the scheduler of Linux. > > > Best regards, > -- > Tomasz Figa > Samsung Poland R&D Center > SW Solution Development, Linux Platform -barry
diff --git a/Documentation/devicetree/bindings/arm/samsung-boards.txt b/Documentation/devicetree/bindings/arm/samsung-boards.txt index 0bf68be..2168ed3 100644 --- a/Documentation/devicetree/bindings/arm/samsung-boards.txt +++ b/Documentation/devicetree/bindings/arm/samsung-boards.txt @@ -6,3 +6,13 @@ 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: + - compatible: only "samsung,secure-firmware" is currently supported + - reg: address of non-secure SYSRAM used for communication with firmware + + firmware@0203F000 { + compatible = "samsung,secure-firmware"; + reg = <0x0203F000 0x1000>; + }; diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile index 5c1de47..b464333 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 dac146d..5f1d393 100644 --- a/arch/arm/mach-exynos/common.h +++ b/arch/arm/mach-exynos/common.h @@ -22,6 +22,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..15d3c87 --- /dev/null +++ b/arch/arm/mach-exynos/firmware.c @@ -0,0 +1,67 @@ +/* + * 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 <linux/of_address.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 int exynos_cpu_boot(int cpu) +{ + exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0); + return 0; +} + +static int exynos_cpu_boot_reg(int cpu, void __iomem **ptr) +{ + *ptr = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu; + return 0; +} + +static const struct firmware_ops exynos_firmware_ops = { + .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()) { + struct device_node *nd; + const __be32 *addr; + + nd = of_find_compatible_node(NULL, NULL, + "samsung,secure-firmware"); + if (!nd) + return; + + addr = of_get_address(nd, 0, NULL, NULL); + if (!addr) { + pr_err("%s: No address specified.\n", __func__); + 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 eadf4b5..977bd39 100644 --- a/arch/arm/mach-exynos/mach-exynos4-dt.c +++ b/arch/arm/mach-exynos/mach-exynos4-dt.c @@ -103,6 +103,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,
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@0203F000 { compatible = "samsung,secure-firmware"; reg = <0x0203F000 0x1000>; }; /* ... */ }; Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> Signed-off-by: Tomasz Figa <t.figa@samsung.com> --- .../devicetree/bindings/arm/samsung-boards.txt | 10 ++++ arch/arm/mach-exynos/Makefile | 1 + arch/arm/mach-exynos/common.h | 2 + arch/arm/mach-exynos/firmware.c | 67 ++++++++++++++++++++++ arch/arm/mach-exynos/mach-exynos4-dt.c | 1 + 5 files changed, 81 insertions(+) create mode 100644 arch/arm/mach-exynos/firmware.c