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

Bin Meng bmeng.cn at gmail.com
Wed Dec 31 08:43:14 CET 2014


Hi Simon,

On Wed, Dec 31, 2014 at 2:48 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 30 December 2014 at 07:53, 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>
>>
>> ---
>>
>> 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 <sjg at 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


More information about the U-Boot mailing list