[U-Boot] [PATCH v2 4/7] fdt: Add several apis to decode pci device node
Simon Glass
sjg at chromium.org
Sun Dec 28 02:55:37 CET 2014
Hi Bin,
On 27 December 2014 at 05:10, Bin Meng <bmeng.cn at 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 at 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
More information about the U-Boot
mailing list