Hi Bin,
On 5 December 2014 at 02:40, Bin Meng <bmeng...@gmail.com> wrote:
> Hi Simon,
>
> On Fri, Dec 5, 2014 at 7:48 AM, Simon Glass <s...@chromium.org> wrote:
>> Hi Bin,
>>
>> On 4 December 2014 at 08:02, 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>
>>> ---
>>> 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...@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...@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...@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...@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...@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...@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...@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...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
