[U-Boot] [PATCH v2 16/27] x86: Add basic support to queensbay platform and crownbay board
Simon Glass
sjg at chromium.org
Fri Dec 12 04:53:38 CET 2014
Hi Bin,
On 11 December 2014 at 20:34, Bin Meng <bmeng.cn at gmail.com> wrote:
> 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.
But couldn't it be called from C with an assembler shim?
My objective is to make the assembler code as short as possible and
call board_init_f() as soon as possible. So can we no call this later
in the init sequence?
>
>>> +
>>> +.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.
It's not writeable though, right? So in what sense it is a 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.
OK.
>
>>> +}
>>> +
>>> +/*
>>> + * 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
Regards,
Simon
More information about the U-Boot
mailing list