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

2014-09-03 Thread Gabriel Huau 

Hi York,

On 09/03/2014 11:00 AM, York Sun wrote:

On 07/28/2014 08:15 AM, Stefano Babic wrote:

Hi Gabriel,

On 26/07/2014 20:35, Gabriel Huau wrote:

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





--- a/common/board_f.c
+++ b/common/board_f.c
@@ -34,6 +34,9 @@
  #ifdef CONFIG_MPC5xxx
  #include 
  #endif
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#include 
+#endif
  
  #include 

  #include 
@@ -43,9 +46,6 @@
  #include 
  #include 
  #include 
-#ifdef CONFIG_MP
-#include 
-#endif
  #include 
  #ifdef CONFIG_X86
  #include 
@@ -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


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?

Regards,
Gabriel
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2014-09-03 Thread Gabriel Huau 

On 09/03/2014 11:36 AM, York Sun wrote:

On 09/03/2014 11:26 AM, Gabriel Huau wrote:

Hi York,

On 09/03/2014 11:00 AM, York Sun wrote:

On 07/28/2014 08:15 AM, Stefano Babic wrote:

Hi Gabriel,

On 26/07/2014 20:35, Gabriel Huau wrote:

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





--- a/common/board_f.c
+++ b/common/board_f.c
@@ -34,6 +34,9 @@
   #ifdef CONFIG_MPC5xxx
   #include 
   #endif
+#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#include 
+#endif
   
   #include 

   #include 
@@ -43,9 +46,6 @@
   #include 
   #include 
   #include 
-#ifdef CONFIG_MP
-#include 
-#endif
   #include 
   #ifdef CONFIG_X86
   #include 
@@ -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

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

+#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#include 
+#endif

York



Oh! I see what you mean, my bad, sorry.
I'll propose the fix as soon as possible to revert the modification on 
the include guard and modify the one for the asm/mp.h


Regards,
Gabriel
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2014-09-03 Thread York Sun 
On 09/03/2014 11:26 AM, Gabriel Huau wrote:
> Hi York,
> 
> On 09/03/2014 11:00 AM, York Sun wrote:
>> On 07/28/2014 08:15 AM, Stefano Babic wrote:
>>> Hi Gabriel,
>>>
>>> On 26/07/2014 20:35, Gabriel Huau wrote:
 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 
 ---
 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

>> 
>>
 --- a/common/board_f.c
 +++ b/common/board_f.c
 @@ -34,6 +34,9 @@
   #ifdef CONFIG_MPC5xxx
   #include 
   #endif
 +#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
 +#include 
 +#endif
   
   #include 
   #include 
 @@ -43,9 +46,6 @@
   #include 
   #include 
   #include 
 -#ifdef CONFIG_MP
 -#include 
 -#endif
   #include 
   #ifdef CONFIG_X86
   #include 
 @@ -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
> 
> 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

+#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
+#include 
+#endif

York

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2014-09-03 Thread York Sun 
On 07/28/2014 08:15 AM, Stefano Babic wrote:
> Hi Gabriel,
> 
> On 26/07/2014 20:35, Gabriel Huau wrote:
>> 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 
>> ---
>> 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
>>



>> --- a/common/board_f.c
>> +++ b/common/board_f.c
>> @@ -34,6 +34,9 @@
>>  #ifdef CONFIG_MPC5xxx
>>  #include 
>>  #endif
>> +#if (defined(CONFIG_MPC86xx) || defined(CONFIG_E500))
>> +#include 
>> +#endif
>>  
>>  #include 
>>  #include 
>> @@ -43,9 +46,6 @@
>>  #include 
>>  #include 
>>  #include 
>> -#ifdef CONFIG_MP
>> -#include 
>> -#endif
>>  #include 
>>  #ifdef CONFIG_X86
>>  #include 
>> @@ -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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2014-07-28 Thread Stefano Babic 
Hi Gabriel,

On 26/07/2014 20:35, Gabriel Huau wrote:
> 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 
> ---
> 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
> 
> 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 000..9f034d6
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/mx6/mp.c
> @@ -0,0 +1,87 @@
> +/*
> + * (C) Copyright 2014
> + * Gabriel Huau 
> + *
> + * (C) Copyright 2009 Freescale Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:  GPL-2.0+
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#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;
> +}
> 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< +#define SRC_SCR_CORE_2_RESET_OFFSET 15
> +#define SRC_SCR_CORE_2_RESET_MASK   (1< +#define SRC_SCR_CORE_3_RESET_OFFSET 16
> +#define SRC_SCR_CORE_3_RESET_MASK   (1< +#define SRC_SCR_CORE_1_