[PATCH 1/3] ARM: Xilinx: add timer and on chip memory support for SMP

[PATCH 1/3] ARM: Xilinx: add timer and on chip memory support for SMP

[John Linn]

These changes are required to support SMP.

This includes an update so that the non-local timer is set
to run on the 1st CPU, the addition of local timer support
and support for on chip memory.

Signed-off-by: John Linn <john.linn@xilinx.com>
---
 arch/arm/mach-xilinx/common.c                  |   16 ++++++++++-
 arch/arm/mach-xilinx/include/mach/irqs.h       |    1 +
 arch/arm/mach-xilinx/include/mach/xilinx_soc.h |   20 +++++++++++++
 arch/arm/mach-xilinx/localtimer.c              |   36 ++++++++++++++++++++++++
 arch/arm/mach-xilinx/timer.c                   |    4 +-
 5 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/mach-xilinx/localtimer.c

diff --git a/arch/arm/mach-xilinx/common.c b/arch/arm/mach-xilinx/common.c
index 83b549d..2695b39 100644
--- a/arch/arm/mach-xilinx/common.c
+++ b/arch/arm/mach-xilinx/common.c
@@ -101,7 +101,21 @@ static struct map_desc io_desc[] __initdata = {
 		.type		= MT_DEVICE,
 	},
 #endif
-
+	/* create a mapping for the OCM  (256K) leaving a hole for the
+	 * interrupt vectors which are handled in the kernel
+	 */
+	{
+		.virtual	= OCM_LOW_VIRT,
+		.pfn		= __phys_to_pfn(OCM_LOW_PHYS),
+		.length		= (192 * SZ_1K),
+		.type		= MT_DEVICE_CACHED,
+	},
+	{
+		.virtual	= OCM_HIGH_VIRT,
+		.pfn		= __phys_to_pfn(OCM_HIGH_PHYS),
+		.length		= (60 * SZ_1K),
+		.type		= MT_DEVICE,
+	},
 };
 
 /**
diff --git a/arch/arm/mach-xilinx/include/mach/irqs.h b/arch/arm/mach-xilinx/include/mach/irqs.h
index 47a8162..8c41b22 100644
--- a/arch/arm/mach-xilinx/include/mach/irqs.h
+++ b/arch/arm/mach-xilinx/include/mach/irqs.h
@@ -22,6 +22,7 @@
  * GIC Interrupts
  */
 
+#define IRQ_SCU_CPU_TIMER	29
 #define IRQ_GIC_SPI_START	32
 #define IRQ_TIMERCOUNTER0	42
 #define IRQ_UART0		59
diff --git a/arch/arm/mach-xilinx/include/mach/xilinx_soc.h b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
index d181c5c..a55a46c 100644
--- a/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
+++ b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
@@ -31,13 +31,33 @@
 #define SCU_PERIPH_PHYS			0xF8F00000
 #define SCU_PERIPH_VIRT			SCU_PERIPH_PHYS
 
+#define OCM_LOW_PHYS			0xFFFC0000
+#define OCM_LOW_VIRT			OCM_LOW_PHYS
+
+#define OCM_HIGH_PHYS			0xFFFF1000
+#define OCM_HIGH_VIRT			OCM_HIGH_PHYS
+
 /* The following are intended for the devices that are mapped early */
 
 #define TTC0_BASE			IOMEM(TTC0_VIRT)
 #define SCU_PERIPH_BASE			IOMEM(SCU_PERIPH_VIRT)
 #define SCU_GIC_CPU_BASE		(SCU_PERIPH_BASE + 0x100)
+#define SCU_CPU_TIMER_BASE		(SCU_PERIPH_BASE + 0x600)
 #define SCU_GIC_DIST_BASE		(SCU_PERIPH_BASE + 0x1000)
 #define PL310_L2CC_BASE			IOMEM(PL310_L2CC_VIRT)
+#define OCM_LOW_BASE			IOMEM(OCM_LOW_VIRT)
+#define OCM_HIGH_BASE			IOMEM(OCM_HIGH_VIRT)
+
+/* There are a couple ram addresses needed for communication between the boot
+ * loader software and the linux kernel with multiple cpus in the kernel (SMP).
+ * The memory addresses are in the high on-chip RAM and these addresses are
+ * mapped flat (virtual = physical). The memory must be mapped early and
+ * non-cached.
+ */
+
+#define BOOT_ADDR_OFFSET	0xEFF0
+#define BOOT_LOCK_OFFSET	0xEFF4
+#define BOOT_LOCK_KEY		0xFACECAFE
 
 /*
  * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
diff --git a/arch/arm/mach-xilinx/localtimer.c b/arch/arm/mach-xilinx/localtimer.c
new file mode 100644
index 0000000..4bd0a0d
--- /dev/null
+++ b/arch/arm/mach-xilinx/localtimer.c
@@ -0,0 +1,36 @@
+/*
+ * arch/arm/mach-xilinx/localtimer.c
+ *
+ * Both cortex-a9 cores have their own timer in it's CPU domain.
+ *
+ * Copyright (C) 2011 Xilinx, Inc.
+ *
+ * This file is based on arch/arm/plat-versatile/localtimer.c
+ *
+ * Copyright (C) 2002 ARM Ltd.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+#include <linux/init.h>
+#include <asm/smp_twd.h>
+#include <mach/xilinx_soc.h>
+
+/*
+ * Setup the local clock events for a CPU.
+ */
+int __cpuinit local_timer_setup(struct clock_event_device *evt)
+{
+	twd_base = SCU_CPU_TIMER_BASE;
+
+	evt->irq = IRQ_SCU_CPU_TIMER;
+	twd_timer_setup(evt);
+	return 0;
+}
+
diff --git a/arch/arm/mach-xilinx/timer.c b/arch/arm/mach-xilinx/timer.c
index b2a72f0..65c336a 100644
--- a/arch/arm/mach-xilinx/timer.c
+++ b/arch/arm/mach-xilinx/timer.c
@@ -283,9 +283,9 @@ static void __init xttcpss_timer_init(void)
 	xttcpss_clockevent.min_delta_ns =
 		clockevent_delta2ns(1, &xttcpss_clockevent);
 
-	/* Indicate that clock event can be used on any of the CPUs */
+	/* Indicate that clock event is on 1st CPU as SMP boot needs it */
 
-	xttcpss_clockevent.cpumask = cpu_all_mask;
+	xttcpss_clockevent.cpumask = cpumask_of(0);
 	clockevents_register_device(&xttcpss_clockevent);
 }
 
-- 
1.5.4.7



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH 1/3] ARM: Xilinx: add timer and on chip memory support for SMP

[Michal Simek]

Hi John,

John Linn wrote:
> These changes are required to support SMP.
> 
> This includes an update so that the non-local timer is set
> to run on the 1st CPU, the addition of local timer support
> and support for on chip memory.
> 
> Signed-off-by: John Linn <john.linn@xilinx.com>
> ---
>  arch/arm/mach-xilinx/common.c                  |   16 ++++++++++-
>  arch/arm/mach-xilinx/include/mach/irqs.h       |    1 +
>  arch/arm/mach-xilinx/include/mach/xilinx_soc.h |   20 +++++++++++++
>  arch/arm/mach-xilinx/localtimer.c              |   36 ++++++++++++++++++++++++
>  arch/arm/mach-xilinx/timer.c                   |    4 +-
>  5 files changed, 74 insertions(+), 3 deletions(-)
>  create mode 100644 arch/arm/mach-xilinx/localtimer.c
> 
> diff --git a/arch/arm/mach-xilinx/common.c b/arch/arm/mach-xilinx/common.c
> index 83b549d..2695b39 100644
> --- a/arch/arm/mach-xilinx/common.c
> +++ b/arch/arm/mach-xilinx/common.c
> @@ -101,7 +101,21 @@ static struct map_desc io_desc[] __initdata = {
>  		.type		= MT_DEVICE,
>  	},
>  #endif
> -
> +	/* create a mapping for the OCM  (256K) leaving a hole for the
> +	 * interrupt vectors which are handled in the kernel
> +	 */
> +	{
> +		.virtual	= OCM_LOW_VIRT,
> +		.pfn		= __phys_to_pfn(OCM_LOW_PHYS),
> +		.length		= (192 * SZ_1K),
> +		.type		= MT_DEVICE_CACHED,
> +	},
> +	{
> +		.virtual	= OCM_HIGH_VIRT,
> +		.pfn		= __phys_to_pfn(OCM_HIGH_PHYS),
> +		.length		= (60 * SZ_1K),
> +		.type		= MT_DEVICE,
> +	},
>  };
>  
>  /**
> diff --git a/arch/arm/mach-xilinx/include/mach/irqs.h b/arch/arm/mach-xilinx/include/mach/irqs.h
> index 47a8162..8c41b22 100644
> --- a/arch/arm/mach-xilinx/include/mach/irqs.h
> +++ b/arch/arm/mach-xilinx/include/mach/irqs.h
> @@ -22,6 +22,7 @@
>   * GIC Interrupts
>   */
>  
> +#define IRQ_SCU_CPU_TIMER	29
>  #define IRQ_GIC_SPI_START	32
>  #define IRQ_TIMERCOUNTER0	42
>  #define IRQ_UART0		59
> diff --git a/arch/arm/mach-xilinx/include/mach/xilinx_soc.h b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> index d181c5c..a55a46c 100644
> --- a/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> +++ b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> @@ -31,13 +31,33 @@
>  #define SCU_PERIPH_PHYS			0xF8F00000
>  #define SCU_PERIPH_VIRT			SCU_PERIPH_PHYS
>  
> +#define OCM_LOW_PHYS			0xFFFC0000
> +#define OCM_LOW_VIRT			OCM_LOW_PHYS
> +
> +#define OCM_HIGH_PHYS			0xFFFF1000
> +#define OCM_HIGH_VIRT			OCM_HIGH_PHYS
> +
>  /* The following are intended for the devices that are mapped early */
>  
>  #define TTC0_BASE			IOMEM(TTC0_VIRT)
>  #define SCU_PERIPH_BASE			IOMEM(SCU_PERIPH_VIRT)
>  #define SCU_GIC_CPU_BASE		(SCU_PERIPH_BASE + 0x100)
> +#define SCU_CPU_TIMER_BASE		(SCU_PERIPH_BASE + 0x600)
>  #define SCU_GIC_DIST_BASE		(SCU_PERIPH_BASE + 0x1000)
>  #define PL310_L2CC_BASE			IOMEM(PL310_L2CC_VIRT)
> +#define OCM_LOW_BASE			IOMEM(OCM_LOW_VIRT)
> +#define OCM_HIGH_BASE			IOMEM(OCM_HIGH_VIRT)
> +
> +/* There are a couple ram addresses needed for communication between the boot
> + * loader software and the linux kernel with multiple cpus in the kernel (SMP).
> + * The memory addresses are in the high on-chip RAM and these addresses are
> + * mapped flat (virtual = physical). The memory must be mapped early and
> + * non-cached.
> + */
> +
> +#define BOOT_ADDR_OFFSET	0xEFF0
> +#define BOOT_LOCK_OFFSET	0xEFF4
> +#define BOOT_LOCK_KEY		0xFACECAFE
>  
>  /*
>   * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
> diff --git a/arch/arm/mach-xilinx/localtimer.c b/arch/arm/mach-xilinx/localtimer.c
> new file mode 100644
> index 0000000..4bd0a0d
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/localtimer.c
> @@ -0,0 +1,36 @@
> +/*
> + * arch/arm/mach-xilinx/localtimer.c
> + *
> + * Both cortex-a9 cores have their own timer in it's CPU domain.
> + *
> + * Copyright (C) 2011 Xilinx, Inc.
> + *
> + * This file is based on arch/arm/plat-versatile/localtimer.c
> + *
> + * Copyright (C) 2002 ARM Ltd.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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.
> + */
> +#include <linux/init.h>
> +#include <asm/smp_twd.h>
> +#include <mach/xilinx_soc.h>
> +
> +/*
> + * Setup the local clock events for a CPU.
> + */
> +int __cpuinit local_timer_setup(struct clock_event_device *evt)
> +{
> +	twd_base = SCU_CPU_TIMER_BASE;
> +
> +	evt->irq = IRQ_SCU_CPU_TIMER;
> +	twd_timer_setup(evt);
> +	return 0;
> +}

I have question here about SCU_CPU_TIMER_BASE.
I have looked at your arm-next branch and you are using there TTC0_BASE
at 0xF8001000 address which is external timer.
Here you are using SCU internal timer at 0xF8F00600.

You are using TTC with three counters (from your comment).
T1: Timer 1, clocksource for generic timekeeping
T2: Timer 2, clockevent source for hrtimers
T3: Timer 3, <unused>

 From twd_timer_setup I see that it looks like that only new clockevent is 
registered.

Please correct me if I am wrong.
Would it be possible to use timer 3 for the second cpu? I expect that your code 
just register new clockevent device. System will contain 2 CPU that's why this 
shouldn't be a problem.

TGLX: Is it correct assumption that SMP system has one clocksource and one 
clockevent device for every CPU in the system? (Sorry I don't have any 
experience with SMP)

I don't know if there is an option to use only internal CPU timers for Linux and 
keep ttc for other purpose.

BTW: I think that will be better to spit this patch to two. The first for local 
timer and the second for OCM.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

[PATCH 1/3] ARM: Xilinx: add timer and on chip memory support for SMP

[John Linn]

> -----Original Message-----
> From: Michal Simek [mailto:monstr at monstr.eu]
> Sent: Tuesday, April 05, 2011 2:05 AM
> To: John Linn
> Cc: linux-arm-kernel at lists.infradead.org;
linux-kernel at vger.kernel.org;
> linux at arm.linux.org.uk; catalin.marinas at arm.com; glikely at secretlab.ca;
> jamie at jamieiles.com; arnd at arndb.de; Thomas Gleixner
> Subject: Re: [PATCH 1/3] ARM: Xilinx: add timer and on chip memory
> support for SMP
> 
> Hi John,
> 
> John Linn wrote:
> > These changes are required to support SMP.
> >
> > This includes an update so that the non-local timer is set
> > to run on the 1st CPU, the addition of local timer support
> > and support for on chip memory.
> >
> > Signed-off-by: John Linn <john.linn@xilinx.com>
> > ---
> >  arch/arm/mach-xilinx/common.c                  |   16 ++++++++++-
> >  arch/arm/mach-xilinx/include/mach/irqs.h       |    1 +
> >  arch/arm/mach-xilinx/include/mach/xilinx_soc.h |   20 +++++++++++++
> >  arch/arm/mach-xilinx/localtimer.c              |   36
> ++++++++++++++++++++++++
> >  arch/arm/mach-xilinx/timer.c                   |    4 +-
> >  5 files changed, 74 insertions(+), 3 deletions(-)
> >  create mode 100644 arch/arm/mach-xilinx/localtimer.c
> >
> > diff --git a/arch/arm/mach-xilinx/common.c b/arch/arm/mach-
> xilinx/common.c
> > index 83b549d..2695b39 100644
> > --- a/arch/arm/mach-xilinx/common.c
> > +++ b/arch/arm/mach-xilinx/common.c
> > @@ -101,7 +101,21 @@ static struct map_desc io_desc[] __initdata = {
> >  		.type		= MT_DEVICE,
> >  	},
> >  #endif
> > -
> > +	/* create a mapping for the OCM  (256K) leaving a hole for the
> > +	 * interrupt vectors which are handled in the kernel
> > +	 */
> > +	{
> > +		.virtual	= OCM_LOW_VIRT,
> > +		.pfn		= __phys_to_pfn(OCM_LOW_PHYS),
> > +		.length		= (192 * SZ_1K),
> > +		.type		= MT_DEVICE_CACHED,
> > +	},
> > +	{
> > +		.virtual	= OCM_HIGH_VIRT,
> > +		.pfn		= __phys_to_pfn(OCM_HIGH_PHYS),
> > +		.length		= (60 * SZ_1K),
> > +		.type		= MT_DEVICE,
> > +	},
> >  };
> >
> >  /**
> > diff --git a/arch/arm/mach-xilinx/include/mach/irqs.h
> b/arch/arm/mach-xilinx/include/mach/irqs.h
> > index 47a8162..8c41b22 100644
> > --- a/arch/arm/mach-xilinx/include/mach/irqs.h
> > +++ b/arch/arm/mach-xilinx/include/mach/irqs.h
> > @@ -22,6 +22,7 @@
> >   * GIC Interrupts
> >   */
> >
> > +#define IRQ_SCU_CPU_TIMER	29
> >  #define IRQ_GIC_SPI_START	32
> >  #define IRQ_TIMERCOUNTER0	42
> >  #define IRQ_UART0		59
> > diff --git a/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> > index d181c5c..a55a46c 100644
> > --- a/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> > +++ b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> > @@ -31,13 +31,33 @@
> >  #define SCU_PERIPH_PHYS			0xF8F00000
> >  #define SCU_PERIPH_VIRT			SCU_PERIPH_PHYS
> >
> > +#define OCM_LOW_PHYS			0xFFFC0000
> > +#define OCM_LOW_VIRT			OCM_LOW_PHYS
> > +
> > +#define OCM_HIGH_PHYS			0xFFFF1000
> > +#define OCM_HIGH_VIRT			OCM_HIGH_PHYS
> > +
> >  /* The following are intended for the devices that are mapped early
> */
> >
> >  #define TTC0_BASE			IOMEM(TTC0_VIRT)
> >  #define SCU_PERIPH_BASE			IOMEM(SCU_PERIPH_VIRT)
> >  #define SCU_GIC_CPU_BASE		(SCU_PERIPH_BASE + 0x100)
> > +#define SCU_CPU_TIMER_BASE		(SCU_PERIPH_BASE + 0x600)
> >  #define SCU_GIC_DIST_BASE		(SCU_PERIPH_BASE + 0x1000)
> >  #define PL310_L2CC_BASE			IOMEM(PL310_L2CC_VIRT)
> > +#define OCM_LOW_BASE			IOMEM(OCM_LOW_VIRT)
> > +#define OCM_HIGH_BASE			IOMEM(OCM_HIGH_VIRT)
> > +
> > +/* There are a couple ram addresses needed for communication
between
> the boot
> > + * loader software and the linux kernel with multiple cpus in the
> kernel (SMP).
> > + * The memory addresses are in the high on-chip RAM and these
> addresses are
> > + * mapped flat (virtual = physical). The memory must be mapped
early
> and
> > + * non-cached.
> > + */
> > +
> > +#define BOOT_ADDR_OFFSET	0xEFF0
> > +#define BOOT_LOCK_OFFSET	0xEFF4
> > +#define BOOT_LOCK_KEY		0xFACECAFE
> >
> >  /*
> >   * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
> > diff --git a/arch/arm/mach-xilinx/localtimer.c b/arch/arm/mach-
> xilinx/localtimer.c
> > new file mode 100644
> > index 0000000..4bd0a0d
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/localtimer.c
> > @@ -0,0 +1,36 @@
> > +/*
> > + * arch/arm/mach-xilinx/localtimer.c
> > + *
> > + * Both cortex-a9 cores have their own timer in it's CPU domain.
> > + *
> > + * Copyright (C) 2011 Xilinx, Inc.
> > + *
> > + * This file is based on arch/arm/plat-versatile/localtimer.c
> > + *
> > + * Copyright (C) 2002 ARM Ltd.
> > + *
> > + * This software is licensed under the terms of the GNU General
> Public
> > + * License version 2, as published by the Free Software Foundation,
> and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * 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.
> > + */
> > +#include <linux/init.h>
> > +#include <asm/smp_twd.h>
> > +#include <mach/xilinx_soc.h>
> > +
> > +/*
> > + * Setup the local clock events for a CPU.
> > + */
> > +int __cpuinit local_timer_setup(struct clock_event_device *evt)
> > +{
> > +	twd_base = SCU_CPU_TIMER_BASE;
> > +
> > +	evt->irq = IRQ_SCU_CPU_TIMER;
> > +	twd_timer_setup(evt);
> > +	return 0;
> > +}
> 
> I have question here about SCU_CPU_TIMER_BASE.
> I have looked at your arm-next branch and you are using there
TTC0_BASE
> at 0xF8001000 address which is external timer.
> Here you are using SCU internal timer at 0xF8F00600.
> 
> You are using TTC with three counters (from your comment).
> T1: Timer 1, clocksource for generic timekeeping
> T2: Timer 2, clockevent source for hrtimers
> T3: Timer 3, <unused>
> 
>  From twd_timer_setup I see that it looks like that only new
clockevent
> is
> registered.
> 
> Please correct me if I am wrong.
> Would it be possible to use timer 3 for the second cpu? I expect that
> your code
> just register new clockevent device. System will contain 2 CPU that's
> why this
> shouldn't be a problem.
> 

I would think it's possible (not an expert in the timers either), but
the 
SCU timer in each CPU is not being used so it seems logical to use it
and 
leave the other timer for other uses.  

I have scratched my head a number 
of times in the past trying to get a better understanding of what's the
best
use models for this.

> TGLX: Is it correct assumption that SMP system has one clocksource and
> one
> clockevent device for every CPU in the system? (Sorry I don't have any
> experience with SMP)

I think that's right myself, but TGLX can say so for sure.

> 
> I don't know if there is an option to use only internal CPU timers for
> Linux and
> keep ttc for other purpose.

It seems like I had looked at that quite some time ago and the SCU
global
timer wasn't being used for that, but made sense to me (again not an
expert).

> 
> BTW: I think that will be better to spit this patch to two. The first
> for local
> timer and the second for OCM.

Ok, my thinking was this whole series has to go together for SMP, it was
only 
busted up to make review easier, but I see your point.

Thanks for the input,
John

> 
> Thanks,
> Michal
> 
> --
> Michal Simek, Ing. (M.Eng)
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel 2.6 Microblaze Linux -
> http://www.monstr.eu/fdt/
> Microblaze U-BOOT custodian


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH 2/3] ARM: Xilinx: add SMP specific support files

[John Linn]

These files are the core processing for SMP which are similar
to most other platforms SMP. The biggest difference is the
way the 2nd CPU is started.

Signed-off-by: John Linn <john.linn@xilinx.com>
---
 arch/arm/mach-xilinx/common.h           |    2 +
 arch/arm/mach-xilinx/headsmp.S          |   59 ++++++++++++
 arch/arm/mach-xilinx/include/mach/smp.h |   30 ++++++
 arch/arm/mach-xilinx/platsmp.c          |  159 +++++++++++++++++++++++++++++++
 4 files changed, 250 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-xilinx/headsmp.S
 create mode 100644 arch/arm/mach-xilinx/include/mach/smp.h
 create mode 100644 arch/arm/mach-xilinx/platsmp.c

diff --git a/arch/arm/mach-xilinx/common.h b/arch/arm/mach-xilinx/common.h
index 71f4ebc..ecd8d65 100644
--- a/arch/arm/mach-xilinx/common.h
+++ b/arch/arm/mach-xilinx/common.h
@@ -25,6 +25,8 @@ void __init xilinx_system_init(void);
 void __init xilinx_irq_init(void);
 void __init xilinx_map_io(void);
 
+void xilinx_secondary_startup(void);
+
 extern struct sys_timer xttcpss_sys_timer;
 
 #endif
diff --git a/arch/arm/mach-xilinx/headsmp.S b/arch/arm/mach-xilinx/headsmp.S
new file mode 100644
index 0000000..bdf9a16
--- /dev/null
+++ b/arch/arm/mach-xilinx/headsmp.S
@@ -0,0 +1,59 @@
+/*
+ * arch/arm/mach-xilinx/headsmp.S
+ *
+ * Secondary CPU startup routine source file.
+ *
+ * Copyright (C) 2011 Xilinx, Inc.
+ *
+ * This file is based on arm omap smp platform file and arm
+ * realview smp platform.
+ *
+ * Copyright (C) 2009 Texas Instruments, Inc.
+ * Copyright (c) 2003 ARM Limited.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#include <linux/linkage.h>
+#include <linux/init.h>
+#include <mach/xilinx_soc.h>
+#include <mach/io.h>
+
+	__INIT
+
+/*
+ * Xilinx specific entry point for secondary CPU to jump from ROM
+ * code.  This routine provides a wait loop in which secondary core
+ * is held until we're ready for it to initialise. The primary core
+ * will update the boot lock with the boot key when it's ready for
+ * the secondary CPU to boot.
+ *
+ * The WFE must be in the loop when using this code with a probe
+ * because any operations cause debug events which can wake up CPU1
+ * falsely.
+ */
+ENTRY(xilinx_secondary_startup)
+	mov	r0, #0
+	ldr	r1, =(OCM_HIGH_PHYS + BOOT_LOCK_OFFSET)
+	ldr	r0, =BOOT_LOCK_KEY
+	mov	r3, #0
+hold:
+	wfe
+	add	r3, #1			@ only track number of wakeups
+					@ for diagnostics
+	ldr	r2, [r1]
+	cmp	r2, r0
+	bne	hold
+
+	/*
+	 * CPU0 released CPU1 by writing the key to the lock, go ahead
+	 * and start CPU1 running the kernel
+	 */
+	b	secondary_startup
diff --git a/arch/arm/mach-xilinx/include/mach/smp.h b/arch/arm/mach-xilinx/include/mach/smp.h
new file mode 100644
index 0000000..c3aae15
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/smp.h
@@ -0,0 +1,30 @@
+/* arch/arm/mach-xilinx/include/mach/smp.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * based on arch/arm/mach-realview/include/mach/smp.h
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#ifndef __MACH_SMP_H__
+#define __MACH_SMP_H__
+
+#include <asm/hardware/gic.h>
+
+/*
+ * We use IRQ1 as the IPI
+ */
+static inline void smp_cross_call(const struct cpumask *mask, int ipi)
+{
+	gic_raise_softirq(mask, ipi);
+}
+
+#endif
diff --git a/arch/arm/mach-xilinx/platsmp.c b/arch/arm/mach-xilinx/platsmp.c
new file mode 100644
index 0000000..be2d55c
--- /dev/null
+++ b/arch/arm/mach-xilinx/platsmp.c
@@ -0,0 +1,159 @@
+/*
+ * arch/arm/mach-xilinx/platsmp.c
+ *
+ * This file contains Xilinx specific SMP code, used to start up
+ * the second processor.
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * based on linux/arch/arm/mach-realview/platsmp.c
+ *
+ * Copyright (C) 2002 ARM Ltd.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+#include <linux/jiffies.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <asm/cacheflush.h>
+#include <asm/smp_scu.h>
+#include <mach/xilinx_soc.h>
+#include <mach/smp.h>
+#include "common.h"
+
+static void __iomem *scu_base = SCU_PERIPH_BASE;
+
+static DEFINE_SPINLOCK(boot_lock);
+
+/* Secondary CPU kernel startup is a 2 phase process.
+ * 1st phase is transition from a boot loader to the kernel, but
+ * then wait not starting the kernel yet. 2nd phase starts the
+ * the kernel. In both phases, the secondary CPU waits for an
+ * event before it continues.
+ */
+
+void __cpuinit platform_secondary_init(unsigned int cpu)
+{
+	/*
+	 * if any interrupts are already enabled for the primary
+	 * core (e.g. timer irq), then they will not have been enabled
+	 * for us: do so
+	 */
+	gic_secondary_init(0);
+
+	/*
+	 * Synchronise with the boot thread.
+	 */
+	spin_lock(&boot_lock);
+	spin_unlock(&boot_lock);
+}
+
+int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	unsigned long timeout;
+
+	/*
+	 * set synchronisation state between this boot processor
+	 * and the secondary one
+	 */
+	spin_lock(&boot_lock);
+
+	printk(KERN_INFO "Xilinx SMP: booting CPU1 now\n");
+
+	/*
+	 * Update boot lock register with the boot key to allow the
+	 * secondary processor to start the kernel after an SEV.
+	 */
+	__raw_writel(BOOT_LOCK_KEY, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
+
+	/* Flush the kernel cache to ensure that the page tables are
+	 * available for the secondary CPU to use and make sure that
+	 * the write buffer is drained before doing an SEV.
+	 */
+	flush_cache_all();
+	smp_wmb();
+
+	/*
+	 * Send an event to wake the secondary core from WFE state.
+	 */
+	sev();
+
+	timeout = jiffies + (1 * HZ);
+	while (time_before(jiffies, timeout))
+		;
+
+	/*
+	 * now the secondary core is starting up let it run its
+	 * calibrations, then wait for it to finish
+	 */
+	spin_unlock(&boot_lock);
+
+	return 0;
+}
+
+/*
+ * Initialise the CPU possible map early - this describes the CPUs
+ * which may be present or become present in the system.
+ */
+void __init smp_init_cpus(void)
+{
+	unsigned int i, ncores;
+
+	ncores = scu_base ? scu_get_core_count(scu_base) : 1;
+
+	/* sanity check */
+	if (ncores > NR_CPUS) {
+		printk(KERN_WARNING
+		       "Realview: no. of cores (%d) greater than configured "
+		       "maximum of %d - clipping\n",
+		       ncores, NR_CPUS);
+		ncores = NR_CPUS;
+	}
+
+	for (i = 0; i < ncores; i++)
+		set_cpu_possible(i, true);
+}
+
+void __init platform_smp_prepare_cpus(unsigned int max_cpus)
+{
+	int i;
+
+	/*
+	 * Initialise the present map, which describes the set of CPUs
+	 * actually populated@the present time.
+	 */
+	for (i = 0; i < max_cpus; i++)
+		set_cpu_present(i, true);
+
+	scu_enable(scu_base);
+
+	/* Initialize the boot lock register to prevent CPU1 from
+	   starting the kernel before CPU0 is ready for that.
+	*/
+	__raw_writel(0, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
+
+	/*
+	 * Write the address of secondary startup routine into the
+	 * boot address. The secondary CPU will use this value
+	 * to get into the kernel after it's awake from WFE state.
+	 *
+	 * Note the physical address is needed as the secondary CPU
+	 * will not have the MMU on yet. A barrier is added to ensure
+	 * that write buffer is drained.
+	 */
+	__raw_writel(virt_to_phys(xilinx_secondary_startup),
+					OCM_HIGH_BASE + BOOT_ADDR_OFFSET);
+	smp_wmb();
+
+	/*
+	 * Send an event to wake the secondary core from WFE state.
+	 */
+	sev();
+}
-- 
1.5.4.7



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH 2/3] ARM: Xilinx: add SMP specific support files

[Catalin Marinas]

John,

A few suggestions on barriers below.

On 3 April 2011 22:00, John Linn <john.linn@xilinx.com> wrote:
> diff --git a/arch/arm/mach-xilinx/platsmp.c b/arch/arm/mach-xilinx/platsmp.c
> new file mode 100644
> index 0000000..be2d55c
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/platsmp.c
...
> +int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> + ? ? ? unsigned long timeout;
> +
> + ? ? ? /*
> + ? ? ? ?* set synchronisation state between this boot processor
> + ? ? ? ?* and the secondary one
> + ? ? ? ?*/
> + ? ? ? spin_lock(&boot_lock);
> +
> + ? ? ? printk(KERN_INFO "Xilinx SMP: booting CPU1 now\n");
> +
> + ? ? ? /*
> + ? ? ? ?* Update boot lock register with the boot key to allow the
> + ? ? ? ?* secondary processor to start the kernel after an SEV.
> + ? ? ? ?*/
> + ? ? ? __raw_writel(BOOT_LOCK_KEY, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
> +
> + ? ? ? /* Flush the kernel cache to ensure that the page tables are
> + ? ? ? ?* available for the secondary CPU to use and make sure that
> + ? ? ? ?* the write buffer is drained before doing an SEV.
> + ? ? ? ?*/
> + ? ? ? flush_cache_all();
> + ? ? ? smp_wmb();
> +
> + ? ? ? /*
> + ? ? ? ?* Send an event to wake the secondary core from WFE state.
> + ? ? ? ?*/
> + ? ? ? sev();

You could flush the cache before writing the boot lock register in
case the secondary wakes up from WFE. You also need a wmb() rather
than the smp_wmb(). The latter only works in the inner shareable
domain but the secondary CPU hasn't initialised the MMU yet.

Something like below:

flush_cache_all();
__raw_writel(BOOT_LOCK_KEY, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
mb();
sev();

> +void __init platform_smp_prepare_cpus(unsigned int max_cpus)
> +{
> + ? ? ? int i;
> +
> + ? ? ? /*
> + ? ? ? ?* Initialise the present map, which describes the set of CPUs
> + ? ? ? ?* actually populated at the present time.
> + ? ? ? ?*/
> + ? ? ? for (i = 0; i < max_cpus; i++)
> + ? ? ? ? ? ? ? set_cpu_present(i, true);
> +
> + ? ? ? scu_enable(scu_base);
> +
> + ? ? ? /* Initialize the boot lock register to prevent CPU1 from
> + ? ? ? ? ?starting the kernel before CPU0 is ready for that.
> + ? ? ? */
> + ? ? ? __raw_writel(0, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
> +
> + ? ? ? /*
> + ? ? ? ?* Write the address of secondary startup routine into the
> + ? ? ? ?* boot address. The secondary CPU will use this value
> + ? ? ? ?* to get into the kernel after it's awake from WFE state.
> + ? ? ? ?*
> + ? ? ? ?* Note the physical address is needed as the secondary CPU
> + ? ? ? ?* will not have the MMU on yet. A barrier is added to ensure
> + ? ? ? ?* that write buffer is drained.
> + ? ? ? ?*/
> + ? ? ? __raw_writel(virt_to_phys(xilinx_secondary_startup),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? OCM_HIGH_BASE + BOOT_ADDR_OFFSET);
> + ? ? ? smp_wmb();

Same here, use a wmb() or mb() before the sev().

-- 
Catalin

[PATCH 2/3] ARM: Xilinx: add SMP specific support files

[John Linn]

> -----Original Message-----
> From: catalin.marinas at gmail.com [mailto:catalin.marinas at gmail.com] On
> Behalf Of Catalin Marinas
> Sent: Monday, April 04, 2011 9:13 AM
> To: John Linn
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> linux at arm.linux.org.uk; glikely at secretlab.ca; jamie at jamieiles.com;
> arnd at arndb.de
> Subject: Re: [PATCH 2/3] ARM: Xilinx: add SMP specific support files
> 
> John,
> 
> A few suggestions on barriers below.
> 
> On 3 April 2011 22:00, John Linn <john.linn@xilinx.com> wrote:
> > diff --git a/arch/arm/mach-xilinx/platsmp.c b/arch/arm/mach-
> xilinx/platsmp.c
> > new file mode 100644
> > index 0000000..be2d55c
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/platsmp.c
> ...
> > +int __cpuinit boot_secondary(unsigned int cpu, struct task_struct
> *idle)
> > +{
> > + ? ? ? unsigned long timeout;
> > +
> > + ? ? ? /*
> > + ? ? ? ?* set synchronisation state between this boot processor
> > + ? ? ? ?* and the secondary one
> > + ? ? ? ?*/
> > + ? ? ? spin_lock(&boot_lock);
> > +
> > + ? ? ? printk(KERN_INFO "Xilinx SMP: booting CPU1 now\n");
> > +
> > + ? ? ? /*
> > + ? ? ? ?* Update boot lock register with the boot key to allow the
> > + ? ? ? ?* secondary processor to start the kernel after an SEV.
> > + ? ? ? ?*/
> > + ? ? ? __raw_writel(BOOT_LOCK_KEY, OCM_HIGH_BASE +
> BOOT_LOCK_OFFSET);
> > +
> > + ? ? ? /* Flush the kernel cache to ensure that the page tables are
> > + ? ? ? ?* available for the secondary CPU to use and make sure that
> > + ? ? ? ?* the write buffer is drained before doing an SEV.
> > + ? ? ? ?*/
> > + ? ? ? flush_cache_all();
> > + ? ? ? smp_wmb();
> > +
> > + ? ? ? /*
> > + ? ? ? ?* Send an event to wake the secondary core from WFE state.
> > + ? ? ? ?*/
> > + ? ? ? sev();
> 
> You could flush the cache before writing the boot lock register in
> case the secondary wakes up from WFE. 

Understood. Since we control when it gets into the kernel completely I guess I wasn't worried about it, maybe I should have been.  Easy enough to change, not a big deal.

> You also need a wmb() rather
> than the smp_wmb(). The latter only works in the inner shareable
> domain but the secondary CPU hasn't initialised the MMU yet.

Ok, great, appreciate the help there.

> 
> Something like below:
> 
> flush_cache_all();
> __raw_writel(BOOT_LOCK_KEY, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
> mb();
> sev();
> 
> > +void __init platform_smp_prepare_cpus(unsigned int max_cpus)
> > +{
> > + ? ? ? int i;
> > +
> > + ? ? ? /*
> > + ? ? ? ?* Initialise the present map, which describes the set of
> CPUs
> > + ? ? ? ?* actually populated at the present time.
> > + ? ? ? ?*/
> > + ? ? ? for (i = 0; i < max_cpus; i++)
> > + ? ? ? ? ? ? ? set_cpu_present(i, true);
> > +
> > + ? ? ? scu_enable(scu_base);
> > +
> > + ? ? ? /* Initialize the boot lock register to prevent CPU1 from
> > + ? ? ? ? ?starting the kernel before CPU0 is ready for that.
> > + ? ? ? */
> > + ? ? ? __raw_writel(0, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
> > +
> > + ? ? ? /*
> > + ? ? ? ?* Write the address of secondary startup routine into the
> > + ? ? ? ?* boot address. The secondary CPU will use this value
> > + ? ? ? ?* to get into the kernel after it's awake from WFE state.
> > + ? ? ? ?*
> > + ? ? ? ?* Note the physical address is needed as the secondary CPU
> > + ? ? ? ?* will not have the MMU on yet. A barrier is added to ensure
> > + ? ? ? ?* that write buffer is drained.
> > + ? ? ? ?*/
> > + ? ? ? __raw_writel(virt_to_phys(xilinx_secondary_startup),
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? OCM_HIGH_BASE +
> BOOT_ADDR_OFFSET);
> > + ? ? ? smp_wmb();
> 
> Same here, use a wmb() or mb() before the sev().
> 

Yes.

Thanks for the input and taking time to review.  I'll wait a bit to see if any 
other input before spinning a new patch set.

-- John

> --
> Catalin


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH 2/3] ARM: Xilinx: add SMP specific support files

[Russell King - ARM Linux]

On Sun, Apr 03, 2011 at 03:00:17PM -0600, John Linn wrote:
> +#include <linux/jiffies.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <asm/cacheflush.h>
> +#include <asm/smp_scu.h>
> +#include <mach/xilinx_soc.h>
> +#include <mach/smp.h>
> +#include "common.h"
> +
> +static void __iomem *scu_base = SCU_PERIPH_BASE;

You don't need this if its known at build time.

> +
> +static DEFINE_SPINLOCK(boot_lock);
> +
> +/* Secondary CPU kernel startup is a 2 phase process.
> + * 1st phase is transition from a boot loader to the kernel, but
> + * then wait not starting the kernel yet. 2nd phase starts the
> + * the kernel. In both phases, the secondary CPU waits for an
> + * event before it continues.
> + */
> +
> +void __cpuinit platform_secondary_init(unsigned int cpu)
> +{
> +	/*
> +	 * if any interrupts are already enabled for the primary
> +	 * core (e.g. timer irq), then they will not have been enabled
> +	 * for us: do so
> +	 */
> +	gic_secondary_init(0);
> +
> +	/*
> +	 * Synchronise with the boot thread.
> +	 */
> +	spin_lock(&boot_lock);
> +	spin_unlock(&boot_lock);
> +}
> +
> +int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> +	unsigned long timeout;
> +
> +	/*
> +	 * set synchronisation state between this boot processor
> +	 * and the secondary one
> +	 */
> +	spin_lock(&boot_lock);
> +
> +	printk(KERN_INFO "Xilinx SMP: booting CPU1 now\n");

Aren't the messages which are already printed enough?

> +
> +	/*
> +	 * Update boot lock register with the boot key to allow the
> +	 * secondary processor to start the kernel after an SEV.
> +	 */
> +	__raw_writel(BOOT_LOCK_KEY, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
> +
> +	/* Flush the kernel cache to ensure that the page tables are
> +	 * available for the secondary CPU to use and make sure that
> +	 * the write buffer is drained before doing an SEV.
> +	 */
> +	flush_cache_all();
> +	smp_wmb();

The page tables and everything else required should already be visible
to the secondary CPU - if not that points to a bug in the generic code.
All that you should need to do here is to make sure your write above
is visible to the secondary CPU (and the address for it to boot from).

> +
> +	/*
> +	 * Send an event to wake the secondary core from WFE state.
> +	 */
> +	sev();
> +
> +	timeout = jiffies + (1 * HZ);
> +	while (time_before(jiffies, timeout))
> +		;

Have you no way to determine that the secondary CPU has started?

> +void __init smp_init_cpus(void)
> +{
> +	unsigned int i, ncores;
> +
> +	ncores = scu_base ? scu_get_core_count(scu_base) : 1;

	unsigned int i, ncores = scu_get_core_count(SCU_PERIPH_BASE);

> +
> +	/* sanity check */
> +	if (ncores > NR_CPUS) {
> +		printk(KERN_WARNING
> +		       "Realview: no. of cores (%d) greater than configured "
> +		       "maximum of %d - clipping\n",
> +		       ncores, NR_CPUS);
> +		ncores = NR_CPUS;
> +	}

Firstly, you're not Realview.  Secondly, I think this kind of check should
be inside scu_get_core_count() if it has to exist.

> +
> +	for (i = 0; i < ncores; i++)
> +		set_cpu_possible(i, true);
> +}
> +
> +void __init platform_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +	int i;
> +
> +	/*
> +	 * Initialise the present map, which describes the set of CPUs
> +	 * actually populated at the present time.
> +	 */
> +	for (i = 0; i < max_cpus; i++)
> +		set_cpu_present(i, true);
> +
> +	scu_enable(scu_base);
> +
> +	/* Initialize the boot lock register to prevent CPU1 from
> +	   starting the kernel before CPU0 is ready for that.
> +	*/
> +	__raw_writel(0, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
> +
> +	/*
> +	 * Write the address of secondary startup routine into the
> +	 * boot address. The secondary CPU will use this value
> +	 * to get into the kernel after it's awake from WFE state.
> +	 *
> +	 * Note the physical address is needed as the secondary CPU
> +	 * will not have the MMU on yet. A barrier is added to ensure
> +	 * that write buffer is drained.
> +	 */
> +	__raw_writel(virt_to_phys(xilinx_secondary_startup),
> +					OCM_HIGH_BASE + BOOT_ADDR_OFFSET);
> +	smp_wmb();
> +
> +	/*
> +	 * Send an event to wake the secondary core from WFE state.
> +	 */
> +	sev();

This looks like it shouldn't be necessary - you send an event in the
boot_secondary() path which should be enough to get the CPU going.

[PATCH 2/3] ARM: Xilinx: add SMP specific support files

[John Linn]

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: Monday, April 04, 2011 10:50 AM
> To: John Linn
> Cc: linux-arm-kernel at lists.infradead.org;
linux-kernel at vger.kernel.org;
> catalin.marinas at arm.com; glikely at secretlab.ca; jamie at jamieiles.com;
> arnd at arndb.de
> Subject: Re: [PATCH 2/3] ARM: Xilinx: add SMP specific support files
> 
> On Sun, Apr 03, 2011 at 03:00:17PM -0600, John Linn wrote:
> > +#include <linux/jiffies.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +#include <asm/cacheflush.h>
> > +#include <asm/smp_scu.h>
> > +#include <mach/xilinx_soc.h>
> > +#include <mach/smp.h>
> > +#include "common.h"
> > +
> > +static void __iomem *scu_base = SCU_PERIPH_BASE;
> 
> You don't need this if its known at build time.

Ok.

> 
> > +
> > +static DEFINE_SPINLOCK(boot_lock);
> > +
> > +/* Secondary CPU kernel startup is a 2 phase process.
> > + * 1st phase is transition from a boot loader to the kernel, but
> > + * then wait not starting the kernel yet. 2nd phase starts the
> > + * the kernel. In both phases, the secondary CPU waits for an
> > + * event before it continues.
> > + */
> > +
> > +void __cpuinit platform_secondary_init(unsigned int cpu)
> > +{
> > +	/*
> > +	 * if any interrupts are already enabled for the primary
> > +	 * core (e.g. timer irq), then they will not have been enabled
> > +	 * for us: do so
> > +	 */
> > +	gic_secondary_init(0);
> > +
> > +	/*
> > +	 * Synchronise with the boot thread.
> > +	 */
> > +	spin_lock(&boot_lock);
> > +	spin_unlock(&boot_lock);
> > +}
> > +
> > +int __cpuinit boot_secondary(unsigned int cpu, struct task_struct
> *idle)
> > +{
> > +	unsigned long timeout;
> > +
> > +	/*
> > +	 * set synchronisation state between this boot processor
> > +	 * and the secondary one
> > +	 */
> > +	spin_lock(&boot_lock);
> > +
> > +	printk(KERN_INFO "Xilinx SMP: booting CPU1 now\n");
> 
> Aren't the messages which are already printed enough?

This was helpful, but can be deleted too, not a big deal.

> 
> > +
> > +	/*
> > +	 * Update boot lock register with the boot key to allow the
> > +	 * secondary processor to start the kernel after an SEV.
> > +	 */
> > +	__raw_writel(BOOT_LOCK_KEY, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
> > +
> > +	/* Flush the kernel cache to ensure that the page tables are
> > +	 * available for the secondary CPU to use and make sure that
> > +	 * the write buffer is drained before doing an SEV.
> > +	 */
> > +	flush_cache_all();
> > +	smp_wmb();
> 
> The page tables and everything else required should already be visible
> to the secondary CPU - if not that points to a bug in the generic
code.
> All that you should need to do here is to make sure your write above
> is visible to the secondary CPU (and the address for it to boot from).

I didn't have the flush in early during debug and found it was needed.
I haven't tried it for quite some time without, I'll give it a try as
maybe some generic code was updated to fix a problem since then.

I noticed it in other platforms was the reason I thought it was really
needed.

> 
> > +
> > +	/*
> > +	 * Send an event to wake the secondary core from WFE state.
> > +	 */
> > +	sev();
> > +
> > +	timeout = jiffies + (1 * HZ);
> > +	while (time_before(jiffies, timeout))
> > +		;
> 
> Have you no way to determine that the secondary CPU has started?

Not right now. I can look at the ability to handshake back so that 
it's not based on time.  I'll to see what some other platforms do with
that.

I can see that it would help boot time not to do it this way.

It was based it off of some platforms doing it like this and it may 
not have been the best pattern to use.

> 
> > +void __init smp_init_cpus(void)
> > +{
> > +	unsigned int i, ncores;
> > +
> > +	ncores = scu_base ? scu_get_core_count(scu_base) : 1;
> 
> 	unsigned int i, ncores = scu_get_core_count(SCU_PERIPH_BASE);
> 
> > +
> > +	/* sanity check */
> > +	if (ncores > NR_CPUS) {
> > +		printk(KERN_WARNING
> > +		       "Realview: no. of cores (%d) greater than
configured
> "
> > +		       "maximum of %d - clipping\n",
> > +		       ncores, NR_CPUS);
> > +		ncores = NR_CPUS;
> > +	}
> 
> Firstly, you're not Realview.  Secondly, I think this kind of check
> should
> be inside scu_get_core_count() if it has to exist.
> 

I should have caught that. Ok it can be moved inside.  

The NR_CPUS thing is not real clear to me so I'll have to investigate
more as we 
only have 2 CPUs, yet the default in the kernel configs is generally 4. 

I know there has been a big deal about defconfigs so I was trying not to
add more
problems by adding one for Xilinx platform, but maybe I have
misunderstood that issue.

> > +
> > +	for (i = 0; i < ncores; i++)
> > +		set_cpu_possible(i, true);
> > +}
> > +
> > +void __init platform_smp_prepare_cpus(unsigned int max_cpus)
> > +{
> > +	int i;
> > +
> > +	/*
> > +	 * Initialise the present map, which describes the set of CPUs
> > +	 * actually populated at the present time.
> > +	 */
> > +	for (i = 0; i < max_cpus; i++)
> > +		set_cpu_present(i, true);
> > +
> > +	scu_enable(scu_base);
> > +
> > +	/* Initialize the boot lock register to prevent CPU1 from
> > +	   starting the kernel before CPU0 is ready for that.
> > +	*/
> > +	__raw_writel(0, OCM_HIGH_BASE + BOOT_LOCK_OFFSET);
> > +
> > +	/*
> > +	 * Write the address of secondary startup routine into the
> > +	 * boot address. The secondary CPU will use this value
> > +	 * to get into the kernel after it's awake from WFE state.
> > +	 *
> > +	 * Note the physical address is needed as the secondary CPU
> > +	 * will not have the MMU on yet. A barrier is added to ensure
> > +	 * that write buffer is drained.
> > +	 */
> > +	__raw_writel(virt_to_phys(xilinx_secondary_startup),
> > +					OCM_HIGH_BASE +
BOOT_ADDR_OFFSET);
> > +	smp_wmb();
> > +
> > +	/*
> > +	 * Send an event to wake the secondary core from WFE state.
> > +	 */
> > +	sev();
> 
> This looks like it shouldn't be necessary - you send an event in the
> boot_secondary() path which should be enough to get the CPU going.

It's a 2 stage process, the 1st SEV gets the 2nd CPU out of the boot rom
and into the kernel, the 2nd SEV lets it start running the kernel.

At some point this was based on a similar pattern that I saw in other
platforms.

Thanks for your time and input,
John

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH 3/3] ARM: Xilinx: update of infrastructure for SMP

[John Linn]

Only minor updates of Kconfig and Makefile to support
the local timer and SMP.

Signed-off-by: John Linn <john.linn@xilinx.com>
---
 arch/arm/Kconfig              |    2 +-
 arch/arm/mach-xilinx/Makefile |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index aa7ac3b..50283ae 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1338,7 +1338,7 @@ config SMP
 	depends on REALVIEW_EB_ARM11MP || REALVIEW_EB_A9MP || \
 		 MACH_REALVIEW_PB11MP || MACH_REALVIEW_PBX || ARCH_OMAP4 || \
 		 ARCH_EXYNOS4 || ARCH_TEGRA || ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || \
-		 ARCH_MSM_SCORPIONMP || ARCH_SHMOBILE
+		 ARCH_MSM_SCORPIONMP || ARCH_SHMOBILE || ARCH_XILINX
 	select USE_GENERIC_SMP_HELPERS
 	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
 	help
diff --git a/arch/arm/mach-xilinx/Makefile b/arch/arm/mach-xilinx/Makefile
index 62787ff..2f34007 100644
--- a/arch/arm/mach-xilinx/Makefile
+++ b/arch/arm/mach-xilinx/Makefile
@@ -5,3 +5,5 @@
 # Common support
 obj-y 				:= common.o timer.o
 obj-$(CONFIG_MACH_XILINX_EP107)	+= board_ep107.o
+obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
+obj-$(CONFIG_LOCAL_TIMERS)	+= localtimer.o
-- 
1.5.4.7



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.