[U-Boot] [PATCH 17/25] x86: Add basic support to queensbay platform and crownbay board

Simon Glass sjg at chromium.org
Fri Dec 5 16:17:05 CET 2014


Hi Bin,

On 5 December 2014 at 02:40, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Fri, Dec 5, 2014 at 7:48 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> On 4 December 2014 at 08:02, 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>
>>> ---
>>>  arch/x86/cpu/queensbay/Makefile   |  9 +++++
>>>  arch/x86/cpu/queensbay/tnc.c      | 48 +++++++++++++++++++++++
>>>  arch/x86/cpu/queensbay/tnc_car.S  | 75 ++++++++++++++++++++++++++++++++++++
>>>  arch/x86/cpu/queensbay/tnc_dram.c | 81 +++++++++++++++++++++++++++++++++++++++
>>>  arch/x86/cpu/queensbay/tnc_pci.c  | 62 ++++++++++++++++++++++++++++++
>>>  board/intel/crownbay/MAINTAINERS  |  6 +++
>>>  board/intel/crownbay/Makefile     |  7 ++++
>>>  board/intel/crownbay/crownbay.c   | 20 ++++++++++
>>>  board/intel/crownbay/start.S      |  9 +++++
>>>  9 files changed, 317 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..c0d19aa
>>> --- /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>
>>> +
>>> +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, 0xcf9);
>>
>> I think there is a constant somewhere for cf9?
>
> Yes, I found one in arch/x86/include/asm/processor.h and will use that.
>
>>> +}
>>> +
>>> +void board_final_cleanup(void)
>>> +{
>>> +       EFI_STATUS status;
>>> +
>>> +       /* call into FspNotify */
>>
>> Remove blank line
>
> OK.
>
>>> +
>>> +       debug("Calling into FSP (notify phase EnumInitPhaseReadyToBoot): ");
>>> +       status = FspNotifyWrapper(NULL, EnumInitPhaseReadyToBoot);
>>
>> Are we stuck with these mixed case identifiers?
>
> I may have to fix FSP support library coding style issues in v2.
>
>>> +       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..722de7f
>>> --- /dev/null
>>> +++ b/arch/x86/cpu/queensbay/tnc_car.S
>>> @@ -0,0 +1,75 @@
>>> +/*
>>> + * 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
>>
>> Is there no way to save it anywhere?
>
> Unfortunately I did not find a way to save the ebp through the FSP
> call so far. The FSP documentation mentions that FSP tries to save
> bootloader's stack before call to FspInit to a temporary memory block
> after the call, however I found it is not working as documented. I may
> look into this in the future, but for now I think we have to let it
> be.

Not great, but score 1 for the binary blobs.

>
>>> +
>>> +car_init_start:
>>> +       post_code(POST_CAR_START)
>>> +       lea     find_fsp_header_stack, %esp
>>> +       jmp     FindFspHeader
>>> +
>>> +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
>>> +       post_code(POST_CAR_FAILURE)
>>> +
>>> +die:
>>> +       hlt
>>> +       jmp     die
>>> +       hlt
>>> +
>>> +continue:
>>> +       post_code(POST_CAR_CPU_CACHE)
>>> +       jmp     car_init_ret
>>> +
>>> +       .balign 4
>>> +find_fsp_header_stack:
>>> +       .long   find_fsp_header_ret
>>> +
>>> +       .balign 4
>>> +temp_ram_init_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"
>>
>> Does the microcode have to be applied before we even have temporary RAM?
>
> Unfortunately the microcode is required to be a parameter of the call
> to TempRamInit, which is defined by the FSP specification.
>
>>
>>> +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..f7b9f19
>>> --- /dev/null
>>> +++ b/arch/x86/cpu/queensbay/tnc_dram.c
>>> @@ -0,0 +1,81 @@
>>> +/*
>>> + * 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_LIST(hob)) {
>>> +               if (hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
>>> +                       if (hob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY ||
>>> +                           hob.ResourceDescriptor->ResourceType == EFI_RESOURCE_MEMORY_RESERVED) {
>>> +                               ram_size += hob.ResourceDescriptor->ResourceLength;
>>> +                       }
>>> +               }
>>> +               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;
>>> +}
>>> +
>>> +/*
>>> + * 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 GetUsableLowMemTop(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_LIST(hob)) {
>>> +               if (hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
>>> +                       if (hob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) {
>>> +                               entries[num_entries].addr = hob.ResourceDescriptor->PhysicalStart;
>>
>> Can we make these identifiers follow style?
>
> I may have to fix FSP support library coding style issues in v2.
>
>>> +                               entries[num_entries].size = hob.ResourceDescriptor->ResourceLength;
>>> +                               entries[num_entries].type = E820_RAM;
>>> +                       } else if (hob.ResourceDescriptor->ResourceType == EFI_RESOURCE_MEMORY_RESERVED) {
>>> +                               entries[num_entries].addr = hob.ResourceDescriptor->PhysicalStart;
>>> +                               entries[num_entries].size = hob.ResourceDescriptor->ResourceLength;
>>> +                               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..13d4691
>>> --- /dev/null
>>> +++ b/arch/x86/cpu/queensbay/tnc_pci.c
>>> @@ -0,0 +1,62 @@
>>> +/*
>>> + * 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;
>>> +
>>> +       /* call into FspNotify */
>>
>> Remove blank line
>
> OK.
>
>>> +
>>> +       debug("Calling into FSP (notify phase EnumInitPhaseAfterPciEnumeration): ");
>>> +       status = FspNotifyWrapper(NULL, EnumInitPhaseAfterPciEnumeration);
>>> +       if (status != FSP_SUCCESS)
>>> +               debug("fail, error code %x\n", status);
>>> +       else
>>> +               debug("OK\n");
>>> +
>>> +       return 0;
>>> +}
>>> diff --git a/board/intel/crownbay/MAINTAINERS b/board/intel/crownbay/MAINTAINERS
>>> new file mode 100644
>>> index 0000000..1eb6869
>>> --- /dev/null
>>> +++ b/board/intel/crownbay/MAINTAINERS
>>> @@ -0,0 +1,6 @@
>>> +INTEL CROWNBAY BOARD
>>> +M:     Bin Meng <bmeng.cn at gmail.com>
>>> +S:     Maintained
>>> +F:     board/intel/crownbay/
>>> +F:     include/configs/crownbay.h
>>> +F:     configs/crownbay_defconfig
>>> diff --git a/board/intel/crownbay/Makefile b/board/intel/crownbay/Makefile
>>> new file mode 100644
>>> index 0000000..aeb219b
>>> --- /dev/null
>>> +++ b/board/intel/crownbay/Makefile
>>> @@ -0,0 +1,7 @@
>>> +#
>>> +# Copyright (C) 2014, Bin Meng <bmeng.cn at gmail.com>
>>> +#
>>> +# SPDX-License-Identifier:     GPL-2.0+
>>> +#
>>> +
>>> +obj-y  += crownbay.o start.o
>>> diff --git a/board/intel/crownbay/crownbay.c b/board/intel/crownbay/crownbay.c
>>> new file mode 100644
>>> index 0000000..bbd2596
>>> --- /dev/null
>>> +++ b/board/intel/crownbay/crownbay.c
>>> @@ -0,0 +1,20 @@
>>> +/*
>>> + * Copyright (C) 2014, Bin Meng <bmeng.cn at gmail.com>
>>> + *
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <asm/pnp_def.h>
>>> +#include <smsc_lpc47m.h>
>>> +
>>> +#define SERIAL_DEV PNP_DEV(0x2e, 4)
>>> +#define UART0_BASE 0x3f8
>>
>> I think there is a constant for this somewhere already?
>
> It was once defined in arch/x86/include/asm/ibmpc.h, however it was
> deleted by commit e98a03c "dm: x86: Convert coreboot serial to use
> driver model". Should I add that back?

Not thrilled with the filename ibmpc.h. If you think of a better name
please rename it. in any case yes please add it back...

>
>>> +
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +int board_early_init_f(void)
>>> +{
>>> +       lpc47m_enable_serial(SERIAL_DEV, UART0_BASE);
>>
>> blank line before return. Why do we need to do this so early?
>>
>
> board_early_init_f() is the last routine which can be used to do such
> thing before call to serial_init().

OK.

Regards,
Simon


More information about the U-Boot mailing list