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

[Bin Meng]

Hi Simon,

On Sat, Feb 20, 2021 at 7:55 PM Simon Glass <sjg at chromium.org> wrote:
>
> 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

I think we can completely remove these APIs once PPC PCI is converted
to DM. We can revisit this later.

> nice to have a test.
>
> Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Bin