Skip to contentSkip to conversation

mach-shmobile: Emma Mobile EV2 SMP prototype code

PATCH In review Magnus Damm 5 participants 9 messages May 9, 2012 – May 11, 2012

Subsystems (4)
Arm (32-bit) Arm sub-architectures SuperH Arm/RISC-V/Renesas architecture
lore
Share & tools
lore

Magnus Damm

author maintainer
#0 lore raw

PATCH 1/1 mach-shmobile: Emma Mobile EV2 SMP prototype code

From: Magnus Damm <[email protected]>

Prototype code that adds SMP support to EMEV2.

At this point only the most basic form of SMP operation is supported. TWD and CPU Hotplug support is excluded.

Based on V1 of the Emma Mobile EV2 code. Needs to be reworked to coexist with DT. Work in progress.

Not-yet-signed-off-by Magnus Damm <[email protected]>

---

Depends on the following series:
"[PATCH 00/02] mach-shmobile: Emma Mobile EV2 - first shot" And the following patches:
"[PATCH] serial8250-em: Emma Mobile UART driver V2" "[PATCH] clocksource: em_sti: Emma Mobile STI driver"

 arch/arm/mach-shmobile/Makefile              |    1 
 arch/arm/mach-shmobile/include/mach/common.h |    6 +
 arch/arm/mach-shmobile/platsmp.c             |   16 ++++
 arch/arm/mach-shmobile/setup-emev2.c         |    7 +
 arch/arm/mach-shmobile/smp-emev2.c           |   95 ++++++++++++++++++++++++++
 5 files changed, 125 insertions(+)
arch/arm/mach-shmobile/Makefile +1 -0
@@ -19,6 +19,7 @@ smp-y				:= platsmp.o headsmp.o
 smp-$(CONFIG_HOTPLUG_CPU)	+= hotplug.o
 smp-$(CONFIG_ARCH_SH73A0)	+= smp-sh73a0.o
 smp-$(CONFIG_ARCH_R8A7779)	+= smp-r8a7779.o
+smp-$(CONFIG_ARCH_EMEV2)	+= smp-emev2.o
 
 # Pinmux setup
 pfc-y				:=
arch/arm/mach-shmobile/include/mach/common.h +6 -0
@@ -91,4 +91,10 @@ extern void emev2_add_early_devices(void
 extern void emev2_add_standard_devices(void);
 extern void emev2_clock_init(void);
 
+extern unsigned int emev2_get_core_count(void);
+extern int emev2_platform_cpu_kill(unsigned int cpu);
+extern void emev2_secondary_init(unsigned int cpu);
+extern int emev2_boot_secondary(unsigned int cpu);
+extern void emev2_smp_prepare_cpus(void);
+
 #endif /* __ARCH_MACH_COMMON_H */
arch/arm/mach-shmobile/platsmp.c +16 -0
@@ -22,6 +22,7 @@
 
 #define is_sh73a0() (machine_is_ag5evm() || machine_is_kota2())
 #define is_r8a7779() machine_is_marzen()
+#define is_emev2() machine_is_kzm9d()
 
 static unsigned int __init shmobile_smp_get_core_count(void)
 {

@@ -31,6 +32,9 @@ static unsigned int __init shmobile_smp_
 	if (is_r8a7779())
 		return r8a7779_get_core_count();
 
+	if (is_emev2())
+		return emev2_get_core_count();
+
 	return 1;
 }
 

@@ -41,6 +45,9 @@ static void __init shmobile_smp_prepare_
 
 	if (is_r8a7779())
 		r8a7779_smp_prepare_cpus();
+
+	if (is_emev2())
+		emev2_smp_prepare_cpus();
 }
 
 int shmobile_platform_cpu_kill(unsigned int cpu)

@@ -48,6 +55,9 @@ int shmobile_platform_cpu_kill(unsigned
 	if (is_r8a7779())
 		return r8a7779_platform_cpu_kill(cpu);
 
+	if (is_emev2())
+		return emev2_platform_cpu_kill(cpu);
+
 	return 1;
 }
 

@@ -60,6 +70,9 @@ void __cpuinit platform_secondary_init(u
 
 	if (is_r8a7779())
 		r8a7779_secondary_init(cpu);
+
+	if (is_emev2())
+		emev2_secondary_init(cpu);
 }
 
 int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle)

@@ -70,6 +83,9 @@ int __cpuinit boot_secondary(unsigned in
 	if (is_r8a7779())
 		return r8a7779_boot_secondary(cpu);
 
+	if (is_emev2())
+		return emev2_boot_secondary(cpu);
+
 	return -ENOSYS;
 }
arch/arm/mach-shmobile/setup-emev2.c +7 -0
@@ -47,6 +47,13 @@ static struct map_desc emev2_io_desc[] _
 		.length		= SZ_128K,
 		.type		= MT_DEVICE
 	},
+	/* 2M mapping for SCU + L2 controller */
+	{
+		.virtual	= 0xf0000000,
+		.pfn		= __phys_to_pfn(0x1e000000),
+		.length		= SZ_2M,
+		.type		= MT_DEVICE
+	},
 };
 
 void __init emev2_map_io(void)
arch/arm/mach-shmobile/smp-emev2.c new file +95 -0

95 lines not on this page · raw message

#1 lore raw
Quoted 13 lines · Magnus Damm

static unsigned int __init shmobile_smp_get_core_count(void) {

arch/arm/mach-shmobile/platsmp.c +3 -0
@@ -31,6 +32,9 @@ static unsigned int __init shmobile_smp_
 	if (is_r8a7779())
 		return r8a7779_get_core_count();
 
+	if (is_emev2())
+		return emev2_get_core_count();
+
 	return 1;
 }

arch/arm/mach-shmobile/platsmp.c static void __init shmobile_smp_prepare_ · line 53

Quoted 10 lines
arch/arm/mach-shmobile/platsmp.c +3 -0
@@ -41,6 +45,9 @@ static void __init shmobile_smp_prepare_
 
 	if (is_r8a7779())
 		r8a7779_smp_prepare_cpus();
+
+	if (is_emev2())
+		emev2_smp_prepare_cpus();
 }
 
 int shmobile_platform_cpu_kill(unsigned int cpu)
...

This shows that we really want an abstraction for soc-specific SMP ops even within one platform, and we'll need the same thing for multiplatform.

Marc Zyngier already proposed a solution for this last year, but I think we couldn't agree on the details back then before he lost interest. I think we should pick that up again and get it into 3.6 so the code above can be simplified and we can do the multiplatform solution. We'll probably discuss the details in Hong Kong in a couple of weeks, so there is no point in changing it now, but I'd hope that you can migrate this to whatever we come up with in the following merge window.

Quoted 1 line

+#define SMU_GENERAL_REG0 IOMEM(0xe01107c0)

I would keep this together with the other SMU handling code and export a function to set it, rather than hardcoding the address.

Quoted 6 lines
arch/arm/mach-shmobile/smp-emev2.c new file +6 -0
+static DEFINE_SPINLOCK(scu_lock);
+static unsigned long tmp;
+
+static void modify_scu_cpu_psr(unsigned long set, unsigned long clr)
+{
+	void __iomem *scu_base = scu_base_addr();

arch/arm/mach-shmobile/smp-emev2.c line 54

Quoted 10 lines
arch/arm/mach-shmobile/smp-emev2.c new file +10 -0
+
+	spin_lock(&scu_lock);
+	tmp = readl(scu_base + 8);
+	tmp &= ~clr;
+	tmp |= set;
+	spin_unlock(&scu_lock);
+
+	/* disable cache coherency after releasing the lock */
+	writel(tmp, scu_base + 8);
+}

This looks strange: why is tmp a file-level static variable rather than local to the modify_scu_cpu_psr function? Why do you do the writel without the spinlock held?

Arnd

#2 lore raw
Quoted 28 lines · Arnd Bergmann
Quoted 13 lines · Magnus Damm

static unsigned int __init shmobile_smp_get_core_count(void) {

arch/arm/mach-shmobile/platsmp.c +3 -0
@@ -31,6 +32,9 @@ static unsigned int __init shmobile_smp_
 	if (is_r8a7779())
 		return r8a7779_get_core_count();
 
+	if (is_emev2())
+		return emev2_get_core_count();
+
 	return 1;
 }

arch/arm/mach-shmobile/platsmp.c static void __init shmobile_smp_prepare_ · line 53

Quoted 10 lines
arch/arm/mach-shmobile/platsmp.c +3 -0
@@ -41,6 +45,9 @@ static void __init shmobile_smp_prepare_
 
 	if (is_r8a7779())
 		r8a7779_smp_prepare_cpus();
+
+	if (is_emev2())
+		emev2_smp_prepare_cpus();
 }
 
 int shmobile_platform_cpu_kill(unsigned int cpu)
...

This shows that we really want an abstraction for soc-specific SMP ops even within one platform, and we'll need the same thing for multiplatform.

Quoted 7 lines

Marc Zyngier already proposed a solution for this last year, but I think we couldn't agree on the details back then before he lost interest. I think we should pick that up again and get it into 3.6 so the code above can be simplified and we can do the multiplatform solution. We'll probably discuss the details in Hong Kong in a couple of weeks, so there is no point in changing it now, but I'd hope that you can migrate this to whatever we come up with in the following merge window.

I'm happy to revive the series if there is an interest.

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#3 lore raw
Quoted 37 lines · Marc Zyngier
Quoted 28 lines · Arnd Bergmann
Quoted 13 lines · Magnus Damm

static unsigned int __init shmobile_smp_get_core_count(void) {

arch/arm/mach-shmobile/platsmp.c +3 -0
@@ -31,6 +32,9 @@ static unsigned int __init shmobile_smp_
 	if (is_r8a7779())
 		return r8a7779_get_core_count();
 
+	if (is_emev2())
+		return emev2_get_core_count();
+
 	return 1;
 }

arch/arm/mach-shmobile/platsmp.c static void __init shmobile_smp_prepare_ · line 53

Quoted 10 lines
arch/arm/mach-shmobile/platsmp.c +3 -0
@@ -41,6 +45,9 @@ static void __init shmobile_smp_prepare_
 
 	if (is_r8a7779())
 		r8a7779_smp_prepare_cpus();
+
+	if (is_emev2())
+		emev2_smp_prepare_cpus();
 }
 
 int shmobile_platform_cpu_kill(unsigned int cpu)
...

This shows that we really want an abstraction for soc-specific SMP ops even within one platform, and we'll need the same thing for multiplatform.

Quoted 7 lines

Marc Zyngier already proposed a solution for this last year, but I think we couldn't agree on the details back then before he lost interest. I think we should pick that up again and get it into 3.6 so the code above can be simplified and we can do the multiplatform solution. We'll probably discuss the details in Hong Kong in a couple of weeks, so there is no point in changing it now, but I'd hope that you can migrate this to whatever we come up with in the following merge window.

Quoted 1 line

I'm happy to revive the series if there is an interest.

Ok, good. I think we were almost there the last time, but I don't know if Russell still had any objections. Magnus, can you comment on the "[PATCH v6 09/15] ARM: SoC: convert shmobile SMP to SoC descriptor" patch from February to see if it fits your needs?

FWIW, I would actually prefer merging the 'struct arm_soc_desc', 'struct arm_soc_smp_init_ops' and 'struct arm_soc_smp_ops' structures into a single 'struct smp_ops' for simplicity. While that would no longer allow us to put more stuff in there, I also don't see an urgent need to do so. I also don't mind the code that we had in version 6.

Arnd

#4 lore raw
Quoted 3 lines · Arnd Bergmann

FWIW, I would actually prefer merging the 'struct arm_soc_desc', 'struct arm_soc_smp_init_ops' and 'struct arm_soc_smp_ops' structures into a single 'struct smp_ops' for simplicity.

You can't easily validate correct usage of __init marked code from the rest if everything is pulled in the same struct. This is why I asked they be split.

Nicolas

#5 lore raw
Quoted 9 lines · Nicolas Pitre
Quoted 3 lines · Arnd Bergmann

FWIW, I would actually prefer merging the 'struct arm_soc_desc', 'struct arm_soc_smp_init_ops' and 'struct arm_soc_smp_ops' structures into a single 'struct smp_ops' for simplicity.

You can't easily validate correct usage of __init marked code from the rest if everything is pulled in the same struct. This is why I asked they be split.

Ah, I see. However, in version 6 of the patch set, each platform was marking both structures as __initdata, which seems to make your argument pointless because we don't actually validate the sections.

Arnd

author maintainer
#6 lore raw
In reply to Arnd Bergmann
Quoted 38 lines · Arnd Bergmann
Quoted 37 lines · Marc Zyngier
Quoted 35 lines · Arnd Bergmann
Quoted 13 lines · Magnus Damm

?static unsigned int __init shmobile_smp_get_core_count(void) ?{

(quoted diff) +3 -0
@@ -31,6 +32,9 @@ static unsigned int __init shmobile_smp_
 ? ?if (is_r8a7779())
 ? ? ? ? ? ?return r8a7779_get_core_count();
 
+ ?if (is_emev2())
+ ? ? ? ? ?return emev2_get_core_count();
+
 ? ?return 1;
 ?}

Review static void __init shmobile_smp_prepare_ · line 53

Quoted 10 lines
(quoted diff) +3 -0
@@ -41,6 +45,9 @@ static void __init shmobile_smp_prepare_
 
 ? ?if (is_r8a7779())
 ? ? ? ? ? ?r8a7779_smp_prepare_cpus();
+
+ ?if (is_emev2())
+ ? ? ? ? ?emev2_smp_prepare_cpus();
 ?}
 
 ?int shmobile_platform_cpu_kill(unsigned int cpu)
...

This shows that we really want an abstraction for soc-specific SMP ops even within one platform, and we'll need the same thing for multiplatform.

Marc Zyngier already proposed a solution for this last year, but I think we couldn't agree on the details back then before he lost interest. I think we should pick that up again and get it into 3.6 so the code above can be simplified and we can do the multiplatform solution. We'll probably discuss the details in Hong Kong in a couple of weeks, so there is no point in changing it now, but I'd hope that you can migrate this to whatever we come up with in the following merge window.

Quoted 6 lines
Quoted 1 line

I'm happy to revive the series if there is an interest.

Ok, good. I think we were almost there the last time, but I don't know if Russell still had any objections. Magnus, can you comment on the "[PATCH v6 09/15] ARM: SoC: convert shmobile SMP to SoC descriptor" patch from February to see if it fits your needs?

I took the liberty to give some comments here instead. I've gone through the patch and it looks good in general. I have not tested it but I'd be happy to fix whatever fallout that may happen.

The patch is pretty close to be a perfect fit from my point of view. This is definitely a step in the right direction. I am however a bit hesitant with how the CPU hotplug code ended up, but at the same time I don't really have any better suggestions. I think I simply need to convert it to be a full per-soc implementation.

Thanks!

/ magnus

#7 lore raw
In reply to Magnus Damm
Quoted 9 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Prototype code that adds SMP support to EMEV2.

At this point only the most basic form of SMP operation is supported. TWD and CPU Hotplug support is excluded.

Based on V1 of the Emma Mobile EV2 code. Needs to be reworked to coexist with DT. Work in progress.

Hi Magnus,

I tried out this patch on top of the next branch of Rafael's renesas tree (5658c94) plus V2 of your Emma Mobile EV2 code but I am only seeing one CPU. Do I need some other tweaks?

The list of patches I have applied is as follows:

mach-shmobile: Emma Mobile EV2 SMP prototype code
ARM: mach-types: Manually add KZM-A9-GT
mach-shmobile: KZM9D board prototype support
mach-shmobile: Emma Mobile EV2 SoC base support
serial8250-em: Add DT support
serial8250-em: clk_get() IS_ERR() error handling fix
serial8250-em: Emma Mobile UART driver V2
serial8250: Introduce serial8250_register_8250_port()
serial8250: Clean up default map and dl code
serial8250: Use dl_read()/dl_write() on RM9K
serial8250: Use dl_read()/dl_write() on Alchemy
serial8250: Add dl_read()/dl_write() callbacks
8250.c: less than 2400 baud fix.
serial/8250_pci: Clear FIFOs for Intel ME Serial Over Lan device on BI tegra, serial8250: add ->handle_break() uart_port op
clocksource: em_sti: Add DT support
clocksource: em_sti: Emma Mobile STI driver V2
clockevents: Make clockevents_config() a global symbol

And yes, I did enable CONFIG_SMP :)

Tested-by
#8 lore raw
Quoted 8 lines · Simon Horman
Quoted 7 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Prototype code that adds SMP support to EMEV2.

At this point only the most basic form of SMP operation is supported. TWD and CPU Hotplug support is excluded.

Quoted 8 lines
Quoted 2 lines

Based on V1 of the Emma Mobile EV2 code. Needs to be reworked to coexist with DT. Work in progress.

Hi Magnus,

I tried out this patch on top of the next branch of Rafael's renesas tree (5658c94) plus V2 of your Emma Mobile EV2 code but I am only seeing one CPU. Do I need some other tweaks?

Hi Magnus,

it turns out that I had maxcpus=1 set on the command line. After removing that all seems well.

Tested-by Simon Horman <[email protected]>