[PATCH v2] lib: utils: fdt: fix "ranges" translation

[Anup Patel]

On Fri, Jul 11, 2025 at 12:02 AM Max Hsu <max.hsu at sifive.com> wrote:
>
> According to the Device Tree Spec, Chapter 2.3.8 "ranges" [1]:
> The parent address size will be determined from the #address-cells
> property of the node that defines the parent’s address space.
>
> In fdt_translate_address(), which considered the parent address size
> is the child address size, this commit fix the two address sizes
> and parsing the address independently.
>
> Signed-off-by: Max Hsu <max.hsu at sifive.com>

LGTM.

Reviewed-by: Anup Patel <anup at brainfault.org>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

> ---
> v1 fix will introduce another bug in the following case.
>
> The node in DTS depth 1 if execute fdt_get_node_addr_size(), inside
> the function, the fdt_translate_address() will try to get the child
> "#address-cells" and "#size-cells" in DTS depth 0, and the parent
> "#address-cells" in DTS depth -1. which will cause early abort.
> With fdt_translate_address() early abort, the parameter "addr" didn't
> got any assignment of the address value. This let
> fdt_get_node_addr_size() return with unknown value of parameter "addr".
>
> For the v2 fix, which will check the "ranges" first, since the above
> case shouldn't happen in the DTS as the "ranges" should be existed in
> DTS depth more than 0. This fix also improve execution cycle.
> ---
> Changes in v2:
> - fix v1 bug mentioned in the cover letter.
> - Link to v1: https://lore.kernel.org/r/20250708-dev-maxh-master_fdt_helper-v1-1-866c165f1f85@sifive.com
> ---
>  lib/utils/fdt/fdt_helper.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> index 0f4859c1a721bc7c2871bacefe27caba7780f752..799fd48dc3abf0a3c1963f84dd6bde271e6c08d7 100644
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -84,23 +84,27 @@ static int fdt_translate_address(const void *fdt, uint64_t reg, int parent,
>                                  uint64_t *addr)
>  {
>         int i, rlen;
> -       int cell_addr, cell_size;
> +       int cell_parent_addr, cell_child_addr, cell_size;
>         const fdt32_t *ranges;
>         uint64_t offset, caddr = 0, paddr = 0, rsize = 0;
>
> -       cell_addr = fdt_address_cells(fdt, parent);
> -       if (cell_addr < 1)
> -               return SBI_ENODEV;
> -
> -       cell_size = fdt_size_cells(fdt, parent);
> -       if (cell_size < 0)
> -               return SBI_ENODEV;
> -
>         ranges = fdt_getprop(fdt, parent, "ranges", &rlen);
>         if (ranges && rlen > 0) {
> -               for (i = 0; i < cell_addr; i++)
> +               cell_child_addr = fdt_address_cells(fdt, parent);
> +               if (cell_child_addr < 1)
> +                       return SBI_ENODEV;
> +
> +               cell_parent_addr = fdt_address_cells(fdt, fdt_parent_offset(fdt, parent));
> +               if (cell_parent_addr < 1)
> +                       return SBI_ENODEV;
> +
> +               cell_size = fdt_size_cells(fdt, parent);
> +               if (cell_size < 0)
> +                       return SBI_ENODEV;
> +
> +               for (i = 0; i < cell_child_addr; i++)
>                         caddr = (caddr << 32) | fdt32_to_cpu(*ranges++);
> -               for (i = 0; i < cell_addr; i++)
> +               for (i = 0; i < cell_parent_addr; i++)
>                         paddr = (paddr << 32) | fdt32_to_cpu(*ranges++);
>                 for (i = 0; i < cell_size; i++)
>                         rsize = (rsize << 32) | fdt32_to_cpu(*ranges++);
>
> ---
> base-commit: a32a91069119e7a5aa31e6bc51d5e00860be3d80
> change-id: 20250708-dev-maxh-master_fdt_helper-46d2eeeb07cd
>
> Best regards,
> --
> Max Hsu <max.hsu at sifive.com>
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi