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

gabriel huau
2014-07-15T14:13:25Z

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
>