| Message ID | 1612710687-56493-4-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Priyanka Jain |
| Headers | show |
| Series | ppc: qemu: Convert qemu-ppce500 to driver model | expand |
Hi Bin, On Sun, 7 Feb 2021 at 08:11, Bin Meng <bmeng.cn@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@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. Regards, Simon
Hi Simon, On Mon, Feb 8, 2021 at 12:21 PM Simon Glass <sjg@chromium.org> wrote: > > Hi Bin, > > On Sun, 7 Feb 2021 at 08:11, Bin Meng <bmeng.cn@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@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. Regards, Bin
Hi Bin, On Sun, 7 Feb 2021 at 22:17, Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Simon, > > On Mon, Feb 8, 2021 at 12:21 PM Simon Glass <sjg@chromium.org> wrote: > > > > Hi Bin, > > > > On Sun, 7 Feb 2021 at 08:11, Bin Meng <bmeng.cn@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@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
diff --git a/common/fdt_support.c b/common/fdt_support.c index 638eca9..b3aa865 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -1602,22 +1602,31 @@ 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> */ + 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:
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@gmail.com> --- common/fdt_support.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)