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

[Simon Glass]

Hi Andy,

On 20 April 2017 at 03:41, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Andy,
>
> On Thu, Apr 20, 2017 at 3:23 PM, Andy Shevchenko
> <andy.shevchenko at gmail.com> wrote:
> > 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?
> >>
>
> 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 <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.
> >
>
> 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