[U-Boot] [PATCH v1] cpu: Add Intel Tangier support

[Andy Shevchenko]

On Thu, Apr 20, 2017 at 5:59 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Andy,
>
> On Tue, Apr 18, 2017 at 10:21 PM, Andy Shevchenko
> <andriy.shevchenko at linux.intel.com> wrote:
>> From: Felipe Balbi <felipe.balbi at linux.intel.com>
>>
>> Add Intel Tangier SoC support.
>>
>> Intel Tangier SoC is a core part of Intel Merrifield platform. For
>> example, Intel Edison board is based on such platform.
>>
>> The patch is based on work done by the following people (in alphabetical
>> order):
>>         Aiden Park <aiden.park at intel.com>
>>         Dukjoon Jeon <dukjoon.jeon at intel.com>
>>         eric.park <eric.park at intel.com>
>>         Fabien Chereau <fabien.chereau at intel.com>
>>         Scott D Phillips <scott.d.phillips at intel.com>
>>         Sebastien Colleur <sebastienx.colleur at intel.com>
>>         Steve Sakoman <steve.sakoman at intel.com>
>>         Vincent Tinelli <vincent.tinelli at intel.com>

>> +#include <common.h>
>> +#include <malloc.h>
>> +#include <asm/bootparam.h>
>> +#include <asm/e820.h>
>> +#include <asm/global_data.h>
>> +#include <asm/processor.h>
>> +#include <asm/sections.h>
>> +#include <asm/sfi.h>
>> +#include <asm/u-boot-x86.h>
>
> Can you double check if all these header files are needed here?

Will do.

>> +/* Memory type definitions */
>> +enum sfi_mem_type {
>> +       SFI_MEM_RESERVED,
>> +       SFI_LOADER_CODE,
>> +       SFI_LOADER_DATA,
>> +       SFI_BOOT_SERVICE_CODE,
>> +       SFI_BOOT_SERVICE_DATA,
>> +       SFI_RUNTIME_SERVICE_CODE,
>> +       SFI_RUNTIME_SERVICE_DATA,
>> +       SFI_MEM_CONV,
>> +       SFI_MEM_UNUSABLE,
>> +       SFI_ACPI_RECLAIM,
>> +       SFI_ACPI_NVS,
>> +       SFI_MEM_MMIO,
>> +       SFI_MEM_IOPORT,
>> +       SFI_PAL_CODE,
>> +       SFI_MEM_TYPEMAX,
>> +};
>> +
>
> Should this be moved to sfi.h?

If no one objects, I would do this.

>
>> +#define SFI_BASE_ADDR          0x000E0000
>> +#define SFI_LENGTH             0x00020000
>> +#define SFI_TABLE_LENGTH       16
>> +
>
> Can you add some comments here? I guess U-Boot on tangier is not the
> 1st stage bootloader. It boots from the 1st stage bootloader and get
> memory information via SFI table which 1st stage bootloader provides?
>

>> diff --git a/arch/x86/cpu/tangier/tangier.c b/arch/x86/cpu/tangier/tangier.c

>> diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h
>> new file mode 100644
>> index 0000000000..7de4c08e36
>> --- /dev/null
>> +++ b/arch/x86/include/asm/dma-mapping.h
>> @@ -0,0 +1,41 @@
>> +/*
>> + * (C) Copyright 2007
>> + * Stelian Pop <stelian at popies.net>
>> + * Lead Tech Design <www.leadtechdesign.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +#ifndef __ASM_X86_DMA_MAPPING_H
>> +#define __ASM_X86_DMA_MAPPING_H
>> +
>> +#define        dma_mapping_error(x, y) 0
>> +
>> +enum dma_data_direction {
>> +       DMA_BIDIRECTIONAL       = 0,
>> +       DMA_TO_DEVICE           = 1,
>> +       DMA_FROM_DEVICE         = 2,
>> +};
>> +
>> +static inline void *dma_alloc_coherent(size_t len, unsigned long *handle)
>> +{
>> +       *handle = (unsigned long)memalign(ARCH_DMA_MINALIGN, len);
>> +       return (void *)*handle;
>> +}
>> +
>> +static inline void dma_free_coherent(void *addr)
>> +{
>> +       free(addr);
>> +}
>> +
>> +static inline unsigned long dma_map_single(volatile void *vaddr, size_t len,
>> +                                          enum dma_data_direction dir)
>> +{
>> +       return (unsigned long)vaddr;
>> +}
>> +
>> +static inline void dma_unmap_single(volatile void *vaddr, size_t len,
>> +                                   unsigned long paddr)
>> +{
>> +}
>> +
>> +#endif /* __ASM_X86_DMA_MAPPING_H */
>> --
>
> Why is this dma-mapping.h file needed? For x86, all memory are
> coherent, which is indicated in your implementation as well
> (malloc/free are used)

dwc3 is used on many platforms and SoCs, so, it's irrelevant to x86.
Other option is to uglify dwc3 driver. I dunno it would be a good idea.

-- 
With Best Regards,
Andy Shevchenko