[U-Boot] [PATCH v2 0/7] x86: Support pci based uart as the U-Boot serial console

[U-Boot] [PATCH v2 0/7] x86: Support pci based uart as the U-Boot serial console

[Bin Meng]

This series add support to the ns16550 compatible pci devices.

Newer x86 Platform Controller Hub chipset (like Topcliff, BayTrail)
starts to integrate ns16550 compatible pci uart devices. In order to
use them, we have to scan the pci bus and allocate memory/io address
in the early phase. A gd->hose is added to save the pci bus controller
hose in the early phase so that pci apis can be used.

On Intel Crown Bay board, there are 4 DB9 connectors, one of which
is from the superio legacy serial port and the other 3 are connected
to the Topcliff PCH UART devices. In order to use them as the U-Boot
serial console, we need describe those devices in the board's dts
file per Open Firmware PCI bus bindings and specify it as the console
via the 'stdout-path' in the chosen node. Several APIs are added in
fdtdec.c to provide help for decoding the pci device nodes.

Changes in v2:
- Add a commit message
- New patch to make pci apis usable before relocation
- New patch to add several apis to decode pci device node
- New patch to support ns16550 compatible pci uart devices
- New patch to use ePAPR defined properties for x86-uart
- New patch to add pci devices in crownbay.dts
- Drop v1 patch: Add an API for finding pci devices in the early phase
- Drop v1 patch: Support PCI UART in the x86_serial driver
- Drop v1 patch: Add PCI UART related defines in crownbay.h

Bin Meng (7):
  x86: Add missing DECLARE_GLOBAL_DATA_PTR for pci.c
  x86: Support pci bus scan in the early phase
  pci: Make pci apis usable before relocation
  fdt: Add several apis to decode pci device node
  serial: ns16550: Support ns16550 compatible pci uart devices
  x86: Use ePAPR defined properties for x86-uart
  x86: crownbay: Add pci devices in the dts file

 arch/x86/cpu/pci.c                 |  11 ++-
 arch/x86/dts/crownbay.dts          |  81 +++++++++++++++++++
 arch/x86/dts/serial.dtsi           |   5 +-
 arch/x86/include/asm/global_data.h |   1 -
 arch/x86/include/asm/pci.h         |   2 +-
 drivers/pci/pci.c                  |  25 ++++--
 drivers/serial/ns16550.c           |  33 +++++++-
 drivers/serial/serial_x86.c        |   8 +-
 include/asm-generic/global_data.h  |   6 ++
 include/fdtdec.h                   | 108 ++++++++++++++++++++++---
 lib/fdtdec.c                       | 157 +++++++++++++++++++++++++++++++++----
 11 files changed, 393 insertions(+), 44 deletions(-)

-- 
1.8.2.1

[U-Boot] [PATCH v2 1/7] x86: Add missing DECLARE_GLOBAL_DATA_PTR for pci.c

[Bin Meng]

arch/x86/cpu/pci.c has access to the U-Boot global data thus
DECLARE_GLOBAL_DATA_PTR is needed.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- Add a commit message

 arch/x86/cpu/pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c
index f3492c3..404fbb6 100644
--- a/arch/x86/cpu/pci.c
+++ b/arch/x86/cpu/pci.c
@@ -15,6 +15,8 @@
 #include <pci.h>
 #include <asm/pci.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static struct pci_controller x86_hose;
 
 int pci_early_init_hose(struct pci_controller **hosep)
-- 
1.8.2.1

[U-Boot] [PATCH v2 2/7] x86: Support pci bus scan in the early phase

[Bin Meng]

On x86, some peripherals on pci buses need to be accessed in the
early phase (eg: pci uart) with a valid pci memory/io address,
thus scan the pci bus and do the corresponding resource allocation.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

Changes in v2: None

 arch/x86/cpu/pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c
index 404fbb6..1eee08b 100644
--- a/arch/x86/cpu/pci.c
+++ b/arch/x86/cpu/pci.c
@@ -29,6 +29,7 @@ int pci_early_init_hose(struct pci_controller **hosep)
 
 	board_pci_setup_hose(hose);
 	pci_setup_type1(hose);
+	hose->last_busno = pci_hose_scan(hose);
 	gd->arch.hose = hose;
 	*hosep = hose;
 
-- 
1.8.2.1

[U-Boot] [PATCH v2 3/7] pci: Make pci apis usable before relocation

[Bin Meng]

Introduce a gd->hose to save the pci hose in the early phase so that
apis in drivers/pci/pci.c can be used before relocation. Architecture
codes need assign a valid gd->hose in the early phase.

Some variables are declared as static so change them to be either
stack variable or global data member so that they can be used before
relocation, except the 'indent' used by CONFIG_PCI_SCAN_SHOW which
just affects some print format.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- New patch to make pci apis usable before relocation

 arch/x86/cpu/pci.c                 |  8 ++++----
 arch/x86/include/asm/global_data.h |  1 -
 arch/x86/include/asm/pci.h         |  2 +-
 drivers/pci/pci.c                  | 25 +++++++++++++++++--------
 include/asm-generic/global_data.h  |  6 ++++++
 5 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c
index 1eee08b..ab1aaaa 100644
--- a/arch/x86/cpu/pci.c
+++ b/arch/x86/cpu/pci.c
@@ -30,7 +30,7 @@ int pci_early_init_hose(struct pci_controller **hosep)
 	board_pci_setup_hose(hose);
 	pci_setup_type1(hose);
 	hose->last_busno = pci_hose_scan(hose);
-	gd->arch.hose = hose;
+	gd->hose = hose;
 	*hosep = hose;
 
 	return 0;
@@ -51,7 +51,7 @@ void pci_init_board(void)
 	struct pci_controller *hose = &x86_hose;
 
 	/* Stop using the early hose */
-	gd->arch.hose = NULL;
+	gd->hose = NULL;
 
 	board_pci_setup_hose(hose);
 	pci_setup_type1(hose);
@@ -64,8 +64,8 @@ void pci_init_board(void)
 
 static struct pci_controller *get_hose(void)
 {
-	if (gd->arch.hose)
-		return gd->arch.hose;
+	if (gd->hose)
+		return gd->hose;
 
 	return pci_bus_to_hose(0);
 }
diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
index 03d491a..aeab3e5 100644
--- a/arch/x86/include/asm/global_data.h
+++ b/arch/x86/include/asm/global_data.h
@@ -43,7 +43,6 @@ struct arch_global_data {
 	uint32_t tsc_mhz;		/* TSC frequency in MHz */
 	void *new_fdt;			/* Relocated FDT */
 	uint32_t bist;			/* Built-in self test value */
-	struct pci_controller *hose;	/* PCI hose for early use */
 	enum pei_boot_mode_t pei_boot_mode;
 	const struct pch_gpio_map *gpio_map;	/* board GPIO map */
 	struct memory_info meminfo;	/* Memory information */
diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index ac1a808..c30dd4c 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -29,7 +29,7 @@ void board_pci_setup_hose(struct pci_controller *hose);
  * pci_early_init_hose() - Set up PCI host before relocation
  *
  * This allocates memory for, sets up and returns the PCI hose. It can be
- * called before relocation. The hose will be stored in gd->arch.hose for
+ * called before relocation. The hose will be stored in gd->hose for
  * later use, but will become invalid one DRAM is available.
  */
 int pci_early_init_hose(struct pci_controller **hosep);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 3daf73c..83fd9a0 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -19,6 +19,8 @@
 #include <asm/io.h>
 #include <pci.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 #define PCI_HOSE_OP(rw, size, type)					\
 int pci_hose_##rw##_config_##size(struct pci_controller *hose,		\
 				  pci_dev_t dev,			\
@@ -123,6 +125,14 @@ void *pci_map_bar(pci_dev_t pdev, int bar, int flags)
 
 static struct pci_controller* hose_head;
 
+struct pci_controller *pci_get_hose_head(void)
+{
+	if (gd->hose)
+		return gd->hose;
+
+	return hose_head;
+}
+
 void pci_register_hose(struct pci_controller* hose)
 {
 	struct pci_controller **phose = &hose_head;
@@ -139,7 +149,7 @@ struct pci_controller *pci_bus_to_hose(int bus)
 {
 	struct pci_controller *hose;
 
-	for (hose = hose_head; hose; hose = hose->next) {
+	for (hose = pci_get_hose_head(); hose; hose = hose->next) {
 		if (bus >= hose->first_busno && bus <= hose->last_busno)
 			return hose;
 	}
@@ -152,7 +162,7 @@ struct pci_controller *find_hose_by_cfg_addr(void *cfg_addr)
 {
 	struct pci_controller *hose;
 
-	for (hose = hose_head; hose; hose = hose->next) {
+	for (hose = pci_get_hose_head(); hose; hose = hose->next) {
 		if (hose->cfg_addr == cfg_addr)
 			return hose;
 	}
@@ -162,7 +172,7 @@ struct pci_controller *find_hose_by_cfg_addr(void *cfg_addr)
 
 int pci_last_busno(void)
 {
-	struct pci_controller *hose = hose_head;
+	struct pci_controller *hose = pci_get_hose_head();
 
 	if (!hose)
 		return -1;
@@ -181,7 +191,7 @@ pci_dev_t pci_find_devices(struct pci_device_id *ids, int index)
 	pci_dev_t bdf;
 	int i, bus, found_multi = 0;
 
-	for (hose = hose_head; hose; hose = hose->next) {
+	for (hose = pci_get_hose_head(); hose; hose = hose->next) {
 #ifdef CONFIG_SYS_SCSI_SCAN_BUS_REVERSE
 		for (bus = hose->last_busno; bus >= hose->first_busno; bus--)
 #else
@@ -233,7 +243,7 @@ pci_dev_t pci_find_devices(struct pci_device_id *ids, int index)
 
 pci_dev_t pci_find_device(unsigned int vendor, unsigned int device, int index)
 {
-	static struct pci_device_id ids[2] = {{}, {0, 0}};
+	struct pci_device_id ids[2] = { {}, {0, 0} };
 
 	ids[0].vendor = vendor;
 	ids[0].device = device;
@@ -709,11 +719,10 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
 int pci_hose_scan(struct pci_controller *hose)
 {
 #if defined(CONFIG_PCI_BOOTDELAY)
-	static int pcidelay_done;
 	char *s;
 	int i;
 
-	if (!pcidelay_done) {
+	if (!gd->pcidelay_done) {
 		/* wait "pcidelay" ms (if defined)... */
 		s = getenv("pcidelay");
 		if (s) {
@@ -721,7 +730,7 @@ int pci_hose_scan(struct pci_controller *hose)
 			for (i = 0; i < val; i++)
 				udelay(1000);
 		}
-		pcidelay_done = 1;
+		gd->pcidelay_done = 1;
 	}
 #endif /* CONFIG_PCI_BOOTDELAY */
 
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 9c5a1e1..3d14d5f 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -91,6 +91,12 @@ typedef struct global_data {
 	unsigned long malloc_limit;	/* limit address */
 	unsigned long malloc_ptr;	/* current address */
 #endif
+#ifdef CONFIG_PCI
+	struct pci_controller *hose;	/* PCI hose for early use */
+#endif
+#ifdef CONFIG_PCI_BOOTDELAY
+	int pcidelay_done;
+#endif
 	struct udevice *cur_serial_dev;	/* current serial device */
 	struct arch_global_data arch;	/* architecture-specific data */
 } gd_t;
-- 
1.8.2.1

[U-Boot] [PATCH v2 4/7] fdt: Add several apis to decode pci device node

[Bin Meng]

This commit adds several APIs to decode PCI device node according to
the Open Firmware PCI bus bindings, including:
- fdtdec_get_pci_addr() for encoded pci address
- fdtdec_get_pci_vendev() for vendor id and device id
- fdtdec_get_pci_bdf() for pci device bdf triplet
- fdtdec_get_pci_bar32() for pci device register bar

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- New patch to add several apis to decode pci device node

 include/fdtdec.h | 108 ++++++++++++++++++++++++++++++++++----
 lib/fdtdec.c     | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 240 insertions(+), 25 deletions(-)

diff --git a/include/fdtdec.h b/include/fdtdec.h
index d2b665c..2b2652f 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -50,6 +50,49 @@ struct fdt_resource {
 	fdt_addr_t end;
 };
 
+enum fdt_pci_space {
+	FDT_PCI_SPACE_CONFIG = 0,
+	FDT_PCI_SPACE_IO = 0x01000000,
+	FDT_PCI_SPACE_MEM32 = 0x02000000,
+	FDT_PCI_SPACE_MEM64 = 0x03000000,
+	FDT_PCI_SPACE_MEM32_PREF = 0x42000000,
+	FDT_PCI_SPACE_MEM64_PREF = 0x43000000,
+};
+
+#define FDT_PCI_ADDR_CELLS	3
+#define FDT_PCI_SIZE_CELLS	2
+#define FDT_PCI_REG_SIZE	\
+	((FDT_PCI_ADDR_CELLS + FDT_PCI_SIZE_CELLS) * sizeof(u32))
+
+/*
+ * The Open Firmware spec defines PCI physical address as follows:
+ *
+ *          bits# 31 .... 24 23 .... 16 15 .... 08 07 .... 00
+ *
+ * phys.hi  cell:  npt000ss   bbbbbbbb   dddddfff   rrrrrrrr
+ * phys.mid cell:  hhhhhhhh   hhhhhhhh   hhhhhhhh   hhhhhhhh
+ * phys.lo  cell:  llllllll   llllllll   llllllll   llllllll
+ *
+ * where:
+ *
+ * n:        is 0 if the address is relocatable, 1 otherwise
+ * p:        is 1 if addressable region is prefetchable, 0 otherwise
+ * t:        is 1 if the address is aliased (for non-relocatable I/O) below 1MB
+ *           (for Memory), or below 64KB (for relocatable I/O)
+ * ss:       is the space code, denoting the address space
+ * bbbbbbbb: is the 8-bit Bus Number
+ * ddddd:    is the 5-bit Device Number
+ * fff:      is the 3-bit Function Number
+ * rrrrrrrr: is the 8-bit Register Number
+ * hhhhhhhh: is a 32-bit unsigned number
+ * llllllll: is a 32-bit unsigned number
+ */
+struct fdt_pci_addr {
+	u32	phys_hi;
+	u32	phys_mid;
+	u32	phys_lo;
+};
+
 /**
  * Compute the size of a resource.
  *
@@ -252,6 +295,60 @@ fdt_addr_t fdtdec_get_addr_size(const void *blob, int node,
 		const char *prop_name, fdt_size_t *sizep);
 
 /**
+ * Look at an address property in a node and return the pci address which
+ * corresponds to the given type in the form of fdt_pci_addr.
+ * The property must hold one fdt_pci_addr with a lengh.
+ *
+ * @param blob		FDT blob
+ * @param node		node to examine
+ * @param type		pci address type (FDT_PCI_SPACE_xxx)
+ * @param prop_name	name of property to find
+ * @param addr		returns pci address in the form of fdt_pci_addr
+ * @return 0 if ok, negative on error
+ */
+int fdtdec_get_pci_addr(const void *blob, int node, enum fdt_pci_space type,
+		const char *prop_name, struct fdt_pci_addr *addr);
+
+/**
+ * Look at the compatible property of a device node that represents a PCI
+ * device and extract pci vendor id and device id from it.
+ *
+ * @param blob		FDT blob
+ * @param node		node to examine
+ * @param vendor	vendor id of the pci device
+ * @param device	device id of the pci device
+ * @return 0 if ok, negative on error
+ */
+int fdtdec_get_pci_vendev(const void *blob, int node,
+		u16 *vendor, u16 *device);
+
+/**
+ * Look at the pci address of a device node that represents a PCI device
+ * and parse the bus, device and function number from it.
+ *
+ * @param blob		FDT blob
+ * @param node		node to examine
+ * @param addr		pci address in the form of fdt_pci_addr
+ * @param bdf		returns bus, device, function triplet
+ * @return 0 if ok, negative on error
+ */
+int fdtdec_get_pci_bdf(const void *blob, int node,
+		struct fdt_pci_addr *addr, pci_dev_t *bdf);
+
+/**
+ * Look at the pci address of a device node that represents a PCI device
+ * and return base address of the pci device's registers.
+ *
+ * @param blob		FDT blob
+ * @param node		node to examine
+ * @param addr		pci address in the form of fdt_pci_addr
+ * @param bar		returns base address of the pci device's registers
+ * @return 0 if ok, negative on error
+ */
+int fdtdec_get_pci_bar32(const void *blob, int node,
+		struct fdt_pci_addr *addr, u32 *bar);
+
+/**
  * Look up a 32-bit integer property in a node and return it. The property
  * must have at least 4 bytes of data. The value of the first cell is
  * returned.
@@ -677,17 +774,6 @@ int fdt_get_named_resource(const void *fdt, int node, const char *property,
 			   struct fdt_resource *res);
 
 /**
- * Look at the reg property of a device node that represents a PCI device
- * and parse the bus, device and function number from it.
- *
- * @param fdt		FDT blob
- * @param node		node to examine
- * @param bdf		returns bus, device, function triplet
- * @return 0 if ok, negative on error
- */
-int fdtdec_pci_get_bdf(const void *fdt, int node, int *bdf);
-
-/**
  * Decode a named region within a memory bank of a given type.
  *
  * This function handles selection of a memory region. The region is
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 9d86dba..e40430e 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -121,6 +121,149 @@ fdt_addr_t fdtdec_get_addr(const void *blob, int node,
 	return fdtdec_get_addr_size(blob, node, prop_name, NULL);
 }
 
+#ifdef CONFIG_PCI
+int fdtdec_get_pci_addr(const void *blob, int node, enum fdt_pci_space type,
+		const char *prop_name, struct fdt_pci_addr *addr)
+{
+	const u32 *cell;
+	int len;
+
+	debug("%s: %s: ", __func__, prop_name);
+
+	/*
+	 * If we follow the pci bus bindings strictly, we should check
+	 * the value of the node's parant node's #address-cells and
+	 * #size-cells. They need to be 3 and 2 accordingly. However,
+	 * for simplicity we skip the check here.
+	 */
+	cell = fdt_getprop(blob, node, prop_name, &len);
+
+	if (cell && ((len % FDT_PCI_REG_SIZE) == 0)) {
+		int num = len / FDT_PCI_REG_SIZE;
+		int i;
+
+		for (i = 0; i < num; i++) {
+			if ((fdt_addr_to_cpu(*cell) & type) == type) {
+				addr->phys_hi = fdt_addr_to_cpu(cell[0]);
+				addr->phys_mid = fdt_addr_to_cpu(cell[1]);
+				addr->phys_lo = fdt_addr_to_cpu(cell[2]);
+				break;
+			} else {
+				cell += (FDT_PCI_ADDR_CELLS +
+					 FDT_PCI_SIZE_CELLS);
+			}
+		}
+
+		if (i == num)
+			goto fail;
+
+		return 0;
+	}
+
+fail:
+	debug("(not found)\n");
+	return -ENOENT;
+}
+
+int fdtdec_get_pci_vendev(const void *blob, int node, u16 *vendor, u16 *device)
+{
+	const char *list, *end;
+	int len;
+
+	list = fdt_getprop(blob, node, "compatible", &len);
+	if (!list)
+		return -ENOENT;
+
+	end = list + len;
+	while (list < end) {
+		int l = strlen(list);
+		char *s, v[5], d[5];
+
+		s = strstr(list, "pci");
+		/* check if the string is something like pciVVVV,DDDD */
+		if (s && s[7] == ',') {
+			s += 3;
+			strlcpy(v, s, 5);
+			s += 5;
+			strlcpy(d, s, 5);
+
+			*vendor = simple_strtol(v, NULL, 16);
+			*device = simple_strtol(d, NULL, 16);
+
+			return 0;
+		} else {
+			list += (l + 1);
+		}
+	}
+
+	return -ENOENT;
+}
+
+int fdtdec_get_pci_bdf(const void *blob, int node,
+		struct fdt_pci_addr *addr, pci_dev_t *bdf)
+{
+	u16 dt_vendor, dt_device, vendor, device;
+	int ret;
+
+	/* get vendor id & device id from the compatible string */
+	ret = fdtdec_get_pci_vendev(blob, node, &dt_vendor, &dt_device);
+	if (ret)
+		return ret;
+
+	/* extract the bdf from fdt_pci_addr */
+	*bdf = addr->phys_hi & 0xffff00;
+
+	/* read vendor id & device id based on bdf */
+	pci_read_config_word(*bdf, PCI_VENDOR_ID, &vendor);
+	pci_read_config_word(*bdf, PCI_DEVICE_ID, &device);
+
+	/*
+	 * Note there are two places in the device tree to fully describe
+	 * a pci device: one is via compatible string with a format of
+	 * "pciVVVV,DDDD" and the other one is the bdf numbers encoded in
+	 * the device node's reg address property. We read the vendor id
+	 * and device id based on bdf and compare the values with the
+	 * "VVVV,DDDD". If they are the same, then we are good to use bdf
+	 * to read device's bar. But if they are different, we have to rely
+	 * on the vendor id and device id extracted from the compatible
+	 * string and locate the real bdf by pci_find_device(). This is
+	 * because normally we may only know device's device number and
+	 * function number when writing device tree. The bus number is
+	 * dynamically assigned during the pci enumeration process.
+	 */
+	if ((dt_vendor != vendor) || (dt_device != device)) {
+		*bdf = pci_find_device(dt_vendor, dt_device, 0);
+		if (*bdf == -1)
+			return -ENODEV;
+	}
+
+	return 0;
+}
+
+int fdtdec_get_pci_bar32(const void *blob, int node,
+		struct fdt_pci_addr *addr, u32 *bar)
+{
+	pci_dev_t bdf;
+	int bn;
+	int ret;
+
+	/* get pci devices's bdf */
+	ret = fdtdec_get_pci_bdf(blob, node, addr, &bdf);
+	if (ret)
+		return ret;
+
+	/* extract the bar number from fdt_pci_addr */
+	bn = addr->phys_hi & 0xff;
+	if ((bn < PCI_BASE_ADDRESS_0) || (bn > PCI_CARDBUS_CIS))
+		return -EINVAL;
+
+	bn = (bn - PCI_BASE_ADDRESS_0) / 4;
+	*bar = pci_read_bar32(pci_bus_to_hose(PCI_BUS(bdf)), bdf, bn);
+
+	return 0;
+}
+#endif
+
 uint64_t fdtdec_get_uint64(const void *blob, int node, const char *prop_name,
 		uint64_t default_val)
 {
@@ -790,20 +933,6 @@ int fdt_get_named_resource(const void *fdt, int node, const char *property,
 	return fdt_get_resource(fdt, node, property, index, res);
 }
 
-int fdtdec_pci_get_bdf(const void *fdt, int node, int *bdf)
-{
-	const fdt32_t *prop;
-	int len;
-
-	prop = fdt_getprop(fdt, node, "reg", &len);
-	if (!prop)
-		return len;
-
-	*bdf = fdt32_to_cpu(*prop) & 0xffffff;
-
-	return 0;
-}
-
 int fdtdec_decode_memory_region(const void *blob, int config_node,
 				const char *mem_type, const char *suffix,
 				fdt_addr_t *basep, fdt_size_t *sizep)
-- 
1.8.2.1

[U-Boot] [PATCH v2 5/7] serial: ns16550: Support ns16550 compatible pci uart devices

[Bin Meng]

There are many pci uart devices which are ns16550 compatible. We can
describe them in the board dts file and use it as the U-Boot serial
console as specified in the chosen node 'stdout-path' property.

Those pci uart devices can have their register be memory mapped, or
i/o mapped. The driver will try to use memory mapped register if the
reg property in the node has an entry to describe the memory mapped
register, otherwise i/o mapped register will be used.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- New patch to support ns16550 compatible pci uart devices

 drivers/serial/ns16550.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index af5beba..2001fac 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -289,10 +289,41 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 	struct ns16550_platdata *plat = dev->platdata;
 	fdt_addr_t addr;
 
+	/* try plb device first */
 	addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
-	if (addr == FDT_ADDR_T_NONE)
+	if (addr == FDT_ADDR_T_NONE) {
+#ifdef CONFIG_PCI
+		/* then try pci device */
+		struct fdt_pci_addr pci_addr;
+		u32 bar;
+		int ret;
+
+		/* we prefer to use memory mapped register */
+		ret = fdtdec_get_pci_addr(gd->fdt_blob, dev->of_offset,
+					  FDT_PCI_SPACE_MEM32, "reg",
+					  &pci_addr);
+		if (ret) {
+			/* try if there is any io mapped register */
+			ret = fdtdec_get_pci_addr(gd->fdt_blob,
+						  dev->of_offset,
+						  FDT_PCI_SPACE_IO,
+						  "reg", &pci_addr);
+			if (ret)
+				return ret;
+		}
+
+		ret = fdtdec_get_pci_bar32(gd->fdt_blob, dev->of_offset,
+					   &pci_addr, &bar);
+		if (ret)
+			return ret;
+
+		addr = bar;
+		goto cont;
+#endif
 		return -EINVAL;
+	}
 
+cont:
 	plat->base = addr;
 	plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
 					 "reg-shift", 1);
-- 
1.8.2.1

[U-Boot] [PATCH v2 6/7] x86: Use ePAPR defined properties for x86-uart

[Bin Meng]

Use ePAPR defined properties for x86-uart: clock-frequency and
current-speed. Assign the value of clock-frequency in device tree
to plat->clock of x86-uart instead of using hardcoded number.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- New patch to use ePAPR defined properties for x86-uart

 arch/x86/dts/serial.dtsi    | 5 ++---
 drivers/serial/serial_x86.c | 8 +++++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/dts/serial.dtsi b/arch/x86/dts/serial.dtsi
index ebdda76..9b097f4 100644
--- a/arch/x86/dts/serial.dtsi
+++ b/arch/x86/dts/serial.dtsi
@@ -3,8 +3,7 @@
 		compatible = "x86-uart";
 		reg = <0x3f8 8>;
 		reg-shift = <0>;
-		io-mapped = <1>;
-		multiplier = <1>;
-		baudrate = <115200>;
+		clock-frequency = <1843200>;
+		current-speed = <115200>;
 	};
 };
diff --git a/drivers/serial/serial_x86.c b/drivers/serial/serial_x86.c
index e81e035..4bf6062 100644
--- a/drivers/serial/serial_x86.c
+++ b/drivers/serial/serial_x86.c
@@ -6,9 +6,12 @@
 
 #include <common.h>
 #include <dm.h>
+#include <fdtdec.h>
 #include <ns16550.h>
 #include <serial.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static const struct udevice_id x86_serial_ids[] = {
 	{ .compatible = "x86-uart" },
 	{ }
@@ -22,10 +25,13 @@ static int x86_serial_ofdata_to_platdata(struct udevice *dev)
 	ret = ns16550_serial_ofdata_to_platdata(dev);
 	if (ret)
 		return ret;
-	plat->clock = 1843200;
+
+	plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+				     "clock-frequency", 1843200);
 
 	return 0;
 }
+
 U_BOOT_DRIVER(serial_ns16550) = {
 	.name	= "serial_x86",
 	.id	= UCLASS_SERIAL,
-- 
1.8.2.1

[U-Boot] [PATCH v2 7/7] x86: crownbay: Add pci devices in the dts file

[Bin Meng]

The Topcliff PCH has 4 UART devices integrated (Device 10, Funciton
1/2/3/4). Add the corresponding device nodes in the crownbay.dts per
Open Firmware PCI bus bindings.

Also a comment block is added for the 'stdout-path' property in the
chosen node, mentioning that by default the legacy superio serial
port (io addr 0x3f8) is still used on Crown Bay as the console port.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- New patch to add pci devices in crownbay.dts
- Drop v1 patch: Add an API for finding pci devices in the early phase
- Drop v1 patch: Support PCI UART in the x86_serial driver
- Drop v1 patch: Add PCI UART related defines in crownbay.h

 arch/x86/dts/crownbay.dts | 81 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/arch/x86/dts/crownbay.dts b/arch/x86/dts/crownbay.dts
index 97f7a52..a42e0e4 100644
--- a/arch/x86/dts/crownbay.dts
+++ b/arch/x86/dts/crownbay.dts
@@ -32,6 +32,14 @@
 	};
 
 	chosen {
+		/*
+		 * By default the legacy superio serial port is used as the
+		 * U-Boot serial console. If we want to use UART from Topcliff
+		 * PCH as the console, change this property to &pciuart#.
+		 *
+		 * For example, stdout-path = &pciuart0 will use the first
+		 * UART on Topcliff PCH.
+		 */
 		stdout-path = "/serial";
 	};
 
@@ -52,4 +60,77 @@
 		};
 	};
 
+	pci {
+		#address-cells = <3>;
+		#size-cells = <2>;
+		compatible = "intel,pci";
+		device_type = "pci";
+
+		pcie at 17,0 {
+			#address-cells = <3>;
+			#size-cells = <2>;
+			compatible = "intel,pci";
+			device_type = "pci";
+
+			topcliff at 0,0 {
+				#address-cells = <3>;
+				#size-cells = <2>;
+				compatible = "intel,pci";
+				device_type = "pci";
+
+				pciuart0: uart at a,1 {
+					compatible = "pci8086,8811.0",
+							"pci8086,8811",
+							"pciclass070002",
+							"pciclass0700",
+							"x86-uart";
+					reg = <0x00025100 0x0 0x0 0x0 0x0
+					       0x01025110 0x0 0x0 0x0 0x0>;
+					reg-shift = <0>;
+					clock-frequency = <1843200>;
+					current-speed = <115200>;
+				};
+
+				pciuart1: uart at a,2 {
+					compatible = "pci8086,8812.0",
+							"pci8086,8812",
+							"pciclass070002",
+							"pciclass0700",
+							"x86-uart";
+					reg = <0x00025200 0x0 0x0 0x0 0x0
+					       0x01025210 0x0 0x0 0x0 0x0>;
+					reg-shift = <0>;
+					clock-frequency = <1843200>;
+					current-speed = <115200>;
+				};
+
+				pciuart2: uart at a,3 {
+					compatible = "pci8086,8813.0",
+							"pci8086,8813",
+							"pciclass070002",
+							"pciclass0700",
+							"x86-uart";
+					reg = <0x00025300 0x0 0x0 0x0 0x0
+					       0x01025310 0x0 0x0 0x0 0x0>;
+					reg-shift = <0>;
+					clock-frequency = <1843200>;
+					current-speed = <115200>;
+				};
+
+				pciuart3: uart at a,4 {
+					compatible = "pci8086,8814.0",
+							"pci8086,8814",
+							"pciclass070002",
+							"pciclass0700",
+							"x86-uart";
+					reg = <0x00025400 0x0 0x0 0x0 0x0
+					       0x01025410 0x0 0x0 0x0 0x0>;
+					reg-shift = <0>;
+					clock-frequency = <1843200>;
+					current-speed = <115200>;
+				};
+			};
+		};
+	};
+
 };
-- 
1.8.2.1

[U-Boot] [PATCH v2 1/7] x86: Add missing DECLARE_GLOBAL_DATA_PTR for pci.c

[Simon Glass]

On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
> arch/x86/cpu/pci.c has access to the U-Boot global data thus
> DECLARE_GLOBAL_DATA_PTR is needed.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

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

>
> ---
>
> Changes in v2:
> - Add a commit message
>
>  arch/x86/cpu/pci.c | 2 ++
>  1 file changed, 2 insertions(+)

[U-Boot] [PATCH v2 2/7] x86: Support pci bus scan in the early phase

[Simon Glass]

On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
> On x86, some peripherals on pci buses need to be accessed in the
> early phase (eg: pci uart) with a valid pci memory/io address,
> thus scan the pci bus and do the corresponding resource allocation.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
> Changes in v2: None

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

>
>  arch/x86/cpu/pci.c | 1 +
>  1 file changed, 1 insertion(+)

[U-Boot] [PATCH v2 3/7] pci: Make pci apis usable before relocation

[Simon Glass]

On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
> Introduce a gd->hose to save the pci hose in the early phase so that
> apis in drivers/pci/pci.c can be used before relocation. Architecture
> codes need assign a valid gd->hose in the early phase.
>
> Some variables are declared as static so change them to be either
> stack variable or global data member so that they can be used before
> relocation, except the 'indent' used by CONFIG_PCI_SCAN_SHOW which
> just affects some print format.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>

I was actually thinking of making it available only for x86, but in
fact it makes sense so provide this as a global facility. The less
arch-specific stuff we add to global_data the better.

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

> ---
>
> Changes in v2:
> - New patch to make pci apis usable before relocation
>
>  arch/x86/cpu/pci.c                 |  8 ++++----
>  arch/x86/include/asm/global_data.h |  1 -
>  arch/x86/include/asm/pci.h         |  2 +-
>  drivers/pci/pci.c                  | 25 +++++++++++++++++--------
>  include/asm-generic/global_data.h  |  6 ++++++
>  5 files changed, 28 insertions(+), 14 deletions(-)

[U-Boot] [PATCH v2 4/7] fdt: Add several apis to decode pci device node

[Simon Glass]

Hi Bin,

On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
> This commit adds several APIs to decode PCI device node according to
> the Open Firmware PCI bus bindings, including:
> - fdtdec_get_pci_addr() for encoded pci address
> - fdtdec_get_pci_vendev() for vendor id and device id
> - fdtdec_get_pci_bdf() for pci device bdf triplet
> - fdtdec_get_pci_bar32() for pci device register bar
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

Looks good - some minor comments below.

>
> ---
>
> Changes in v2:
> - New patch to add several apis to decode pci device node
>
>  include/fdtdec.h | 108 ++++++++++++++++++++++++++++++++++----
>  lib/fdtdec.c     | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 240 insertions(+), 25 deletions(-)
>
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index d2b665c..2b2652f 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -50,6 +50,49 @@ struct fdt_resource {
>         fdt_addr_t end;
>  };
>
> +enum fdt_pci_space {
> +       FDT_PCI_SPACE_CONFIG = 0,
> +       FDT_PCI_SPACE_IO = 0x01000000,
> +       FDT_PCI_SPACE_MEM32 = 0x02000000,
> +       FDT_PCI_SPACE_MEM64 = 0x03000000,
> +       FDT_PCI_SPACE_MEM32_PREF = 0x42000000,
> +       FDT_PCI_SPACE_MEM64_PREF = 0x43000000,
> +};
> +
> +#define FDT_PCI_ADDR_CELLS     3
> +#define FDT_PCI_SIZE_CELLS     2
> +#define FDT_PCI_REG_SIZE       \
> +       ((FDT_PCI_ADDR_CELLS + FDT_PCI_SIZE_CELLS) * sizeof(u32))
> +
> +/*
> + * The Open Firmware spec defines PCI physical address as follows:
> + *
> + *          bits# 31 .... 24 23 .... 16 15 .... 08 07 .... 00
> + *
> + * phys.hi  cell:  npt000ss   bbbbbbbb   dddddfff   rrrrrrrr
> + * phys.mid cell:  hhhhhhhh   hhhhhhhh   hhhhhhhh   hhhhhhhh
> + * phys.lo  cell:  llllllll   llllllll   llllllll   llllllll
> + *
> + * where:
> + *
> + * n:        is 0 if the address is relocatable, 1 otherwise
> + * p:        is 1 if addressable region is prefetchable, 0 otherwise
> + * t:        is 1 if the address is aliased (for non-relocatable I/O) below 1MB
> + *           (for Memory), or below 64KB (for relocatable I/O)
> + * ss:       is the space code, denoting the address space
> + * bbbbbbbb: is the 8-bit Bus Number
> + * ddddd:    is the 5-bit Device Number
> + * fff:      is the 3-bit Function Number
> + * rrrrrrrr: is the 8-bit Register Number
> + * hhhhhhhh: is a 32-bit unsigned number
> + * llllllll: is a 32-bit unsigned number
> + */
> +struct fdt_pci_addr {
> +       u32     phys_hi;
> +       u32     phys_mid;
> +       u32     phys_lo;
> +};
> +
>  /**
>   * Compute the size of a resource.
>   *
> @@ -252,6 +295,60 @@ fdt_addr_t fdtdec_get_addr_size(const void *blob, int node,
>                 const char *prop_name, fdt_size_t *sizep);
>
>  /**
> + * Look at an address property in a node and return the pci address which
> + * corresponds to the given type in the form of fdt_pci_addr.
> + * The property must hold one fdt_pci_addr with a lengh.
> + *
> + * @param blob         FDT blob
> + * @param node         node to examine
> + * @param type         pci address type (FDT_PCI_SPACE_xxx)
> + * @param prop_name    name of property to find
> + * @param addr         returns pci address in the form of fdt_pci_addr
> + * @return 0 if ok, negative on error
> + */
> +int fdtdec_get_pci_addr(const void *blob, int node, enum fdt_pci_space type,
> +               const char *prop_name, struct fdt_pci_addr *addr);
> +
> +/**
> + * Look at the compatible property of a device node that represents a PCI
> + * device and extract pci vendor id and device id from it.
> + *
> + * @param blob         FDT blob
> + * @param node         node to examine
> + * @param vendor       vendor id of the pci device
> + * @param device       device id of the pci device
> + * @return 0 if ok, negative on error
> + */
> +int fdtdec_get_pci_vendev(const void *blob, int node,
> +               u16 *vendor, u16 *device);
> +
> +/**
> + * Look at the pci address of a device node that represents a PCI device
> + * and parse the bus, device and function number from it.
> + *
> + * @param blob         FDT blob
> + * @param node         node to examine
> + * @param addr         pci address in the form of fdt_pci_addr
> + * @param bdf          returns bus, device, function triplet
> + * @return 0 if ok, negative on error
> + */
> +int fdtdec_get_pci_bdf(const void *blob, int node,
> +               struct fdt_pci_addr *addr, pci_dev_t *bdf);
> +
> +/**
> + * Look at the pci address of a device node that represents a PCI device
> + * and return base address of the pci device's registers.
> + *
> + * @param blob         FDT blob
> + * @param node         node to examine
> + * @param addr         pci address in the form of fdt_pci_addr
> + * @param bar          returns base address of the pci device's registers
> + * @return 0 if ok, negative on error
> + */
> +int fdtdec_get_pci_bar32(const void *blob, int node,
> +               struct fdt_pci_addr *addr, u32 *bar);
> +
> +/**
>   * Look up a 32-bit integer property in a node and return it. The property
>   * must have at least 4 bytes of data. The value of the first cell is
>   * returned.
> @@ -677,17 +774,6 @@ int fdt_get_named_resource(const void *fdt, int node, const char *property,
>                            struct fdt_resource *res);
>
>  /**
> - * Look at the reg property of a device node that represents a PCI device
> - * and parse the bus, device and function number from it.
> - *
> - * @param fdt          FDT blob
> - * @param node         node to examine
> - * @param bdf          returns bus, device, function triplet
> - * @return 0 if ok, negative on error
> - */
> -int fdtdec_pci_get_bdf(const void *fdt, int node, int *bdf);
> -
> -/**
>   * Decode a named region within a memory bank of a given type.
>   *
>   * This function handles selection of a memory region. The region is
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 9d86dba..e40430e 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -121,6 +121,149 @@ fdt_addr_t fdtdec_get_addr(const void *blob, int node,
>         return fdtdec_get_addr_size(blob, node, prop_name, NULL);
>  }
>
> +#ifdef CONFIG_PCI
> +int fdtdec_get_pci_addr(const void *blob, int node, enum fdt_pci_space type,
> +               const char *prop_name, struct fdt_pci_addr *addr)
> +{
> +       const u32 *cell;
> +       int len;
> +
> +       debug("%s: %s: ", __func__, prop_name);
> +
> +       /*
> +        * If we follow the pci bus bindings strictly, we should check
> +        * the value of the node's parant node's #address-cells and

parent

> +        * #size-cells. They need to be 3 and 2 accordingly. However,
> +        * for simplicity we skip the check here.
> +        */
> +       cell = fdt_getprop(blob, node, prop_name, &len);

I think it would be better to return -ENOENT if cell is NULL and
-EINVAL in the case where the number of cells is not a multiple of 5.

> +
> +       if (cell && ((len % FDT_PCI_REG_SIZE) == 0)) {
> +               int num = len / FDT_PCI_REG_SIZE;
> +               int i;
> +
> +               for (i = 0; i < num; i++) {
> +                       if ((fdt_addr_to_cpu(*cell) & type) == type) {
> +                               addr->phys_hi = fdt_addr_to_cpu(cell[0]);
> +                               addr->phys_mid = fdt_addr_to_cpu(cell[1]);
> +                               addr->phys_lo = fdt_addr_to_cpu(cell[2]);
> +                               break;
> +                       } else {
> +                               cell += (FDT_PCI_ADDR_CELLS +
> +                                        FDT_PCI_SIZE_CELLS);
> +                       }

You need a debug() in here to print stuff out, and a way of getting a
\n at the end.

> +               }
> +
> +               if (i == num)
> +                       goto fail;
> +
> +               return 0;
> +       }
> +
> +fail:
> +       debug("(not found)\n");
> +       return -ENOENT;
> +}
> +
> +int fdtdec_get_pci_vendev(const void *blob, int node, u16 *vendor, u16 *device)
> +{
> +       const char *list, *end;
> +       int len;
> +
> +       list = fdt_getprop(blob, node, "compatible", &len);
> +       if (!list)
> +               return -ENOENT;
> +
> +       end = list + len;
> +       while (list < end) {
> +               int l = strlen(list);

s/l/len/

> +               char *s, v[5], d[5];
> +
> +               s = strstr(list, "pci");
> +               /* check if the string is something like pciVVVV,DDDD */
> +               if (s && s[7] == ',') {

Should check that end - list > 7 otherwise s[7] might not exist. Also
how about checking for the '.'?

> +                       s += 3;
> +                       strlcpy(v, s, 5);
> +                       s += 5;
> +                       strlcpy(d, s, 5);
> +
> +                       *vendor = simple_strtol(v, NULL, 16);
> +                       *device = simple_strtol(d, NULL, 16);

I suspect you can avoid this - just use simple_strtol() directly on
the correct places in the string. The function will automatically stop
when it sees no more hex digits.

> +
> +                       return 0;
> +               } else {
> +                       list += (l + 1);
> +               }
> +       }
> +
> +       return -ENOENT;
> +}
> +
> +int fdtdec_get_pci_bdf(const void *blob, int node,
> +               struct fdt_pci_addr *addr, pci_dev_t *bdf)
> +{
> +       u16 dt_vendor, dt_device, vendor, device;
> +       int ret;
> +
> +       /* get vendor id & device id from the compatible string */
> +       ret = fdtdec_get_pci_vendev(blob, node, &dt_vendor, &dt_device);
> +       if (ret)
> +               return ret;
> +
> +       /* extract the bdf from fdt_pci_addr */
> +       *bdf = addr->phys_hi & 0xffff00;
> +
> +       /* read vendor id & device id based on bdf */
> +       pci_read_config_word(*bdf, PCI_VENDOR_ID, &vendor);
> +       pci_read_config_word(*bdf, PCI_DEVICE_ID, &device);

Check return values

> +
> +       /*
> +        * Note there are two places in the device tree to fully describe
> +        * a pci device: one is via compatible string with a format of
> +        * "pciVVVV,DDDD" and the other one is the bdf numbers encoded in
> +        * the device node's reg address property. We read the vendor id
> +        * and device id based on bdf and compare the values with the
> +        * "VVVV,DDDD". If they are the same, then we are good to use bdf
> +        * to read device's bar. But if they are different, we have to rely
> +        * on the vendor id and device id extracted from the compatible
> +        * string and locate the real bdf by pci_find_device(). This is
> +        * because normally we may only know device's device number and
> +        * function number when writing device tree. The bus number is
> +        * dynamically assigned during the pci enumeration process.
> +        */
> +       if ((dt_vendor != vendor) || (dt_device != device)) {
> +               *bdf = pci_find_device(dt_vendor, dt_device, 0);
> +               if (*bdf == -1)
> +                       return -ENODEV;
> +       }
> +
> +       return 0;
> +}
> +
> +int fdtdec_get_pci_bar32(const void *blob, int node,
> +               struct fdt_pci_addr *addr, u32 *bar)
> +{
> +       pci_dev_t bdf;
> +       int bn;

s/bn/barnum/ since it is self-documenting.

> +       int ret;
> +
> +       /* get pci devices's bdf */
> +       ret = fdtdec_get_pci_bdf(blob, node, addr, &bdf);
> +       if (ret)
> +               return ret;
> +
> +       /* extract the bar number from fdt_pci_addr */
> +       bn = addr->phys_hi & 0xff;
> +       if ((bn < PCI_BASE_ADDRESS_0) || (bn > PCI_CARDBUS_CIS))
> +               return -EINVAL;
> +
> +       bn = (bn - PCI_BASE_ADDRESS_0) / 4;
> +       *bar = pci_read_bar32(pci_bus_to_hose(PCI_BUS(bdf)), bdf, bn);
> +
> +       return 0;
> +}
> +#endif
> +
>  uint64_t fdtdec_get_uint64(const void *blob, int node, const char *prop_name,
>                 uint64_t default_val)
>  {
> @@ -790,20 +933,6 @@ int fdt_get_named_resource(const void *fdt, int node, const char *property,
>         return fdt_get_resource(fdt, node, property, index, res);
>  }
>
> -int fdtdec_pci_get_bdf(const void *fdt, int node, int *bdf)
> -{
> -       const fdt32_t *prop;
> -       int len;
> -
> -       prop = fdt_getprop(fdt, node, "reg", &len);
> -       if (!prop)
> -               return len;
> -
> -       *bdf = fdt32_to_cpu(*prop) & 0xffffff;
> -
> -       return 0;
> -}
> -
>  int fdtdec_decode_memory_region(const void *blob, int config_node,
>                                 const char *mem_type, const char *suffix,
>                                 fdt_addr_t *basep, fdt_size_t *sizep)
> --
> 1.8.2.1
>

Regards,
Simon

[U-Boot] [PATCH v2 5/7] serial: ns16550: Support ns16550 compatible pci uart devices

[Simon Glass]

Hi Bin,

On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
> There are many pci uart devices which are ns16550 compatible. We can
> describe them in the board dts file and use it as the U-Boot serial
> console as specified in the chosen node 'stdout-path' property.
>
> Those pci uart devices can have their register be memory mapped, or

memory-mapped

> i/o mapped. The driver will try to use memory mapped register if the

i/o-mapped

s/memory mapped/the memory-mapped/

> reg property in the node has an entry to describe the memory mapped

memory-mapped

> register, otherwise i/o mapped register will be used.

i/o-mapped

>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v2:
> - New patch to support ns16550 compatible pci uart devices
>
>  drivers/serial/ns16550.c | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index af5beba..2001fac 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -289,10 +289,41 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
>         struct ns16550_platdata *plat = dev->platdata;
>         fdt_addr_t addr;
>
> +       /* try plb device first */

What is plb?

>         addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
> -       if (addr == FDT_ADDR_T_NONE)
> +       if (addr == FDT_ADDR_T_NONE) {
> +#ifdef CONFIG_PCI
> +               /* then try pci device */
> +               struct fdt_pci_addr pci_addr;
> +               u32 bar;
> +               int ret;
> +
> +               /* we prefer to use memory mapped register */

a memory-mapped

> +               ret = fdtdec_get_pci_addr(gd->fdt_blob, dev->of_offset,
> +                                         FDT_PCI_SPACE_MEM32, "reg",
> +                                         &pci_addr);
> +               if (ret) {
> +                       /* try if there is any io mapped register */
> +                       ret = fdtdec_get_pci_addr(gd->fdt_blob,
> +                                                 dev->of_offset,
> +                                                 FDT_PCI_SPACE_IO,
> +                                                 "reg", &pci_addr);
> +                       if (ret)
> +                               return ret;
> +               }
> +
> +               ret = fdtdec_get_pci_bar32(gd->fdt_blob, dev->of_offset,
> +                                          &pci_addr, &bar);
> +               if (ret)
> +                       return ret;
> +
> +               addr = bar;
> +               goto cont;
> +#endif
>                 return -EINVAL;
> +       }

Instead of the above 4 lines, move the #ifdef CONFIG_PCI up one line, then here:

   }
if (addr == FDT_ADDR_T_NONE)
   return -EINVAL;

This avoids the goto.

>
> +cont:
>         plat->base = addr;
>         plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>                                          "reg-shift", 1);
> --
> 1.8.2.1
>

Regards,
Simon

[U-Boot] [PATCH v2 6/7] x86: Use ePAPR defined properties for x86-uart

[Simon Glass]

On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
> Use ePAPR defined properties for x86-uart: clock-frequency and
> current-speed. Assign the value of clock-frequency in device tree
> to plat->clock of x86-uart instead of using hardcoded number.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

>
> ---
>
> Changes in v2:
> - New patch to use ePAPR defined properties for x86-uart
>
>  arch/x86/dts/serial.dtsi    | 5 ++---
>  drivers/serial/serial_x86.c | 8 +++++++-
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/dts/serial.dtsi b/arch/x86/dts/serial.dtsi
> index ebdda76..9b097f4 100644
> --- a/arch/x86/dts/serial.dtsi
> +++ b/arch/x86/dts/serial.dtsi
> @@ -3,8 +3,7 @@
>                 compatible = "x86-uart";
>                 reg = <0x3f8 8>;
>                 reg-shift = <0>;
> -               io-mapped = <1>;
> -               multiplier = <1>;
> -               baudrate = <115200>;
> +               clock-frequency = <1843200>;
> +               current-speed = <115200>;

Where is current speed used? If it is needed, please update the
binding at doc/device-tree-bindings/serial/ns16550.txt

>         };
>  };
> diff --git a/drivers/serial/serial_x86.c b/drivers/serial/serial_x86.c
> index e81e035..4bf6062 100644
> --- a/drivers/serial/serial_x86.c
> +++ b/drivers/serial/serial_x86.c
> @@ -6,9 +6,12 @@
>
>  #include <common.h>
>  #include <dm.h>
> +#include <fdtdec.h>
>  #include <ns16550.h>
>  #include <serial.h>
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  static const struct udevice_id x86_serial_ids[] = {
>         { .compatible = "x86-uart" },
>         { }
> @@ -22,10 +25,13 @@ static int x86_serial_ofdata_to_platdata(struct udevice *dev)
>         ret = ns16550_serial_ofdata_to_platdata(dev);
>         if (ret)
>                 return ret;
> -       plat->clock = 1843200;
> +
> +       plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> +                                    "clock-frequency", 1843200);
>
>         return 0;
>  }
> +
>  U_BOOT_DRIVER(serial_ns16550) = {
>         .name   = "serial_x86",
>         .id     = UCLASS_SERIAL,
> --
> 1.8.2.1
>

Regards,
Simon

[U-Boot] [PATCH v2 7/7] x86: crownbay: Add pci devices in the dts file

[Simon Glass]

On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
> The Topcliff PCH has 4 UART devices integrated (Device 10, Funciton
> 1/2/3/4). Add the corresponding device nodes in the crownbay.dts per
> Open Firmware PCI bus bindings.
>
> Also a comment block is added for the 'stdout-path' property in the
> chosen node, mentioning that by default the legacy superio serial
> port (io addr 0x3f8) is still used on Crown Bay as the console port.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v2:
> - New patch to add pci devices in crownbay.dts
> - Drop v1 patch: Add an API for finding pci devices in the early phase
> - Drop v1 patch: Support PCI UART in the x86_serial driver
> - Drop v1 patch: Add PCI UART related defines in crownbay.h
>
>  arch/x86/dts/crownbay.dts | 81 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)

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

This looks like a very useful series. It should support just about any
serial port we might want to use.

Regards,
Simon

[U-Boot] [PATCH v2 5/7] serial: ns16550: Support ns16550 compatible pci uart devices

[Bin Meng]

Hi Simon,

On Sun, Dec 28, 2014 at 9:55 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
>> There are many pci uart devices which are ns16550 compatible. We can
>> describe them in the board dts file and use it as the U-Boot serial
>> console as specified in the chosen node 'stdout-path' property.
>>
>> Those pci uart devices can have their register be memory mapped, or
>
> memory-mapped
>
>> i/o mapped. The driver will try to use memory mapped register if the
>
> i/o-mapped
>
> s/memory mapped/the memory-mapped/
>
>> reg property in the node has an entry to describe the memory mapped
>
> memory-mapped
>
>> register, otherwise i/o mapped register will be used.
>
> i/o-mapped
>
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>
>> ---
>>
>> Changes in v2:
>> - New patch to support ns16550 compatible pci uart devices
>>
>>  drivers/serial/ns16550.c | 33 ++++++++++++++++++++++++++++++++-
>>  1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
>> index af5beba..2001fac 100644
>> --- a/drivers/serial/ns16550.c
>> +++ b/drivers/serial/ns16550.c
>> @@ -289,10 +289,41 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
>>         struct ns16550_platdata *plat = dev->platdata;
>>         fdt_addr_t addr;
>>
>> +       /* try plb device first */
>
> What is plb?

I mean Processor Local Bus, a concept I believe is popular in the
embedded powerpc world, but I realized it might not be that popuar to
other architectures. Maybe I could just remove it.

>>         addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
>> -       if (addr == FDT_ADDR_T_NONE)
>> +       if (addr == FDT_ADDR_T_NONE) {
>> +#ifdef CONFIG_PCI
>> +               /* then try pci device */
>> +               struct fdt_pci_addr pci_addr;
>> +               u32 bar;
>> +               int ret;
>> +
>> +               /* we prefer to use memory mapped register */
>
> a memory-mapped
>
>> +               ret = fdtdec_get_pci_addr(gd->fdt_blob, dev->of_offset,
>> +                                         FDT_PCI_SPACE_MEM32, "reg",
>> +                                         &pci_addr);
>> +               if (ret) {
>> +                       /* try if there is any io mapped register */
>> +                       ret = fdtdec_get_pci_addr(gd->fdt_blob,
>> +                                                 dev->of_offset,
>> +                                                 FDT_PCI_SPACE_IO,
>> +                                                 "reg", &pci_addr);
>> +                       if (ret)
>> +                               return ret;
>> +               }
>> +
>> +               ret = fdtdec_get_pci_bar32(gd->fdt_blob, dev->of_offset,
>> +                                          &pci_addr, &bar);
>> +               if (ret)
>> +                       return ret;
>> +
>> +               addr = bar;
>> +               goto cont;
>> +#endif
>>                 return -EINVAL;
>> +       }
>
> Instead of the above 4 lines, move the #ifdef CONFIG_PCI up one line, then here:
>
>    }
> if (addr == FDT_ADDR_T_NONE)
>    return -EINVAL;
>
> This avoids the goto.

Yep, this is better. Will fix it.

>>
>> +cont:
>>         plat->base = addr;
>>         plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>>                                          "reg-shift", 1);
>> --
>> 1.8.2.1
>>

Regards,
Bin

[U-Boot] [PATCH v2 6/7] x86: Use ePAPR defined properties for x86-uart

[Bin Meng]

Hi Simon,

On Sun, Dec 28, 2014 at 9:55 AM, Simon Glass <sjg@chromium.org> wrote:
> On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Use ePAPR defined properties for x86-uart: clock-frequency and
>> current-speed. Assign the value of clock-frequency in device tree
>> to plat->clock of x86-uart instead of using hardcoded number.
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
>>
>> ---
>>
>> Changes in v2:
>> - New patch to use ePAPR defined properties for x86-uart
>>
>>  arch/x86/dts/serial.dtsi    | 5 ++---
>>  drivers/serial/serial_x86.c | 8 +++++++-
>>  2 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/dts/serial.dtsi b/arch/x86/dts/serial.dtsi
>> index ebdda76..9b097f4 100644
>> --- a/arch/x86/dts/serial.dtsi
>> +++ b/arch/x86/dts/serial.dtsi
>> @@ -3,8 +3,7 @@
>>                 compatible = "x86-uart";
>>                 reg = <0x3f8 8>;
>>                 reg-shift = <0>;
>> -               io-mapped = <1>;
>> -               multiplier = <1>;
>> -               baudrate = <115200>;
>> +               clock-frequency = <1843200>;
>> +               current-speed = <115200>;
>
> Where is current speed used? If it is needed, please update the
> binding at doc/device-tree-bindings/serial/ns16550.txt
>

The current-speed is the baud rate of the serial port. Currently it is
put there for ePAPR completeness and not used by U-Boot as U-Boot is
using gd->baudrate for the serial console.

[snip]

Regards,
Bin

[U-Boot] [PATCH v2 5/7] serial: ns16550: Support ns16550 compatible pci uart devices

[Simon Glass]

Hi Bin,

On 28 December 2014 at 23:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Sun, Dec 28, 2014 at 9:55 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Bin,
>>
>> On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> There are many pci uart devices which are ns16550 compatible. We can
>>> describe them in the board dts file and use it as the U-Boot serial
>>> console as specified in the chosen node 'stdout-path' property.
>>>
>>> Those pci uart devices can have their register be memory mapped, or
>>
>> memory-mapped
>>
>>> i/o mapped. The driver will try to use memory mapped register if the
>>
>> i/o-mapped
>>
>> s/memory mapped/the memory-mapped/
>>
>>> reg property in the node has an entry to describe the memory mapped
>>
>> memory-mapped
>>
>>> register, otherwise i/o mapped register will be used.
>>
>> i/o-mapped
>>
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>
>>> ---
>>>
>>> Changes in v2:
>>> - New patch to support ns16550 compatible pci uart devices
>>>
>>>  drivers/serial/ns16550.c | 33 ++++++++++++++++++++++++++++++++-
>>>  1 file changed, 32 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
>>> index af5beba..2001fac 100644
>>> --- a/drivers/serial/ns16550.c
>>> +++ b/drivers/serial/ns16550.c
>>> @@ -289,10 +289,41 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
>>>         struct ns16550_platdata *plat = dev->platdata;
>>>         fdt_addr_t addr;
>>>
>>> +       /* try plb device first */
>>
>> What is plb?
>
> I mean Processor Local Bus, a concept I believe is popular in the
> embedded powerpc world, but I realized it might not be that popuar to
> other architectures. Maybe I could just remove it.

Seems fine, but might want to write it out in full.

>
>>>         addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
>>> -       if (addr == FDT_ADDR_T_NONE)
>>> +       if (addr == FDT_ADDR_T_NONE) {
>>> +#ifdef CONFIG_PCI
>>> +               /* then try pci device */
>>> +               struct fdt_pci_addr pci_addr;
>>> +               u32 bar;
>>> +               int ret;
>>> +
>>> +               /* we prefer to use memory mapped register */
>>
>> a memory-mapped
>>
>>> +               ret = fdtdec_get_pci_addr(gd->fdt_blob, dev->of_offset,
>>> +                                         FDT_PCI_SPACE_MEM32, "reg",
>>> +                                         &pci_addr);
>>> +               if (ret) {
>>> +                       /* try if there is any io mapped register */
>>> +                       ret = fdtdec_get_pci_addr(gd->fdt_blob,
>>> +                                                 dev->of_offset,
>>> +                                                 FDT_PCI_SPACE_IO,
>>> +                                                 "reg", &pci_addr);
>>> +                       if (ret)
>>> +                               return ret;
>>> +               }
>>> +
>>> +               ret = fdtdec_get_pci_bar32(gd->fdt_blob, dev->of_offset,
>>> +                                          &pci_addr, &bar);
>>> +               if (ret)
>>> +                       return ret;
>>> +
>>> +               addr = bar;
>>> +               goto cont;
>>> +#endif
>>>                 return -EINVAL;
>>> +       }
>>
>> Instead of the above 4 lines, move the #ifdef CONFIG_PCI up one line, then here:
>>
>>    }
>> if (addr == FDT_ADDR_T_NONE)
>>    return -EINVAL;
>>
>> This avoids the goto.
>
> Yep, this is better. Will fix it.
>
>>>
>>> +cont:
>>>         plat->base = addr;
>>>         plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>>>                                          "reg-shift", 1);
>>> --
>>> 1.8.2.1

Regards,
Simon

[U-Boot] [PATCH v2 6/7] x86: Use ePAPR defined properties for x86-uart

[Simon Glass]

Hi Bin,

On 29 December 2014 at 00:45, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Sun, Dec 28, 2014 at 9:55 AM, Simon Glass <sjg@chromium.org> wrote:
>> On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Use ePAPR defined properties for x86-uart: clock-frequency and
>>> current-speed. Assign the value of clock-frequency in device tree
>>> to plat->clock of x86-uart instead of using hardcoded number.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>
>>>
>>> ---
>>>
>>> Changes in v2:
>>> - New patch to use ePAPR defined properties for x86-uart
>>>
>>>  arch/x86/dts/serial.dtsi    | 5 ++---
>>>  drivers/serial/serial_x86.c | 8 +++++++-
>>>  2 files changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/x86/dts/serial.dtsi b/arch/x86/dts/serial.dtsi
>>> index ebdda76..9b097f4 100644
>>> --- a/arch/x86/dts/serial.dtsi
>>> +++ b/arch/x86/dts/serial.dtsi
>>> @@ -3,8 +3,7 @@
>>>                 compatible = "x86-uart";
>>>                 reg = <0x3f8 8>;
>>>                 reg-shift = <0>;
>>> -               io-mapped = <1>;
>>> -               multiplier = <1>;
>>> -               baudrate = <115200>;
>>> +               clock-frequency = <1843200>;
>>> +               current-speed = <115200>;
>>
>> Where is current speed used? If it is needed, please update the
>> binding at doc/device-tree-bindings/serial/ns16550.txt
>>
>
> The current-speed is the baud rate of the serial port. Currently it is
> put there for ePAPR completeness and not used by U-Boot as U-Boot is
> using gd->baudrate for the serial console.

OK I see, thanks.

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

[U-Boot] [PATCH v2 4/7] fdt: Add several apis to decode pci device node

[Bin Meng]

Hi Simon,

On Sun, Dec 28, 2014 at 9:55 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 27 December 2014 at 05:10, Bin Meng <bmeng.cn@gmail.com> wrote:
>> This commit adds several APIs to decode PCI device node according to
>> the Open Firmware PCI bus bindings, including:
>> - fdtdec_get_pci_addr() for encoded pci address
>> - fdtdec_get_pci_vendev() for vendor id and device id
>> - fdtdec_get_pci_bdf() for pci device bdf triplet
>> - fdtdec_get_pci_bar32() for pci device register bar
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> Looks good - some minor comments below.
>
>>
>> ---
>>
>> Changes in v2:
>> - New patch to add several apis to decode pci device node
>>
>>  include/fdtdec.h | 108 ++++++++++++++++++++++++++++++++++----
>>  lib/fdtdec.c     | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
>>  2 files changed, 240 insertions(+), 25 deletions(-)
>>
>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>> index d2b665c..2b2652f 100644
>> --- a/include/fdtdec.h
>> +++ b/include/fdtdec.h
>> @@ -50,6 +50,49 @@ struct fdt_resource {
>>         fdt_addr_t end;
>>  };
>>
>> +enum fdt_pci_space {
>> +       FDT_PCI_SPACE_CONFIG = 0,
>> +       FDT_PCI_SPACE_IO = 0x01000000,
>> +       FDT_PCI_SPACE_MEM32 = 0x02000000,
>> +       FDT_PCI_SPACE_MEM64 = 0x03000000,
>> +       FDT_PCI_SPACE_MEM32_PREF = 0x42000000,
>> +       FDT_PCI_SPACE_MEM64_PREF = 0x43000000,
>> +};
>> +
>> +#define FDT_PCI_ADDR_CELLS     3
>> +#define FDT_PCI_SIZE_CELLS     2
>> +#define FDT_PCI_REG_SIZE       \
>> +       ((FDT_PCI_ADDR_CELLS + FDT_PCI_SIZE_CELLS) * sizeof(u32))
>> +
>> +/*
>> + * The Open Firmware spec defines PCI physical address as follows:
>> + *
>> + *          bits# 31 .... 24 23 .... 16 15 .... 08 07 .... 00
>> + *
>> + * phys.hi  cell:  npt000ss   bbbbbbbb   dddddfff   rrrrrrrr
>> + * phys.mid cell:  hhhhhhhh   hhhhhhhh   hhhhhhhh   hhhhhhhh
>> + * phys.lo  cell:  llllllll   llllllll   llllllll   llllllll
>> + *
>> + * where:
>> + *
>> + * n:        is 0 if the address is relocatable, 1 otherwise
>> + * p:        is 1 if addressable region is prefetchable, 0 otherwise
>> + * t:        is 1 if the address is aliased (for non-relocatable I/O) below 1MB
>> + *           (for Memory), or below 64KB (for relocatable I/O)
>> + * ss:       is the space code, denoting the address space
>> + * bbbbbbbb: is the 8-bit Bus Number
>> + * ddddd:    is the 5-bit Device Number
>> + * fff:      is the 3-bit Function Number
>> + * rrrrrrrr: is the 8-bit Register Number
>> + * hhhhhhhh: is a 32-bit unsigned number
>> + * llllllll: is a 32-bit unsigned number
>> + */
>> +struct fdt_pci_addr {
>> +       u32     phys_hi;
>> +       u32     phys_mid;
>> +       u32     phys_lo;
>> +};
>> +
>>  /**
>>   * Compute the size of a resource.
>>   *
>> @@ -252,6 +295,60 @@ fdt_addr_t fdtdec_get_addr_size(const void *blob, int node,
>>                 const char *prop_name, fdt_size_t *sizep);
>>
>>  /**
>> + * Look at an address property in a node and return the pci address which
>> + * corresponds to the given type in the form of fdt_pci_addr.
>> + * The property must hold one fdt_pci_addr with a lengh.
>> + *
>> + * @param blob         FDT blob
>> + * @param node         node to examine
>> + * @param type         pci address type (FDT_PCI_SPACE_xxx)
>> + * @param prop_name    name of property to find
>> + * @param addr         returns pci address in the form of fdt_pci_addr
>> + * @return 0 if ok, negative on error
>> + */
>> +int fdtdec_get_pci_addr(const void *blob, int node, enum fdt_pci_space type,
>> +               const char *prop_name, struct fdt_pci_addr *addr);
>> +
>> +/**
>> + * Look at the compatible property of a device node that represents a PCI
>> + * device and extract pci vendor id and device id from it.
>> + *
>> + * @param blob         FDT blob
>> + * @param node         node to examine
>> + * @param vendor       vendor id of the pci device
>> + * @param device       device id of the pci device
>> + * @return 0 if ok, negative on error
>> + */
>> +int fdtdec_get_pci_vendev(const void *blob, int node,
>> +               u16 *vendor, u16 *device);
>> +
>> +/**
>> + * Look at the pci address of a device node that represents a PCI device
>> + * and parse the bus, device and function number from it.
>> + *
>> + * @param blob         FDT blob
>> + * @param node         node to examine
>> + * @param addr         pci address in the form of fdt_pci_addr
>> + * @param bdf          returns bus, device, function triplet
>> + * @return 0 if ok, negative on error
>> + */
>> +int fdtdec_get_pci_bdf(const void *blob, int node,
>> +               struct fdt_pci_addr *addr, pci_dev_t *bdf);
>> +
>> +/**
>> + * Look at the pci address of a device node that represents a PCI device
>> + * and return base address of the pci device's registers.
>> + *
>> + * @param blob         FDT blob
>> + * @param node         node to examine
>> + * @param addr         pci address in the form of fdt_pci_addr
>> + * @param bar          returns base address of the pci device's registers
>> + * @return 0 if ok, negative on error
>> + */
>> +int fdtdec_get_pci_bar32(const void *blob, int node,
>> +               struct fdt_pci_addr *addr, u32 *bar);
>> +
>> +/**
>>   * Look up a 32-bit integer property in a node and return it. The property
>>   * must have at least 4 bytes of data. The value of the first cell is
>>   * returned.
>> @@ -677,17 +774,6 @@ int fdt_get_named_resource(const void *fdt, int node, const char *property,
>>                            struct fdt_resource *res);
>>
>>  /**
>> - * Look at the reg property of a device node that represents a PCI device
>> - * and parse the bus, device and function number from it.
>> - *
>> - * @param fdt          FDT blob
>> - * @param node         node to examine
>> - * @param bdf          returns bus, device, function triplet
>> - * @return 0 if ok, negative on error
>> - */
>> -int fdtdec_pci_get_bdf(const void *fdt, int node, int *bdf);
>> -
>> -/**
>>   * Decode a named region within a memory bank of a given type.
>>   *
>>   * This function handles selection of a memory region. The region is
>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> index 9d86dba..e40430e 100644
>> --- a/lib/fdtdec.c
>> +++ b/lib/fdtdec.c
>> @@ -121,6 +121,149 @@ fdt_addr_t fdtdec_get_addr(const void *blob, int node,
>>         return fdtdec_get_addr_size(blob, node, prop_name, NULL);
>>  }
>>
>> +#ifdef CONFIG_PCI
>> +int fdtdec_get_pci_addr(const void *blob, int node, enum fdt_pci_space type,
>> +               const char *prop_name, struct fdt_pci_addr *addr)
>> +{
>> +       const u32 *cell;
>> +       int len;
>> +
>> +       debug("%s: %s: ", __func__, prop_name);
>> +
>> +       /*
>> +        * If we follow the pci bus bindings strictly, we should check
>> +        * the value of the node's parant node's #address-cells and
>
> parent

Fixed in v3.

>> +        * #size-cells. They need to be 3 and 2 accordingly. However,
>> +        * for simplicity we skip the check here.
>> +        */
>> +       cell = fdt_getprop(blob, node, prop_name, &len);
>
> I think it would be better to return -ENOENT if cell is NULL and
> -EINVAL in the case where the number of cells is not a multiple of 5.

Will do in v3.

>> +
>> +       if (cell && ((len % FDT_PCI_REG_SIZE) == 0)) {
>> +               int num = len / FDT_PCI_REG_SIZE;
>> +               int i;
>> +
>> +               for (i = 0; i < num; i++) {
>> +                       if ((fdt_addr_to_cpu(*cell) & type) == type) {
>> +                               addr->phys_hi = fdt_addr_to_cpu(cell[0]);
>> +                               addr->phys_mid = fdt_addr_to_cpu(cell[1]);
>> +                               addr->phys_lo = fdt_addr_to_cpu(cell[2]);
>> +                               break;
>> +                       } else {
>> +                               cell += (FDT_PCI_ADDR_CELLS +
>> +                                        FDT_PCI_SIZE_CELLS);
>> +                       }
>
> You need a debug() in here to print stuff out, and a way of getting a
> \n at the end.

Will do in v3.

>> +               }
>> +
>> +               if (i == num)
>> +                       goto fail;
>> +
>> +               return 0;
>> +       }
>> +
>> +fail:
>> +       debug("(not found)\n");
>> +       return -ENOENT;
>> +}
>> +
>> +int fdtdec_get_pci_vendev(const void *blob, int node, u16 *vendor, u16 *device)
>> +{
>> +       const char *list, *end;
>> +       int len;
>> +
>> +       list = fdt_getprop(blob, node, "compatible", &len);
>> +       if (!list)
>> +               return -ENOENT;
>> +
>> +       end = list + len;
>> +       while (list < end) {
>> +               int l = strlen(list);
>
> s/l/len/

Will do in v3.

>> +               char *s, v[5], d[5];
>> +
>> +               s = strstr(list, "pci");
>> +               /* check if the string is something like pciVVVV,DDDD */
>> +               if (s && s[7] == ',') {
>
> Should check that end - list > 7 otherwise s[7] might not exist. Also
> how about checking for the '.'?

Will do in v3.

>> +                       s += 3;
>> +                       strlcpy(v, s, 5);
>> +                       s += 5;
>> +                       strlcpy(d, s, 5);
>> +
>> +                       *vendor = simple_strtol(v, NULL, 16);
>> +                       *device = simple_strtol(d, NULL, 16);
>
> I suspect you can avoid this - just use simple_strtol() directly on
> the correct places in the string. The function will automatically stop
> when it sees no more hex digits.

Yes, you are right. Will fix this.

>> +
>> +                       return 0;
>> +               } else {
>> +                       list += (l + 1);
>> +               }
>> +       }
>> +
>> +       return -ENOENT;
>> +}
>> +
>> +int fdtdec_get_pci_bdf(const void *blob, int node,
>> +               struct fdt_pci_addr *addr, pci_dev_t *bdf)
>> +{
>> +       u16 dt_vendor, dt_device, vendor, device;
>> +       int ret;
>> +
>> +       /* get vendor id & device id from the compatible string */
>> +       ret = fdtdec_get_pci_vendev(blob, node, &dt_vendor, &dt_device);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* extract the bdf from fdt_pci_addr */
>> +       *bdf = addr->phys_hi & 0xffff00;
>> +
>> +       /* read vendor id & device id based on bdf */
>> +       pci_read_config_word(*bdf, PCI_VENDOR_ID, &vendor);
>> +       pci_read_config_word(*bdf, PCI_DEVICE_ID, &device);
>
> Check return values

I think there is no need to check return values, as if there is
anything wrong with the pci_read_config_word() call, the vendor/device
will be set to 0xffff, which are invalid values and will fail at the
codes below.

>> +
>> +       /*
>> +        * Note there are two places in the device tree to fully describe
>> +        * a pci device: one is via compatible string with a format of
>> +        * "pciVVVV,DDDD" and the other one is the bdf numbers encoded in
>> +        * the device node's reg address property. We read the vendor id
>> +        * and device id based on bdf and compare the values with the
>> +        * "VVVV,DDDD". If they are the same, then we are good to use bdf
>> +        * to read device's bar. But if they are different, we have to rely
>> +        * on the vendor id and device id extracted from the compatible
>> +        * string and locate the real bdf by pci_find_device(). This is
>> +        * because normally we may only know device's device number and
>> +        * function number when writing device tree. The bus number is
>> +        * dynamically assigned during the pci enumeration process.
>> +        */
>> +       if ((dt_vendor != vendor) || (dt_device != device)) {
>> +               *bdf = pci_find_device(dt_vendor, dt_device, 0);
>> +               if (*bdf == -1)
>> +                       return -ENODEV;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +int fdtdec_get_pci_bar32(const void *blob, int node,
>> +               struct fdt_pci_addr *addr, u32 *bar)
>> +{
>> +       pci_dev_t bdf;
>> +       int bn;
>
> s/bn/barnum/ since it is self-documenting.

Will do in v3.

[snip]

Regards,
Bin