Hi Simon, On Wed, Dec 31, 2014 at 2:48 AM, Simon Glass <s...@chromium.org> wrote: > Hi Bin, > > On 30 December 2014 at 07:53, Bin Meng <bmeng...@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...@gmail.com> >> >> --- >> >> Changes in v3: >> - Fixed a typo: parant -> parent >> - Return better error code in fdtdec_get_pci_addr() >> - Add some debug output in fdtdec_get_pci_addr() >> - Reuse variable 'len' instead of creating a new one 'l' >> - Check compatible string length and existence of '.' >> - Using simple_strtol() directly on the compatible sub-string >> - Change variable 'bn' to 'barnum' which is self-documenting >> >> Changes in v2: >> - New patch to add several apis to decode pci device node >> >> include/fdtdec.h | 108 ++++++++++++++++++++++++++++++++---- >> lib/fdtdec.c | 166 >> ++++++++++++++++++++++++++++++++++++++++++++++++++----- >> 2 files changed, 249 insertions(+), 25 deletions(-) > > Acked-by: Simon Glass <s...@chromium.org> > > See question below... > >> >> 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..a703f57 100644 >> --- a/lib/fdtdec.c >> +++ b/lib/fdtdec.c >> @@ -121,6 +121,158 @@ 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; >> + int ret = -ENOENT; >> + >> + debug("%s: %s: ", __func__, prop_name); >> + >> + /* >> + * If we follow the pci bus bindings strictly, we should check >> + * the value of the node's parent 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) >> + goto fail; >> + >> + if ((len % FDT_PCI_REG_SIZE) == 0) { >> + int num = len / FDT_PCI_REG_SIZE; >> + int i; >> + >> + for (i = 0; i < num; i++) { >> + debug("pci address #%d: %08lx %08lx %08lx\n", i, >> + (ulong)fdt_addr_to_cpu(cell[0]), >> + (ulong)fdt_addr_to_cpu(cell[1]), >> + (ulong)fdt_addr_to_cpu(cell[2])); >> + 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; >> + } else { >> + ret = -EINVAL; >> + } >> + >> +fail: >> + debug("(not found)\n"); >> + return ret; >> +} >> + >> +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) { >> + char *s; >> + >> + len = strlen(list); >> + if (len > strlen("pciVVVV,DDDD")) { >> + s = strstr(list, "pci"); >> + /* check if the string is something like >> pciVVVV,DDDD */ > > You are also requiring a revision ID here, but that doesn't seem to be > absolutely required by the spec. This is fine if that's what you want, > but should probably add a '.R'R at the end of of the comment. Another > option would be to allow s[12] to be either '.' or \0. The subsystem > ID is not supported which is fine.
I originally thought the pciVVVV,DDDD.RR should always be there in the device tree, but I think checking s[12] against \0 is not bad. > Please let me know which way you'd like to go - and either I can fix > it up before applying, or you can send a new patch. I will send a new patch to check s[12] against \0. [snip] Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot