| Message ID | 1405204264-10922-1-git-send-email-contact@huau-gabriel.fr |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
Dear Gabriel Huau, In message <1405204264-10922-1-git-send-email-contact@huau-gabriel.fr> you 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. 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? ... > +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; > + } 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. > +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; > + } Ditto. Such code duplication does not scale. Please rework to avoid the switch. > + 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); Ditto here. And can you please explain why you are using __raw_writel() here? > + 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); 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
Hi Wolfgang, On 07/13/2014 02:58 AM, Wolfgang Denk wrote: > Dear Gabriel Huau, > > In message <1405204264-10922-1-git-send-email-contact@huau-gabriel.fr> you 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. > 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? > > ... Nope, but it makes sense for only Dual and Quad version. I'll update the commit message to be more specific. >> +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; >> + } > 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. Agreed. >> +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; >> + } > Ditto. Such code duplication does not scale. Please rework to avoid > the switch. > >> + 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); > Ditto here. > > And can you please explain why you are using __raw_writel() here? No particular reason, I'll update with the generic macro. >> + 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); > 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 I'll send a version 6 with your correction. Regards, Gabriel
Hi Gabriel, On 13/07/2014 00:31, 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 > 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. > - 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. > 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)) > diff --git a/arch/arm/cpu/armv7/mx6/Makefile b/arch/arm/cpu/armv7/mx6/Makefile > index 6dc9f8e..bf6effc 100644 > --- a/arch/arm/cpu/armv7/mx6/Makefile > +++ b/arch/arm/cpu/armv7/mx6/Makefile > @@ -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 > diff --git a/arch/arm/cpu/armv7/mx6/mp.c b/arch/arm/cpu/armv7/mx6/mp.c > new file mode 100644 > index 0000000..85003d3 > --- /dev/null > +++ b/arch/arm/cpu/armv7/mx6/mp.c > @@ -0,0 +1,134 @@ > +/* > + * (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/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; > +} > diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c > index f20bdeb..19429b2 100644 > --- a/arch/arm/cpu/armv7/mx6/soc.c > +++ b/arch/arm/cpu/armv7/mx6/soc.c > @@ -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; > diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h > index a69a753..a90cbe9 100644 > --- a/arch/arm/include/asm/arch-mx6/imx-regs.h > +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h > @@ -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; > diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h > index 42d30f5..7cbc133 100644 > --- a/arch/arm/include/asm/arch-mx6/sys_proto.h > +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h > @@ -14,6 +14,7 @@ > #define soc_rev() (get_cpu_rev() & 0xFF) > #define is_soc_rev(rev) (soc_rev() - rev) > > +#define is_soc_rev(rev) ((get_cpu_rev() & 0xFF) - rev) You see, is_soc_rev() is defined twice ! > u32 get_cpu_rev(void); > > /* returns MXC_CPU_ value */ > diff --git a/arch/arm/include/asm/mp.h b/arch/arm/include/asm/mp.h > new file mode 100644 > index 0000000..8cf8d88 > --- /dev/null > +++ b/arch/arm/include/asm/mp.h > @@ -0,0 +1,11 @@ > +/* > + * (C) Copyright 2014 > + * Gabriel Huau <contact@huau-gabriel.fr> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#ifndef _ASM_MP_H_ > +#define _ASM_MP_H_ > + > +#endif > diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h > index e4a5cc5..135a3f5 100644 > --- a/include/configs/mx6_common.h > +++ b/include/configs/mx6_common.h > @@ -28,4 +28,6 @@ > #define CONFIG_SYS_PL310_BASE L2_PL310_BASE > #endif > > +#define CONFIG_MP > + > #endif > Best regards, Stefano Babic
On 07/15/2014 12:49 AM, Stefano Babic wrote: > Hi Gabriel, > > On 13/07/2014 00:31, 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 >> 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. My bad during the merge conflict, I'll fix that for the next version of the patch. >> - 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. > >> 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)) 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. > Best regards, > Stefano Babic >
Hi Gabriel, On 15/07/2014 16:13, gabriel huau wrote: >> 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)) > > 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
Agreed, I misunderstood sorry. I'll do the modification for the next version of the patch. Thanks! Regards, Gabriel On 07/15/2014 12:35 PM, Stefano Babic wrote: > Hi Gabriel, > > On 15/07/2014 16:13, gabriel huau wrote: > >>> 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)) >> 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 >
diff --git a/arch/arm/cpu/armv7/mx6/Makefile b/arch/arm/cpu/armv7/mx6/Makefile index 6dc9f8e..bf6effc 100644 --- a/arch/arm/cpu/armv7/mx6/Makefile +++ b/arch/arm/cpu/armv7/mx6/Makefile @@ -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 diff --git a/arch/arm/cpu/armv7/mx6/mp.c b/arch/arm/cpu/armv7/mx6/mp.c new file mode 100644 index 0000000..85003d3 --- /dev/null +++ b/arch/arm/cpu/armv7/mx6/mp.c @@ -0,0 +1,134 @@ +/* + * (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/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; +} diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c index f20bdeb..19429b2 100644 --- a/arch/arm/cpu/armv7/mx6/soc.c +++ b/arch/arm/cpu/armv7/mx6/soc.c @@ -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; diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h index a69a753..a90cbe9 100644 --- a/arch/arm/include/asm/arch-mx6/imx-regs.h +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h @@ -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; diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h index 42d30f5..7cbc133 100644 --- a/arch/arm/include/asm/arch-mx6/sys_proto.h +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h @@ -14,6 +14,7 @@ #define soc_rev() (get_cpu_rev() & 0xFF) #define is_soc_rev(rev) (soc_rev() - rev) +#define is_soc_rev(rev) ((get_cpu_rev() & 0xFF) - rev) u32 get_cpu_rev(void); /* returns MXC_CPU_ value */ diff --git a/arch/arm/include/asm/mp.h b/arch/arm/include/asm/mp.h new file mode 100644 index 0000000..8cf8d88 --- /dev/null +++ b/arch/arm/include/asm/mp.h @@ -0,0 +1,11 @@ +/* + * (C) Copyright 2014 + * Gabriel Huau <contact@huau-gabriel.fr> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _ASM_MP_H_ +#define _ASM_MP_H_ + +#endif diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h index e4a5cc5..135a3f5 100644 --- a/include/configs/mx6_common.h +++ b/include/configs/mx6_common.h @@ -28,4 +28,6 @@ #define CONFIG_SYS_PL310_BASE L2_PL310_BASE #endif +#define CONFIG_MP + #endif
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 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