[PATCH 1/2] riscv: Use correct version of fdtdec_get_addr_size
fdtdec_get_addr_size uses a fixed value for address_cells & size_cells which may not work correctly always. fdtdec_get_addr_size_no_parent will automatically calculate the cell sizes from parent but not optimized especially when parent node is already available with the caller.
Use fdtdec_get_addr_size_auto_parent that automatically calculate the cell sizes and optimized for the given usecase.
Signed-off-by: Atish Patra atish.patra@wdc.com --- arch/riscv/lib/fdt_fixup.c | 2 +- lib/fdtdec.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index 6db48ad04a56..f2ec37b57b15 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -44,7 +44,7 @@ int riscv_fdt_copy_resv_mem_node(const void *src, void *dst) fdt_for_each_subnode(node, src, offset) { name = fdt_get_name(src, node, NULL);
- addr = fdtdec_get_addr_size_auto_noparent(src, node, + addr = fdtdec_get_addr_size_auto_parent(src, offset, node, "reg", 0, &size, false); if (addr == FDT_ADDR_T_NONE) { diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1f2b763acc31..b62eb142ccc3 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1296,7 +1296,8 @@ int fdtdec_add_reserved_memory(void *blob, const char *basename, const char *name = fdt_get_name(blob, node, NULL); phys_addr_t addr, size;
- addr = fdtdec_get_addr_size(blob, node, "reg", &size); + addr = fdtdec_get_addr_size_auto_parent(blob, parent, node, + "reg", 0, &size, false); if (addr == FDT_ADDR_T_NONE) { debug("failed to read address/size for %s\n", name); continue;
Not all errors are fatal. If a reserved memory node already exists in the destination device tree, we can continue to boot without failing.
Signed-off-by: Atish Patra atish.patra@wdc.com --- arch/riscv/lib/fdt_fixup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index f2ec37b57b15..00b84dccbef0 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -62,7 +62,7 @@ int riscv_fdt_copy_resv_mem_node(const void *src, void *dst) pmp_mem.end = addr + size - 1; err = fdtdec_add_reserved_memory(dst, basename, &pmp_mem, &phandle); - if (err < 0) { + if (err < 0 && err != FDT_ERR_EXISTS) { printf("failed to add reserved memory: %d\n", err); return err; }
Hi Atish,
On Thu, Jun 18, 2020 at 7:51 AM Atish Patra atish.patra@wdc.com wrote:
fdtdec_get_addr_size uses a fixed value for address_cells & size_cells which may not work correctly always. fdtdec_get_addr_size_no_parent will automatically calculate the cell sizes from parent but not optimized especially when parent node is already available with the caller.
Use fdtdec_get_addr_size_auto_parent that automatically calculate the cell sizes and optimized for the given usecase.
Signed-off-by: Atish Patra atish.patra@wdc.com
arch/riscv/lib/fdt_fixup.c | 2 +- lib/fdtdec.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index 6db48ad04a56..f2ec37b57b15 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -44,7 +44,7 @@ int riscv_fdt_copy_resv_mem_node(const void *src, void *dst) fdt_for_each_subnode(node, src, offset) { name = fdt_get_name(src, node, NULL);
addr = fdtdec_get_addr_size_auto_noparent(src, node,
addr = fdtdec_get_addr_size_auto_parent(src, offset, node, "reg", 0, &size, false); if (addr == FDT_ADDR_T_NONE) {diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1f2b763acc31..b62eb142ccc3 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1296,7 +1296,8 @@ int fdtdec_add_reserved_memory(void *blob, const char *basename, const char *name = fdt_get_name(blob, node, NULL); phys_addr_t addr, size;
addr = fdtdec_get_addr_size(blob, node, "reg", &size);
addr = fdtdec_get_addr_size_auto_parent(blob, parent, node,"reg", 0, &size, false);
There is already a patch for this change (although slightly different, but fixed the same thing) http://patchwork.ozlabs.org/project/uboot/patch/1591767391-2669-2-git-send-e...
Please also send them in separate patches in the future, as one is RISC-V specific and the other one is for generic codes.
Regards, Bin
On Wed, Jun 17, 2020 at 5:46 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Atish,
On Thu, Jun 18, 2020 at 7:51 AM Atish Patra atish.patra@wdc.com wrote:
fdtdec_get_addr_size uses a fixed value for address_cells & size_cells which may not work correctly always. fdtdec_get_addr_size_no_parent will automatically calculate the cell sizes from parent but not optimized especially when parent node is already available with the caller.
Use fdtdec_get_addr_size_auto_parent that automatically calculate the cell sizes and optimized for the given usecase.
Signed-off-by: Atish Patra atish.patra@wdc.com
arch/riscv/lib/fdt_fixup.c | 2 +- lib/fdtdec.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index 6db48ad04a56..f2ec37b57b15 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -44,7 +44,7 @@ int riscv_fdt_copy_resv_mem_node(const void *src, void *dst) fdt_for_each_subnode(node, src, offset) { name = fdt_get_name(src, node, NULL);
addr = fdtdec_get_addr_size_auto_noparent(src, node,
addr = fdtdec_get_addr_size_auto_parent(src, offset, node, "reg", 0, &size, false); if (addr == FDT_ADDR_T_NONE) {diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1f2b763acc31..b62eb142ccc3 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1296,7 +1296,8 @@ int fdtdec_add_reserved_memory(void *blob, const char *basename, const char *name = fdt_get_name(blob, node, NULL); phys_addr_t addr, size;
addr = fdtdec_get_addr_size(blob, node, "reg", &size);
addr = fdtdec_get_addr_size_auto_parent(blob, parent, node,"reg", 0, &size, false);There is already a patch for this change (although slightly different, but fixed the same thing) http://patchwork.ozlabs.org/project/uboot/patch/1591767391-2669-2-git-send-e...
Ah I missed that. I used this version compared to fdtdec_get_addr_size_fixed because of the following comment in the header file.
"You probably don't want to use this function directly except to parse non-standard properties, and never to parse the "reg" property. Instead, use one of the "auto" variants below, which automatically honor the #address-cells and #size-cells properties in the parent node."
Do you want to update your patch or I can send v2 by splitting riscv & generic changes ? Please let me know your preference.
Please also send them in separate patches in the future, as one is RISC-V specific and the other one is for generic codes.
Regards, Bin
Hi Atish,
On Thu, Jun 18, 2020 at 3:04 PM Atish Patra atishp@atishpatra.org wrote:
On Wed, Jun 17, 2020 at 5:46 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Atish,
On Thu, Jun 18, 2020 at 7:51 AM Atish Patra atish.patra@wdc.com wrote:
fdtdec_get_addr_size uses a fixed value for address_cells & size_cells which may not work correctly always. fdtdec_get_addr_size_no_parent will automatically calculate the cell sizes from parent but not optimized especially when parent node is already available with the caller.
Use fdtdec_get_addr_size_auto_parent that automatically calculate the cell sizes and optimized for the given usecase.
Signed-off-by: Atish Patra atish.patra@wdc.com
arch/riscv/lib/fdt_fixup.c | 2 +- lib/fdtdec.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index 6db48ad04a56..f2ec37b57b15 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -44,7 +44,7 @@ int riscv_fdt_copy_resv_mem_node(const void *src, void *dst) fdt_for_each_subnode(node, src, offset) { name = fdt_get_name(src, node, NULL);
addr = fdtdec_get_addr_size_auto_noparent(src, node,
addr = fdtdec_get_addr_size_auto_parent(src, offset, node, "reg", 0, &size, false); if (addr == FDT_ADDR_T_NONE) {diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1f2b763acc31..b62eb142ccc3 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1296,7 +1296,8 @@ int fdtdec_add_reserved_memory(void *blob, const char *basename, const char *name = fdt_get_name(blob, node, NULL); phys_addr_t addr, size;
addr = fdtdec_get_addr_size(blob, node, "reg", &size);
addr = fdtdec_get_addr_size_auto_parent(blob, parent, node,"reg", 0, &size, false);There is already a patch for this change (although slightly different, but fixed the same thing) http://patchwork.ozlabs.org/project/uboot/patch/1591767391-2669-2-git-send-e...
Ah I missed that. I used this version compared to fdtdec_get_addr_size_fixed because of the following comment in the header file.
"You probably don't want to use this function directly except to parse non-standard properties, and never to parse the "reg" property. Instead, use one of the "auto" variants below, which automatically honor the #address-cells and #size-cells properties in the parent node."
Do you want to update your patch or I can send v2 by splitting riscv & generic changes ? Please let me know your preference.
I intended to use fdtdec_get_addr_size_fixed() because na and ns are already known at this point. Calling fdtdec_get_addr_size_auto_parent() duplicates the efforts of getting values of na and ns, and is not necessary.
Please also send them in separate patches in the future, as one is RISC-V specific and the other one is for generic codes.
Regards, Bin
On Thu, Jun 18, 2020 at 12:19 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Atish,
On Thu, Jun 18, 2020 at 3:04 PM Atish Patra atishp@atishpatra.org wrote:
On Wed, Jun 17, 2020 at 5:46 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Atish,
On Thu, Jun 18, 2020 at 7:51 AM Atish Patra atish.patra@wdc.com wrote:
fdtdec_get_addr_size uses a fixed value for address_cells & size_cells which may not work correctly always. fdtdec_get_addr_size_no_parent will automatically calculate the cell sizes from parent but not optimized especially when parent node is already available with the caller.
Use fdtdec_get_addr_size_auto_parent that automatically calculate the cell sizes and optimized for the given usecase.
Signed-off-by: Atish Patra atish.patra@wdc.com
arch/riscv/lib/fdt_fixup.c | 2 +- lib/fdtdec.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index 6db48ad04a56..f2ec37b57b15 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -44,7 +44,7 @@ int riscv_fdt_copy_resv_mem_node(const void *src, void *dst) fdt_for_each_subnode(node, src, offset) { name = fdt_get_name(src, node, NULL);
addr = fdtdec_get_addr_size_auto_noparent(src, node,
addr = fdtdec_get_addr_size_auto_parent(src, offset, node, "reg", 0, &size, false); if (addr == FDT_ADDR_T_NONE) {diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1f2b763acc31..b62eb142ccc3 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1296,7 +1296,8 @@ int fdtdec_add_reserved_memory(void *blob, const char *basename, const char *name = fdt_get_name(blob, node, NULL); phys_addr_t addr, size;
addr = fdtdec_get_addr_size(blob, node, "reg", &size);
addr = fdtdec_get_addr_size_auto_parent(blob, parent, node,"reg", 0, &size, false);There is already a patch for this change (although slightly different, but fixed the same thing) http://patchwork.ozlabs.org/project/uboot/patch/1591767391-2669-2-git-send-e...
Ah I missed that. I used this version compared to fdtdec_get_addr_size_fixed because of the following comment in the header file.
"You probably don't want to use this function directly except to parse non-standard properties, and never to parse the "reg" property. Instead, use one of the "auto" variants below, which automatically honor the #address-cells and #size-cells properties in the parent node."
Do you want to update your patch or I can send v2 by splitting riscv & generic changes ? Please let me know your preference.
I intended to use fdtdec_get_addr_size_fixed() because na and ns are already known at this point. Calling fdtdec_get_addr_size_auto_parent() duplicates the efforts of getting values of na and ns, and is not necessary.
Yes. But the duplication effort is just reading two parameters. Anyways, it's not a big deal. I will drop the generic fix from my patch and resent after rebasing on top of your series.
Please also send them in separate patches in the future, as one is RISC-V specific and the other one is for generic codes.
Regards, Bin
participants (3)
-
Atish Patra -
Atish Patra -
Bin Meng