Hi Bin, On 11 December 2014 at 20:34, Bin Meng <bmeng...@gmail.com> wrote: > Hi Simon, > > On Fri, Dec 12, 2014 at 11:18 AM, Simon Glass <s...@chromium.org> wrote: >> Hi Bin, >> >> On 9 December 2014 at 07:50, Bin Meng <bmeng...@gmail.com> wrote: >>> Implement minimum required functions for the basic support to >>> queensbay platform and crownbay board. >>> >>> Signed-off-by: Bin Meng <bmeng...@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...@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...@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...@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...@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...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot