[PATCH v2 03/38] common: fdt_support: Support special case of PCI address in fdt_read_prop()

[Simon Glass]

Hi Bin,

On Thu, 18 Feb 2021 at 08:58, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> At present fdt_read_prop() can only handle 1 or 2 cells. It is
> called by fdt_read_range() which may be used to read PCI address
> from <ranges> for a PCI bus node where the number of PCI address
> cell is 3. The <ranges> property is an array of:
>
>   { <child address> <parent address> <size in child address space> }
>
> When trying to read <child address> from a PCI bus node using
> fdt_read_prop(), as the codes below:
>
>     /* Read <child address> */
>     if (child_addr) {
>         r = fdt_read_prop(ranges, ranges_len, cell, child_addr,
>                           acells);
>         if (r)
>             return r;
>     }
>
> it will fail, because the PCI child address is made up of 3 cells
> but fdt_read_prop() cannot handle it. We advance the cell offset
> by 1 so that the <child address> can be correctly read.
>
> This adds the special handling of such case.
>
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>
> ---
>
> Changes in v2:
> - add more details in the commit message, and put some comments
>   in the codes to explain why
>
>  common/fdt_support.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 638eca9..17c54a3 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -1602,22 +1602,36 @@ u64 fdt_get_base_address(const void *fdt, int node)
>  }
>
>  /*
> - * Read a property of size <prop_len>. Currently only supports 1 or 2 cells.
> + * Read a property of size <prop_len>. Currently only supports 1 or 2 cells,
> + * or 3 cells specially for a PCI address.
>   */
>  static int fdt_read_prop(const fdt32_t *prop, int prop_len, int cell_off,
>                          uint64_t *val, int cells)
>  {
> -       const fdt32_t *prop32 = &prop[cell_off];
> -       const unaligned_fdt64_t *prop64 = (const fdt64_t *)&prop[cell_off];
> +       const fdt32_t *prop32;
> +       const unaligned_fdt64_t *prop64;
>
>         if ((cell_off + cells) > prop_len)
>                 return -FDT_ERR_NOSPACE;
>
> +       prop32 = &prop[cell_off];
> +
> +       /*
> +        * Special handling for PCI address in PCI bus <ranges>
> +        *
> +        * PCI child address is made up of 3 cells. Advance the cell offset
> +        * by 1 so that the PCI child address can be correctly read.
> +        */
> +       if (cells == 3)
> +               cell_off += 1;
> +       prop64 = (const fdt64_t *)&prop[cell_off];
> +
>         switch (cells) {
>         case 1:
>                 *val = fdt32_to_cpu(*prop32);
>                 break;
>         case 2:
> +       case 3:
>                 *val = fdt64_to_cpu(*prop64);
>                 break;
>         default:
> --
> 2.7.4
>

I'm not sure whether this is needed once PPC PCI is converted to DM.
If not, then perhaps we can revert it later. If it is then it would be
nice to have a test.

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon