Hi Andy,
On Thu, Apr 20, 2017 at 3:23 PM, Andy Shevchenko
<andy.shevche...@gmail.com> wrote:
> On Thu, Apr 20, 2017 at 5:59 AM, Bin Meng <bmeng...@gmail.com> wrote:
>> Hi Andy,
>>
>> On Tue, Apr 18, 2017 at 10:21 PM, Andy Shevchenko
>> <andriy.shevche...@linux.intel.com> wrote:
>>> From: Felipe Balbi <felipe.ba...@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.p...@intel.com>
>>> Dukjoon Jeon <dukjoon.j...@intel.com>
>>> eric.park <eric.p...@intel.com>
>>> Fabien Chereau <fabien.cher...@intel.com>
>>> Scott D Phillips <scott.d.phill...@intel.com>
>>> Sebastien Colleur <sebastienx.coll...@intel.com>
>>> Steve Sakoman <steve.sako...@intel.com>
>>> Vincent Tinelli <vincent.tine...@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?
>>
Can you confirm if my above comments are correct? If so, please add
some comments.
>
>>> 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 <stel...@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.
>
OK, so please split this patch into two, Leave the adding of
dma-mapping.h file to a single patch, and state that this is required
on some cross-platform drivers.
Regards,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
