[PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support

[PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support

[Li Yang]

Add support to disable and re-enable individual cores at runtime
on MPC85xx/QorIQ SMP machines.

This makes suspend/resume possible for SMP systems, as the power management
code on SMP always disable non-boot cpus on suspend.

MPC85xx machines use ePAPR spin-table in boot page for CPU kick-off.
This patch brings the bootpage and spin-table from bootloader into
kernel because the bootpage in bootloader might have been lost at
runtime.  Also add support to boot from physical address larger than
32-bit.

Signed-off-by: Yutaka Ando <y.ando@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/Kconfig                   |    2 +-
 arch/powerpc/kernel/Makefile           |    2 +-
 arch/powerpc/kernel/head_fsl_booke.S   |   32 +++++
 arch/powerpc/kernel/smp.c              |    4 +-
 arch/powerpc/platforms/85xx/Makefile   |    4 +-
 arch/powerpc/platforms/85xx/bootpage.S |  206 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/85xx/smp.c      |  222 ++++++++++++++++++++++++++------
 7 files changed, 428 insertions(+), 44 deletions(-)
 create mode 100644 arch/powerpc/platforms/85xx/bootpage.S

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e625e9e..b1982dd 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -320,7 +320,7 @@ config SWIOTLB
 
 config HOTPLUG_CPU
 	bool "Support for enabling/disabling CPUs"
-	depends on SMP && HOTPLUG && EXPERIMENTAL && (PPC_PSERIES || PPC_PMAC)
+	depends on SMP && HOTPLUG && EXPERIMENTAL && (PPC_PSERIES || PPC_PMAC || E500)
 	---help---
 	  Say Y here to be able to disable and re-enable individual
 	  CPUs at runtime on SMP machines.
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 36c30f3..bb20496 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -56,7 +56,7 @@ obj-$(CONFIG_IBMEBUS)           += ibmebus.o
 obj-$(CONFIG_GENERIC_TBSYNC)	+= smp-tbsync.o
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
 ifeq ($(CONFIG_PPC32),y)
-obj-$(CONFIG_E500)		+= idle_e500.o
+obj-$(CONFIG_E500)		+= idle_e500.o l2cr_85xx.o
 endif
 obj-$(CONFIG_6xx)		+= idle_6xx.o l2cr_6xx.o cpu_setup_6xx.o
 obj-$(CONFIG_TAU)		+= tau_6xx.o
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 529b817..61d9c46 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -23,6 +23,7 @@
  *	PowerPC 44x support, Matt Porter <mporter@kernel.crashing.org>
  *    Copyright 2004 Freescale Semiconductor, Inc
  *	PowerPC e500 modifications, Kumar Gala <galak@kernel.crashing.org>
+ *    Copyright 2008, 2010 Freescale Semiconductor, Inc.
  *
  * 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
@@ -254,6 +255,37 @@ _ENTRY(__early_start)
 	lwz	r11, 0(r12);		/* Get Linux PTE */
 #endif
 
+_GLOBAL(flush_disable_L1)
+/*
+ * Flush L1 d-cache, invalidate and disable d-cache,
+ * invalidate and disable i-cache
+ */
+	mflr	r10
+	bl	flush_dcache_L1	/* Flush L1 d-cache */
+	mtlr	r10
+
+	mfspr	r4, SPRN_L1CSR0	/* Invalidate and disable d-cache */
+	li	r5, 2
+	rlwimi	r4, r5, 0, 3
+
+	msync
+	isync
+	mtspr	SPRN_L1CSR0, r4
+	isync
+
+1:	mfspr	r4, SPRN_L1CSR0	/* Wait for the invalidate to finish */
+	andi.	r4, r4, 2
+	bne	1b
+
+	mfspr	r4, SPRN_L1CSR1	/* Invalidate and disable i-cache */
+	li	r5, 2
+	rlwimi	r4, r5, 0, 3
+
+	mtspr	SPRN_L1CSR1, r4
+	isync
+
+	blr
+
 /*
  * Interrupt vector entry code
  *
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 68034bb..321cf2e 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -317,6 +317,8 @@ int generic_cpu_disable(void)
 	set_cpu_online(cpu, false);
 #ifdef CONFIG_PPC64
 	vdso_data->processorCount--;
+#endif
+#if defined(CONFIG_PPC64) || defined(CONFIG_E500)
 	fixup_irqs(cpu_online_mask);
 #endif
 	return 0;
@@ -336,7 +338,7 @@ int generic_cpu_enable(unsigned int cpu)
 	while (!cpu_online(cpu))
 		cpu_relax();
 
-#ifdef CONFIG_PPC64
+#if defined(CONFIG_PPC64) || defined(CONFIG_E500)
 	fixup_irqs(cpu_online_mask);
 	/* counter the irq disable in fixup_irqs */
 	local_irq_enable();
diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index dd70db7..6bbcf22 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -1,7 +1,9 @@
 #
 # Makefile for the PowerPC 85xx linux kernel.
 #
-obj-$(CONFIG_SMP) += smp.o
+obj-$(CONFIG_SMP)         += smp.o
+obj-$(CONFIG_HOTPLUG_CPU) += bootpage.o
+obj-$(CONFIG_SUSPEND)     += suspend-asm.o
 
 obj-$(CONFIG_MPC8540_ADS) += mpc85xx_ads.o
 obj-$(CONFIG_MPC8560_ADS) += mpc85xx_ads.o
diff --git a/arch/powerpc/platforms/85xx/bootpage.S b/arch/powerpc/platforms/85xx/bootpage.S
new file mode 100644
index 0000000..ff0ca10
--- /dev/null
+++ b/arch/powerpc/platforms/85xx/bootpage.S
@@ -0,0 +1,206 @@
+/*
+ * Copyright 2008-2010 Freescale Semiconductor, Inc.
+ * Kumar Gala <kumar.gala@freescale.com>
+ * This file is taken from u-boot
+ *
+ * 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;
+ */
+#include <linux/init.h>
+#include <linux/threads.h>
+#include <asm/processor.h>
+#include <asm/page.h>
+#include <asm/mmu.h>
+#include <asm/pgtable.h>
+#include <asm/cputable.h>
+#include <asm/thread_info.h>
+#include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/cache.h>
+
+/* To boot secondary cpus, we need a place for them to start up.
+ * Normally, they start at 0xfffffffc, but that's usually the
+ * firmware, and we don't want to have to run the firmware again.
+ * Instead, the primary cpu will set the BPTR to point here to
+ * this page.  We then set up the core, and head to
+ * start_secondary.  Note that this means that the code below
+ * must never exceed 1023 instructions (the branch at the end
+ * would then be the 1024th).
+ */
+	.globl	__secondary_start_page
+	.align	12
+__secondary_start_page:
+	lis	r3, 0x8000		/* enable machine check */
+#ifndef CONFIG_PPC_E500MC
+	ori	r3,r3,0x4000		/* enable Timebase */
+#endif
+#ifdef CONFIG_PHYS_64BIT
+	/* for 36-bit addressing */
+	ori	r3,r3,0x0080		/* enable MAS7 updates */
+#endif
+	mtspr	SPRN_HID0,r3
+
+#ifndef CONFIG_PPC_E500MC
+	li	r3,0x3000		/* Addr streaming & broadcast */
+	mtspr	SPRN_HID1,r3
+#endif
+
+	/* Enable branch prediction */
+	li	r3,0x201
+	mtspr	SPRN_BUCSR,r3
+
+	/* Ensure TB is 0 */
+	li	r3,0
+	mttbl	r3
+	mttbu	r3
+
+	mfspr	r0,SPRN_L1CSR1
+	ori	r0,r0,0x0003		/* Enable/invalidate the I-Cache */
+	mtspr	SPRN_L1CSR1,r0
+	isync
+
+
+	mfspr	r0,SPRN_L1CSR0
+	ori	r0,r0,0x0003		/* Enable/invalidate the D-Cache */
+	msync
+	isync
+	mtspr	SPRN_L1CSR0,r0
+	isync
+
+#define toreset(x) (x - __secondary_start_page + 0xfffff000)
+
+	/* get our PIR to figure out our table entry */
+	lis	r3,toreset(__spin_table)@h
+	ori	r3,r3,toreset(__spin_table)@l
+
+	/* r10 has the base address for the entry */
+	mfspr	r0,SPRN_PIR
+#ifdef CONFIG_PPC_E500MC
+	rlwinm	r4,r0,27,27,31
+#else
+	mr	r4,r0
+#endif
+	slwi	r8,r4,5
+	add	r10,r3,r8
+
+#define EPAPR_MAGIC		(0x45504150)
+#define ENTRY_ADDR_UPPER	0
+#define ENTRY_ADDR_LOWER	4
+#define ENTRY_R3_UPPER		8
+#define ENTRY_R3_LOWER		12
+#define ENTRY_RESV		16
+#define ENTRY_PIR		20
+#define ENTRY_R6_UPPER		24
+#define ENTRY_R6_LOWER		28
+#define ENTRY_SIZE		32
+
+	/* setup the entry */
+	li	r3,0
+	li	r8,1
+	stw	r0,ENTRY_PIR(r10)
+	stw	r3,ENTRY_ADDR_UPPER(r10)
+	stw	r8,ENTRY_ADDR_LOWER(r10)
+	stw	r3,ENTRY_R3_UPPER(r10)
+	stw	r4,ENTRY_R3_LOWER(r10)
+	stw	r3,ENTRY_R6_UPPER(r10)
+	stw	r3,ENTRY_R6_LOWER(r10)
+
+	/* setup mapping for AS = 1, and jump there */
+	lis	r11,(MAS0_TLBSEL(1)|MAS0_ESEL(1))@h
+	mtspr	SPRN_MAS0,r11
+	lis	r11,(MAS1_VALID|MAS1_IPROT)@h
+	ori	r11,r11,(MAS1_TS|MAS1_TSIZE(BOOK3E_PAGESZ_4K))@l
+	mtspr	SPRN_MAS1,r11
+	lis	r11,(0xfffff000|MAS2_I|MAS2_G)@h
+	ori	r11,r11,(0xfffff000|MAS2_I|MAS2_G)@l
+	mtspr	SPRN_MAS2,r11
+	lis	r11,(0xfffff000|MAS3_SX|MAS3_SW|MAS3_SR)@h
+	ori	r11,r11,(0xfffff000|MAS3_SX|MAS3_SW|MAS3_SR)@l
+	mtspr	SPRN_MAS3,r11
+	tlbwe
+
+	bl	1f
+1:	mflr	r11
+	addi	r11,r11,28
+	mfmsr	r13
+	ori	r12,r13,MSR_IS|MSR_DS@l
+
+	mtspr	SPRN_SRR0,r11
+	mtspr	SPRN_SRR1,r12
+	rfi
+
+	/* spin waiting for addr */
+2:
+	lwz	r4,ENTRY_ADDR_LOWER(r10)
+	andi.	r11,r4,1
+	bne	2b
+	isync
+
+	/* get the upper bits of the addr */
+	lwz	r11,ENTRY_ADDR_UPPER(r10)
+
+	/* setup branch addr */
+	mtspr	SPRN_SRR0,r4
+
+	/* mark the entry as released */
+	li	r8,3
+	stw	r8,ENTRY_ADDR_LOWER(r10)
+
+	/* mask by ~64M to setup our tlb we will jump to */
+	rlwinm	r12,r4,0,0,5
+
+	/* setup r3, r4, r5, r6, r7, r8, r9 */
+	lwz	r3,ENTRY_R3_LOWER(r10)
+	li	r4,0
+	li	r5,0
+	lwz	r6,ENTRY_R6_LOWER(r10)
+	lis	r7,(64*1024*1024)@h
+	li	r8,0
+	li	r9,0
+
+	/* load up the pir */
+	lwz	r0,ENTRY_PIR(r10)
+	mtspr	SPRN_PIR,r0
+	mfspr	r0,SPRN_PIR
+	stw	r0,ENTRY_PIR(r10)
+
+	mtspr	SPRN_IVPR,r12
+/*
+ * Coming here, we know the cpu has one TLB mapping in TLB1[0]
+ * which maps 0xfffff000-0xffffffff one-to-one.  We set up a
+ * second mapping that maps addr 1:1 for 64M, and then we jump to
+ * addr
+ */
+	lis	r10,(MAS0_TLBSEL(1)|MAS0_ESEL(0))@h
+	mtspr	SPRN_MAS0,r10
+	lis	r10,(MAS1_VALID|MAS1_IPROT)@h
+	ori	r10,r10,(MAS1_TSIZE(BOOK3E_PAGESZ_64M))@l
+	mtspr	SPRN_MAS1,r10
+	/* WIMGE = 0b00000 for now */
+	mtspr	SPRN_MAS2,r12
+	ori	r12,r12,(MAS3_SX|MAS3_SW|MAS3_SR)
+	mtspr	SPRN_MAS3,r12
+#ifdef CONFIG_PHYS_64BIT
+	mtspr	SPRN_MAS7,r11
+#endif
+	tlbwe
+
+/* Now we have another mapping for this page, so we jump to that
+ * mapping
+ */
+	mtspr	SPRN_SRR1,r13
+	rfi
+
+	.align L1_CACHE_SHIFT
+	.globl __spin_table
+__spin_table:
+	.space NR_CPUS*ENTRY_SIZE
+
+	/* Fill in the empty space.  The actual reset vector is
+	 * the last word of the page */
+__secondary_start_code_end:
+	.space 4092 - (__secondary_start_code_end - __secondary_start_page)
+
+__secondary_reset_vector:
+	b	__secondary_start_page
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
index 1e8aec8..2ef3e8e 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/kexec.h>
 #include <linux/highmem.h>
+#include <linux/cpu.h>
 
 #include <asm/machdep.h>
 #include <asm/pgtable.h>
@@ -28,26 +29,116 @@
 #include <sysdev/fsl_soc.h>
 #include <sysdev/mpic.h>
 
+#define MPC85xx_BPTR_OFF		0x00020
+#define MPC85xx_BPTR_EN			0x80000000
+#define MPC85xx_ECM_EEBPCR_OFF		0x01010
+#define MPC85xx_PIC_PIR_OFF		0x41090
+
+extern void mpc85xx_cpu_down(void) __attribute__((noreturn));
 extern void __early_start(void);
+extern void __secondary_start_page(void);
+extern volatile unsigned long __spin_table;
+
+struct epapr_entry {
+	u32	addr_h;
+	u32	addr_l;
+	u32	r3_h;
+	u32	r3_l;
+	u32	reserved;
+	u32	pir;
+	u32	r6_h;
+	u32	r6_l;
+};
 
-#define BOOT_ENTRY_ADDR_UPPER	0
-#define BOOT_ENTRY_ADDR_LOWER	1
-#define BOOT_ENTRY_R3_UPPER	2
-#define BOOT_ENTRY_R3_LOWER	3
-#define BOOT_ENTRY_RESV		4
-#define BOOT_ENTRY_PIR		5
-#define BOOT_ENTRY_R6_UPPER	6
-#define BOOT_ENTRY_R6_LOWER	7
-#define NUM_BOOT_ENTRY		8
-#define SIZE_BOOT_ENTRY		(NUM_BOOT_ENTRY * sizeof(u32))
+/* access per cpu vars from generic smp.c */
+DECLARE_PER_CPU(int, cpu_state);
 
-static void __init
+#ifdef CONFIG_HOTPLUG_CPU
+static void __cpuinit
+smp_85xx_mach_cpu_die(void)
+{
+	__get_cpu_var(cpu_state) = CPU_DEAD;
+	smp_wmb();
+
+	local_irq_disable();
+	idle_task_exit();
+	mtspr(SPRN_TSR, TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS);
+	mtspr(SPRN_TCR, 0);
+	mpc85xx_cpu_down();
+}
+
+static void __cpuinit
+smp_85xx_reset_core(int nr)
+{
+	__iomem u32 *ecm_vaddr;
+	__iomem u32 *pic_vaddr;
+	u32 pcr, pir, cpu;
+
+	cpu = (1 << 24) << nr;
+	ecm_vaddr = ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4);
+	pcr = in_be32(ecm_vaddr);
+	if (pcr & cpu) {
+		pic_vaddr = ioremap(get_immrbase() + MPC85xx_PIC_PIR_OFF, 4);
+		pir = in_be32(pic_vaddr);
+		/* reset assert */
+		pir |= (1 << nr);
+		out_be32(pic_vaddr, pir);
+		pir = in_be32(pic_vaddr);
+		pir &= ~(1 << nr);
+		/* reset negate */
+		out_be32(pic_vaddr, pir);
+		(void)in_be32(pic_vaddr);
+		iounmap(pic_vaddr);
+	} else {
+		out_be32(ecm_vaddr, pcr | cpu);
+		(void)in_be32(ecm_vaddr);
+	}
+	iounmap(ecm_vaddr);
+}
+
+static int __cpuinit
+smp_85xx_map_bootpg(unsigned long pa)
+{
+	__iomem u32 *bootpg_ptr;
+	u32 bptr;
+
+	/* Get the BPTR */
+	bootpg_ptr = ioremap(get_immrbase() + MPC85xx_BPTR_OFF, 4);
+
+	/* Set the BPTR to the secondary boot page */
+	(void)in_be32(bootpg_ptr);
+
+	bptr = (MPC85xx_BPTR_EN | (pa >> 12));
+	out_be32(bootpg_ptr, bptr);
+	(void)in_be32(bootpg_ptr);
+	iounmap(bootpg_ptr);
+	return 0;
+}
+
+static int __cpuinit
+smp_85xx_unmap_bootpg(void)
+{
+	__iomem u32 *bootpg_ptr;
+
+	/* Get the BPTR */
+	bootpg_ptr = ioremap(get_immrbase() + MPC85xx_BPTR_OFF, 4);
+
+	/* Restore the BPTR */
+	if (in_be32(bootpg_ptr) & MPC85xx_BPTR_EN) {
+		out_be32(bootpg_ptr, 0);
+		(void)in_be32(bootpg_ptr);
+	}
+	iounmap(bootpg_ptr);
+	return 0;
+}
+#endif
+
+static void __cpuinit
 smp_85xx_kick_cpu(int nr)
 {
 	unsigned long flags;
-	const u64 *cpu_rel_addr;
-	__iomem u32 *bptr_vaddr;
-	struct device_node *np;
+	phys_addr_t cpu_rel_addr;
+	__iomem struct epapr_entry *epapr;
 	int n = 0;
 	int ioremappable;
 
@@ -55,41 +146,83 @@ smp_85xx_kick_cpu(int nr)
 
 	pr_debug("smp_85xx_kick_cpu: kick CPU #%d\n", nr);
 
-	np = of_get_cpu_node(nr, NULL);
-	cpu_rel_addr = of_get_property(np, "cpu-release-addr", NULL);
+	if (system_state < SYSTEM_RUNNING) {
+		/* booting, using __spin_table from u-boot */
+		struct device_node *np;
+		const u64 *prop;
 
-	if (cpu_rel_addr == NULL) {
-		printk(KERN_ERR "No cpu-release-addr for cpu %d\n", nr);
-		return;
-	}
+		np = of_get_cpu_node(nr, NULL);
+		if (np == NULL)
+			return;
 
-	/*
-	 * A secondary core could be in a spinloop in the bootpage
-	 * (0xfffff000), somewhere in highmem, or somewhere in lowmem.
-	 * The bootpage and highmem can be accessed via ioremap(), but
-	 * we need to directly access the spinloop if its in lowmem.
-	 */
-	ioremappable = *cpu_rel_addr > virt_to_phys(high_memory);
+		prop = of_get_property(np, "cpu-release-addr", NULL);
+		if (prop == NULL) {
+			of_node_put(np);
+			printk(KERN_ERR "No cpu-release-addr for cpu %d\n", nr);
+			return;
+		}
+		cpu_rel_addr = (phys_addr_t)*prop;
+		of_node_put(np);
+
+		/*
+		 * A secondary core could be in a spinloop in the bootpage
+		 * (0xfffff000), somewhere in highmem, or somewhere in lowmem.
+		 * The bootpage and highmem can be accessed via ioremap(), but
+		 * we need to directly access the spinloop if its in lowmem.
+		 */
+		ioremappable = cpu_rel_addr > virt_to_phys(high_memory);
+
+		if (ioremappable)
+			epapr = ioremap(cpu_rel_addr,
+						sizeof(struct epapr_entry));
+		else
+			epapr = phys_to_virt(cpu_rel_addr);
+
+		local_irq_save(flags);
+	} else {
+#ifdef CONFIG_HOTPLUG_CPU
+		/* spin table in kernel, no need to remap */
+		ioremappable = 0;
+		epapr = (void *)&__spin_table + nr * sizeof(struct epapr_entry);
 
-	/* Map the spin table */
-	if (ioremappable)
-		bptr_vaddr = ioremap(*cpu_rel_addr, SIZE_BOOT_ENTRY);
-	else
-		bptr_vaddr = phys_to_virt(*cpu_rel_addr);
+		/* prevent bootpage from being accessed by others */
+		local_irq_save(flags);
+
+		smp_85xx_map_bootpg(__pa(__secondary_start_page));
 
-	local_irq_save(flags);
+		smp_85xx_reset_core(nr);
 
-	out_be32(bptr_vaddr + BOOT_ENTRY_PIR, nr);
+		/* wait until core(nr) is ready... */
+		while ((in_be32(&epapr->addr_l) != 1) && (++n < 1000))
+			udelay(100);
+
+		if (n == 1000) {
+			pr_err("timeout waiting for core%d to reset\n",
+					nr);
+			goto out;
+		}
+#else
+		pr_err("runtime kick cpu not supported\n");
+		return;
+#endif
+	}
+
+	out_be32(&epapr->pir, nr);
 #ifdef CONFIG_PPC32
-	out_be32(bptr_vaddr + BOOT_ENTRY_ADDR_LOWER, __pa(__early_start));
+	/* clear the acknowledge status */
+	__secondary_hold_acknowledge = -1;
+	out_be32(&epapr->addr_l, __pa(__early_start));
 
 	if (!ioremappable)
-		flush_dcache_range((ulong)bptr_vaddr,
-				(ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
+		flush_dcache_range((ulong)epapr,
+				(ulong)epapr + sizeof(struct epapr_entry));
 
 	/* Wait a bit for the CPU to ack. */
+	n = 0;
 	while ((__secondary_hold_acknowledge != nr) && (++n < 1000))
 		mdelay(1);
+	if (n == 1000)
+		pr_err("timeout waiting for core%d to ack\n", nr);
 #else
 	out_be64((u64 *)(bptr_vaddr + BOOT_ENTRY_ADDR_UPPER),
 		__pa((u64)*((unsigned long long *) generic_secondary_smp_init)));
@@ -97,12 +230,15 @@ smp_85xx_kick_cpu(int nr)
 	smp_generic_kick_cpu(nr);
 #endif
 
+out:
+#ifdef CONFIG_HOTPLUG_CPU
+	if (system_state >= SYSTEM_RUNNING)
+		smp_85xx_unmap_bootpg();
+#endif
 	local_irq_restore(flags);
 
 	if (ioremappable)
-		iounmap(bptr_vaddr);
-
-	pr_debug("waited %d msecs for CPU #%d.\n", n, nr);
+		iounmap(epapr);
 }
 
 static void __init
@@ -232,6 +368,12 @@ void __init mpc85xx_smp_init(void)
 
 	BUG_ON(!smp_85xx_ops.message_pass);
 
+#ifdef CONFIG_HOTPLUG_CPU
+	smp_85xx_ops.cpu_disable   = generic_cpu_disable;
+	smp_85xx_ops.cpu_die	= generic_cpu_die;
+	ppc_md.cpu_die		= smp_85xx_mach_cpu_die;
+#endif
+
 	smp_ops = &smp_85xx_ops;
 
 #ifdef CONFIG_KEXEC
-- 
1.6.6-rc1.GIT

[PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support

[Zhao Chenhui]

From: Li Yang <leoli@freescale.com>

Add support to disable and re-enable individual cores at runtime
on MPC85xx/QorIQ SMP machines. Currently support e500 core.

MPC85xx machines use ePAPR spin-table in boot page for CPU kick-off.
This patch uses the boot page from bootloader to boot core at runtime.
It supports 32-bit and 36-bit physical address.

Add delay in generic_cpu_die() to wait core reset.

Signed-off-by: Li Yang <leoli@freescale.com>
Signed-off-by: Jin Qing <b24347@freescale.com>
Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
---
 arch/powerpc/Kconfig                 |    5 +-
 arch/powerpc/kernel/head_fsl_booke.S |   28 +++++
 arch/powerpc/kernel/smp.c            |    8 +-
 arch/powerpc/platforms/85xx/smp.c    |  220 +++++++++++++++++++++++++++++-----
 4 files changed, 226 insertions(+), 35 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 47682b6..dc7feba 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -212,7 +212,7 @@ config ARCH_HIBERNATION_POSSIBLE
 config ARCH_SUSPEND_POSSIBLE
 	def_bool y
 	depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \
-		   (PPC_85xx && !SMP) || PPC_86xx || PPC_PSERIES || 44x || 40x
+		   PPC_85xx || PPC_86xx || PPC_PSERIES || 44x || 40x
 
 config PPC_DCR_NATIVE
 	bool
@@ -323,7 +323,8 @@ config SWIOTLB
 
 config HOTPLUG_CPU
 	bool "Support for enabling/disabling CPUs"
-	depends on SMP && HOTPLUG && EXPERIMENTAL && (PPC_PSERIES || PPC_PMAC)
+	depends on SMP && HOTPLUG && EXPERIMENTAL && (PPC_PSERIES || \
+		PPC_PMAC || E500)
 	---help---
 	  Say Y here to be able to disable and re-enable individual
 	  CPUs at runtime on SMP machines.
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 5084592..d13ae54 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -899,6 +899,34 @@ _GLOBAL(flush_dcache_L1)
 
 	blr
 
+/* Flush L1 d-cache, invalidate and disable d-cache and i-cache */
+_GLOBAL(flush_disable_L1)
+	mflr	r10
+	bl	flush_dcache_L1	/* Flush L1 d-cache */
+	mtlr	r10
+
+	mfspr	r4, SPRN_L1CSR0	/* Invalidate and disable d-cache */
+	li	r5, 2
+	rlwimi	r4, r5, 0, 3
+
+	msync
+	isync
+	mtspr	SPRN_L1CSR0, r4
+	isync
+
+1:	mfspr	r4, SPRN_L1CSR0	/* Wait for the invalidate to finish */
+	andi.	r4, r4, 2
+	bne	1b
+
+	mfspr	r4, SPRN_L1CSR1	/* Invalidate and disable i-cache */
+	li	r5, 2
+	rlwimi	r4, r5, 0, 3
+
+	mtspr	SPRN_L1CSR1, r4
+	isync
+
+	blr
+
 #ifdef CONFIG_SMP
 /* When we get here, r24 needs to hold the CPU # */
 	.globl __secondary_start
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 7bf2187..12a54f0 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -381,8 +381,14 @@ void generic_cpu_die(unsigned int cpu)
 
 	for (i = 0; i < 100; i++) {
 		smp_rmb();
-		if (per_cpu(cpu_state, cpu) == CPU_DEAD)
+		if (per_cpu(cpu_state, cpu) == CPU_DEAD) {
+			/*
+			 * After another core sets cpu_state to CPU_DEAD,
+			 * it needs some time to die.
+			 */
+			msleep(10);
 			return;
+		}
 		msleep(100);
 	}
 	printk(KERN_ERR "CPU%d didn't die...\n", cpu);
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
index 9b0de9c..5a54fc1 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/kexec.h>
 #include <linux/highmem.h>
+#include <linux/cpu.h>
 
 #include <asm/machdep.h>
 #include <asm/pgtable.h>
@@ -30,26 +31,141 @@
 
 extern void __early_start(void);
 
-#define BOOT_ENTRY_ADDR_UPPER	0
-#define BOOT_ENTRY_ADDR_LOWER	1
-#define BOOT_ENTRY_R3_UPPER	2
-#define BOOT_ENTRY_R3_LOWER	3
-#define BOOT_ENTRY_RESV		4
-#define BOOT_ENTRY_PIR		5
-#define BOOT_ENTRY_R6_UPPER	6
-#define BOOT_ENTRY_R6_LOWER	7
-#define NUM_BOOT_ENTRY		8
-#define SIZE_BOOT_ENTRY		(NUM_BOOT_ENTRY * sizeof(u32))
-
-static int __init
-smp_85xx_kick_cpu(int nr)
+#define MPC85xx_BPTR_OFF		0x00020
+#define MPC85xx_BPTR_EN			0x80000000
+#define MPC85xx_BPTR_BOOT_PAGE_MASK	0x00ffffff
+#define MPC85xx_BRR_OFF			0xe0e4
+#define MPC85xx_ECM_EEBPCR_OFF		0x01010
+#define MPC85xx_PIC_PIR_OFF		0x41090
+
+struct epapr_entry {
+	u32	addr_h;
+	u32	addr_l;
+	u32	r3_h;
+	u32	r3_l;
+	u32	reserved;
+	u32	pir;
+	u32	r6_h;
+	u32	r6_l;
+};
+
+static int is_corenet;
+static void __cpuinit smp_85xx_setup_cpu(int cpu_nr);
+
+#if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PPC32)
+extern void flush_disable_L1(void);
+
+static void __cpuinit smp_85xx_mach_cpu_die(void)
+{
+	unsigned int cpu = smp_processor_id();
+	register u32 tmp;
+
+	local_irq_disable();
+	idle_task_exit();
+	generic_set_cpu_dead(cpu);
+	smp_wmb();
+
+	mtspr(SPRN_TSR, TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS);
+	mtspr(SPRN_TCR, 0);
+
+	flush_disable_L1();
+
+	tmp = 0;
+	if (cpu_has_feature(CPU_FTR_CAN_NAP))
+		tmp = HID0_NAP;
+	else if (cpu_has_feature(CPU_FTR_CAN_DOZE))
+		tmp = HID0_DOZE;
+	if (tmp) {
+		tmp |= mfspr(SPRN_HID0) & ~(HID0_DOZE|HID0_NAP|HID0_SLEEP);
+
+		smp_mb();
+		isync();
+		mtspr(SPRN_HID0, tmp);
+		isync();
+
+		tmp = mfmsr();
+		tmp |= MSR_WE;
+		smp_mb();
+		mtmsr(tmp);
+		isync();
+	}
+
+	for (;;);
+}
+
+static void __cpuinit smp_85xx_reset_core(int nr)
+{
+	__iomem u32 *vaddr, *pir_vaddr;
+	u32 val, cpu_mask;
+
+	/* If CoreNet platform, use BRR as release register. */
+	if (is_corenet) {
+		cpu_mask = 1 << nr;
+		vaddr = ioremap(get_immrbase() + MPC85xx_BRR_OFF, 4);
+	} else {
+		cpu_mask = 1 << (24 + nr);
+		vaddr = ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4);
+	}
+	val = in_be32(vaddr);
+	if (!(val & cpu_mask)) {
+		out_be32(vaddr, val | cpu_mask);
+	} else {
+		/* reset core */
+		pir_vaddr = ioremap(get_immrbase() + MPC85xx_PIC_PIR_OFF, 4);
+		val = in_be32(pir_vaddr);
+		/* reset assert */
+		val |= (1 << nr);
+		out_be32(pir_vaddr, val);
+		val = in_be32(pir_vaddr);
+		val &= ~(1 << nr);
+		/* reset negate */
+		out_be32(pir_vaddr, val);
+		iounmap(pir_vaddr);
+	}
+	iounmap(vaddr);
+}
+
+static int __cpuinit smp_85xx_map_bootpg(u32 page)
+{
+	__iomem u32 *bootpg_ptr;
+	u32 bptr;
+
+	/* Get the BPTR */
+	bootpg_ptr = ioremap(get_immrbase() + MPC85xx_BPTR_OFF, 4);
+
+	/* Set the BPTR to the secondary boot page */
+	bptr = MPC85xx_BPTR_EN | (page & MPC85xx_BPTR_BOOT_PAGE_MASK);
+	out_be32(bootpg_ptr, bptr);
+
+	iounmap(bootpg_ptr);
+	return 0;
+}
+
+static int __cpuinit smp_85xx_unmap_bootpg(void)
+{
+	__iomem u32 *bootpg_ptr;
+
+	/* Get the BPTR */
+	bootpg_ptr = ioremap(get_immrbase() + MPC85xx_BPTR_OFF, 4);
+
+	/* Restore the BPTR */
+	if (in_be32(bootpg_ptr) & MPC85xx_BPTR_EN)
+		out_be32(bootpg_ptr, 0);
+
+	iounmap(bootpg_ptr);
+	return 0;
+}
+#endif
+
+static int __cpuinit smp_85xx_kick_cpu(int nr)
 {
 	unsigned long flags;
 	const u64 *cpu_rel_addr;
-	__iomem u32 *bptr_vaddr;
+	__iomem struct epapr_entry *epapr;
 	struct device_node *np;
-	int n = 0;
+	int n = 0, hw_cpu = get_hard_smp_processor_id(nr);
 	int ioremappable;
+	int ret = 0;
 
 	WARN_ON (nr < 0 || nr >= NR_CPUS);
 
@@ -73,46 +189,79 @@ smp_85xx_kick_cpu(int nr)
 
 	/* Map the spin table */
 	if (ioremappable)
-		bptr_vaddr = ioremap(*cpu_rel_addr, SIZE_BOOT_ENTRY);
+		epapr = ioremap(*cpu_rel_addr, sizeof(struct epapr_entry));
 	else
-		bptr_vaddr = phys_to_virt(*cpu_rel_addr);
+		epapr = phys_to_virt(*cpu_rel_addr);
 
 	local_irq_save(flags);
 
-	out_be32(bptr_vaddr + BOOT_ENTRY_PIR, nr);
+	out_be32(&epapr->pir, hw_cpu);
 #ifdef CONFIG_PPC32
-	out_be32(bptr_vaddr + BOOT_ENTRY_ADDR_LOWER, __pa(__early_start));
+#ifdef CONFIG_HOTPLUG_CPU
+	if (system_state == SYSTEM_RUNNING) {
+		out_be32(&epapr->addr_l, 0);
+		smp_85xx_map_bootpg((u32)(*cpu_rel_addr >> PAGE_SHIFT));
+
+		smp_85xx_reset_core(hw_cpu);
+
+		/* wait until core is ready... */
+		n = 0;
+		while ((in_be32(&epapr->addr_l) != 1) && (++n < 1000))
+			udelay(100);
+		if (n > 1000) {
+			pr_err("timeout waiting for core%d to reset\n",	nr);
+			ret = -ENOENT;
+			goto out;
+		}
+		/*  clear the acknowledge status */
+		__secondary_hold_acknowledge = -1;
+
+		smp_85xx_unmap_bootpg();
+	}
+#endif
+	out_be32(&epapr->addr_l, __pa(__early_start));
 
 	if (!ioremappable)
-		flush_dcache_range((ulong)bptr_vaddr,
-				(ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
+		flush_dcache_range((ulong)epapr,
+				(ulong)epapr + sizeof(struct epapr_entry));
 
 	/* Wait a bit for the CPU to ack. */
-	while ((__secondary_hold_acknowledge != nr) && (++n < 1000))
+	n = 0;
+	while ((__secondary_hold_acknowledge != hw_cpu) && (++n < 1000))
 		mdelay(1);
+	if (n > 1000) {
+		pr_err("timeout waiting for core%d to ack\n", nr);
+		ret = -ENOENT;
+		goto out;
+	}
+out:
 #else
 	smp_generic_kick_cpu(nr);
 
-	out_be64((u64 *)(bptr_vaddr + BOOT_ENTRY_ADDR_UPPER),
+	out_be64((u64 *)(&epapr->addr_h),
 		__pa((u64)*((unsigned long long *) generic_secondary_smp_init)));
 
 	if (!ioremappable)
-		flush_dcache_range((ulong)bptr_vaddr,
-				(ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
+		flush_dcache_range((ulong)epapr,
+				(ulong)epapr + sizeof(struct epapr_entry));
 #endif
-
 	local_irq_restore(flags);
 
 	if (ioremappable)
-		iounmap(bptr_vaddr);
+		iounmap(epapr);
 
 	pr_debug("waited %d msecs for CPU #%d.\n", n, nr);
 
-	return 0;
+	return ret;
 }
 
 struct smp_ops_t smp_85xx_ops = {
 	.kick_cpu = smp_85xx_kick_cpu,
+	.setup_cpu	= smp_85xx_setup_cpu,
+#ifdef CONFIG_HOTPLUG_CPU
+	.cpu_disable	= generic_cpu_disable,
+	.cpu_die	= generic_cpu_die,
+#endif
 	.give_timebase	= smp_generic_give_timebase,
 	.take_timebase	= smp_generic_take_timebase,
 };
@@ -214,8 +363,7 @@ static void mpc85xx_smp_machine_kexec(struct kimage *image)
 }
 #endif /* CONFIG_KEXEC */
 
-static void __init
-smp_85xx_setup_cpu(int cpu_nr)
+static void __cpuinit smp_85xx_setup_cpu(int cpu_nr)
 {
 	if (smp_85xx_ops.probe == smp_mpic_probe)
 		mpic_setup_this_cpu();
@@ -228,14 +376,18 @@ void __init mpc85xx_smp_init(void)
 {
 	struct device_node *np;
 
-	smp_85xx_ops.setup_cpu = smp_85xx_setup_cpu;
-
 	np = of_find_node_by_type(NULL, "open-pic");
 	if (np) {
 		smp_85xx_ops.probe = smp_mpic_probe;
 		smp_85xx_ops.message_pass = smp_mpic_message_pass;
 	}
 
+	/* Check if the chip is based on CoreNet platform. */
+	is_corenet = 0;
+	np = of_find_compatible_node(NULL, NULL, "fsl,qoriq-device-config-1.0");
+	if (np)
+		is_corenet = 1;
+
 	if (cpu_has_feature(CPU_FTR_DBELL)) {
 		/*
 		 * If left NULL, .message_pass defaults to
@@ -244,6 +396,10 @@ void __init mpc85xx_smp_init(void)
 		smp_85xx_ops.cause_ipi = doorbell_cause_ipi;
 	}
 
+#ifdef CONFIG_HOTPLUG_CPU
+	ppc_md.cpu_die		= smp_85xx_mach_cpu_die;
+#endif
+
 	smp_ops = &smp_85xx_ops;
 
 #ifdef CONFIG_KEXEC
-- 
1.6.4.1

Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support

[Scott Wood]

On 11/04/2011 07:31 AM, Zhao Chenhui wrote:
> From: Li Yang <leoli@freescale.com>
> 
> Add support to disable and re-enable individual cores at runtime
> on MPC85xx/QorIQ SMP machines. Currently support e500 core.
> 
> MPC85xx machines use ePAPR spin-table in boot page for CPU kick-off.
> This patch uses the boot page from bootloader to boot core at runtime.
> It supports 32-bit and 36-bit physical address.

Note that there is no guarantee that the bootloader can handle you
resetting a core.  In ePAPR the spin table is a one-time release
mechanism, not a core reset mechanism.  If this has a U-Boot dependency,
document that.

>  #ifdef CONFIG_SMP
>  /* When we get here, r24 needs to hold the CPU # */
>  	.globl __secondary_start
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 7bf2187..12a54f0 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -381,8 +381,14 @@ void generic_cpu_die(unsigned int cpu)
>  
>  	for (i = 0; i < 100; i++) {
>  		smp_rmb();
> -		if (per_cpu(cpu_state, cpu) == CPU_DEAD)
> +		if (per_cpu(cpu_state, cpu) == CPU_DEAD) {
> +			/*
> +			 * After another core sets cpu_state to CPU_DEAD,
> +			 * it needs some time to die.
> +			 */
> +			msleep(10);
>  			return;
> +		}
>  		msleep(100);

It would be better to do this as a call into platform-specific code than
can check registers to determine whether the core has checked out (in
our case, whether it has entered nap) -- or to do a suitable delay for
that platform if this isn't possible.

> diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
> index 9b0de9c..5a54fc1 100644
> --- a/arch/powerpc/platforms/85xx/smp.c
> +++ b/arch/powerpc/platforms/85xx/smp.c
> @@ -17,6 +17,7 @@
>  #include <linux/of.h>
>  #include <linux/kexec.h>
>  #include <linux/highmem.h>
> +#include <linux/cpu.h>
>  
>  #include <asm/machdep.h>
>  #include <asm/pgtable.h>
> @@ -30,26 +31,141 @@
>  
>  extern void __early_start(void);
>  
> -#define BOOT_ENTRY_ADDR_UPPER	0
> -#define BOOT_ENTRY_ADDR_LOWER	1
> -#define BOOT_ENTRY_R3_UPPER	2
> -#define BOOT_ENTRY_R3_LOWER	3
> -#define BOOT_ENTRY_RESV		4
> -#define BOOT_ENTRY_PIR		5
> -#define BOOT_ENTRY_R6_UPPER	6
> -#define BOOT_ENTRY_R6_LOWER	7
> -#define NUM_BOOT_ENTRY		8
> -#define SIZE_BOOT_ENTRY		(NUM_BOOT_ENTRY * sizeof(u32))
> -
> -static int __init
> -smp_85xx_kick_cpu(int nr)
> +#define MPC85xx_BPTR_OFF		0x00020
> +#define MPC85xx_BPTR_EN			0x80000000
> +#define MPC85xx_BPTR_BOOT_PAGE_MASK	0x00ffffff
> +#define MPC85xx_BRR_OFF			0xe0e4
> +#define MPC85xx_ECM_EEBPCR_OFF		0x01010
> +#define MPC85xx_PIC_PIR_OFF		0x41090
> +
> +struct epapr_entry {

ePAPR is more than just the spin table.  Call it something like
epapr_spin_table.

> +	u32	addr_h;
> +	u32	addr_l;
> +	u32	r3_h;
> +	u32	r3_l;
> +	u32	reserved;
> +	u32	pir;
> +	u32	r6_h;
> +	u32	r6_l;
> +};

Get rid of r6, it is not part of the ePAPR spin table.

> +static int is_corenet;
> +static void __cpuinit smp_85xx_setup_cpu(int cpu_nr);
> +
> +#if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PPC32)

Why PPC32?

> +extern void flush_disable_L1(void);

If this isn't already in a header file, put it in one.

> +static void __cpuinit smp_85xx_mach_cpu_die(void)
> +{
> +	unsigned int cpu = smp_processor_id();
> +	register u32 tmp;
> +
> +	local_irq_disable();
> +	idle_task_exit();
> +	generic_set_cpu_dead(cpu);
> +	smp_wmb();
> +
> +	mtspr(SPRN_TSR, TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS);
> +	mtspr(SPRN_TCR, 0);

If clearing TSR matters at all (I'm not sure that it does), first clear
TCR, then TSR.

> +	flush_disable_L1();

You'll also need to take down L2 on e500mc.

> +	tmp = 0;
> +	if (cpu_has_feature(CPU_FTR_CAN_NAP))
> +		tmp = HID0_NAP;
> +	else if (cpu_has_feature(CPU_FTR_CAN_DOZE))
> +		tmp = HID0_DOZE;

Those FTR bits are for what we can do in idle, and can be cleared if the
user sets CONFIG_BDI_SWITCH.

On 85xx we always want to nap here, and at least on e500mc it seems to
be mandatory.  From the p5020 RM description of PIR:

> For proper system operation, a core should be reset in this way only if the core is already in nap or sleep
> state. Because a core in either state cannot perform the necessary write to cause a hard reset, a core cannot
> put itself into hard reset.

Note that on e500mc we don't use HID0/MSR_WE to enter nap, we need to
hit the CCSR register.  And unless you can somehow guarantee that only
one core at a time is doing this, we'll need some oher core to actually
place us in nap (since once we enter nap we're not running so can't
release a lock).

> +	if (tmp) {
> +		tmp |= mfspr(SPRN_HID0) & ~(HID0_DOZE|HID0_NAP|HID0_SLEEP);
> +
> +		smp_mb();

smp_mb()?  This is always SMP...  It looks like you meant some specific
sync instruction as part of an architected sequence, so just use that.

> +		isync();
> +		mtspr(SPRN_HID0, tmp);
> +		isync();
> +
> +		tmp = mfmsr();
> +		tmp |= MSR_WE;
> +		smp_mb();
> +		mtmsr(tmp);
> +		isync();
> +	}
> +
> +	for (;;);
> +}
> +
> +static void __cpuinit smp_85xx_reset_core(int nr)
> +{
> +	__iomem u32 *vaddr, *pir_vaddr;
> +	u32 val, cpu_mask;
> +
> +	/* If CoreNet platform, use BRR as release register. */
> +	if (is_corenet) {
> +		cpu_mask = 1 << nr;
> +		vaddr = ioremap(get_immrbase() + MPC85xx_BRR_OFF, 4);
> +	} else {
> +		cpu_mask = 1 << (24 + nr);
> +		vaddr = ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4);
> +	}

Please use the device tree node, not get_immrbase().

> +	val = in_be32(vaddr);
> +	if (!(val & cpu_mask)) {
> +		out_be32(vaddr, val | cpu_mask);
> +	} else {
> +		/* reset core */
> +		pir_vaddr = ioremap(get_immrbase() + MPC85xx_PIC_PIR_OFF, 4);
> +		val = in_be32(pir_vaddr);
> +		/* reset assert */
> +		val |= (1 << nr);
> +		out_be32(pir_vaddr, val);

Use setbits32().

> +		val = in_be32(pir_vaddr);
> +		val &= ~(1 << nr);
> +		/* reset negate */
> +		out_be32(pir_vaddr, val);

clrbits32().

Is there any amount of time we need to keep the reset pin asserted?

> +		iounmap(pir_vaddr);
> +	}
> +	iounmap(vaddr);
> +}
> +
> +static int __cpuinit smp_85xx_map_bootpg(u32 page)
> +{
> +	__iomem u32 *bootpg_ptr;
> +	u32 bptr;
> +
> +	/* Get the BPTR */
> +	bootpg_ptr = ioremap(get_immrbase() + MPC85xx_BPTR_OFF, 4);
> +
> +	/* Set the BPTR to the secondary boot page */
> +	bptr = MPC85xx_BPTR_EN | (page & MPC85xx_BPTR_BOOT_PAGE_MASK);
> +	out_be32(bootpg_ptr, bptr);
> +
> +	iounmap(bootpg_ptr);
> +	return 0;
> +}

Shouldn't the boot page already be set by U-Boot?

> +static int __cpuinit smp_85xx_kick_cpu(int nr)
>  {
>  	unsigned long flags;
>  	const u64 *cpu_rel_addr;
> -	__iomem u32 *bptr_vaddr;
> +	__iomem struct epapr_entry *epapr;
>  	struct device_node *np;
> -	int n = 0;
> +	int n = 0, hw_cpu = get_hard_smp_processor_id(nr);
>  	int ioremappable;
> +	int ret = 0;
>  
>  	WARN_ON (nr < 0 || nr >= NR_CPUS);
>  
> @@ -73,46 +189,79 @@ smp_85xx_kick_cpu(int nr)
>  
>  	/* Map the spin table */
>  	if (ioremappable)
> -		bptr_vaddr = ioremap(*cpu_rel_addr, SIZE_BOOT_ENTRY);
> +		epapr = ioremap(*cpu_rel_addr, sizeof(struct epapr_entry));
>  	else
> -		bptr_vaddr = phys_to_virt(*cpu_rel_addr);
> +		epapr = phys_to_virt(*cpu_rel_addr);
>  
>  	local_irq_save(flags);
>  
> -	out_be32(bptr_vaddr + BOOT_ENTRY_PIR, nr);
> +	out_be32(&epapr->pir, hw_cpu);
>  #ifdef CONFIG_PPC32
> -	out_be32(bptr_vaddr + BOOT_ENTRY_ADDR_LOWER, __pa(__early_start));
> +#ifdef CONFIG_HOTPLUG_CPU
> +	if (system_state == SYSTEM_RUNNING) {
> +		out_be32(&epapr->addr_l, 0);
> +		smp_85xx_map_bootpg((u32)(*cpu_rel_addr >> PAGE_SHIFT));

Why is this inside PPC32?

> +		smp_85xx_reset_core(hw_cpu);
> +
> +		/* wait until core is ready... */
> +		n = 0;
> +		while ((in_be32(&epapr->addr_l) != 1) && (++n < 1000))
> +			udelay(100);
> +		if (n > 1000) {

if (n == 1000)

or

if (in_be32(&epapr->addr_l) != 1)

> +			pr_err("timeout waiting for core%d to reset\n",	nr);
> +			ret = -ENOENT;
> +			goto out;
> +		}
> +		/*  clear the acknowledge status */
> +		__secondary_hold_acknowledge = -1;
> +
> +		smp_85xx_unmap_bootpg();
> +	}
> +#endif
> +	out_be32(&epapr->addr_l, __pa(__early_start));
>  
>  	if (!ioremappable)
> -		flush_dcache_range((ulong)bptr_vaddr,
> -				(ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
> +		flush_dcache_range((ulong)epapr,
> +				(ulong)epapr + sizeof(struct epapr_entry));
>  
>  	/* Wait a bit for the CPU to ack. */
> -	while ((__secondary_hold_acknowledge != nr) && (++n < 1000))
> +	n = 0;
> +	while ((__secondary_hold_acknowledge != hw_cpu) && (++n < 1000))
>  		mdelay(1);
> +	if (n > 1000) {

if (n == 1000)

or

if (__secondary_hold_acknowledge != hw_cpu)

> +		pr_err("timeout waiting for core%d to ack\n", nr);

pr_err("%s: timeout waiting for core %d to ack\n", __func__, nr);

Likewise elsewhere.  Maybe also/instead mention hw_cpu.

> +		ret = -ENOENT;
> +		goto out;
> +	}
> +out:
>  #else
>  	smp_generic_kick_cpu(nr);
>  
> -	out_be64((u64 *)(bptr_vaddr + BOOT_ENTRY_ADDR_UPPER),
> +	out_be64((u64 *)(&epapr->addr_h),
>  		__pa((u64)*((unsigned long long *) generic_secondary_smp_init)));
>  
>  	if (!ioremappable)
> -		flush_dcache_range((ulong)bptr_vaddr,
> -				(ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
> +		flush_dcache_range((ulong)epapr,
> +				(ulong)epapr + sizeof(struct epapr_entry));

We don't wait for the core to come up on 64-bit?

> @@ -228,14 +376,18 @@ void __init mpc85xx_smp_init(void)
>  {
>  	struct device_node *np;
>  
> -	smp_85xx_ops.setup_cpu = smp_85xx_setup_cpu;
> -
>  	np = of_find_node_by_type(NULL, "open-pic");
>  	if (np) {
>  		smp_85xx_ops.probe = smp_mpic_probe;
>  		smp_85xx_ops.message_pass = smp_mpic_message_pass;
>  	}
>  
> +	/* Check if the chip is based on CoreNet platform. */
> +	is_corenet = 0;
> +	np = of_find_compatible_node(NULL, NULL, "fsl,qoriq-device-config-1.0");
> +	if (np)
> +		is_corenet = 1;

Please also check for the non-corenet guts node.  If you don't find
either, disable the mechanism -- you're probably running under a hypervisor.

-Scott

RE: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support

[Li Yang-R58472]

>Subject: Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
>
>On 11/04/2011 07:31 AM, Zhao Chenhui wrote:
>> From: Li Yang <leoli@freescale.com>
>>
>> Add support to disable and re-enable individual cores at runtime
>> on MPC85xx/QorIQ SMP machines. Currently support e500 core.
>>
>> MPC85xx machines use ePAPR spin-table in boot page for CPU kick-off.
>> This patch uses the boot page from bootloader to boot core at runtime.
>> It supports 32-bit and 36-bit physical address.
>
>Note that there is no guarantee that the bootloader can handle you
>resetting a core.  In ePAPR the spin table is a one-time release
>mechanism, not a core reset mechanism.  If this has a U-Boot dependency,
>document that.

Actually we suggested to add a spin table in kernel so that we won't have t=
he dependency on u-boot.  But there seems to be some opposite opinion in th=
e internal discussion.  I personally prefer to remove the u-boot dependency=
 and add the mechanism in kernel.

>
>>  #ifdef CONFIG_SMP
>>  /* When we get here, r24 needs to hold the CPU # */
>>  	.globl __secondary_start
>> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
>> index 7bf2187..12a54f0 100644
>> --- a/arch/powerpc/kernel/smp.c
>> +++ b/arch/powerpc/kernel/smp.c
>> @@ -381,8 +381,14 @@ void generic_cpu_die(unsigned int cpu)
>>
>>  	for (i =3D 0; i < 100; i++) {
>>  		smp_rmb();
>> -		if (per_cpu(cpu_state, cpu) =3D=3D CPU_DEAD)
>> +		if (per_cpu(cpu_state, cpu) =3D=3D CPU_DEAD) {
>> +			/*
>> +			 * After another core sets cpu_state to CPU_DEAD,
>> +			 * it needs some time to die.
>> +			 */
>> +			msleep(10);
>>  			return;
>> +		}
>>  		msleep(100);
>
>It would be better to do this as a call into platform-specific code than
>can check registers to determine whether the core has checked out (in
>our case, whether it has entered nap) -- or to do a suitable delay for
>that platform if this isn't possible.

It will be easier if we can add a platform specific cpu_die instead of usin=
g the generic one?

>
>> diff --git a/arch/powerpc/platforms/85xx/smp.c
>b/arch/powerpc/platforms/85xx/smp.c
>> index 9b0de9c..5a54fc1 100644
>> --- a/arch/powerpc/platforms/85xx/smp.c
>> +++ b/arch/powerpc/platforms/85xx/smp.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/of.h>
>>  #include <linux/kexec.h>
>>  #include <linux/highmem.h>
>> +#include <linux/cpu.h>
>>
>>  #include <asm/machdep.h>
>>  #include <asm/pgtable.h>
>> @@ -30,26 +31,141 @@
>>
>>  extern void __early_start(void);
>>
>> -#define BOOT_ENTRY_ADDR_UPPER	0
>> -#define BOOT_ENTRY_ADDR_LOWER	1
>> -#define BOOT_ENTRY_R3_UPPER	2
>> -#define BOOT_ENTRY_R3_LOWER	3
>> -#define BOOT_ENTRY_RESV		4
>> -#define BOOT_ENTRY_PIR		5
>> -#define BOOT_ENTRY_R6_UPPER	6
>> -#define BOOT_ENTRY_R6_LOWER	7
>> -#define NUM_BOOT_ENTRY		8
>> -#define SIZE_BOOT_ENTRY		(NUM_BOOT_ENTRY * sizeof(u32))
>> -
>> -static int __init
>> -smp_85xx_kick_cpu(int nr)
>> +#define MPC85xx_BPTR_OFF		0x00020
>> +#define MPC85xx_BPTR_EN			0x80000000
>> +#define MPC85xx_BPTR_BOOT_PAGE_MASK	0x00ffffff
>> +#define MPC85xx_BRR_OFF			0xe0e4
>> +#define MPC85xx_ECM_EEBPCR_OFF		0x01010
>> +#define MPC85xx_PIC_PIR_OFF		0x41090
>> +
>> +struct epapr_entry {
>
>ePAPR is more than just the spin table.  Call it something like
>epapr_spin_table.
>
>> +	u32	addr_h;
>> +	u32	addr_l;
>> +	u32	r3_h;
>> +	u32	r3_l;
>> +	u32	reserved;
>> +	u32	pir;
>> +	u32	r6_h;
>> +	u32	r6_l;
>> +};
>
>Get rid of r6, it is not part of the ePAPR spin table.

Agree.

>
>> +static int is_corenet;
>> +static void __cpuinit smp_85xx_setup_cpu(int cpu_nr);
>> +
>> +#if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PPC32)
>
>Why PPC32?

Not sure if it is the same for e5500.  E5500 support will be verified later=
.

>
>> +extern void flush_disable_L1(void);
>
>If this isn't already in a header file, put it in one.
>
>> +static void __cpuinit smp_85xx_mach_cpu_die(void)
>> +{
>> +	unsigned int cpu =3D smp_processor_id();
>> +	register u32 tmp;
>> +
>> +	local_irq_disable();
>> +	idle_task_exit();
>> +	generic_set_cpu_dead(cpu);
>> +	smp_wmb();
>> +
>> +	mtspr(SPRN_TSR, TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS);
>> +	mtspr(SPRN_TCR, 0);
>
>If clearing TSR matters at all (I'm not sure that it does), first clear
>TCR, then TSR.
>
>> +	flush_disable_L1();
>
>You'll also need to take down L2 on e500mc.

Right.  E500mc support is beyond this patch series.  We will work on it aft=
er the e500v2 support is finalized.

>
>> +	tmp =3D 0;
>> +	if (cpu_has_feature(CPU_FTR_CAN_NAP))
>> +		tmp =3D HID0_NAP;
>> +	else if (cpu_has_feature(CPU_FTR_CAN_DOZE))
>> +		tmp =3D HID0_DOZE;
>
>Those FTR bits are for what we can do in idle, and can be cleared if the
>user sets CONFIG_BDI_SWITCH.

It is powersave_nap variable shows what we can do in idle.  I think it's co=
rrect to use the CPU_FTR_CAN_* macro as CPU capability.

>
>On 85xx we always want to nap here, and at least on e500mc it seems to
>be mandatory.  From the p5020 RM description of PIR:
>
>> For proper system operation, a core should be reset in this way only if
>the core is already in nap or sleep
>> state. Because a core in either state cannot perform the necessary write
>to cause a hard reset, a core cannot
>> put itself into hard reset.
>
>Note that on e500mc we don't use HID0/MSR_WE to enter nap, we need to
>hit the CCSR register.  And unless you can somehow guarantee that only
>one core at a time is doing this, we'll need some oher core to actually
>place us in nap (since once we enter nap we're not running so can't
>release a lock).
>
>> +	if (tmp) {
>> +		tmp |=3D mfspr(SPRN_HID0) & ~(HID0_DOZE|HID0_NAP|HID0_SLEEP);
>> +
>> +		smp_mb();
>
>smp_mb()?  This is always SMP...  It looks like you meant some specific
>sync instruction as part of an architected sequence, so just use that.
>
>> +		isync();
>> +		mtspr(SPRN_HID0, tmp);
>> +		isync();
>> +
>> +		tmp =3D mfmsr();
>> +		tmp |=3D MSR_WE;
>> +		smp_mb();
>> +		mtmsr(tmp);
>> +		isync();
>> +	}
>> +
>> +	for (;;);
>> +}
>> +
>> +static void __cpuinit smp_85xx_reset_core(int nr)
>> +{
>> +	__iomem u32 *vaddr, *pir_vaddr;
>> +	u32 val, cpu_mask;
>> +
>> +	/* If CoreNet platform, use BRR as release register. */
>> +	if (is_corenet) {
>> +		cpu_mask =3D 1 << nr;
>> +		vaddr =3D ioremap(get_immrbase() + MPC85xx_BRR_OFF, 4);
>> +	} else {
>> +		cpu_mask =3D 1 << (24 + nr);
>> +		vaddr =3D ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4);
>> +	}
>
>Please use the device tree node, not get_immrbase().

The get_immrbase() implementation uses the device tree node.

>
>> +	val =3D in_be32(vaddr);
>> +	if (!(val & cpu_mask)) {
>> +		out_be32(vaddr, val | cpu_mask);
>> +	} else {
>> +		/* reset core */
>> +		pir_vaddr =3D ioremap(get_immrbase() + MPC85xx_PIC_PIR_OFF, 4);
>> +		val =3D in_be32(pir_vaddr);
>> +		/* reset assert */
>> +		val |=3D (1 << nr);
>> +		out_be32(pir_vaddr, val);
>
>Use setbits32().
>
>> +		val =3D in_be32(pir_vaddr);
>> +		val &=3D ~(1 << nr);
>> +		/* reset negate */
>> +		out_be32(pir_vaddr, val);
>
>clrbits32().
>
>Is there any amount of time we need to keep the reset pin asserted?

We don't know, but it's already working without any wait.

>
>> +		iounmap(pir_vaddr);
>> +	}
>> +	iounmap(vaddr);
>> +}
>> +
>> +static int __cpuinit smp_85xx_map_bootpg(u32 page)
>> +{
>> +	__iomem u32 *bootpg_ptr;
>> +	u32 bptr;
>> +
>> +	/* Get the BPTR */
>> +	bootpg_ptr =3D ioremap(get_immrbase() + MPC85xx_BPTR_OFF, 4);
>> +
>> +	/* Set the BPTR to the secondary boot page */
>> +	bptr =3D MPC85xx_BPTR_EN | (page & MPC85xx_BPTR_BOOT_PAGE_MASK);
>> +	out_be32(bootpg_ptr, bptr);
>> +
>> +	iounmap(bootpg_ptr);
>> +	return 0;
>> +}
>
>Shouldn't the boot page already be set by U-Boot?

The register should be lost after a deep sleep.   Better to do it again.

>
>> +static int __cpuinit smp_85xx_kick_cpu(int nr)
>>  {
>>  	unsigned long flags;
>>  	const u64 *cpu_rel_addr;
>> -	__iomem u32 *bptr_vaddr;
>> +	__iomem struct epapr_entry *epapr;
>>  	struct device_node *np;
>> -	int n =3D 0;
>> +	int n =3D 0, hw_cpu =3D get_hard_smp_processor_id(nr);
>>  	int ioremappable;
>> +	int ret =3D 0;
>>
>>  	WARN_ON (nr < 0 || nr >=3D NR_CPUS);
>>
>> @@ -73,46 +189,79 @@ smp_85xx_kick_cpu(int nr)
>>
>>  	/* Map the spin table */
>>  	if (ioremappable)
>> -		bptr_vaddr =3D ioremap(*cpu_rel_addr, SIZE_BOOT_ENTRY);
>> +		epapr =3D ioremap(*cpu_rel_addr, sizeof(struct epapr_entry));
>>  	else
>> -		bptr_vaddr =3D phys_to_virt(*cpu_rel_addr);
>> +		epapr =3D phys_to_virt(*cpu_rel_addr);
>>
>>  	local_irq_save(flags);
>>
>> -	out_be32(bptr_vaddr + BOOT_ENTRY_PIR, nr);
>> +	out_be32(&epapr->pir, hw_cpu);
>>  #ifdef CONFIG_PPC32
>> -	out_be32(bptr_vaddr + BOOT_ENTRY_ADDR_LOWER, __pa(__early_start));
>> +#ifdef CONFIG_HOTPLUG_CPU
>> +	if (system_state =3D=3D SYSTEM_RUNNING) {
>> +		out_be32(&epapr->addr_l, 0);
>> +		smp_85xx_map_bootpg((u32)(*cpu_rel_addr >> PAGE_SHIFT));
>
>Why is this inside PPC32?

Not tested on 64-bit yet.  Might require a different implementation on PPC6=
4.

>
>> +		smp_85xx_reset_core(hw_cpu);
>> +
>> +		/* wait until core is ready... */
>> +		n =3D 0;
>> +		while ((in_be32(&epapr->addr_l) !=3D 1) && (++n < 1000))
>> +			udelay(100);
>> +		if (n > 1000) {
>
>if (n =3D=3D 1000)
>
>or
>
>if (in_be32(&epapr->addr_l) !=3D 1)
>
>> +			pr_err("timeout waiting for core%d to reset\n",	nr);
>> +			ret =3D -ENOENT;
>> +			goto out;
>> +		}
>> +		/*  clear the acknowledge status */
>> +		__secondary_hold_acknowledge =3D -1;
>> +
>> +		smp_85xx_unmap_bootpg();
>> +	}
>> +#endif
>> +	out_be32(&epapr->addr_l, __pa(__early_start));
>>
>>  	if (!ioremappable)
>> -		flush_dcache_range((ulong)bptr_vaddr,
>> -				(ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
>> +		flush_dcache_range((ulong)epapr,
>> +				(ulong)epapr + sizeof(struct epapr_entry));
>>
>>  	/* Wait a bit for the CPU to ack. */
>> -	while ((__secondary_hold_acknowledge !=3D nr) && (++n < 1000))
>> +	n =3D 0;
>> +	while ((__secondary_hold_acknowledge !=3D hw_cpu) && (++n < 1000))
>>  		mdelay(1);
>> +	if (n > 1000) {
>
>if (n =3D=3D 1000)
>
>or
>
>if (__secondary_hold_acknowledge !=3D hw_cpu)
>
>> +		pr_err("timeout waiting for core%d to ack\n", nr);
>
>pr_err("%s: timeout waiting for core %d to ack\n", __func__, nr);
>
>Likewise elsewhere.  Maybe also/instead mention hw_cpu.
>
>> +		ret =3D -ENOENT;
>> +		goto out;
>> +	}
>> +out:
>>  #else
>>  	smp_generic_kick_cpu(nr);
>>
>> -	out_be64((u64 *)(bptr_vaddr + BOOT_ENTRY_ADDR_UPPER),
>> +	out_be64((u64 *)(&epapr->addr_h),
>>  		__pa((u64)*((unsigned long long *)
>generic_secondary_smp_init)));
>>
>>  	if (!ioremappable)
>> -		flush_dcache_range((ulong)bptr_vaddr,
>> -				(ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
>> +		flush_dcache_range((ulong)epapr,
>> +				(ulong)epapr + sizeof(struct epapr_entry));
>
>We don't wait for the core to come up on 64-bit?

Not sure about it.  But at least the normal boot up should be tested on P50=
20, right?

>
>> @@ -228,14 +376,18 @@ void __init mpc85xx_smp_init(void)
>>  {
>>  	struct device_node *np;
>>
>> -	smp_85xx_ops.setup_cpu =3D smp_85xx_setup_cpu;
>> -
>>  	np =3D of_find_node_by_type(NULL, "open-pic");
>>  	if (np) {
>>  		smp_85xx_ops.probe =3D smp_mpic_probe;
>>  		smp_85xx_ops.message_pass =3D smp_mpic_message_pass;
>>  	}
>>
>> +	/* Check if the chip is based on CoreNet platform. */
>> +	is_corenet =3D 0;
>> +	np =3D of_find_compatible_node(NULL, NULL, "fsl,qoriq-device-config-
>1.0");
>> +	if (np)
>> +		is_corenet =3D 1;
>
>Please also check for the non-corenet guts node.  If you don't find
>either, disable the mechanism -- you're probably running under a
>hypervisor.

Ok.

- Leo

Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support

[Scott Wood]

On 11/08/2011 04:05 AM, Li Yang-R58472 wrote:
>> Subject: Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
>>
>> On 11/04/2011 07:31 AM, Zhao Chenhui wrote:
>>> +static int is_corenet;
>>> +static void __cpuinit smp_85xx_setup_cpu(int cpu_nr);
>>> +
>>> +#if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PPC32)
>>
>> Why PPC32?
> 
> Not sure if it is the same for e5500.  E5500 support will be verified later.

It's better to have 64-bit silently do nothing here?

>>> +	flush_disable_L1();
>>
>> You'll also need to take down L2 on e500mc.
> 
> Right.  E500mc support is beyond this patch series.  We will work on it after the e500v2 support is finalized.

I saw some support with "is_corenet"...  If we don't support e500mc,
make sure the code doesn't try to run on e500mc.

This isn't an e500v2-only BSP you're putting the code into. :-)

>>> +	tmp = 0;
>>> +	if (cpu_has_feature(CPU_FTR_CAN_NAP))
>>> +		tmp = HID0_NAP;
>>> +	else if (cpu_has_feature(CPU_FTR_CAN_DOZE))
>>> +		tmp = HID0_DOZE;
>>
>> Those FTR bits are for what we can do in idle, and can be cleared if the
>> user sets CONFIG_BDI_SWITCH.
> 
> It is powersave_nap variable shows what we can do in idle.

No, that shows what the user wants to do in idle.

> I think it's correct to use the CPU_FTR_CAN_* macro as CPU capability.

This is 85xx-specific code.  We always can, and want to, nap here
(though the way we enter nap will be different on e500mc and up) -- even
if it's broken to use it for idle (such as on p4080, which would miss
doorbells as wake events).

>>> +static void __cpuinit smp_85xx_reset_core(int nr)
>>> +{
>>> +	__iomem u32 *vaddr, *pir_vaddr;
>>> +	u32 val, cpu_mask;
>>> +
>>> +	/* If CoreNet platform, use BRR as release register. */
>>> +	if (is_corenet) {
>>> +		cpu_mask = 1 << nr;
>>> +		vaddr = ioremap(get_immrbase() + MPC85xx_BRR_OFF, 4);
>>> +	} else {
>>> +		cpu_mask = 1 << (24 + nr);
>>> +		vaddr = ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4);
>>> +	}
>>
>> Please use the device tree node, not get_immrbase().
> 
> The get_immrbase() implementation uses the device tree node.

I mean the guts node.  get_immrbase() should be avoided where possible.

Also, some people might care about latency to enter/exit deep sleep.
Searching through the device tree at this point (rather than on init)
slows that down.

>>> +static int __cpuinit smp_85xx_map_bootpg(u32 page)
>>> +{
>>> +	__iomem u32 *bootpg_ptr;
>>> +	u32 bptr;
>>> +
>>> +	/* Get the BPTR */
>>> +	bootpg_ptr = ioremap(get_immrbase() + MPC85xx_BPTR_OFF, 4);
>>> +
>>> +	/* Set the BPTR to the secondary boot page */
>>> +	bptr = MPC85xx_BPTR_EN | (page & MPC85xx_BPTR_BOOT_PAGE_MASK);
>>> +	out_be32(bootpg_ptr, bptr);
>>> +
>>> +	iounmap(bootpg_ptr);
>>> +	return 0;
>>> +}
>>
>> Shouldn't the boot page already be set by U-Boot?
> 
> The register should be lost after a deep sleep.   Better to do it again.

How can it be lost after a deep sleep if we're relying on it to hold our
wakeup code?

It's not "better to do it again" if we're making a bad assumption about
the code and the table being in the same page.

>>>  	local_irq_save(flags);
>>>
>>> -	out_be32(bptr_vaddr + BOOT_ENTRY_PIR, nr);
>>> +	out_be32(&epapr->pir, hw_cpu);
>>>  #ifdef CONFIG_PPC32
>>> -	out_be32(bptr_vaddr + BOOT_ENTRY_ADDR_LOWER, __pa(__early_start));
>>> +#ifdef CONFIG_HOTPLUG_CPU
>>> +	if (system_state == SYSTEM_RUNNING) {
>>> +		out_be32(&epapr->addr_l, 0);
>>> +		smp_85xx_map_bootpg((u32)(*cpu_rel_addr >> PAGE_SHIFT));
>>
>> Why is this inside PPC32?
> 
> Not tested on 64-bit yet.  Might require a different implementation on PPC64.

Please make a reasonable effort to do things in a way that works on
both.  It shouldn't be a big deal to test that it doesn't break booting
on p5020.

>>>  	if (!ioremappable)
>>> -		flush_dcache_range((ulong)bptr_vaddr,
>>> -				(ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
>>> +		flush_dcache_range((ulong)epapr,
>>> +				(ulong)epapr + sizeof(struct epapr_entry));
>>
>> We don't wait for the core to come up on 64-bit?
> 
> Not sure about it.  But at least the normal boot up should be tested on P5020, right?

Well, that's a special case in that it only has one secondary. :-)

Or we could be getting lucky with timing.  It's not a new issue with
this patch, it just looks odd.

-Scott

RE: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support

[Li Yang-R58472]

>-----Original Message-----
>From: Wood Scott-B07421
>Sent: Wednesday, November 09, 2011 4:58 AM
>To: Li Yang-R58472
>Cc: Wood Scott-B07421; Zhao Chenhui-B35336; linuxppc-dev@lists.ozlabs.org
>Subject: Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
>
>On 11/08/2011 04:05 AM, Li Yang-R58472 wrote:
>>> Subject: Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
>>>
>>> On 11/04/2011 07:31 AM, Zhao Chenhui wrote:
>>>> +static int is_corenet;
>>>> +static void __cpuinit smp_85xx_setup_cpu(int cpu_nr);
>>>> +
>>>> +#if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PPC32)
>>>
>>> Why PPC32?
>>
>> Not sure if it is the same for e5500.  E5500 support will be verified
>later.
>
>It's better to have 64-bit silently do nothing here?
>
>>>> +	flush_disable_L1();
>>>
>>> You'll also need to take down L2 on e500mc.
>>
>> Right.  E500mc support is beyond this patch series.  We will work on it
>after the e500v2 support is finalized.
>
>I saw some support with "is_corenet"...  If we don't support e500mc, make
>sure the code doesn't try to run on e500mc.

The is_corenet code is just a start of the e500mc support and is far from c=
omplete.

>
>This isn't an e500v2-only BSP you're putting the code into. :-)

Yes, I know.  But it will take quite some time to perfect the support for d=
ifferent type of cores.  I'd like to make the effort into stages.  We want =
to push the e500v2 support in first and add support to newer cores later so=
 that we don't need to re-spin the patches again and again.  And the upstre=
am kernel can get the PM functionality at least for e500v2 earlier.  Anyway=
, we need to update the title of the patch to be more specific on e500v2.

>
>>>> +	tmp =3D 0;
>>>> +	if (cpu_has_feature(CPU_FTR_CAN_NAP))
>>>> +		tmp =3D HID0_NAP;
>>>> +	else if (cpu_has_feature(CPU_FTR_CAN_DOZE))
>>>> +		tmp =3D HID0_DOZE;
>>>
>>> Those FTR bits are for what we can do in idle, and can be cleared if
>>> the user sets CONFIG_BDI_SWITCH.
>>
>> It is powersave_nap variable shows what we can do in idle.
>
>No, that shows what the user wants to do in idle.
>
>> I think it's correct to use the CPU_FTR_CAN_* macro as CPU capability.
>
>This is 85xx-specific code.  We always can, and want to, nap here (though
>the way we enter nap will be different on e500mc and up) -- even if it's
>broken to use it for idle (such as on p4080, which would miss doorbells as
>wake events).

Ok.  Will stick to Nap.

>
>>>> +static void __cpuinit smp_85xx_reset_core(int nr) {
>>>> +	__iomem u32 *vaddr, *pir_vaddr;
>>>> +	u32 val, cpu_mask;
>>>> +
>>>> +	/* If CoreNet platform, use BRR as release register. */
>>>> +	if (is_corenet) {
>>>> +		cpu_mask =3D 1 << nr;
>>>> +		vaddr =3D ioremap(get_immrbase() + MPC85xx_BRR_OFF, 4);
>>>> +	} else {
>>>> +		cpu_mask =3D 1 << (24 + nr);
>>>> +		vaddr =3D ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4);
>>>> +	}
>>>
>>> Please use the device tree node, not get_immrbase().
>>
>> The get_immrbase() implementation uses the device tree node.
>
>I mean the guts node.  get_immrbase() should be avoided where possible.

Ok.  I got your point to use offset from guts.

>
>Also, some people might care about latency to enter/exit deep sleep.
>Searching through the device tree at this point (rather than on init)
>slows that down.

Actually the get_immrbase() is smarter than you thought. :) It only search =
the device tree on first call and returns the saved value on follow up call=
s.

>
>>>> +static int __cpuinit smp_85xx_map_bootpg(u32 page) {
>>>> +	__iomem u32 *bootpg_ptr;
>>>> +	u32 bptr;
>>>> +
>>>> +	/* Get the BPTR */
>>>> +	bootpg_ptr =3D ioremap(get_immrbase() + MPC85xx_BPTR_OFF, 4);
>>>> +
>>>> +	/* Set the BPTR to the secondary boot page */
>>>> +	bptr =3D MPC85xx_BPTR_EN | (page & MPC85xx_BPTR_BOOT_PAGE_MASK);
>>>> +	out_be32(bootpg_ptr, bptr);
>>>> +
>>>> +	iounmap(bootpg_ptr);
>>>> +	return 0;
>>>> +}
>>>
>>> Shouldn't the boot page already be set by U-Boot?
>>
>> The register should be lost after a deep sleep.   Better to do it again.
>
>How can it be lost after a deep sleep if we're relying on it to hold our
>wakeup code?

In order to wake up from deep sleep, the boot page has been set to the deep=
 sleep restoration function.  We need to set it back to the bootpage from u=
-boot.

>
>It's not "better to do it again" if we're making a bad assumption about
>the code and the table being in the same page.

Currently we are reusing the whole boot page including the spin-table from =
u-boot.  Are you suggesting to move just the boot page to kernel?

>
>>>>  	local_irq_save(flags);
>>>>
>>>> -	out_be32(bptr_vaddr + BOOT_ENTRY_PIR, nr);
>>>> +	out_be32(&epapr->pir, hw_cpu);
>>>>  #ifdef CONFIG_PPC32
>>>> -	out_be32(bptr_vaddr + BOOT_ENTRY_ADDR_LOWER, __pa(__early_start));
>>>> +#ifdef CONFIG_HOTPLUG_CPU
>>>> +	if (system_state =3D=3D SYSTEM_RUNNING) {
>>>> +		out_be32(&epapr->addr_l, 0);
>>>> +		smp_85xx_map_bootpg((u32)(*cpu_rel_addr >> PAGE_SHIFT));
>>>
>>> Why is this inside PPC32?
>>
>> Not tested on 64-bit yet.  Might require a different implementation on
>PPC64.
>
>Please make a reasonable effort to do things in a way that works on both.
>It shouldn't be a big deal to test that it doesn't break booting on p5020.

Will do.  But in stages.

>
>>>>  	if (!ioremappable)
>>>> -		flush_dcache_range((ulong)bptr_vaddr,
>>>> -				(ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
>>>> +		flush_dcache_range((ulong)epapr,
>>>> +				(ulong)epapr + sizeof(struct epapr_entry));
>>>
>>> We don't wait for the core to come up on 64-bit?
>>
>> Not sure about it.  But at least the normal boot up should be tested on
>P5020, right?
>
>Well, that's a special case in that it only has one secondary. :-)
>
>Or we could be getting lucky with timing.  It's not a new issue with this
>patch, it just looks odd.

We will look into it more when doing the e5500 support.

- Leo

Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support

[Scott Wood]

On Wed, Nov 09, 2011 at 02:31:23AM -0600, Li Yang-R58472 wrote:
> >-----Original Message-----
> >From: Wood Scott-B07421
> >Sent: Wednesday, November 09, 2011 4:58 AM
> >To: Li Yang-R58472
> >Cc: Wood Scott-B07421; Zhao Chenhui-B35336; linuxppc-dev@lists.ozlabs.org
> >Subject: Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
> >
> >On 11/08/2011 04:05 AM, Li Yang-R58472 wrote:
> >>> Subject: Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
> >>>
> >>>> +	flush_disable_L1();
> >>>
> >>> You'll also need to take down L2 on e500mc.
> >>
> >> Right.  E500mc support is beyond this patch series.  We will work on it
> >after the e500v2 support is finalized.
> >
> >I saw some support with "is_corenet"...  If we don't support e500mc, make
> >sure the code doesn't try to run on e500mc.
> 
> The is_corenet code is just a start of the e500mc support and is far from complete.
> 
> >
> >This isn't an e500v2-only BSP you're putting the code into. :-)
> 
> Yes, I know.  But it will take quite some time to perfect the support
> for different type of cores. 

That's fine, I'm just asking for it to be clearer that e500mc/corenet is TODO,
and ideally for the code to reject hotplug if is_corenet is set, until it's
supported.

> >>>> +static int __cpuinit smp_85xx_map_bootpg(u32 page) {
> >>>> +	__iomem u32 *bootpg_ptr;
> >>>> +	u32 bptr;
> >>>> +
> >>>> +	/* Get the BPTR */
> >>>> +	bootpg_ptr = ioremap(get_immrbase() + MPC85xx_BPTR_OFF, 4);
> >>>> +
> >>>> +	/* Set the BPTR to the secondary boot page */
> >>>> +	bptr = MPC85xx_BPTR_EN | (page & MPC85xx_BPTR_BOOT_PAGE_MASK);
> >>>> +	out_be32(bootpg_ptr, bptr);
> >>>> +
> >>>> +	iounmap(bootpg_ptr);
> >>>> +	return 0;
> >>>> +}
> >>>
> >>> Shouldn't the boot page already be set by U-Boot?
> >>
> >> The register should be lost after a deep sleep.   Better to do it again.
> >
> >How can it be lost after a deep sleep if we're relying on it to hold our
> >wakeup code?
> 
> In order to wake up from deep sleep, the boot page has been set to the
> deep sleep restoration function.  We need to set it back to the
> bootpage from u-boot.

Ah.  That deserves a code comment.

> >It's not "better to do it again" if we're making a bad assumption about
> >the code and the table being in the same page.
> 
> Currently we are reusing the whole boot page including the spin-table
> from u-boot.  Are you suggesting to move just the boot page to kernel?

Just that it might be better to make sure that we put the same value in
BPTR that was in it when Linux booted, rather than assume that the page
that holds the spin table itself is the right one.

-Scott

Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support

[Benjamin Herrenschmidt]

On Fri, 2011-11-04 at 20:31 +0800, Zhao Chenhui wrote:

>  #ifdef CONFIG_SMP
>  /* When we get here, r24 needs to hold the CPU # */
>  	.globl __secondary_start
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 7bf2187..12a54f0 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -381,8 +381,14 @@ void generic_cpu_die(unsigned int cpu)
>  
>  	for (i = 0; i < 100; i++) {
>  		smp_rmb();
> -		if (per_cpu(cpu_state, cpu) == CPU_DEAD)
> +		if (per_cpu(cpu_state, cpu) == CPU_DEAD) {
> +			/*
> +			 * After another core sets cpu_state to CPU_DEAD,
> +			 * it needs some time to die.
> +			 */
> +			msleep(10);
>  			return;
> +		}
>  		msleep(100);
>  	}
>  	printk(KERN_ERR "CPU%d didn't die...\n", cpu);

I don't really see why you need to wait. This loop is about waiting for
the CPU to mark itself DEAD, not necessarily to be physically powered
off.

> +struct epapr_entry {
> +	u32	addr_h;
> +	u32	addr_l;
> +	u32	r3_h;
> +	u32	r3_l;
> +	u32	reserved;
> +	u32	pir;
> +	u32	r6_h;
> +	u32	r6_l;
> +};

This should probably be in a generic location.

> +static int is_corenet;

This global is a bit gross...

 ...

> +
> +static void __cpuinit smp_85xx_reset_core(int nr)
> +{
> +	__iomem u32 *vaddr, *pir_vaddr;
> +	u32 val, cpu_mask;
> +
> +	/* If CoreNet platform, use BRR as release register. */
> +	if (is_corenet) {
> +		cpu_mask = 1 << nr;
> +		vaddr = ioremap(get_immrbase() + MPC85xx_BRR_OFF, 4);
> +	} else {
> +		cpu_mask = 1 << (24 + nr);
> +		vaddr = ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4);
> +	}

Instead, can't you instead have two functions using a common helper and
pick/hook the right one ?

Also in what context is that reset_core() called ? Doing ioremap isn't
something you can do at any random time...

Cheers,
Ben.

RE: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support

[Zhao Chenhui-B35336]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmVuamFtaW4gSGVycmVu
c2NobWlkdCBbbWFpbHRvOmJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZ10NCj4gU2VudDogRnJpZGF5
LCBOb3ZlbWJlciAxMSwgMjAxMSAxMjoyMyBQTQ0KPiBUbzogWmhhbyBDaGVuaHVpLUIzNTMzNg0K
PiBDYzogbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRD
SCAyLzddIHBvd2VycGMvODV4eDogYWRkIEhPVFBMVUdfQ1BVIHN1cHBvcnQNCj4gDQo+IE9uIEZy
aSwgMjAxMS0xMS0wNCBhdCAyMDozMSArMDgwMCwgWmhhbyBDaGVuaHVpIHdyb3RlOg0KPiANCj4g
PiAgI2lmZGVmIENPTkZJR19TTVANCj4gPiAgLyogV2hlbiB3ZSBnZXQgaGVyZSwgcjI0IG5lZWRz
IHRvIGhvbGQgdGhlIENQVSAjICovDQo+ID4gIAkuZ2xvYmwgX19zZWNvbmRhcnlfc3RhcnQNCj4g
PiBkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2tlcm5lbC9zbXAuYyBiL2FyY2gvcG93ZXJwYy9r
ZXJuZWwvc21wLmMNCj4gPiBpbmRleCA3YmYyMTg3Li4xMmE1NGYwIDEwMDY0NA0KPiA+IC0tLSBh
L2FyY2gvcG93ZXJwYy9rZXJuZWwvc21wLmMNCj4gPiArKysgYi9hcmNoL3Bvd2VycGMva2VybmVs
L3NtcC5jDQo+ID4gQEAgLTM4MSw4ICszODEsMTQgQEAgdm9pZCBnZW5lcmljX2NwdV9kaWUodW5z
aWduZWQgaW50IGNwdSkNCj4gPg0KPiA+ICAJZm9yIChpID0gMDsgaSA8IDEwMDsgaSsrKSB7DQo+
ID4gIAkJc21wX3JtYigpOw0KPiA+IC0JCWlmIChwZXJfY3B1KGNwdV9zdGF0ZSwgY3B1KSA9PSBD
UFVfREVBRCkNCj4gPiArCQlpZiAocGVyX2NwdShjcHVfc3RhdGUsIGNwdSkgPT0gQ1BVX0RFQUQp
IHsNCj4gPiArCQkJLyoNCj4gPiArCQkJICogQWZ0ZXIgYW5vdGhlciBjb3JlIHNldHMgY3B1X3N0
YXRlIHRvIENQVV9ERUFELA0KPiA+ICsJCQkgKiBpdCBuZWVkcyBzb21lIHRpbWUgdG8gZGllLg0K
PiA+ICsJCQkgKi8NCj4gPiArCQkJbXNsZWVwKDEwKTsNCj4gPiAgCQkJcmV0dXJuOw0KPiA+ICsJ
CX0NCj4gPiAgCQltc2xlZXAoMTAwKTsNCj4gPiAgCX0NCj4gPiAgCXByaW50ayhLRVJOX0VSUiAi
Q1BVJWQgZGlkbid0IGRpZS4uLlxuIiwgY3B1KTsNCj4gDQo+IEkgZG9uJ3QgcmVhbGx5IHNlZSB3
aHkgeW91IG5lZWQgdG8gd2FpdC4gVGhpcyBsb29wIGlzIGFib3V0IHdhaXRpbmcgZm9yDQo+IHRo
ZSBDUFUgdG8gbWFyayBpdHNlbGYgREVBRCwgbm90IG5lY2Vzc2FyaWx5IHRvIGJlIHBoeXNpY2Fs
bHkgcG93ZXJlZA0KPiBvZmYuDQo+IA0KDQpJZiBJIGRvbid0IGFkZCB0aGlzIGRlbGF5LCB0aGUg
a2VybmVsIGhhbmdzIHNvbWV0aW1lcyB3aGVuIGhvdHBsdWdpbmcgYSBjb3JlLg0KSSB3aWxsIG1v
dmUgdGhpcyBmdW5jdGlvbiB0byA4NXh4L3NtcC5jIGluIHRoZSBuZXh0IHJldmlzaW9uLg0KDQo+
ID4gK3N0cnVjdCBlcGFwcl9lbnRyeSB7DQo+ID4gKwl1MzIJYWRkcl9oOw0KPiA+ICsJdTMyCWFk
ZHJfbDsNCj4gPiArCXUzMglyM19oOw0KPiA+ICsJdTMyCXIzX2w7DQo+ID4gKwl1MzIJcmVzZXJ2
ZWQ7DQo+ID4gKwl1MzIJcGlyOw0KPiA+ICsJdTMyCXI2X2g7DQo+ID4gKwl1MzIJcjZfbDsNCj4g
PiArfTsNCj4gDQo+IFRoaXMgc2hvdWxkIHByb2JhYmx5IGJlIGluIGEgZ2VuZXJpYyBsb2NhdGlv
bi4NCj4gDQo+ID4gK3N0YXRpYyBpbnQgaXNfY29yZW5ldDsNCj4gDQo+IFRoaXMgZ2xvYmFsIGlz
IGEgYml0IGdyb3NzLi4uDQo+IA0KPiAgLi4uDQoNCkFncmVlLiBJIHdpbGwgcmVtb3ZlIGl0Lg0K
DQo+IA0KPiA+ICsNCj4gPiArc3RhdGljIHZvaWQgX19jcHVpbml0IHNtcF84NXh4X3Jlc2V0X2Nv
cmUoaW50IG5yKQ0KPiA+ICt7DQo+ID4gKwlfX2lvbWVtIHUzMiAqdmFkZHIsICpwaXJfdmFkZHI7
DQo+ID4gKwl1MzIgdmFsLCBjcHVfbWFzazsNCj4gPiArDQo+ID4gKwkvKiBJZiBDb3JlTmV0IHBs
YXRmb3JtLCB1c2UgQlJSIGFzIHJlbGVhc2UgcmVnaXN0ZXIuICovDQo+ID4gKwlpZiAoaXNfY29y
ZW5ldCkgew0KPiA+ICsJCWNwdV9tYXNrID0gMSA8PCBucjsNCj4gPiArCQl2YWRkciA9IGlvcmVt
YXAoZ2V0X2ltbXJiYXNlKCkgKyBNUEM4NXh4X0JSUl9PRkYsIDQpOw0KPiA+ICsJfSBlbHNlIHsN
Cj4gPiArCQljcHVfbWFzayA9IDEgPDwgKDI0ICsgbnIpOw0KPiA+ICsJCXZhZGRyID0gaW9yZW1h
cChnZXRfaW1tcmJhc2UoKSArIE1QQzg1eHhfRUNNX0VFQlBDUl9PRkYsIDQpOw0KPiA+ICsJfQ0K
PiANCj4gSW5zdGVhZCwgY2FuJ3QgeW91IGluc3RlYWQgaGF2ZSB0d28gZnVuY3Rpb25zIHVzaW5n
IGEgY29tbW9uIGhlbHBlciBhbmQNCj4gcGljay9ob29rIHRoZSByaWdodCBvbmUgPw0KPiANCj4g
QWxzbyBpbiB3aGF0IGNvbnRleHQgaXMgdGhhdCByZXNldF9jb3JlKCkgY2FsbGVkID8gRG9pbmcg
aW9yZW1hcCBpc24ndA0KPiBzb21ldGhpbmcgeW91IGNhbiBkbyBhdCBhbnkgcmFuZG9tIHRpbWUu
Li4NCj4gDQo+IENoZWVycywNCj4gQmVuLg0KPiANCj4gDQoNClRoYW5rcy4gSSB3aWxsIGZpeCB0
aGVtLg0KDQotY2hlbmh1aQ0KDQo=