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

gabriel huau
2014-06-25T00:22:40Z

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 <contact@huau-gabriel.fr>
>> ---
>> 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 0000000..8105fcc
>> --- /dev/null
>> +++ b/arch/arm/cpu/armv7/mx6/mp.c
>> @@ -0,0 +1,131 @@
>> +/*
>> + * (C) Copyright 2014
>> + * Gabriel Huau <contact@huau-gabriel.fr>
>> + *
>> + * (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/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.
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);
>> +
>> + 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<<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;
>
> Best regards,
> Stefano Babic
>
>