[U-Boot] [PATCH v2 0/7] x86: Support pci based uart as the U-Boot serial console
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(-)
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)
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(+)
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;
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(+)
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;
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(-)
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)
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
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 andparent
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
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);
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
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
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
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,
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
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
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
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@17,0 { + #address-cells = <3>; + #size-cells = <2>; + compatible = "intel,pci"; + device_type = "pci"; + + topcliff@0,0 { + #address-cells = <3>; + #size-cells = <2>; + compatible = "intel,pci"; + device_type = "pci"; + + pciuart0: uart@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@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@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@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>; + }; + }; + }; + }; + };
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
participants (2)
-
Bin Meng -
Simon Glass