[U-Boot] [PATCH v2 16/27] x86: Add basic support to queensbay platform and crownbay board

Bin Meng bmeng.cn at gmail.com
Fri Dec 12 04:34:53 CET 2014


Hi Simon,

On Fri, Dec 12, 2014 at 11:18 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 9 December 2014 at 07:50, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Implement minimum required functions for the basic support to
>> queensbay platform and crownbay board.
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>
>> ---
>>
>> Changes in v2:
>> - Replace 0xcf9 with macro PORT_RESET from processor.h
>> - Move FspInit call from start.S to car_init
>> - Add UART0_BASE and UART1_BASE to ibmpc.h
>>
>>  arch/x86/cpu/queensbay/Makefile   |   9 ++++
>>  arch/x86/cpu/queensbay/tnc.c      |  48 ++++++++++++++++++
>>  arch/x86/cpu/queensbay/tnc_car.S  | 103 ++++++++++++++++++++++++++++++++++++++
>>  arch/x86/cpu/queensbay/tnc_dram.c |  78 +++++++++++++++++++++++++++++
>>  arch/x86/cpu/queensbay/tnc_pci.c  |  61 ++++++++++++++++++++++
>>  arch/x86/include/asm/ibmpc.h      |   3 ++
>>  board/intel/crownbay/MAINTAINERS  |   6 +++
>>  board/intel/crownbay/Makefile     |   7 +++
>>  board/intel/crownbay/crownbay.c   |  21 ++++++++
>>  board/intel/crownbay/start.S      |   9 ++++
>>  10 files changed, 345 insertions(+)
>>  create mode 100644 arch/x86/cpu/queensbay/Makefile
>>  create mode 100644 arch/x86/cpu/queensbay/tnc.c
>>  create mode 100644 arch/x86/cpu/queensbay/tnc_car.S
>>  create mode 100644 arch/x86/cpu/queensbay/tnc_dram.c
>>  create mode 100644 arch/x86/cpu/queensbay/tnc_pci.c
>>  create mode 100644 board/intel/crownbay/MAINTAINERS
>>  create mode 100644 board/intel/crownbay/Makefile
>>  create mode 100644 board/intel/crownbay/crownbay.c
>>  create mode 100644 board/intel/crownbay/start.S
>>
>> diff --git a/arch/x86/cpu/queensbay/Makefile b/arch/x86/cpu/queensbay/Makefile
>> new file mode 100644
>> index 0000000..ace04ca
>> --- /dev/null
>> +++ b/arch/x86/cpu/queensbay/Makefile
>> @@ -0,0 +1,9 @@
>> +#
>> +# Copyright (C) 2014, Bin Meng <bmeng.cn at gmail.com>
>> +#
>> +# SPDX-License-Identifier:     GPL-2.0+
>> +#
>> +
>> +obj-y += tnc_car.o tnc_dram.o tnc.o
>> +obj-y += fsp_configs.o fsp_support.o
>> +obj-$(CONFIG_PCI) += tnc_pci.o
>> diff --git a/arch/x86/cpu/queensbay/tnc.c b/arch/x86/cpu/queensbay/tnc.c
>> new file mode 100644
>> index 0000000..b1df57a
>> --- /dev/null
>> +++ b/arch/x86/cpu/queensbay/tnc.c
>> @@ -0,0 +1,48 @@
>> +/*
>> + * Copyright (C) 2014, Bin Meng <bmeng.cn at gmail.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/io.h>
>> +#include <asm/post.h>
>> +#include <asm/arch/fsp/fsp_support.h>
>> +#include <asm/processor.h>
>> +
>> +int arch_cpu_init(void)
>> +{
>> +       post_code(POST_CPU_INIT);
>> +#ifdef CONFIG_SYS_X86_TSC_TIMER
>> +       timer_set_base(rdtsc());
>> +#endif
>> +
>> +       return x86_cpu_init_f();
>> +}
>> +
>> +int print_cpuinfo(void)
>> +{
>> +       post_code(POST_CPU_INFO);
>> +       return default_print_cpuinfo();
>> +}
>> +
>> +void reset_cpu(ulong addr)
>> +{
>> +       /* cold reset */
>> +       outb(0x06, PORT_RESET);
>> +}
>> +
>> +void board_final_cleanup(void)
>> +{
>> +       EFI_STATUS status;
>
> Is this an enum? We should avoid capital types and also typedef /
> #define for types.

This will be fixed in the U-Boot coding convention final cleanup in v3.

>> +
>> +       /* call into FspNotify */
>> +       debug("Calling into FSP (notify phase INIT_PHASE_BOOT): ");
>> +       status = fsp_notify(NULL, INIT_PHASE_BOOT);
>> +       if (status != FSP_SUCCESS)
>> +               debug("fail, error code %x\n", status);
>> +       else
>> +               debug("OK\n");
>> +
>> +       return;
>> +}
>> diff --git a/arch/x86/cpu/queensbay/tnc_car.S b/arch/x86/cpu/queensbay/tnc_car.S
>> new file mode 100644
>> index 0000000..4f39e42
>> --- /dev/null
>> +++ b/arch/x86/cpu/queensbay/tnc_car.S
>> @@ -0,0 +1,103 @@
>> +/*
>> + * Copyright (C) 2014, Bin Meng <bmeng.cn at gmail.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <config.h>
>> +#include <asm/post.h>
>> +
>> +.globl car_init
>> +car_init:
>> +       /*
>> +        * Note: ebp holds the BIST value (built-in self test) so far, but ebp
>> +        * will be destroyed through the FSP call, thus we have to test the
>> +        * BIST value here before we call into FSP.
>> +        */
>> +       test    %ebp, %ebp
>> +       jz      car_init_start
>> +       post_code(POST_BIST_FAILURE)
>> +       jmp     die
>> +
>> +car_init_start:
>> +       post_code(POST_CAR_START)
>> +       lea     find_fsp_header_stack, %esp
>> +       jmp     find_fsp_header
>> +
>> +find_fsp_header_ret:
>> +       /* EAX points to FSP_INFO_HEADER */
>> +       mov     %eax, %ebp
>> +
>> +       /* sanity test */
>> +       cmp     $CONFIG_FSP_LOCATION, %eax
>> +       jb      die
>> +
>> +       /* calculate TempRamInitEntry address */
>> +       mov     0x30(%ebp), %eax
>> +       add     0x1c(%ebp), %eax
>> +
>> +       /* call FSP TempRamInitEntry to setup temporary stack */
>> +       lea     temp_ram_init_stack, %esp
>> +       jmp     *%eax
>> +
>> +temp_ram_init_ret:
>> +       addl    $4, %esp
>> +       cmp     $0, %eax
>> +       jz      continue
>
> Can we make this jnz put the error code below instead of it being in
> the normal path flow?

Sure.

>> +       post_code(POST_CAR_FAILURE)
>> +
>> +die:
>> +       hlt
>> +       jmp     die
>> +       hlt
>> +
>> +continue:
>> +       post_code(POST_CAR_CPU_CACHE)
>> +
>> +       /*
>> +        * The FSP TempRamInit initializes the ecx and edx registers to
>> +        * point to a temporary but writable memory range (Cache-As-RAM).
>> +        * ecx: the start of this temporary memory range,
>> +        * edx: the end of this range.
>> +        */
>> +
>> +       /* stack grows down from top of CAR */
>> +       movl    %edx, %esp
>> +
>> +       movl    $CONFIG_FSP_TEMP_RAM_ADDR, %eax
>> +       xorl    %edx, %edx
>> +       xorl    %ecx, %ecx
>> +       call    fsp_init
>
> Do we have to call fsp_init so early? Could we not call it from
> cpu_init_f() or even later?

Unfortunately yes. According to FSP architecture spec, the fsp_init
will not return to its caller, instead it requires the bootloader to
provide a so-called continuation function to pass into the FSP as a
parameter of fsp_init, and fsp_init will call that continuation
function directly.

>> +
>> +.global fsp_init_done
>> +fsp_init_done:
>> +       /*
>> +        * We come here from FspInit with eax pointing to the HOB list.
>> +        * Save eax to esi temporarily.
>> +        */
>> +       movl    %eax, %esi
>> +       /*
>> +        * Re-initialize the ebp (BIST) to zero, as we already reach here
>> +        * which means we passed BIST testing before.
>> +        */
>> +       xorl    %ebp, %ebp
>> +       jmp     car_init_ret
>> +
>> +       .balign 4
>> +find_fsp_header_stack:
>> +       .long   find_fsp_header_ret
>> +
>> +       .balign 4
>> +temp_ram_init_stack:
>
> Can we use temp_ram_init_vars or similar? This name makes it look like
> it is a temporary stack.

Yes, it is indeed a temporary ROM stack.

>> +       .long   temp_ram_init_ret
>> +       .long   temp_ram_init_params
>> +temp_ram_init_params:
>> +       .long   ucode_start             /* microcode base */
>> +       .long   ucode_size              /* microcode size */
>> +       .long   CONFIG_SYS_MONITOR_BASE /* code region base */
>> +       .long   CONFIG_SYS_MONITOR_LEN  /* code region size */
>> +
>> +       .balign 4
>> +ucode_start:
>> +       .include "arch/x86/cpu/queensbay/M0220661105.inc"
>> +ucode_size = ( . - ucode_start)
>> diff --git a/arch/x86/cpu/queensbay/tnc_dram.c b/arch/x86/cpu/queensbay/tnc_dram.c
>> new file mode 100644
>> index 0000000..8e21374
>> --- /dev/null
>> +++ b/arch/x86/cpu/queensbay/tnc_dram.c
>> @@ -0,0 +1,78 @@
>> +/*
>> + * Copyright (C) 2014, Bin Meng <bmeng.cn at gmail.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/arch/fsp/fsp_support.h>
>> +#include <asm/e820.h>
>> +#include <asm/post.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +int dram_init(void)
>> +{
>> +       phys_size_t ram_size = 0;
>> +       EFI_PEI_HOB_POINTERS hob;
>> +
>> +       hob.raw = gd->arch.hob_list;
>> +       while (!END_OF_HOB(hob)) {
>> +               if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>> +                       if (hob.res_desc->type == RES_SYS_MEM ||
>> +                           hob.res_desc->type == RES_MEM_RESERVED) {
>> +                               ram_size += hob.res_desc->len;
>> +                       }
>> +               }
>> +               hob.raw = GET_NEXT_HOB(hob);
>> +       }
>> +
>> +       gd->ram_size = ram_size;
>> +       post_code(POST_DRAM);
>> +
>> +       return 0;
>> +}
>> +
>> +void dram_init_banksize(void)
>> +{
>> +       gd->bd->bi_dram[0].start = 0;
>> +       gd->bd->bi_dram[0].size = gd->ram_size;
>
> Is it true that RAM starts at address 0 and is contiguous? Can this
> ever go above 4GB?

The maximum DRAM size the Queensbay platform supports is 2GiB. It is
true that RAM starts from 0 and is contiguous for this FSP, except
there is a 1MiB memory hole which is used as the TSEG.

>> +}
>> +
>> +/*
>> + * This function looks for the highest region of memory lower than 4GB which
>> + * has enough space for U-Boot where U-Boot is aligned on a page boundary.
>> + * It overrides the default implementation found elsewhere which simply
>> + * picks the end of ram, wherever that may be. The location of the stack,
>> + * the relocation address, and how far U-Boot is moved by relocation are
>> + * set in the global data structure.
>> + */
>> +ulong board_get_usable_ram_top(ulong total_size)
>> +{
>> +       return get_usable_lowmem_top(gd->arch.hob_list);
>> +}
>> +
>> +unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
>> +{
>> +       unsigned num_entries = 0;
>> +
>> +       EFI_PEI_HOB_POINTERS hob;
>> +
>> +       hob.raw = gd->arch.hob_list;
>> +
>> +       while (!END_OF_HOB(hob)) {
>> +               if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>> +                       entries[num_entries].addr = hob.res_desc->phys_start;
>> +                       entries[num_entries].size = hob.res_desc->len;
>> +
>> +                       if (hob.res_desc->type == RES_SYS_MEM)
>> +                               entries[num_entries].type = E820_RAM;
>> +                       else if (hob.res_desc->type == RES_MEM_RESERVED)
>> +                               entries[num_entries].type = E820_RESERVED;
>> +               }
>> +               hob.raw = GET_NEXT_HOB(hob);
>> +               num_entries++;
>> +       }
>> +
>> +       return num_entries;
>> +}
>> diff --git a/arch/x86/cpu/queensbay/tnc_pci.c b/arch/x86/cpu/queensbay/tnc_pci.c
>> new file mode 100644
>> index 0000000..85254f0
>> --- /dev/null
>> +++ b/arch/x86/cpu/queensbay/tnc_pci.c
>> @@ -0,0 +1,61 @@
>> +/*
>> + * Copyright (C) 2014, Bin Meng <bmeng.cn at gmail.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <pci.h>
>> +#include <asm/pci.h>
>> +#include <asm/arch/fsp/fsp_support.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +void board_pci_setup_hose(struct pci_controller *hose)
>> +{
>> +       hose->first_busno = 0;
>> +       hose->last_busno = 0;
>> +
>> +       /* PCI memory space */
>> +       pci_set_region(hose->regions + 0,
>> +                      CONFIG_PCI_MEM_BUS,
>> +                      CONFIG_PCI_MEM_PHYS,
>> +                      CONFIG_PCI_MEM_SIZE,
>> +                      PCI_REGION_MEM);
>> +
>> +       /* PCI IO space */
>> +       pci_set_region(hose->regions + 1,
>> +                      CONFIG_PCI_IO_BUS,
>> +                      CONFIG_PCI_IO_PHYS,
>> +                      CONFIG_PCI_IO_SIZE,
>> +                      PCI_REGION_IO);
>> +
>> +       pci_set_region(hose->regions + 2,
>> +                      CONFIG_PCI_PREF_BUS,
>> +                      CONFIG_PCI_PREF_PHYS,
>> +                      CONFIG_PCI_PREF_SIZE,
>> +                      PCI_REGION_PREFETCH);
>> +
>> +       pci_set_region(hose->regions + 3,
>> +                      0,
>> +                      0,
>> +                      gd->ram_size,
>> +                      PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>> +
>> +       hose->region_count = 4;
>> +}
>> +
>> +int board_pci_post_scan(struct pci_controller *hose)
>> +{
>> +       EFI_STATUS status;
>
> Use lower case / avoid typedef

Fixed in v3.

[snip]

Regards,
Bin


More information about the U-Boot mailing list