| Message ID | 1613663886-83811-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 and enable additional driver support | expand |
Hi Bin, On Thu, 18 Feb 2021 at 08:58, 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. 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@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@chromium.org> Regards, Simon
Hi Simon, On Sat, Feb 20, 2021 at 7:55 PM Simon Glass <sjg@chromium.org> wrote: > > Hi Bin, > > On Thu, 18 Feb 2021 at 08:58, 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. 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@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@chromium.org> Regards, Bin
>-----Original Message----- >From: Bin Meng <bmeng.cn@gmail.com> >Sent: Thursday, February 18, 2021 9:28 PM >To: Simon Glass <sjg@chromium.org>; Alexander Graf <agraf@csgraf.de>; >Priyanka Jain <priyanka.jain@nxp.com> >Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Tom Rini ><trini@konsulko.com> >Subject: [PATCH v2 03/38] common: fdt_support: Support special case of PCI >address in fdt_read_prop() > >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@gmail.com> > >--- > Reviewed-by: Priyanka Jain <priyanka.jain@nxp.com>
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:
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@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(-)