Hi Simon,
On Mon, Feb 2, 2015 at 12:30 AM, Simon Glass <s...@chromium.org> wrote:
> Hi Bin,
>
> On 29 January 2015 at 02:18, Bin Meng <bmeng...@gmail.com> wrote:
>> Add minimum codes to support Intel Quark SoC. DRAM initialization
>> is not ready yet so a hardcoded gd->ram_size is assigned.
>>
>> Signed-off-by: Bin Meng <bmeng...@gmail.com>
>>
>> ---
>>
>> Changes in v2:
>> - Use machine-specific
>> - Move vairous components' base addresses within Quark SoC to Kconfig
>> - Rebase to u-boot-86/master
>>
>
> All looks OK but for a few nits.
>
>> arch/x86/cpu/quark/Kconfig | 118
>> +++++++++++++++++++++++++++++++++
>> arch/x86/cpu/quark/Makefile | 8 +++
>> arch/x86/cpu/quark/dram.c | 39 +++++++++++
>> arch/x86/cpu/quark/pci.c | 70 +++++++++++++++++++
>> arch/x86/cpu/quark/quark.c | 44 ++++++++++++
>> arch/x86/include/asm/arch-quark/gpio.h | 13 ++++
>> 6 files changed, 292 insertions(+)
>> create mode 100644 arch/x86/cpu/quark/Kconfig
>> create mode 100644 arch/x86/cpu/quark/Makefile
>> create mode 100644 arch/x86/cpu/quark/dram.c
>> create mode 100644 arch/x86/cpu/quark/pci.c
>> create mode 100644 arch/x86/cpu/quark/quark.c
>> create mode 100644 arch/x86/include/asm/arch-quark/gpio.h
>>
>> diff --git a/arch/x86/cpu/quark/Kconfig b/arch/x86/cpu/quark/Kconfig
>> new file mode 100644
>> index 0000000..8bccf09
>> --- /dev/null
>> +++ b/arch/x86/cpu/quark/Kconfig
>> @@ -0,0 +1,118 @@
>> +#
>> +# Copyright (C) 2015, Bin Meng <bmeng...@gmail.com>
>> +#
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +
>> +config INTEL_QUARK
>> + bool
>> + select HAVE_RMU
>> +
>> +if INTEL_QUARK
>> +
>> +config HAVE_RMU
>> + bool "Add a Remote Management Unit (RMU) binary"
>> + help
>> + Select this option to add a Remote Management Unit (RMU) binary
>> + to the resulting U-Boot image. It is a data block (up to 64K) of
>> + machine-specific code which must be put in the flash for the RMU
>> + within the Quark SoC processor to access when powered up before
>> + system BIOS is executed.
>> +
>> +config RMU_FILE
>> + string "Remote Management Unit (RMU) binary filename"
>> + depends on HAVE_RMU
>> + default "rmu.bin"
>> + help
>> + The filename of the file to use as Remote Management Unit (RMU)
>> + binary in the board directory.
>> +
>> +config RMU_ADDR
>> + hex "Remote Management Unit (RMU) binary location"
>> + depends on HAVE_RMU
>> + default 0xfff00000
>> + help
>> + The location of the RMU binary is determined by a strap. It must be
>> + put in flash at a location matching the strap-determined base
>> address.
>> +
>> + The default base address of 0xfff00000 indicates that the binary
>> must
>> + be located at offset 0 from the beginning of a 1MB flash device.
>> +
>> +config HAVE_CMC
>> + bool
>> + default HAVE_RMU
>> +
>> +config CMC_FILE
>> + string
>> + depends on HAVE_CMC
>> + default RMU_FILE
>> +
>> +config CMC_ADDR
>> + hex
>> + depends on HAVE_CMC
>> + default RMU_ADDR
>> +
>> +config ESRAM_BASE
>> + hex
>> + default 0x80000000
>> + help
>> + Embedded SRAM (eSRAM) memory-mapped base address.
>> +
>> +config PCIE_ECAM_BASE
>> + hex
>> + default 0xe0000000
>> +
>> +config RCBA_BASE
>> + hex
>> + default 0xfed1c000
>> + help
>> + Root Complex register block memory-mapped base address.
>> +
>> +config ACPI_PM1_BASE
>> + hex
>> + default 0x1000
>> + help
>> + ACPI PM1 i/o-mapped base address.
>
> What is PM1?
This is defined in ACPI spec. Given U-Boot does not support ACPI yet,
and its full description is documented in the ACPI spec, I don't think
we need describe what it actually does here as this help is not a user
visible Kconfig option.
>> +
>> +config ACPI_P_BASE
>> + hex
>> + default 0x1010
>> + help
>> + ACPI PBLK i/o-mapped base address.
>
> What is PBLK?
Defined in ACPI spec. Maybe I should define this as ACPI_PBLK_BASE.
>> +
>> +config SPI_DMA_BASE
>> + hex
>> + default 0x1020
>> + help
>> + SPI DMA i/o-mapped base address.
>> +
>> +config GPIO_BASE
>> + hex
>> + default 0x1080
>> + help
>> + GPIO i/o-mapped base address.
>> +
>> +config GPE0_BASE
>> + hex
>> + default 0x1100
>> + help
>> + GPE0 i/o-mapped base address.
>
> What is GPE0?
Defined in ACPI spec. Maybe I should describe it as "ACPI GPE0", like above two.
>> +
>> +config WDT_BASE
>> + hex
>> + default 0x1140
>> + help
>> + Watchdog timer i/o-mapped base address.
>> +
>> +config SYS_CAR_ADDR
>> + hex
>> + default ESRAM_BASE
>> +
>> +config SYS_CAR_SIZE
>> + hex
>> + default 0x8000
>> + help
>> + Space in bytes in eSRAM used as Cache-As-ARM (CAR).
>> + Note this size must not exceed eSRAM's total size.
>> +
>> +endif
>> diff --git a/arch/x86/cpu/quark/Makefile b/arch/x86/cpu/quark/Makefile
>> new file mode 100644
>> index 0000000..168c1e6
>> --- /dev/null
>> +++ b/arch/x86/cpu/quark/Makefile
>> @@ -0,0 +1,8 @@
>> +#
>> +# Copyright (C) 2015, Bin Meng <bmeng...@gmail.com>
>> +#
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +
>> +obj-y += car.o dram.o msg_port.o quark.o
>> +obj-$(CONFIG_PCI) += pci.o
>> diff --git a/arch/x86/cpu/quark/dram.c b/arch/x86/cpu/quark/dram.c
>> new file mode 100644
>> index 0000000..fbdc3cd
>> --- /dev/null
>> +++ b/arch/x86/cpu/quark/dram.c
>> @@ -0,0 +1,39 @@
>> +/*
>> + * Copyright (C) 2015, Bin Meng <bmeng...@gmail.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/post.h>
>> +#include <asm/arch/quark.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +int dram_init(void)
>> +{
>> + /* hardcode the DRAM size for now */
>> + gd->ram_size = DRAM_MAX_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.
>
> This comment probably doesn't make sense here.
>
This comment was taken from other codes. So far since we don't have
MRC, it returns a hardcoded number. Do you think I need change this
for this patch and when we support MRC we can add this comment back?
[snip]
Regards,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
