All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
@ 2017-04-18 14:21 Andy Shevchenko
  2017-04-19  0:12 ` Simon Glass
  2017-04-20  2:59 ` Bin Meng
  0 siblings, 2 replies; 12+ messages in thread
From: Andy Shevchenko @ 2017-04-18 14:21 UTC (permalink / raw)
  To: u-boot

From: Felipe Balbi <felipe.balbi@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@intel.com>
	Dukjoon Jeon <dukjoon.jeon@intel.com>
	eric.park <eric.park@intel.com>
	Fabien Chereau <fabien.chereau@intel.com>
	Scott D Phillips <scott.d.phillips@intel.com>
	Sebastien Colleur <sebastienx.colleur@intel.com>
	Steve Sakoman <steve.sakoman@intel.com>
	Vincent Tinelli <vincent.tinelli@intel.com>

Signed-off-by: Vincent Tinelli <vincent.tinelli@intel.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/Kconfig                   |   1 +
 arch/x86/cpu/Makefile              |   1 +
 arch/x86/cpu/tangier/Kconfig       |  20 ++++
 arch/x86/cpu/tangier/Makefile      |   1 +
 arch/x86/cpu/tangier/car.S         |  13 +++
 arch/x86/cpu/tangier/sdram.c       | 234 +++++++++++++++++++++++++++++++++++++
 arch/x86/cpu/tangier/tangier.c     |  36 ++++++
 arch/x86/include/asm/dma-mapping.h |  41 +++++++
 8 files changed, 347 insertions(+)
 create mode 100644 arch/x86/cpu/tangier/Kconfig
 create mode 100644 arch/x86/cpu/tangier/Makefile
 create mode 100644 arch/x86/cpu/tangier/car.S
 create mode 100644 arch/x86/cpu/tangier/sdram.c
 create mode 100644 arch/x86/cpu/tangier/tangier.c
 create mode 100644 arch/x86/include/asm/dma-mapping.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9ead3ebccf..31f2d154d4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -114,6 +114,7 @@ source "arch/x86/cpu/ivybridge/Kconfig"
 source "arch/x86/cpu/qemu/Kconfig"
 source "arch/x86/cpu/quark/Kconfig"
 source "arch/x86/cpu/queensbay/Kconfig"
+source "arch/x86/cpu/tangier/Kconfig"
 
 # architecture-specific options below
 
diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile
index 92a9023b0b..6f3535d216 100644
--- a/arch/x86/cpu/Makefile
+++ b/arch/x86/cpu/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_QEMU) += qemu/
 obj-$(CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE) += ivybridge/
 obj-$(CONFIG_INTEL_QUARK) += quark/
 obj-$(CONFIG_INTEL_QUEENSBAY) += queensbay/
+obj-$(CONFIG_INTEL_TANGIER) += tangier/
 obj-y += lapic.o ioapic.o
 obj-y += irq.o
 ifndef CONFIG_$(SPL_)X86_64
diff --git a/arch/x86/cpu/tangier/Kconfig b/arch/x86/cpu/tangier/Kconfig
new file mode 100644
index 0000000000..3545886a65
--- /dev/null
+++ b/arch/x86/cpu/tangier/Kconfig
@@ -0,0 +1,20 @@
+#
+# Copyright (C) 2015 Intel, Inc
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+config INTEL_TANGIER
+	bool
+	depends on INTEL_MID
+
+config SYS_CAR_ADDR
+	hex
+	default 0x19200000
+
+config SYS_CAR_SIZE
+	hex
+	default 0x4000
+	help
+	  Space in bytes in eSRAM used as Cache-As-RAM (CAR).
+	  Note this size must not exceed eSRAM's total size.
diff --git a/arch/x86/cpu/tangier/Makefile b/arch/x86/cpu/tangier/Makefile
new file mode 100644
index 0000000000..d937737057
--- /dev/null
+++ b/arch/x86/cpu/tangier/Makefile
@@ -0,0 +1 @@
+obj-y += car.o tangier.o sdram.o
diff --git a/arch/x86/cpu/tangier/car.S b/arch/x86/cpu/tangier/car.S
new file mode 100644
index 0000000000..6982106c19
--- /dev/null
+++ b/arch/x86/cpu/tangier/car.S
@@ -0,0 +1,13 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * (C) Copyright 2010-2011
+ * Graeme Russ, <graeme.russ@gmail.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+.section .text
+
+.globl car_init
+car_init:
+	jmp	car_init_ret
diff --git a/arch/x86/cpu/tangier/sdram.c b/arch/x86/cpu/tangier/sdram.c
new file mode 100644
index 0000000000..da046559a1
--- /dev/null
+++ b/arch/x86/cpu/tangier/sdram.c
@@ -0,0 +1,234 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * (C) Copyright 2010,2011
+ * Graeme Russ, <graeme.russ@gmail.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#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>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* 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,
+};
+
+#define SFI_BASE_ADDR		0x000E0000
+#define SFI_LENGTH		0x00020000
+#define SFI_TABLE_LENGTH	16
+
+static int sfi_table_check(struct sfi_table_header *sbh)
+{
+	char chksum = 0;
+	char *pos = (char *)sbh;
+	int i;
+
+	if (sbh->len < SFI_TABLE_LENGTH)
+		return -1;
+
+	if (sbh->len > SFI_LENGTH)
+		return -1;
+
+	for (i = 0; i < sbh->len; i++)
+		chksum += *pos++;
+
+	if (chksum)
+		error("sfi: Invalid checksum\n");
+
+	/* Checksum is OK if zero */
+	return chksum;
+}
+
+static int sfi_table_is_type(struct sfi_table_header *sbh, const char *signature)
+{
+	return !strncmp(sbh->sig, signature, SFI_SIGNATURE_SIZE) &&
+	       !sfi_table_check(sbh);
+}
+
+static struct sfi_table_simple *sfi_get_table_by_sig(unsigned long addr,
+						     const char *signature)
+{
+	struct sfi_table_simple *sb = NULL;
+	int i;
+
+	for (i = 0; i < SFI_LENGTH; i += SFI_TABLE_LENGTH) {
+		sb = (struct sfi_table_simple *)(addr + i);
+		if (sfi_table_is_type(&sb->header, signature))
+			break;
+	}
+
+	if (i >= SFI_LENGTH || !sb)
+		return NULL;
+
+	return sb;
+}
+
+static struct sfi_table_simple *sfi_search_mmap(void)
+{
+	struct sfi_table_header *sbh;
+	struct sfi_table_simple *sb;
+	u32 sys_entry_cnt;
+	u32 i;
+
+	/* Find SYST table */
+	sb = sfi_get_table_by_sig(SFI_BASE_ADDR, SFI_SIG_SYST);
+	if (!sb) {
+		error("failed to locate SYST table\n");
+		return NULL;
+	}
+
+	sys_entry_cnt = (sb->header.len - sizeof(*sbh)) / 8;
+
+	/* Search through each SYST entry for MMAP table */
+	for (i = 0; i < sys_entry_cnt; i++) {
+		sbh = (struct sfi_table_header *)(unsigned long)sb->pentry[i];
+
+		if (sfi_table_is_type(sbh, SFI_SIG_MMAP))
+			return (struct sfi_table_simple *) sbh;
+	}
+
+	error("failed to locate SFI MMAP table\n");
+	return NULL;
+}
+
+#define sfi_for_each_mentry(i, sb, mentry)				\
+	for (i = 0, mentry = (struct sfi_mem_entry *)sb->pentry;	\
+	     i < SFI_GET_NUM_ENTRIES(sb, struct sfi_mem_entry);		\
+	     i++, mentry++)						\
+
+static unsigned sfi_setup_e820(unsigned max_entries, struct e820entry *entries)
+{
+	struct sfi_table_simple *sb;
+	struct sfi_mem_entry *mentry;
+	unsigned long long start, end, size;
+	int type, total = 0;
+	int i;
+
+	sb = sfi_search_mmap();
+	if (!sb)
+		return 0;
+
+	sfi_for_each_mentry(i, sb, mentry) {
+		start = mentry->phys_start;
+		size = mentry->pages << 12;
+		end = start + size;
+
+		if (start > end)
+			continue;
+
+		/* translate SFI mmap type to E820 map type */
+		switch (mentry->type) {
+		case SFI_MEM_CONV:
+			type = E820_RAM;
+			break;
+		case SFI_MEM_UNUSABLE:
+		case SFI_RUNTIME_SERVICE_DATA:
+			continue;
+		default:
+			type = E820_RESERVED;
+		}
+
+		if (total == E820MAX)
+			break;
+		entries[total].addr = start;
+		entries[total].size = size;
+		entries[total].type = type;
+
+		total++;
+	}
+
+	return total;
+}
+
+static int sfi_get_bank_size(void)
+{
+	struct sfi_table_simple *sb;
+	struct sfi_mem_entry *mentry;
+	int bank = 0;
+	int i;
+
+	sb = sfi_search_mmap();
+	if (!sb)
+		return -ENXIO;
+
+	sfi_for_each_mentry(i, sb, mentry) {
+		if (mentry->type != SFI_MEM_CONV)
+			continue;
+
+		gd->bd->bi_dram[bank].start = mentry->phys_start;
+		gd->bd->bi_dram[bank].size = mentry->pages << 12;
+		bank++;
+	}
+
+	return bank;
+}
+
+static phys_size_t sfi_get_ram_size(void)
+{
+	struct sfi_table_simple *sb;
+	struct sfi_mem_entry *mentry;
+	phys_size_t ram = 0;
+	int i;
+
+	sb = sfi_search_mmap();
+	if (!sb)
+		return 0;
+
+	sfi_for_each_mentry(i, sb, mentry) {
+		if (mentry->type != SFI_MEM_CONV)
+			continue;
+
+		ram += (mentry->pages << 12);
+	}
+
+	debug("RAM size %llu\n", ram);
+
+	return ram;
+}
+
+unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
+{
+	return sfi_setup_e820(max_entries, entries);
+}
+
+int dram_init_banksize(void)
+{
+	int ret;
+
+	ret = sfi_get_bank_size();
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+int dram_init(void)
+{
+	gd->ram_size = sfi_get_ram_size();
+
+	return 0;
+}
diff --git a/arch/x86/cpu/tangier/tangier.c b/arch/x86/cpu/tangier/tangier.c
new file mode 100644
index 0000000000..c06767b915
--- /dev/null
+++ b/arch/x86/cpu/tangier/tangier.c
@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * (C) Copyright 2008
+ * Graeme Russ, graeme.russ at gmail.com.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/scu.h>
+#include <asm/u-boot-x86.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * Miscellaneous platform dependent initializations
+ */
+int arch_cpu_init(void)
+{
+	return x86_cpu_init_f();
+}
+
+int checkcpu(void)
+{
+	return 0;
+}
+
+int print_cpuinfo(void)
+{
+	return default_print_cpuinfo();
+}
+
+void reset_cpu(ulong addr)
+{
+	scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
+}
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@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 */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-04-18 14:21 [U-Boot] [PATCH v1] cpu: Add Intel Tangier support Andy Shevchenko
@ 2017-04-19  0:12 ` Simon Glass
  2017-04-19 11:16   ` Andy Shevchenko
  2017-04-20  2:59 ` Bin Meng
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Glass @ 2017-04-19  0:12 UTC (permalink / raw)
  To: u-boot

On 18 April 2017 at 08:21, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> From: Felipe Balbi <felipe.balbi@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@intel.com>
>         Dukjoon Jeon <dukjoon.jeon@intel.com>
>         eric.park <eric.park@intel.com>
>         Fabien Chereau <fabien.chereau@intel.com>
>         Scott D Phillips <scott.d.phillips@intel.com>
>         Sebastien Colleur <sebastienx.colleur@intel.com>
>         Steve Sakoman <steve.sakoman@intel.com>
>         Vincent Tinelli <vincent.tinelli@intel.com>
>
> Signed-off-by: Vincent Tinelli <vincent.tinelli@intel.com>
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/Kconfig                   |   1 +
>  arch/x86/cpu/Makefile              |   1 +
>  arch/x86/cpu/tangier/Kconfig       |  20 ++++
>  arch/x86/cpu/tangier/Makefile      |   1 +
>  arch/x86/cpu/tangier/car.S         |  13 +++
>  arch/x86/cpu/tangier/sdram.c       | 234 +++++++++++++++++++++++++++++++++++++
>  arch/x86/cpu/tangier/tangier.c     |  36 ++++++
>  arch/x86/include/asm/dma-mapping.h |  41 +++++++
>  8 files changed, 347 insertions(+)
>  create mode 100644 arch/x86/cpu/tangier/Kconfig
>  create mode 100644 arch/x86/cpu/tangier/Makefile
>  create mode 100644 arch/x86/cpu/tangier/car.S
>  create mode 100644 arch/x86/cpu/tangier/sdram.c
>  create mode 100644 arch/x86/cpu/tangier/tangier.c
>  create mode 100644 arch/x86/include/asm/dma-mapping.h

Reviewed-by: Simon Glass <sjg@chromium.org>

Please check the chromium copyright though.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-04-19  0:12 ` Simon Glass
@ 2017-04-19 11:16   ` Andy Shevchenko
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2017-04-19 11:16 UTC (permalink / raw)
  To: u-boot

On Tue, 2017-04-18 at 18:12 -0600, Simon Glass wrote:
> On 18 April 2017 at 08:21, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > From: Felipe Balbi <felipe.balbi@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@intel.com>
> >         Dukjoon Jeon <dukjoon.jeon@intel.com>
> >         eric.park <eric.park@intel.com>
> >         Fabien Chereau <fabien.chereau@intel.com>
> >         Scott D Phillips <scott.d.phillips@intel.com>
> >         Sebastien Colleur <sebastienx.colleur@intel.com>
> >         Steve Sakoman <steve.sakoman@intel.com>
> >         Vincent Tinelli <vincent.tinelli@intel.com>
> > 
> > Signed-off-by: Vincent Tinelli <vincent.tinelli@intel.com>
> > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks for review.

> Please check the chromium copyright though.

Actually if you can give an advice what better to do here.

The code is based somehow on some code (origin of which I have never saw
since I have no clue what was the tree and version of it) from coreboot,
but Felipe and me rewrote a major part of it (I can evaluate that about
90% is rewritten / refactored).

So, would it be better to update copyrights, if yes, what could they
look like?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-04-18 14:21 [U-Boot] [PATCH v1] cpu: Add Intel Tangier support Andy Shevchenko
  2017-04-19  0:12 ` Simon Glass
@ 2017-04-20  2:59 ` Bin Meng
  2017-04-20  7:23   ` Andy Shevchenko
  1 sibling, 1 reply; 12+ messages in thread
From: Bin Meng @ 2017-04-20  2:59 UTC (permalink / raw)
  To: u-boot

Hi Andy,

On Tue, Apr 18, 2017 at 10:21 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> From: Felipe Balbi <felipe.balbi@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@intel.com>
>         Dukjoon Jeon <dukjoon.jeon@intel.com>
>         eric.park <eric.park@intel.com>
>         Fabien Chereau <fabien.chereau@intel.com>
>         Scott D Phillips <scott.d.phillips@intel.com>
>         Sebastien Colleur <sebastienx.colleur@intel.com>
>         Steve Sakoman <steve.sakoman@intel.com>
>         Vincent Tinelli <vincent.tinelli@intel.com>
>
> Signed-off-by: Vincent Tinelli <vincent.tinelli@intel.com>
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/Kconfig                   |   1 +
>  arch/x86/cpu/Makefile              |   1 +
>  arch/x86/cpu/tangier/Kconfig       |  20 ++++
>  arch/x86/cpu/tangier/Makefile      |   1 +
>  arch/x86/cpu/tangier/car.S         |  13 +++
>  arch/x86/cpu/tangier/sdram.c       | 234 +++++++++++++++++++++++++++++++++++++
>  arch/x86/cpu/tangier/tangier.c     |  36 ++++++
>  arch/x86/include/asm/dma-mapping.h |  41 +++++++
>  8 files changed, 347 insertions(+)
>  create mode 100644 arch/x86/cpu/tangier/Kconfig
>  create mode 100644 arch/x86/cpu/tangier/Makefile
>  create mode 100644 arch/x86/cpu/tangier/car.S
>  create mode 100644 arch/x86/cpu/tangier/sdram.c
>  create mode 100644 arch/x86/cpu/tangier/tangier.c
>  create mode 100644 arch/x86/include/asm/dma-mapping.h
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9ead3ebccf..31f2d154d4 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -114,6 +114,7 @@ source "arch/x86/cpu/ivybridge/Kconfig"
>  source "arch/x86/cpu/qemu/Kconfig"
>  source "arch/x86/cpu/quark/Kconfig"
>  source "arch/x86/cpu/queensbay/Kconfig"
> +source "arch/x86/cpu/tangier/Kconfig"
>
>  # architecture-specific options below
>
> diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile
> index 92a9023b0b..6f3535d216 100644
> --- a/arch/x86/cpu/Makefile
> +++ b/arch/x86/cpu/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_QEMU) += qemu/
>  obj-$(CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE) += ivybridge/
>  obj-$(CONFIG_INTEL_QUARK) += quark/
>  obj-$(CONFIG_INTEL_QUEENSBAY) += queensbay/
> +obj-$(CONFIG_INTEL_TANGIER) += tangier/
>  obj-y += lapic.o ioapic.o
>  obj-y += irq.o
>  ifndef CONFIG_$(SPL_)X86_64
> diff --git a/arch/x86/cpu/tangier/Kconfig b/arch/x86/cpu/tangier/Kconfig
> new file mode 100644
> index 0000000000..3545886a65
> --- /dev/null
> +++ b/arch/x86/cpu/tangier/Kconfig
> @@ -0,0 +1,20 @@
> +#
> +# Copyright (C) 2015 Intel, Inc
> +#
> +# SPDX-License-Identifier:     GPL-2.0+
> +#
> +
> +config INTEL_TANGIER
> +       bool
> +       depends on INTEL_MID
> +
> +config SYS_CAR_ADDR
> +       hex
> +       default 0x19200000
> +
> +config SYS_CAR_SIZE
> +       hex
> +       default 0x4000
> +       help
> +         Space in bytes in eSRAM used as Cache-As-RAM (CAR).
> +         Note this size must not exceed eSRAM's total size.
> diff --git a/arch/x86/cpu/tangier/Makefile b/arch/x86/cpu/tangier/Makefile
> new file mode 100644
> index 0000000000..d937737057
> --- /dev/null
> +++ b/arch/x86/cpu/tangier/Makefile
> @@ -0,0 +1 @@
> +obj-y += car.o tangier.o sdram.o
> diff --git a/arch/x86/cpu/tangier/car.S b/arch/x86/cpu/tangier/car.S
> new file mode 100644
> index 0000000000..6982106c19
> --- /dev/null
> +++ b/arch/x86/cpu/tangier/car.S
> @@ -0,0 +1,13 @@
> +/*
> + * Copyright (c) 2011 The Chromium OS Authors.
> + * (C) Copyright 2010-2011
> + * Graeme Russ, <graeme.russ@gmail.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +.section .text
> +
> +.globl car_init
> +car_init:
> +       jmp     car_init_ret
> diff --git a/arch/x86/cpu/tangier/sdram.c b/arch/x86/cpu/tangier/sdram.c
> new file mode 100644
> index 0000000000..da046559a1
> --- /dev/null
> +++ b/arch/x86/cpu/tangier/sdram.c
> @@ -0,0 +1,234 @@
> +/*
> + * Copyright (c) 2011 The Chromium OS Authors.
> + * (C) Copyright 2010,2011
> + * Graeme Russ, <graeme.russ@gmail.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#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?

> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/* 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?

> +#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?

> +static int sfi_table_check(struct sfi_table_header *sbh)
> +{
> +       char chksum = 0;
> +       char *pos = (char *)sbh;
> +       int i;
> +
> +       if (sbh->len < SFI_TABLE_LENGTH)
> +               return -1;
> +
> +       if (sbh->len > SFI_LENGTH)
> +               return -1;
> +
> +       for (i = 0; i < sbh->len; i++)
> +               chksum += *pos++;
> +
> +       if (chksum)
> +               error("sfi: Invalid checksum\n");
> +
> +       /* Checksum is OK if zero */
> +       return chksum;
> +}
> +
> +static int sfi_table_is_type(struct sfi_table_header *sbh, const char *signature)
> +{
> +       return !strncmp(sbh->sig, signature, SFI_SIGNATURE_SIZE) &&
> +              !sfi_table_check(sbh);
> +}
> +
> +static struct sfi_table_simple *sfi_get_table_by_sig(unsigned long addr,
> +                                                    const char *signature)
> +{
> +       struct sfi_table_simple *sb = NULL;
> +       int i;
> +
> +       for (i = 0; i < SFI_LENGTH; i += SFI_TABLE_LENGTH) {
> +               sb = (struct sfi_table_simple *)(addr + i);
> +               if (sfi_table_is_type(&sb->header, signature))
> +                       break;
> +       }
> +
> +       if (i >= SFI_LENGTH || !sb)
> +               return NULL;
> +
> +       return sb;
> +}
> +
> +static struct sfi_table_simple *sfi_search_mmap(void)
> +{
> +       struct sfi_table_header *sbh;
> +       struct sfi_table_simple *sb;
> +       u32 sys_entry_cnt;
> +       u32 i;
> +
> +       /* Find SYST table */
> +       sb = sfi_get_table_by_sig(SFI_BASE_ADDR, SFI_SIG_SYST);
> +       if (!sb) {
> +               error("failed to locate SYST table\n");
> +               return NULL;
> +       }
> +
> +       sys_entry_cnt = (sb->header.len - sizeof(*sbh)) / 8;
> +
> +       /* Search through each SYST entry for MMAP table */
> +       for (i = 0; i < sys_entry_cnt; i++) {
> +               sbh = (struct sfi_table_header *)(unsigned long)sb->pentry[i];
> +
> +               if (sfi_table_is_type(sbh, SFI_SIG_MMAP))
> +                       return (struct sfi_table_simple *) sbh;
> +       }
> +
> +       error("failed to locate SFI MMAP table\n");
> +       return NULL;
> +}
> +
> +#define sfi_for_each_mentry(i, sb, mentry)                             \
> +       for (i = 0, mentry = (struct sfi_mem_entry *)sb->pentry;        \
> +            i < SFI_GET_NUM_ENTRIES(sb, struct sfi_mem_entry);         \
> +            i++, mentry++)                                             \
> +
> +static unsigned sfi_setup_e820(unsigned max_entries, struct e820entry *entries)
> +{
> +       struct sfi_table_simple *sb;
> +       struct sfi_mem_entry *mentry;
> +       unsigned long long start, end, size;
> +       int type, total = 0;
> +       int i;
> +
> +       sb = sfi_search_mmap();
> +       if (!sb)
> +               return 0;
> +
> +       sfi_for_each_mentry(i, sb, mentry) {
> +               start = mentry->phys_start;
> +               size = mentry->pages << 12;
> +               end = start + size;
> +
> +               if (start > end)
> +                       continue;
> +
> +               /* translate SFI mmap type to E820 map type */
> +               switch (mentry->type) {
> +               case SFI_MEM_CONV:
> +                       type = E820_RAM;
> +                       break;
> +               case SFI_MEM_UNUSABLE:
> +               case SFI_RUNTIME_SERVICE_DATA:
> +                       continue;
> +               default:
> +                       type = E820_RESERVED;
> +               }
> +
> +               if (total == E820MAX)
> +                       break;
> +               entries[total].addr = start;
> +               entries[total].size = size;
> +               entries[total].type = type;
> +
> +               total++;
> +       }
> +
> +       return total;
> +}
> +
> +static int sfi_get_bank_size(void)
> +{
> +       struct sfi_table_simple *sb;
> +       struct sfi_mem_entry *mentry;
> +       int bank = 0;
> +       int i;
> +
> +       sb = sfi_search_mmap();
> +       if (!sb)
> +               return -ENXIO;
> +
> +       sfi_for_each_mentry(i, sb, mentry) {
> +               if (mentry->type != SFI_MEM_CONV)
> +                       continue;
> +
> +               gd->bd->bi_dram[bank].start = mentry->phys_start;
> +               gd->bd->bi_dram[bank].size = mentry->pages << 12;
> +               bank++;
> +       }
> +
> +       return bank;
> +}
> +
> +static phys_size_t sfi_get_ram_size(void)
> +{
> +       struct sfi_table_simple *sb;
> +       struct sfi_mem_entry *mentry;
> +       phys_size_t ram = 0;
> +       int i;
> +
> +       sb = sfi_search_mmap();
> +       if (!sb)
> +               return 0;
> +
> +       sfi_for_each_mentry(i, sb, mentry) {
> +               if (mentry->type != SFI_MEM_CONV)
> +                       continue;
> +
> +               ram += (mentry->pages << 12);
> +       }
> +
> +       debug("RAM size %llu\n", ram);
> +
> +       return ram;
> +}
> +
> +unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
> +{
> +       return sfi_setup_e820(max_entries, entries);
> +}
> +
> +int dram_init_banksize(void)
> +{
> +       int ret;
> +
> +       ret = sfi_get_bank_size();
> +       if (ret < 0)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +int dram_init(void)
> +{
> +       gd->ram_size = sfi_get_ram_size();
> +
> +       return 0;
> +}
> diff --git a/arch/x86/cpu/tangier/tangier.c b/arch/x86/cpu/tangier/tangier.c
> new file mode 100644
> index 0000000000..c06767b915
> --- /dev/null
> +++ b/arch/x86/cpu/tangier/tangier.c
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright (c) 2011 The Chromium OS Authors.
> + * (C) Copyright 2008
> + * Graeme Russ, graeme.russ at gmail.com.
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <asm/scu.h>
> +#include <asm/u-boot-x86.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/*
> + * Miscellaneous platform dependent initializations
> + */
> +int arch_cpu_init(void)
> +{
> +       return x86_cpu_init_f();
> +}
> +
> +int checkcpu(void)
> +{
> +       return 0;
> +}
> +
> +int print_cpuinfo(void)
> +{
> +       return default_print_cpuinfo();
> +}
> +
> +void reset_cpu(ulong addr)
> +{
> +       scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
> +}
> 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@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)

Regards,
Bin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-04-20  2:59 ` Bin Meng
@ 2017-04-20  7:23   ` Andy Shevchenko
  2017-04-20  9:41     ` Bin Meng
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2017-04-20  7:23 UTC (permalink / raw)
  To: u-boot

On Thu, Apr 20, 2017 at 5:59 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Andy,
>
> On Tue, Apr 18, 2017 at 10:21 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>> From: Felipe Balbi <felipe.balbi@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@intel.com>
>>         Dukjoon Jeon <dukjoon.jeon@intel.com>
>>         eric.park <eric.park@intel.com>
>>         Fabien Chereau <fabien.chereau@intel.com>
>>         Scott D Phillips <scott.d.phillips@intel.com>
>>         Sebastien Colleur <sebastienx.colleur@intel.com>
>>         Steve Sakoman <steve.sakoman@intel.com>
>>         Vincent Tinelli <vincent.tinelli@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@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-04-20  7:23   ` Andy Shevchenko
@ 2017-04-20  9:41     ` Bin Meng
  2017-06-20 18:51       ` Simon Glass
  2017-07-05 20:06       ` Andy Shevchenko
  0 siblings, 2 replies; 12+ messages in thread
From: Bin Meng @ 2017-04-20  9:41 UTC (permalink / raw)
  To: u-boot

Hi Andy,

On Thu, Apr 20, 2017 at 3:23 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Apr 20, 2017 at 5:59 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Andy,
>>
>> On Tue, Apr 18, 2017 at 10:21 PM, Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>>> From: Felipe Balbi <felipe.balbi@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@intel.com>
>>>         Dukjoon Jeon <dukjoon.jeon@intel.com>
>>>         eric.park <eric.park@intel.com>
>>>         Fabien Chereau <fabien.chereau@intel.com>
>>>         Scott D Phillips <scott.d.phillips@intel.com>
>>>         Sebastien Colleur <sebastienx.colleur@intel.com>
>>>         Steve Sakoman <steve.sakoman@intel.com>
>>>         Vincent Tinelli <vincent.tinelli@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@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-04-20  9:41     ` Bin Meng
@ 2017-06-20 18:51       ` Simon Glass
  2017-06-20 19:02         ` Andy Shevchenko
  2017-07-05 20:06       ` Andy Shevchenko
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Glass @ 2017-06-20 18:51 UTC (permalink / raw)
  To: u-boot

Hi Andy,

On 20 April 2017 at 03:41, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Andy,
>
> On Thu, Apr 20, 2017 at 3:23 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Apr 20, 2017 at 5:59 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> >> Hi Andy,
> >>
> >> On Tue, Apr 18, 2017 at 10:21 PM, Andy Shevchenko
> >> <andriy.shevchenko@linux.intel.com> wrote:
> >>> From: Felipe Balbi <felipe.balbi@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@intel.com>
> >>>         Dukjoon Jeon <dukjoon.jeon@intel.com>
> >>>         eric.park <eric.park@intel.com>
> >>>         Fabien Chereau <fabien.chereau@intel.com>
> >>>         Scott D Phillips <scott.d.phillips@intel.com>
> >>>         Sebastien Colleur <sebastienx.colleur@intel.com>
> >>>         Steve Sakoman <steve.sakoman@intel.com>
> >>>         Vincent Tinelli <vincent.tinelli@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@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-06-20 18:51       ` Simon Glass
@ 2017-06-20 19:02         ` Andy Shevchenko
  2017-06-20 19:35           ` Simon Glass
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2017-06-20 19:02 UTC (permalink / raw)
  To: u-boot

On Tue, Jun 20, 2017 at 9:51 PM, Simon Glass <sjg@chromium.org> wrote:
> On 20 April 2017 at 03:41, Bin Meng <bmeng.cn@gmail.com> wrote:

> Andy, any update on this please? Is it still in progress?

Hi, yes, it is still in progress.
The issue is I have not much time to look at it.

Can you remind me what is the summary regarding power suppliers?

From hardware point of view there no regulators wrt SDHCI host
controllers, only some let's say magic bits that needs to be set or
clear.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-06-20 19:02         ` Andy Shevchenko
@ 2017-06-20 19:35           ` Simon Glass
  2017-07-05 19:27             ` Andy Shevchenko
  0 siblings, 1 reply; 12+ messages in thread
From: Simon Glass @ 2017-06-20 19:35 UTC (permalink / raw)
  To: u-boot

Hi Andy,

On 20 June 2017 at 13:02, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Tue, Jun 20, 2017 at 9:51 PM, Simon Glass <sjg@chromium.org> wrote:
>> On 20 April 2017 at 03:41, Bin Meng <bmeng.cn@gmail.com> wrote:
>
>> Andy, any update on this please? Is it still in progress?
>
> Hi, yes, it is still in progress.
> The issue is I have not much time to look at it.

OK thanks.

> Can you remind me what is the summary regarding power suppliers?
>
> From hardware point of view there no regulators wrt SDHCI host
> controllers, only some let's say magic bits that needs to be set or
> clear.

The issue is just that we cannot call from drivers into board code. If
you don't want to model it as a regulator perhaps you could call into
code in arch/arm/cpu/... to flip the bits?

Regards,
Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-06-20 19:35           ` Simon Glass
@ 2017-07-05 19:27             ` Andy Shevchenko
  2017-07-05 19:34               ` Simon Glass
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2017-07-05 19:27 UTC (permalink / raw)
  To: u-boot

On Tue, 2017-06-20 at 13:35 -0600, Simon Glass wrote:
> Hi Andy,
> 
> On 20 June 2017 at 13:02, Andy Shevchenko <andy.shevchenko@gmail.com>
> wrote:
> > On Tue, Jun 20, 2017 at 9:51 PM, Simon Glass <sjg@chromium.org>
> > wrote:
> > > On 20 April 2017 at 03:41, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > Andy, any update on this please? Is it still in progress?
> > 
> > Hi, yes, it is still in progress.
> > The issue is I have not much time to look at it.
> 
> OK thanks.

I'm about to send new version.
Though, there is one thing to be clear of...

> 
> > Can you remind me what is the summary regarding power suppliers?
> > 
> > From hardware point of view there no regulators wrt SDHCI host
> > controllers, only some let's say magic bits that needs to be set or
> > clear.
> 
> The issue is just that we cannot call from drivers into board code. If
> you don't want to model it as a regulator 

Okay, meaning there is no consensus, U-Boot (as I see currently) is
broken in few ways, one of them is ignoring hardware and proposing to
emulate something which certain board doesn't have. That's not how
things work.

Linux kernel is not the same in terms of DM in this case. In Linux this
board (and SDHCI controllers) is represented differently (by emulating
PCI programming interface). And there the PM code is much more
complicated than here.

> perhaps you could call into
> code in arch/arm/cpu/... to flip the bits?

(x86)

It was at very beginning like that, but since SCU and PMU are
represented as system controllers (which they indeed are), the calling
to them from CPU code would be hackish.

So, the only way I see now is to disable SD card slot on that board
blaming U-Boot upstream DM design for that.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-07-05 19:27             ` Andy Shevchenko
@ 2017-07-05 19:34               ` Simon Glass
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2017-07-05 19:34 UTC (permalink / raw)
  To: u-boot

Hi Andy,

On 5 July 2017 at 13:27, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Tue, 2017-06-20 at 13:35 -0600, Simon Glass wrote:
>> Hi Andy,
>>
>> On 20 June 2017 at 13:02, Andy Shevchenko <andy.shevchenko@gmail.com>
>> wrote:
>> > On Tue, Jun 20, 2017 at 9:51 PM, Simon Glass <sjg@chromium.org>
>> > wrote:
>> > > On 20 April 2017 at 03:41, Bin Meng <bmeng.cn@gmail.com> wrote:
>> > > Andy, any update on this please? Is it still in progress?
>> >
>> > Hi, yes, it is still in progress.
>> > The issue is I have not much time to look at it.
>>
>> OK thanks.
>
> I'm about to send new version.
> Though, there is one thing to be clear of...
>
>>
>> > Can you remind me what is the summary regarding power suppliers?
>> >
>> > From hardware point of view there no regulators wrt SDHCI host
>> > controllers, only some let's say magic bits that needs to be set or
>> > clear.
>>
>> The issue is just that we cannot call from drivers into board code. If
>> you don't want to model it as a regulator
>
> Okay, meaning there is no consensus, U-Boot (as I see currently) is
> broken in few ways, one of them is ignoring hardware and proposing to
> emulate something which certain board doesn't have. That's not how
> things work.
>
> Linux kernel is not the same in terms of DM in this case. In Linux this
> board (and SDHCI controllers) is represented differently (by emulating
> PCI programming interface). And there the PM code is much more
> complicated than here.
>
>> perhaps you could call into
>> code in arch/arm/cpu/... to flip the bits?
>
> (x86)
>
> It was at very beginning like that, but since SCU and PMU are
> represented as system controllers (which they indeed are), the calling
> to them from CPU code would be hackish.
>
> So, the only way I see now is to disable SD card slot on that board
> blaming U-Boot upstream DM design for that.

I don't think this is a super-hard problem. We have resolved much more
tricky ones, so don't despair!

I suggest you send the driver (but disabled) with a comment indicating
where the problem is. We can get it applied and then we can discuss
separately what new infrastructure is needed to resolve this. It has
come up on several threads and I have a few ideas on what we could do.

Regards,
Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v1] cpu: Add Intel Tangier support
  2017-04-20  9:41     ` Bin Meng
  2017-06-20 18:51       ` Simon Glass
@ 2017-07-05 20:06       ` Andy Shevchenko
  1 sibling, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2017-07-05 20:06 UTC (permalink / raw)
  To: u-boot

On Thu, 2017-04-20 at 17:41 +0800, Bin Meng wrote:
> On Thu, Apr 20, 2017 at 3:23 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Apr 20, 2017 at 5:59 AM, Bin Meng <bmeng.cn@gmail.com> 
> > > On Tue, Apr 18, 2017 at 10:21 PM, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:

> > > > +#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.
> 

I didn't get what you are expecting to see here, what kind of comments?

SFI tables are located in first stage boot ROM / "BIOS" (not exactly,
but it's a good approximation for U-Boot). Above is just a range in a
memory to search for them (like some one may do using /dev/mem in
Linux).


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-07-05 20:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 14:21 [U-Boot] [PATCH v1] cpu: Add Intel Tangier support Andy Shevchenko
2017-04-19  0:12 ` Simon Glass
2017-04-19 11:16   ` Andy Shevchenko
2017-04-20  2:59 ` Bin Meng
2017-04-20  7:23   ` Andy Shevchenko
2017-04-20  9:41     ` Bin Meng
2017-06-20 18:51       ` Simon Glass
2017-06-20 19:02         ` Andy Shevchenko
2017-06-20 19:35           ` Simon Glass
2017-07-05 19:27             ` Andy Shevchenko
2017-07-05 19:34               ` Simon Glass
2017-07-05 20:06       ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.