mx6: add support of multi-processor command

Signed-off-by Gabriel Huau <[email protected]>

This allows u-boot to load different OS or Bare Metal application on the different cores of the i.MX6DQ.
For example: we can run Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

Hello Gabriel,

Quoted 6 lines · Gabriel Huau

This allows u-boot to load different OS or Bare Metal application on the different cores of the i.MX6DQ.
For example: we can run Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

funny..

arch/arm/cpu/armv7/mx6/mp.c line 105

Quoted 9 lines

arch/arm/cpu/armv7/mx6/mp.c new file +8 -0

+int is_core_valid(unsigned int nr)
+{
+    if (nr < 0 || nr >= CONFIG_NUM_CPUS)
+        return 0;
+
+    return 1;
+}
+

I am not sure if u-boot has rules for it, but personally I would prefer not to test values which are obvious unsigned for negativity, since it causes unnecessary noise when compiling u-boot with W=1, W=2.

Regards,
Jeroen

new file mode 100644
index 0000000..8105fcc

arch/arm/cpu/armv7/mx6/mp.c new file +6 -0

@@ -0,0 +1,131 @@
+/*
+ * (C) Copyright 2014
+ * Gabriel Huau <[email protected]>
+ *
+ * (C) Copyright 2009 Freescale Semiconductor, Inc.
+ *

arch/arm/cpu/armv7/mx6/mp.c new file +74 -0

+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		reg |= SRC_SCR_CORE_1_RESET_MASK;
+		break;
+
+	case 2:
+		reg |= SRC_SCR_CORE_2_RESET_MASK;
+		break;
+
+	case 3:
+		reg |= SRC_SCR_CORE_3_RESET_MASK;
+		break;
+	}
+
+	/* Software reset of the CPU N */
+	__raw_writel(reg, &src->scr);
+
+	return 0;
+}
+
+int cpu_status(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		printf("core 1: %d\n", !!(reg & SRC_SCR_CORE_1_ENABLE_MASK));
+		break;
+
+	case 2:
+		printf("core 2: %d\n", !!(reg & SRC_SCR_CORE_2_ENABLE_MASK));
+		break;
+
+	case 3:
+		printf("core 3: %d\n", !!(reg & SRC_SCR_CORE_3_ENABLE_MASK));
+		break;
+	}
+
+	return 0;
+}
+
+int cpu_release(int nr, int argc, char *const argv[])
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+	uint32_t boot_addr;
+
+	boot_addr = simple_strtoul(argv[0], NULL, 16);
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		__raw_writel(boot_addr, &src->gpr3);
+		reg |= SRC_SCR_CORE_1_ENABLE_MASK;
+		break;
+
+	case 2:
+		__raw_writel(boot_addr, &src->gpr5);
+		reg |= SRC_SCR_CORE_2_ENABLE_MASK;
+		break;
+
+	case 3:
+		__raw_writel(boot_addr, &src->gpr7);
+		reg |= SRC_SCR_CORE_3_ENABLE_MASK;
+		break;
+	}

arch/arm/cpu/armv7/mx6/mp.c new file +31 -0

+		return 0;
+
+	return 1;
+}
+
+int cpu_disable(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		reg &= ~SRC_SCR_CORE_1_ENABLE_MASK;
+		break;
+
+	case 2:
+		reg &= ~SRC_SCR_CORE_2_ENABLE_MASK;
+		break;
+
+	case 3:
+		reg &= ~SRC_SCR_CORE_3_ENABLE_MASK;
+		break;
+	}
+
+	/* Disable the CPU N */
+	__raw_writel(reg, &src->scr);
+
+	return 0;
+}

arch/arm/include/asm/arch-mx6/imx-regs.h +6 -0

index 1f19727..3f8c2ee 100644
@@ -225,6 +225,19 @@
 
 extern void imx_get_mac_from_fuse(int dev_id, unsigned char *mac);
 
+#define SRC_SCR_CORE_1_RESET_OFFSET     14
+#define SRC_SCR_CORE_1_RESET_MASK       (1<<SRC_SCR_CORE_1_RESET_OFFSET)
+#define SRC_SCR_CORE_2_RESET_OFFSET     15
+#define SRC_SCR_CORE_2_RESET_MASK       (1<<SRC_SCR_CORE_2_RESET_OFFSET)
+#define SRC_SCR_CORE_3_RESET_OFFSET     16
+#define SRC_SCR_CORE_3_RESET_MASK       (1<<SRC_SCR_CORE_3_RESET_OFFSET)

Hi Gabriel,

Quoted 23 lines · Gabriel Huau

This allows u-boot to load different OS or Bare Metal application on the different cores of the i.MX6DQ.
For example: we can run Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

---
Changes for v2:
- Add a commit log message to explain the purpose of this patch
Changes for v3:
- Remove unnecessary check for unsigned values when they are negative

  arch/arm/cpu/armv7/mx6/Makefile          |   1 +
  arch/arm/cpu/armv7/mx6/mp.c              | 131 +++++++++++++++++++++++++++++++
  arch/arm/include/asm/arch-mx6/imx-regs.h |  13 +++
  3 files changed, 145 insertions(+)

create mode 100644 arch/arm/cpu/armv7/mx6/mp.c

arch/arm/cpu/armv7/mx6/Makefile +0 -0

index d7285fc..ec08526 100644
@@ -9,3 +9,4 @@
 

arch/arm/cpu/armv7/mx6/mp.c

Quoted 4 lines

arch/arm/cpu/armv7/mx6/Makefile +1 -0

  obj-y	:= soc.o clock.o
  obj-$(CONFIG_SECURE_BOOT)    += hab.o
+obj-$(CONFIG_MP)             += mp.o

arch/arm/cpu/armv7/mx6/mp.c +0 -0

Do you have a user of this code ? I do not see it at the mopment.

Quoted 16 lines

new file mode 100644
index 0000000..8105fcc

arch/arm/cpu/armv7/mx6/mp.c new file +11 -0

@@ -0,0 +1,131 @@
+/*
+ * (C) Copyright 2014
+ * Gabriel Huau <[email protected]>
+ *
+ * (C) Copyright 2009 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>

arch/arm/cpu/armv7/mx6/mp.c line 16

Quoted 5 lines

arch/arm/cpu/armv7/mx6/mp.c new file +5 -0

+#include <asm/errno.h>
+#include <asm/arch/imx-regs.h>
+
+int cpu_reset(int nr)
+{

As cpu__ are quite too generic and could conflict, I suggest to use more specific names indicating that you are enabling/disabling a core on the cpu. Something like imx_core_reset() or whatever you choose.

Quoted 74 lines

arch/arm/cpu/armv7/mx6/mp.c new file +74 -0

+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		reg |= SRC_SCR_CORE_1_RESET_MASK;
+		break;
+
+	case 2:
+		reg |= SRC_SCR_CORE_2_RESET_MASK;
+		break;
+
+	case 3:
+		reg |= SRC_SCR_CORE_3_RESET_MASK;
+		break;
+	}
+
+	/* Software reset of the CPU N */
+	__raw_writel(reg, &src->scr);
+
+	return 0;
+}
+
+int cpu_status(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		printf("core 1: %d\n", !!(reg & SRC_SCR_CORE_1_ENABLE_MASK));
+		break;
+
+	case 2:
+		printf("core 2: %d\n", !!(reg & SRC_SCR_CORE_2_ENABLE_MASK));
+		break;
+
+	case 3:
+		printf("core 3: %d\n", !!(reg & SRC_SCR_CORE_3_ENABLE_MASK));
+		break;
+	}
+
+	return 0;
+}
+
+int cpu_release(int nr, int argc, char *const argv[])
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+	uint32_t boot_addr;
+
+	boot_addr = simple_strtoul(argv[0], NULL, 16);
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		__raw_writel(boot_addr, &src->gpr3);
+		reg |= SRC_SCR_CORE_1_ENABLE_MASK;
+		break;
+
+	case 2:
+		__raw_writel(boot_addr, &src->gpr5);
+		reg |= SRC_SCR_CORE_2_ENABLE_MASK;
+		break;
+
+	case 3:
+		__raw_writel(boot_addr, &src->gpr7);
+		reg |= SRC_SCR_CORE_3_ENABLE_MASK;
+		break;
+	}

arch/arm/cpu/armv7/mx6/mp.c line 100

Quoted 10 lines

arch/arm/cpu/armv7/mx6/mp.c new file +10 -0

+
+	/* CPU N is ready to start */
+	__raw_writel(reg, &src->scr);
+
+	return 0;
+}
+
+int is_core_valid(unsigned int nr)
+{
+	if (nr >= CONFIG_NUM_CPUS)

And where is defined CONFIG_NUM_CPUS ?

Why do we have to configure it ? The number is related to the running SOC and cannot be changed.

Quoted 45 lines

arch/arm/cpu/armv7/mx6/mp.c new file +31 -0

+		return 0;
+
+	return 1;
+}
+
+int cpu_disable(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		reg &= ~SRC_SCR_CORE_1_ENABLE_MASK;
+		break;
+
+	case 2:
+		reg &= ~SRC_SCR_CORE_2_ENABLE_MASK;
+		break;
+
+	case 3:
+		reg &= ~SRC_SCR_CORE_3_ENABLE_MASK;
+		break;
+	}
+
+	/* Disable the CPU N */
+	__raw_writel(reg, &src->scr);
+
+	return 0;
+}

arch/arm/include/asm/arch-mx6/imx-regs.h +6 -0

index 1f19727..3f8c2ee 100644
@@ -225,6 +225,19 @@
 
  extern void imx_get_mac_from_fuse(int dev_id, unsigned char *mac);
 
+#define SRC_SCR_CORE_1_RESET_OFFSET     14
+#define SRC_SCR_CORE_1_RESET_MASK       (1<<SRC_SCR_CORE_1_RESET_OFFSET)
+#define SRC_SCR_CORE_2_RESET_OFFSET     15
+#define SRC_SCR_CORE_2_RESET_MASK       (1<<SRC_SCR_CORE_2_RESET_OFFSET)
+#define SRC_SCR_CORE_3_RESET_OFFSET     16
+#define SRC_SCR_CORE_3_RESET_MASK       (1<<SRC_SCR_CORE_3_RESET_OFFSET)

arch/arm/include/asm/arch-mx6/imx-regs.h line 243

Quoted 10 lines

arch/arm/include/asm/arch-mx6/imx-regs.h +7 -0

+#define SRC_SCR_CORE_1_ENABLE_OFFSET    22
+#define SRC_SCR_CORE_1_ENABLE_MASK      (1<<SRC_SCR_CORE_1_ENABLE_OFFSET)
+#define SRC_SCR_CORE_2_ENABLE_OFFSET    23
+#define SRC_SCR_CORE_2_ENABLE_MASK      (1<<SRC_SCR_CORE_2_ENABLE_OFFSET)
+#define SRC_SCR_CORE_3_ENABLE_OFFSET    24
+#define SRC_SCR_CORE_3_ENABLE_MASK      (1<<SRC_SCR_CORE_3_ENABLE_OFFSET)
+
  /* System Reset Controller (SRC) */
  struct src {
  	u32	scr;

Best regards,
Stefano Babic

This allows u-boot to load different OS or Bare Metal application on the different cores of the i.MX6DQ.
For example: we can run Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

---
Changes for v2:
- Add a commit log message to explain the purpose of this patch
Changes for v3:
- Remove unnecessary check for unsigned values when they are negative
Changes for v4:
- Add CONFIG_MP to the common mx6 configuration
- Get the number of CPUs dynamically instead of using a macro

 arch/arm/cpu/armv7/mx6/Makefile           |   1 +
 arch/arm/cpu/armv7/mx6/mp.c               | 134 ++++++++++++++++++++++++++++++
 arch/arm/cpu/armv7/mx6/soc.c              |   6 ++
 arch/arm/include/asm/arch-mx6/imx-regs.h  |  13 +++
 arch/arm/include/asm/arch-mx6/sys_proto.h |   2 +
 include/configs/mx6_common.h              |   2 +
 6 files changed, 158 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/mx6/mp.c

This allows u-boot to load different OS or Bare Metal application on the different cores of the i.MX6DQ.
For example: we can run Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

arch/arm/cpu/armv7/mx6/mp.c new file +10 -0

+int cpu_reset(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		reg |= SRC_SCR_CORE_1_RESET_MASK;

arch/arm/cpu/armv7/mx6/mp.c new file +10 -0

+int cpu_status(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		printf("core 1: %d\n", !!(reg & SRC_SCR_CORE_1_ENABLE_MASK));

arch/arm/cpu/armv7/mx6/mp.c new file +9 -0

+	switch (nr) {
+	case 1:
+		__raw_writel(boot_addr, &src->gpr3);
+		reg |= SRC_SCR_CORE_1_ENABLE_MASK;
+		break;
+
+	case 2:
+		__raw_writel(boot_addr, &src->gpr5);
+		reg |= SRC_SCR_CORE_2_ENABLE_MASK;

arch/arm/cpu/armv7/mx6/mp.c new file +8 -0

+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		reg &= ~SRC_SCR_CORE_1_ENABLE_MASK;
+		break;
+
+	case 2:

Dear Gabriel Huau,

Quoted 3 lines · In message <[email protected]> you…

This allows u-boot to load different OS or Bare Metal application on the different cores of the i.MX6DQ.
For example: we can run Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Has this patch really to be specific for the quad core version? Can we not also support the dual core version in the same way?

...

Quoted 15 lines

arch/arm/cpu/armv7/mx6/mp.c new file +15 -0

+int cpu_reset(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		reg |= SRC_SCR_CORE_1_RESET_MASK;
+		break;
+
+	case 2:
+		reg |= SRC_SCR_CORE_2_RESET_MASK;
+		break;

arch/arm/cpu/armv7/mx6/mp.c line 35

Quoted 5 lines

arch/arm/cpu/armv7/mx6/mp.c new file +5 -0

+
+	case 3:
+		reg |= SRC_SCR_CORE_3_RESET_MASK;
+		break;
+	}

I feel this should not be hardwired for 4 cores, and I also think we should avoid using such a switch statement here. All you need is an index into an array.

Quoted 10 lines

arch/arm/cpu/armv7/mx6/mp.c new file +10 -0

+int cpu_status(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		printf("core 1: %d\n", !!(reg & SRC_SCR_CORE_1_ENABLE_MASK));

arch/arm/cpu/armv7/mx6/mp.c line 62

Quoted 10 lines

arch/arm/cpu/armv7/mx6/mp.c new file +10 -0

+		break;
+
+	case 2:
+		printf("core 2: %d\n", !!(reg & SRC_SCR_CORE_2_ENABLE_MASK));
+		break;
+
+	case 3:
+		printf("core 3: %d\n", !!(reg & SRC_SCR_CORE_3_ENABLE_MASK));
+		break;
+	}

Ditto. Such code duplication does not scale. Please rework to avoid the switch.

Quoted 9 lines

arch/arm/cpu/armv7/mx6/mp.c new file +9 -0

+	switch (nr) {
+	case 1:
+		__raw_writel(boot_addr, &src->gpr3);
+		reg |= SRC_SCR_CORE_1_ENABLE_MASK;
+		break;
+
+	case 2:
+		__raw_writel(boot_addr, &src->gpr5);
+		reg |= SRC_SCR_CORE_2_ENABLE_MASK;

arch/arm/cpu/armv7/mx6/mp.c line 94

Quoted 10 lines

arch/arm/cpu/armv7/mx6/mp.c new file +10 -0

+		break;
+
+	case 3:
+		__raw_writel(boot_addr, &src->gpr7);
+		reg |= SRC_SCR_CORE_3_ENABLE_MASK;
+		break;
+	}
+
+	/* CPU N is ready to start */
+	__raw_writel(reg, &src->scr);

Ditto here.

And can you please explain why you are using __raw_writel() here?

Quoted 8 lines

arch/arm/cpu/armv7/mx6/mp.c new file +8 -0

+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		reg &= ~SRC_SCR_CORE_1_ENABLE_MASK;
+		break;
+
+	case 2:

arch/arm/cpu/armv7/mx6/mp.c line 131

Quoted 10 lines

arch/arm/cpu/armv7/mx6/mp.c new file +10 -0

+		reg &= ~SRC_SCR_CORE_2_ENABLE_MASK;
+		break;
+
+	case 3:
+		reg &= ~SRC_SCR_CORE_3_ENABLE_MASK;
+		break;
+	}
+
+	/* Disable the CPU N */
+	__raw_writel(reg, &src->scr);

Again, please avoid the switch.

We have read-modify-write macros which you could use, unless you really have to use the __raw_*() accessors. Why is this needed?

Best regards,

Wolfgang Denk

This allows u-boot to load different OS or Bare Metal application on the different cores of the i.MX6DQ.
For example: we can run Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

---
Changes for v2:
- Add a commit log message to explain the purpose of this patch
Changes for v3:
- Remove unnecessary check for unsigned values when they are negative
Changes for v4:
- Add CONFIG_MP to the common mx6 configuration
- Get the number of CPUs dynamically instead of using a macro
Changes for v5:
- Rebase on the last update of the tree (conflicts solved)

- Add a dummy header to solve build issue regarding the common/board_f.c

 arch/arm/cpu/armv7/mx6/Makefile           |   1 +
 arch/arm/cpu/armv7/mx6/mp.c               | 134 ++++++++++++++++++++++++++++++
 arch/arm/cpu/armv7/mx6/soc.c              |   6 ++
 arch/arm/include/asm/arch-mx6/imx-regs.h  |  13 +++
 arch/arm/include/asm/arch-mx6/sys_proto.h |   1 +
 arch/arm/include/asm/mp.h                 |  11 +++
 include/configs/mx6_common.h              |   2 +
 7 files changed, 168 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/mx6/mp.c
 create mode 100644 arch/arm/include/asm/mp.h

arch/arm/cpu/armv7/mx6/Makefile +1 -0

index 6dc9f8e..bf6effc 100644
@@ -10,3 +10,4 @@
 obj-y	:= soc.o clock.o
 obj-$(CONFIG_SPL_BUILD)	     += ddr.o
 obj-$(CONFIG_SECURE_BOOT)    += hab.o
+obj-$(CONFIG_MP)             += mp.o

arch/arm/cpu/armv7/mx6/mp.c new file +134 -0

new file mode 100644
index 0000000..85003d3
@@ -0,0 +1,134 @@
+/*
+ * (C) Copyright 2014
+ * Gabriel Huau <[email protected]>
+ *
+ * (C) Copyright 2009 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/errno.h>
+#include <asm/arch/sys_proto.h>
+#include <asm/arch/imx-regs.h>
+
+int cpu_reset(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		reg |= SRC_SCR_CORE_1_RESET_MASK;
+		break;
+
+	case 2:
+		reg |= SRC_SCR_CORE_2_RESET_MASK;
+		break;
+
+	case 3:
+		reg |= SRC_SCR_CORE_3_RESET_MASK;
+		break;
+	}
+
+	/* Software reset of the CPU N */
+	__raw_writel(reg, &src->scr);
+
+	return 0;
+}
+
+int cpu_status(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		printf("core 1: %d\n", !!(reg & SRC_SCR_CORE_1_ENABLE_MASK));
+		break;
+
+	case 2:
+		printf("core 2: %d\n", !!(reg & SRC_SCR_CORE_2_ENABLE_MASK));
+		break;
+
+	case 3:
+		printf("core 3: %d\n", !!(reg & SRC_SCR_CORE_3_ENABLE_MASK));
+		break;
+	}
+
+	return 0;
+}
+
+int cpu_release(int nr, int argc, char *const argv[])
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+	uint32_t boot_addr;
+
+	boot_addr = simple_strtoul(argv[0], NULL, 16);
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		__raw_writel(boot_addr, &src->gpr3);
+		reg |= SRC_SCR_CORE_1_ENABLE_MASK;
+		break;
+
+	case 2:
+		__raw_writel(boot_addr, &src->gpr5);
+		reg |= SRC_SCR_CORE_2_ENABLE_MASK;
+		break;
+
+	case 3:
+		__raw_writel(boot_addr, &src->gpr7);
+		reg |= SRC_SCR_CORE_3_ENABLE_MASK;
+		break;
+	}
+
+	/* CPU N is ready to start */
+	__raw_writel(reg, &src->scr);
+
+	return 0;
+}
+
+int is_core_valid(unsigned int core)
+{
+	uint32_t nr_cores = get_nr_cpus();
+
+	if (core > nr_cores)
+		return 0;
+
+	return 1;
+}
+
+int cpu_disable(int nr)
+{
+	uint32_t reg;
+	struct src *src = (struct src *)SRC_BASE_ADDR;
+
+	reg = __raw_readl(&src->scr);
+
+	switch (nr) {
+	case 1:
+		reg &= ~SRC_SCR_CORE_1_ENABLE_MASK;
+		break;
+
+	case 2:
+		reg &= ~SRC_SCR_CORE_2_ENABLE_MASK;
+		break;
+
+	case 3:
+		reg &= ~SRC_SCR_CORE_3_ENABLE_MASK;
+		break;
+	}
+
+	/* Disable the CPU N */
+	__raw_writel(reg, &src->scr);
+
+	return 0;
+}

arch/arm/cpu/armv7/mx6/soc.c +6 -0

index f20bdeb..19429b2 100644
@@ -35,6 +35,12 @@ struct scu_regs {
 	u32	fpga_rev;
 };
 
+u32 get_nr_cpus(void)
+{
+	struct scu_regs *scu = (struct scu_regs *)SCU_BASE_ADDR;
+	return readl(&scu->config) & 3;
+}
+
 u32 get_cpu_rev(void)
 {
 	struct anatop_regs *anatop = (struct anatop_regs *)ANATOP_BASE_ADDR;

arch/arm/include/asm/arch-mx6/imx-regs.h +13 -0

index a69a753..a90cbe9 100644
@@ -227,6 +227,19 @@
 
 extern void imx_get_mac_from_fuse(int dev_id, unsigned char *mac);
 
+#define SRC_SCR_CORE_1_RESET_OFFSET     14
+#define SRC_SCR_CORE_1_RESET_MASK       (1<<SRC_SCR_CORE_1_RESET_OFFSET)
+#define SRC_SCR_CORE_2_RESET_OFFSET     15
+#define SRC_SCR_CORE_2_RESET_MASK       (1<<SRC_SCR_CORE_2_RESET_OFFSET)
+#define SRC_SCR_CORE_3_RESET_OFFSET     16
+#define SRC_SCR_CORE_3_RESET_MASK       (1<<SRC_SCR_CORE_3_RESET_OFFSET)
+#define SRC_SCR_CORE_1_ENABLE_OFFSET    22
+#define SRC_SCR_CORE_1_ENABLE_MASK      (1<<SRC_SCR_CORE_1_ENABLE_OFFSET)
+#define SRC_SCR_CORE_2_ENABLE_OFFSET    23
+#define SRC_SCR_CORE_2_ENABLE_MASK      (1<<SRC_SCR_CORE_2_ENABLE_OFFSET)
+#define SRC_SCR_CORE_3_ENABLE_OFFSET    24
+#define SRC_SCR_CORE_3_ENABLE_MASK      (1<<SRC_SCR_CORE_3_ENABLE_OFFSET)
+
 /* System Reset Controller (SRC) */
 struct src {
 	u32	scr;

arch/arm/include/asm/arch-mx6/sys_proto.h +0 -0

index 42d30f5..7cbc133 100644

arch/arm/include/asm/arch-mx6/sys_proto.h +0 -0

 u32 get_cpu_rev(void);
 
 /* returns MXC_CPU_ value */

arch/arm/include/asm/mp.h new file +11 -0

new file mode 100644
index 0000000..8cf8d88
@@ -0,0 +1,11 @@
+/*
+ * (C) Copyright 2014
+ * Gabriel Huau <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _ASM_MP_H_
+#define _ASM_MP_H_
+
+#endif

include/configs/mx6_common.h +0 -0

index e4a5cc5..135a3f5 100644

Hi Gabriel,

Quoted 5 lines · Gabriel Huau

This allows u-boot to load different OS or Bare Metal application on the different cores of the i.MX6DQ.
For example: we can run Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

Quoted 10 lines

---
Changes for v2:
- Add a commit log message to explain the purpose of this patch
Changes for v3:
- Remove unnecessary check for unsigned values when they are negative
Changes for v4:
- Add CONFIG_MP to the common mx6 configuration
- Get the number of CPUs dynamically instead of using a macro
Changes for v5:
- Rebase on the last update of the tree (conflicts solved)

However, I get several warnings applying your patch:

arch/arm/include/asm/arch/sys_proto.h:17:0: warning: "is_soc_rev" redefined [enabled by default] arch/arm/include/asm/arch/sys_proto.h:15:0: note: this is the location of the previous definition

and:

arch/arm/cpu/armv7/mx6/mp.c:101:2: warning: implicit declaration of function 'get_nr_cpus' [-Wimplicit-function-declaration] In file included from arch/arm/imx-common/cpu.c:15:0:

You muxt fix them.

Quoted 2 lines

- Add a dummy header to solve build issue regarding the common/board_f.c

I do not think this is the best solution. An empty file is a file that is not needed.

Quoted 11 lines

  arch/arm/cpu/armv7/mx6/Makefile           |   1 +
  arch/arm/cpu/armv7/mx6/mp.c               | 134 ++++++++++++++++++++++++++++++
  arch/arm/cpu/armv7/mx6/soc.c              |   6 ++
  arch/arm/include/asm/arch-mx6/imx-regs.h  |  13 +++
  arch/arm/include/asm/arch-mx6/sys_proto.h |   1 +
  arch/arm/include/asm/mp.h                 |  11 +++
  include/configs/mx6_common.h              |   2 +
  7 files changed, 168 insertions(+)

create mode 100644 arch/arm/cpu/armv7/mx6/mp.c
create mode 100644 arch/arm/include/asm/mp.h

I have just investigate a bit. The file is included by common/board_f.c but it is, frankly, quite not used. There are several prototype inside it:

void setup_mp(void);
void cpu_mp_lmb_reserve(struct lmb *lmb);
int is_core_disabled(int nr);

They are not used at all.

u32 determine_mp_bootpg(unsigned int *pagesize);

This is the only one that is used.

Then it makes more sense to drop mp.h from board_f.c and add a prototype for determine_mp_bootpg(). This function is already protected by:

#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))

Best regards,
Stefano Babic

Quoted 17 lines

I have just investigate a bit. The file is included by common/board_f.c but it is, frankly, quite not used. There are several prototype inside it:

void setup_mp(void);
void cpu_mp_lmb_reserve(struct lmb *lmb);
int is_core_disabled(int nr);

They are not used at all.

u32 determine_mp_bootpg(unsigned int *pagesize);

This is the only one that is used.

Then it makes more sense to drop mp.h from board_f.c and add a prototype for determine_mp_bootpg(). This function is already protected by:

Quoted 2 lines

#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))

I agree adding an empty is not necessary the best solution, but I'd rather not to add any cpu/board specific defines in the common/ folder. That's why I think we should keep CONFIG_PM as this is a generic define. If necessary, I can propose another patch to fix it.

Hi Gabriel,

Quoted 17 lines · gabriel huau

Quoted 17 lines

I have just investigate a bit. The file is included by common/board_f.c but it is, frankly, quite not used. There are several prototype inside it:

void setup_mp(void);
void cpu_mp_lmb_reserve(struct lmb *lmb);
int is_core_disabled(int nr);

They are not used at all.

u32 determine_mp_bootpg(unsigned int *pagesize);

This is the only one that is used.

Then it makes more sense to drop mp.h from board_f.c and add a prototype for determine_mp_bootpg(). This function is already protected by:

Quoted 6 lines

Quoted 2 lines

#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))

I agree adding an empty is not necessary the best solution, but I'd rather not to add any cpu/board specific defines in the common/ folder. That's why I think we should keep CONFIG_PM as this is a generic define. If necessary, I can propose another patch to fix it.

I think we are saying the same thing. I agree letting CONFIG_MP, this is not the point. But prototype for determine_mp_bootg() can be moved in a powerpc include file and board_f does not need to include any mp.h

Best regards,
Stefano Babic

This allows u-boot to load different OS or Bare Metal application on different cores of the i.MX6 SoC.
For example: running Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

---
Changes for v2:
- Add a commit log message to explain the purpose of this patch
Changes for v3:
- Remove unnecessary check for unsigned values when they are negative
Changes for v4:
- Add CONFIG_MP to the common mx6 configuration
- Get the number of CPUs dynamically instead of using a macro
Changes for v5:
- Rebase on the last update of the tree (conflicts solved)
Changes for v6:
- Remove useless switch case
- Update board_f to not depend on mp.h unnecessary
- Fix build warnings
- Update commit message

 arch/arm/cpu/armv7/mx6/Makefile           |  1 +
 arch/arm/cpu/armv7/mx6/mp.c               | 87 +++++++++++++++++++++++++++++++
 arch/arm/cpu/armv7/mx6/soc.c              |  6 +++
 arch/arm/include/asm/arch-mx6/imx-regs.h  | 13 +++++
 arch/arm/include/asm/arch-mx6/sys_proto.h |  1 +
 common/board_f.c                          |  8 +--
 include/configs/mx6_common.h              |  2 +
 7 files changed, 114 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/mx6/mp.c

arch/arm/cpu/armv7/mx6/Makefile +1 -0

index 6dc9f8e..bf6effc 100644
@@ -10,3 +10,4 @@
 obj-y	:= soc.o clock.o
 obj-$(CONFIG_SPL_BUILD)	     += ddr.o
 obj-$(CONFIG_SECURE_BOOT)    += hab.o
+obj-$(CONFIG_MP)             += mp.o

arch/arm/cpu/armv7/mx6/mp.c new file +87 -0

new file mode 100644
index 0000000..9f034d6
@@ -0,0 +1,87 @@
+/*
+ * (C) Copyright 2014
+ * Gabriel Huau <[email protected]>
+ *
+ * (C) Copyright 2009 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/errno.h>
+#include <asm/arch/sys_proto.h>
+#include <asm/arch/imx-regs.h>
+
+#define MAX_CPUS 4
+static struct src *src = (struct src *)SRC_BASE_ADDR;
+
+static uint32_t cpu_reset_mask[MAX_CPUS] = {
+	0, /* We don't really want to modify the cpu0 */
+	SRC_SCR_CORE_1_RESET_MASK,
+	SRC_SCR_CORE_2_RESET_MASK,
+	SRC_SCR_CORE_3_RESET_MASK
+};
+
+static uint32_t cpu_ctrl_mask[MAX_CPUS] = {
+	0, /* We don't really want to modify the cpu0 */
+	SRC_SCR_CORE_1_ENABLE_MASK,
+	SRC_SCR_CORE_2_ENABLE_MASK,
+	SRC_SCR_CORE_3_ENABLE_MASK
+};
+
+int cpu_reset(int nr)
+{
+	/* Software reset of the CPU N */
+	src->scr |= cpu_reset_mask[nr];
+	return 0;
+}
+
+int cpu_status(int nr)
+{
+	printf("core %d => %d\n", nr, !!(src->scr & cpu_ctrl_mask[nr]));
+	return 0;
+}
+
+int cpu_release(int nr, int argc, char *const argv[])
+{
+	uint32_t boot_addr;
+
+	boot_addr = simple_strtoul(argv[0], NULL, 16);
+
+	switch (nr) {
+	case 1:
+		src->gpr3 = boot_addr;
+		break;
+	case 2:
+		src->gpr5 = boot_addr;
+		break;
+	case 3:
+		src->gpr7 = boot_addr;
+		break;
+	default:
+		return 1;
+	}
+
+	/* CPU N is ready to start */
+	src->scr |= cpu_ctrl_mask[nr];
+
+	return 0;
+}
+
+int is_core_valid(unsigned int core)
+{
+	uint32_t nr_cores = get_nr_cpus();
+
+	if (core > nr_cores)
+		return 0;
+
+	return 1;
+}
+
+int cpu_disable(int nr)
+{
+	/* Disable the CPU N */
+	src->scr &= ~cpu_ctrl_mask[nr];
+	return 0;
+}

arch/arm/cpu/armv7/mx6/soc.c +6 -0

index f20bdeb..19429b2 100644
@@ -35,6 +35,12 @@ struct scu_regs {
 	u32	fpga_rev;
 };
 
+u32 get_nr_cpus(void)
+{
+	struct scu_regs *scu = (struct scu_regs *)SCU_BASE_ADDR;
+	return readl(&scu->config) & 3;
+}
+
 u32 get_cpu_rev(void)
 {
 	struct anatop_regs *anatop = (struct anatop_regs *)ANATOP_BASE_ADDR;

arch/arm/include/asm/arch-mx6/imx-regs.h +13 -0

index a69a753..a90cbe9 100644
@@ -227,6 +227,19 @@
 
 extern void imx_get_mac_from_fuse(int dev_id, unsigned char *mac);
 
+#define SRC_SCR_CORE_1_RESET_OFFSET     14
+#define SRC_SCR_CORE_1_RESET_MASK       (1<<SRC_SCR_CORE_1_RESET_OFFSET)
+#define SRC_SCR_CORE_2_RESET_OFFSET     15
+#define SRC_SCR_CORE_2_RESET_MASK       (1<<SRC_SCR_CORE_2_RESET_OFFSET)
+#define SRC_SCR_CORE_3_RESET_OFFSET     16
+#define SRC_SCR_CORE_3_RESET_MASK       (1<<SRC_SCR_CORE_3_RESET_OFFSET)
+#define SRC_SCR_CORE_1_ENABLE_OFFSET    22
+#define SRC_SCR_CORE_1_ENABLE_MASK      (1<<SRC_SCR_CORE_1_ENABLE_OFFSET)
+#define SRC_SCR_CORE_2_ENABLE_OFFSET    23
+#define SRC_SCR_CORE_2_ENABLE_MASK      (1<<SRC_SCR_CORE_2_ENABLE_OFFSET)
+#define SRC_SCR_CORE_3_ENABLE_OFFSET    24
+#define SRC_SCR_CORE_3_ENABLE_MASK      (1<<SRC_SCR_CORE_3_ENABLE_OFFSET)
+
 /* System Reset Controller (SRC) */
 struct src {
 	u32	scr;

arch/arm/include/asm/arch-mx6/sys_proto.h +1 -0

index 42d30f5..306d699 100644
@@ -14,6 +14,7 @@
 #define soc_rev() (get_cpu_rev() & 0xFF)
 #define is_soc_rev(rev)        (soc_rev() - rev)
 
+u32 get_nr_cpus(void);
 u32 get_cpu_rev(void);
 
 /* returns MXC_CPU_ value */

common/board_f.c +4 -4

index 6203d85..e1dbdf3 100644
@@ -34,6 +34,9 @@
 #ifdef CONFIG_MPC5xxx
 #include <mpc5xxx.h>
 #endif
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#include <asm/mp.h>
+#endif
 
 #include <os.h>
 #include <post.h>

@@ -43,9 +46,6 @@
 #include <watchdog.h>
 #include <asm/errno.h>
 #include <asm/io.h>
-#ifdef CONFIG_MP
-#include <asm/mp.h>
-#endif
 #include <asm/sections.h>
 #ifdef CONFIG_X86
 #include <asm/init_helpers.h>

@@ -381,7 +381,7 @@ static int setup_dest_addr(void)
 	gd->ram_top = board_get_usable_ram_top(gd->mon_len);
 	gd->relocaddr = gd->ram_top;
 	debug("Ram top: %08lX\n", (ulong)gd->ram_top);
-#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
 	/*
 	 * We need to make sure the location we intend to put secondary core
 	 * boot code is reserved and not used by any part of u-boot

include/configs/mx6_common.h +0 -0

index e4a5cc5..135a3f5 100644

Hi Gabriel,

Quoted 21 lines · Gabriel Huau

This allows u-boot to load different OS or Bare Metal application on different cores of the i.MX6 SoC.
For example: running Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

---
Changes for v2:
- Add a commit log message to explain the purpose of this patch
Changes for v3:
- Remove unnecessary check for unsigned values when they are negative
Changes for v4:
- Add CONFIG_MP to the common mx6 configuration
- Get the number of CPUs dynamically instead of using a macro
Changes for v5:
- Rebase on the last update of the tree (conflicts solved)
Changes for v6:
- Remove useless switch case
- Update board_f to not depend on mp.h unnecessary
- Fix build warnings
- Update commit message

Quoted 20 lines

common/board_f.c +3 -3

@@ -34,6 +34,9 @@
 #ifdef CONFIG_MPC5xxx
 #include <mpc5xxx.h>
 #endif
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#include <asm/mp.h>
+#endif
 
 #include <os.h>
 #include <post.h>

@@ -43,9 +46,6 @@
 #include <watchdog.h>
 #include <asm/errno.h>
 #include <asm/io.h>
-#ifdef CONFIG_MP
-#include <asm/mp.h>
-#endif
 #include <asm/sections.h>

common/board_f.c static int setup_dest_addr(void) · line 384

Quoted 8 lines

common/board_f.c +1 -1

 #ifdef CONFIG_X86
 #include <asm/init_helpers.h>

@@ -381,7 +381,7 @@ static int setup_dest_addr(void)
 	gd->ram_top = board_get_usable_ram_top(gd->mon_len);
 	gd->relocaddr = gd->ram_top;
 	debug("Ram top: %08lX\n", (ulong)gd->ram_top);
-#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))

Quoted 24 lines · Stefano Babic

Hi Gabriel,

Quoted 21 lines · Gabriel Huau

This allows u-boot to load different OS or Bare Metal application on different cores of the i.MX6 SoC.
For example: running Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

---
Changes for v2:
- Add a commit log message to explain the purpose of this patch
Changes for v3:
- Remove unnecessary check for unsigned values when they are negative
Changes for v4:
- Add CONFIG_MP to the common mx6 configuration
- Get the number of CPUs dynamically instead of using a macro
Changes for v5:
- Rebase on the last update of the tree (conflicts solved)
Changes for v6:
- Remove useless switch case
- Update board_f to not depend on mp.h unnecessary
- Fix build warnings
- Update commit message

<snip>

Quoted 20 lines

Quoted 20 lines

common/board_f.c +3 -3

@@ -34,6 +34,9 @@
  #ifdef CONFIG_MPC5xxx
  #include <mpc5xxx.h>
  #endif
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#include <asm/mp.h>
+#endif
 
  #include <os.h>
  #include <post.h>

@@ -43,9 +46,6 @@
  #include <watchdog.h>
  #include <asm/errno.h>
  #include <asm/io.h>
-#ifdef CONFIG_MP
-#include <asm/mp.h>
-#endif
  #include <asm/sections.h>

Quoted 8 lines

common/board_f.c static int setup_dest_addr(void) · line 384

Quoted 8 lines

common/board_f.c +1 -1

  #ifdef CONFIG_X86
  #include <asm/init_helpers.h>

@@ -381,7 +381,7 @@ static int setup_dest_addr(void)
  	gd->ram_top = board_get_usable_ram_top(gd->mon_len);
  	gd->relocaddr = gd->ram_top;
  	debug("Ram top: %08lX\n", (ulong)gd->ram_top);
-#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))

I didn't notice this patch until it got merged. Why dropping CONFIG_MP here? This change breaks these boards

MPC8536DS MPC8536DS_36BIT MPC8536DS_SDCARD MPC8536DS_SPIFLASH qemu-ppce500

York

Hi York,

Quoted 58 lines · York Sun

Quoted 24 lines · Stefano Babic

Hi Gabriel,

Quoted 21 lines · Gabriel Huau

This allows u-boot to load different OS or Bare Metal application on different cores of the i.MX6 SoC.
For example: running Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

---
Changes for v2:
- Add a commit log message to explain the purpose of this patch
Changes for v3:
- Remove unnecessary check for unsigned values when they are negative
Changes for v4:
- Add CONFIG_MP to the common mx6 configuration
- Get the number of CPUs dynamically instead of using a macro
Changes for v5:
- Rebase on the last update of the tree (conflicts solved)
Changes for v6:
- Remove useless switch case
- Update board_f to not depend on mp.h unnecessary
- Fix build warnings
- Update commit message

<snip>

Quoted 20 lines

Quoted 20 lines

common/board_f.c +3 -3

@@ -34,6 +34,9 @@
  #ifdef CONFIG_MPC5xxx
  #include <mpc5xxx.h>
  #endif
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#include <asm/mp.h>
+#endif
 
  #include <os.h>
  #include <post.h>

@@ -43,9 +46,6 @@
  #include <watchdog.h>
  #include <asm/errno.h>
  #include <asm/io.h>
-#ifdef CONFIG_MP
-#include <asm/mp.h>
-#endif
  #include <asm/sections.h>

Quoted 8 lines

common/board_f.c static int setup_dest_addr(void) · line 384

Quoted 8 lines

common/board_f.c +1 -1

  #ifdef CONFIG_X86
  #include <asm/init_helpers.h>

@@ -381,7 +381,7 @@ static int setup_dest_addr(void)
  	gd->ram_top = board_get_usable_ram_top(gd->mon_len);
  	gd->relocaddr = gd->ram_top;
  	debug("Ram top: %08lX\n", (ulong)gd->ram_top);
-#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))

I didn't notice this patch until it got merged. Why dropping CONFIG_MP here? This change breaks these boards

Quoted 3 lines

MPC8536DS MPC8536DS_36BIT MPC8536DS_SDCARD MPC8536DS_SPIFLASH qemu-ppce500

York

Sorry, I didn't see the break for these boards.
'asm/mp.h' is used only for the powerpc board and using CONFIG_MP force the other architecture (or board) to create an empty header. As this header was here to use only specific features (not generic), I removed it and put the include guards only for the boards needed.

Can't we add these boards to the board_f.c?

Quoted 63 lines · Gabriel Huau

Hi York,

Quoted 60 lines · York Sun

Quoted 24 lines · Stefano Babic

Hi Gabriel,

Quoted 21 lines · Gabriel Huau

This allows u-boot to load different OS or Bare Metal application on different cores of the i.MX6 SoC.
For example: running Android on cpu0 and a RT OS like QNX/FreeRTOS on cpu1.

Signed-off-by Gabriel Huau <[email protected]>

---
Changes for v2:
- Add a commit log message to explain the purpose of this patch
Changes for v3:
- Remove unnecessary check for unsigned values when they are negative
Changes for v4:
- Add CONFIG_MP to the common mx6 configuration
- Get the number of CPUs dynamically instead of using a macro
Changes for v5:
- Rebase on the last update of the tree (conflicts solved)
Changes for v6:
- Remove useless switch case
- Update board_f to not depend on mp.h unnecessary
- Fix build warnings
- Update commit message

<snip>

Quoted 20 lines

Quoted 20 lines

common/board_f.c +3 -3

@@ -34,6 +34,9 @@
   #ifdef CONFIG_MPC5xxx
   #include <mpc5xxx.h>
   #endif
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#include <asm/mp.h>
+#endif
 
   #include <os.h>
   #include <post.h>

@@ -43,9 +46,6 @@
   #include <watchdog.h>
   #include <asm/errno.h>
   #include <asm/io.h>
-#ifdef CONFIG_MP
-#include <asm/mp.h>
-#endif
   #include <asm/sections.h>

Quoted 8 lines

common/board_f.c static int setup_dest_addr(void) · line 384

Quoted 8 lines

common/board_f.c +1 -1

   #ifdef CONFIG_X86
   #include <asm/init_helpers.h>

@@ -381,7 +381,7 @@ static int setup_dest_addr(void)
   	gd->ram_top = board_get_usable_ram_top(gd->mon_len);
   	gd->relocaddr = gd->ram_top;
   	debug("Ram top: %08lX\n", (ulong)gd->ram_top);
-#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))

I didn't notice this patch until it got merged. Why dropping CONFIG_MP here? This change breaks these boards

   MPC8536DS MPC8536DS_36BIT MPC8536DS_SDCARD MPC8536DS_SPIFLASH qemu-ppce500

Quoted 9 lines

Quoted 1 line

York

Sorry, I didn't see the break for these boards.
'asm/mp.h' is used only for the powerpc board and using CONFIG_MP force the other architecture (or board) to create an empty header. As this header was here to use only specific features (not generic), I removed it and put the include guards only for the boards needed.

Can't we add these boards to the board_f.c?

I don't mind to change the guard for asm/mp.h. But you shouldn't remove the guard for the function call. Not all powerpc SoCs support SMP. You could use

common/board_f.c +3 -0

+#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#include <asm/mp.h>
+#endif

York