Hi Andy,
On 20 April 2017 at 03:41, Bin Meng <bmeng...@gmail.com> wrote:
>
> 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.
Andy, any update on this please? Is it still in progress?
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
