Re: [U-Boot] [PATCH v3] mx6: add support of multi-processor command

2014-06-24 Thread gabriel huau

Hi Stefano,

On 06/24/2014 05:50 AM, Stefano Babic wrote:

Hi Gabriel,

On 22/06/2014 18:56, Gabriel Huau wrote:

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 
---
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

diff --git a/arch/arm/cpu/armv7/mx6/Makefile b/arch/arm/cpu/armv7/mx6/Makefile
index d7285fc..ec08526 100644
--- a/arch/arm/cpu/armv7/mx6/Makefile
+++ b/arch/arm/cpu/armv7/mx6/Makefile
@@ -9,3 +9,4 @@
  
  obj-y	:= soc.o clock.o

  obj-$(CONFIG_SECURE_BOOT)+= hab.o
+obj-$(CONFIG_MP) += mp.o
diff --git a/arch/arm/cpu/armv7/mx6/mp.c b/arch/arm/cpu/armv7/mx6/mp.c

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

I didn't enable the configuration in the board file so it won't be 
available by default, should I do it? or should we leave the user to add 
this command in their board file?

new file mode 100644
index 000..8105fcc
--- /dev/null
+++ b/arch/arm/cpu/armv7/mx6/mp.c
@@ -0,0 +1,131 @@
+/*
+ * (C) Copyright 2014
+ * Gabriel Huau 
+ *
+ * (C) Copyright 2009 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+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.
The name 'cpu_reset' is not really my choice, when you enable the 
command MP you have to defines these functions (common/cmd_mp.c), I'm 
open to any suggestion but anyway we will have to define these function 
(except if I missed

something).

To avoid any conflict, I've defined them in the SoC code (they did the 
same for PowerPC), so another SoC can define these functions without any 
conflict.



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

By default, I didn't enable the common MP for the mx6, but the 
CONFIG_NUM_CPUS should be defined in the config file of the SoC (Quad => 
4, Dual => 2, Solo => 1).
I think a better way will be to read directly the SCU registers to get 
the current CPU configuration of the board, I'll update the patch.



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

Re: [U-Boot] [PATCH v3] mx6: add support of multi-processor command

2014-06-24 Thread Stefano Babic
Hi Gabriel,

On 22/06/2014 18:56, Gabriel Huau wrote:
> 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 
> ---
> 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
> 
> diff --git a/arch/arm/cpu/armv7/mx6/Makefile b/arch/arm/cpu/armv7/mx6/Makefile
> index d7285fc..ec08526 100644
> --- a/arch/arm/cpu/armv7/mx6/Makefile
> +++ b/arch/arm/cpu/armv7/mx6/Makefile
> @@ -9,3 +9,4 @@
>  
>  obj-y:= soc.o clock.o
>  obj-$(CONFIG_SECURE_BOOT)+= hab.o
> +obj-$(CONFIG_MP) += mp.o
> diff --git a/arch/arm/cpu/armv7/mx6/mp.c b/arch/arm/cpu/armv7/mx6/mp.c

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

> new file mode 100644
> index 000..8105fcc
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/mx6/mp.c
> @@ -0,0 +1,131 @@
> +/*
> + * (C) Copyright 2014
> + * Gabriel Huau 
> + *
> + * (C) Copyright 2009 Freescale Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:  GPL-2.0+
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +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.

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

> + 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;
> +}
> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h 
> b/arch/arm/include/asm/arch-mx6/imx-regs.h
> index 1f19727..3f8c2ee 100644
> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
> @@ -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

[U-Boot] [PATCH v3] mx6: add support of multi-processor command

2014-06-22 Thread 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 
---
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

diff --git a/arch/arm/cpu/armv7/mx6/Makefile b/arch/arm/cpu/armv7/mx6/Makefile
index d7285fc..ec08526 100644
--- a/arch/arm/cpu/armv7/mx6/Makefile
+++ b/arch/arm/cpu/armv7/mx6/Makefile
@@ -9,3 +9,4 @@
 
 obj-y  := soc.o clock.o
 obj-$(CONFIG_SECURE_BOOT)+= hab.o
+obj-$(CONFIG_MP) += mp.o
diff --git a/arch/arm/cpu/armv7/mx6/mp.c b/arch/arm/cpu/armv7/mx6/mp.c
new file mode 100644
index 000..8105fcc
--- /dev/null
+++ b/arch/arm/cpu/armv7/mx6/mp.c
@@ -0,0 +1,131 @@
+/*
+ * (C) Copyright 2014
+ * Gabriel Huau 
+ *
+ * (C) Copyright 2009 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+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 nr)
+{
+   if (nr >= CONFIG_NUM_CPUS)
+   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;
+}
diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h 
b/arch/arm/include/asm/arch-mx6/imx-regs.h
index 1f19727..3f8c2ee 100644
--- a/arch/arm/include/asm/arch-mx6/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
@@ -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