diff mbox

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

Message ID 1419682210-21181-5-git-send-email-bmeng.cn@gmail.com
State Changes Requested
Delegated to: Simon Glass
Headers show

Commit Message

Bin Meng Dec. 27, 2014, 12:10 p.m. UTC
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(-)

Comments

Simon Glass Dec. 28, 2014, 1:55 a.m. UTC | #1
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
Bin Meng Dec. 30, 2014, 2:45 p.m. UTC | #2
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
diff mbox

Patch

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)