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

[Simon Glass]

Hi Bin,

On Sun, 7 Feb 2021 at 22:17, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Feb 8, 2021 at 12:21 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Sun, 7 Feb 2021 at 08:11, 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.
> > >
> > > This adds the special handling of such case.
> > >
> > > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> > > ---
> > >
> > >  common/fdt_support.c | 15 ++++++++++++---
> > >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > I think this is missing why it is needed. Also needs an update to
> > function to the comment in the header file.
> >
>
> I will try to add more details in v2.
>
> The reason why this is needed is because in fdt_read_range() may be
> used to read PCI address from <ranges>.
> 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.

OK, that could go in the commit message and a function comment
somewhere about the case.

There is similar code in decode_regions(), so we have two places that
read ranges.

Regards,
Simon