[PATCH] mach-shmobile: Emma Mobile EV2 SMP prototype code

[PATCH] mach-shmobile: Emma Mobile EV2 SMP prototype code

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

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 <damm@opensource.se>
---

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

--- 0012/arch/arm/mach-shmobile/Makefile
+++ work/arch/arm/mach-shmobile/Makefile	2012-05-09 16:43:48.000000000 +0900
@@ -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				:--- 0011/arch/arm/mach-shmobile/include/mach/common.h
+++ work/arch/arm/mach-shmobile/include/mach/common.h	2012-05-09 16:43:48.000000000 +0900
@@ -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 */
--- 0001/arch/arm/mach-shmobile/platsmp.c
+++ work/arch/arm/mach-shmobile/platsmp.c	2012-05-09 16:43:48.000000000 +0900
@@ -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;
 }
 
--- 0011/arch/arm/mach-shmobile/setup-emev2.c
+++ work/arch/arm/mach-shmobile/setup-emev2.c	2012-05-09 16:43:48.000000000 +0900
@@ -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)
--- /dev/null
+++ work/arch/arm/mach-shmobile/smp-emev2.c	2012-05-09 16:44:26.000000000 +0900
@@ -0,0 +1,95 @@
+/*
+ * SMP support for Emma Mobile EV2
+ *
+ * Copyright (C) 2012  Renesas Solutions Corp.
+ * Copyright (C) 2012  Magnus Damm
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/smp.h>
+#include <linux/spinlock.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <mach/common.h>
+#include <asm/smp_plat.h>
+#include <asm/smp_scu.h>
+#include <asm/hardware/gic.h>
+#include <asm/cacheflush.h>
+
+#define SMU_GENERAL_REG0	IOMEM(0xe01107c0)
+
+static void __iomem *scu_base_addr(void)
+{
+	return IOMEM(0xf0000000);
+}
+
+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();
+
+	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);
+}
+
+unsigned int __init emev2_get_core_count(void)
+{
+	void __iomem *scu_base = scu_base_addr();
+
+	return scu_get_core_count(scu_base);
+}
+
+int emev2_platform_cpu_kill(unsigned int cpu)
+{
+	return 0; /* not supported yet */
+}
+
+void __cpuinit emev2_secondary_init(unsigned int cpu)
+{
+	gic_secondary_init(0);
+}
+
+int __cpuinit emev2_boot_secondary(unsigned int cpu)
+{
+	cpu = cpu_logical_map(cpu);
+
+	/* enable cache coherency */
+	modify_scu_cpu_psr(0, 3 << (cpu * 8));
+
+	/* Tell ROM loader about our vector (in headsmp.S) */
+	writel(__pa(shmobile_secondary_vector), SMU_GENERAL_REG0);
+
+	gic_raise_softirq(cpumask_of(cpu), 1);
+	return 0;
+}
+
+void __init emev2_smp_prepare_cpus(void)
+{
+	int cpu = cpu_logical_map(0);
+
+	scu_enable(scu_base_addr());
+
+	/* enable cache coherency on CPU0 */
+	modify_scu_cpu_psr(0, 3 << (cpu * 8));
+}

Re: [PATCH] mach-shmobile: Emma Mobile EV2 SMP prototype code

[Arnd Bergmann]

On Wednesday 09 May 2012, Magnus Damm wrote:
>  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)
> ...

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.

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

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

Re: [PATCH] mach-shmobile: Emma Mobile EV2 SMP prototype code

[Marc Zyngier]

On 09/05/12 13:12, Arnd Bergmann wrote:
> On Wednesday 09 May 2012, Magnus Damm wrote:
>>  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)
>> ...
> 
> 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.

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

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH] mach-shmobile: Emma Mobile EV2 SMP prototype code

[Arnd Bergmann]

On Wednesday 09 May 2012, Marc Zyngier wrote:
> On 09/05/12 13:12, Arnd Bergmann wrote:
> > On Wednesday 09 May 2012, Magnus Damm wrote:
> >>  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)
> >> ...
> > 
> > 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.
> 
> 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

Re: [PATCH] mach-shmobile: Emma Mobile EV2 SMP prototype code

[Nicolas Pitre]

On Wed, 9 May 2012, Arnd Bergmann wrote:

> 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

Re: [PATCH] mach-shmobile: Emma Mobile EV2 SMP prototype code

[Arnd Bergmann]

On Wednesday 09 May 2012, Nicolas Pitre wrote:
> On Wed, 9 May 2012, Arnd Bergmann wrote:
> 
> > 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

Re: [PATCH] mach-shmobile: Emma Mobile EV2 SMP prototype code

[Magnus Damm]

On Wed, May 9, 2012 at 10:45 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 09 May 2012, Marc Zyngier wrote:
>> On 09/05/12 13:12, Arnd Bergmann wrote:
>> > On Wednesday 09 May 2012, Magnus Damm wrote:
>> >>  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)
>> >> ...
>> >
>> > 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.
>>
>> 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

Re: [PATCH] mach-shmobile: Emma Mobile EV2 SMP prototype code

[Simon Horman]

On Wed, May 09, 2012 at 04:54:07PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> 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 :)

Re: [PATCH] mach-shmobile: Emma Mobile EV2 SMP prototype code

[Simon Horman]

On Fri, May 11, 2012 at 04:13:08PM +0900, Simon Horman wrote:
> On Wed, May 09, 2012 at 04:54:07PM +0900, Magnus Damm wrote:
> > From: Magnus Damm <damm@opensource.se>
> > 
> > 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?

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 <horms@verge.net.au>