| Message ID | 20200619015150.27745-2-atish.patra@wdc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Assorted fixes related to reserved memory | expand |
Hi Atish, On Fri, Jun 19, 2020 at 9:52 AM Atish Patra <atish.patra at wdc.com> wrote: > > 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 at 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 6db48ad04a56..91524d9a5ae9 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) { This FDT_ERR_EXISTS error code is possibly returned by: node = fdt_add_subnode(blob, parent, name); if (node < 0) return node; But if it exists, I believe fdtdec_add_reserved_memory() already returns 0 because they are likely to have the same range of memory block start address and size, no? > printf("failed to add reserved memory: %d\n", err); > return err; > } > -- Regards, Bin
On Tue, Jun 23, 2020 at 12:24 AM Bin Meng <bmeng.cn at gmail.com> wrote: > > Hi Atish, > > On Fri, Jun 19, 2020 at 9:52 AM Atish Patra <atish.patra at wdc.com> wrote: > > > > 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 at 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 6db48ad04a56..91524d9a5ae9 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) { > > This FDT_ERR_EXISTS error code is possibly returned by: > > node = fdt_add_subnode(blob, parent, name); > if (node < 0) > return node; > > But if it exists, I believe fdtdec_add_reserved_memory() already > returns 0 because they are likely to have the same range of memory > block start address and size, no? > Currently, yes. As libfdt and fdtdec_add_reserved_memory external to this code, functionality can change without modifying fdt_fixup.c knowingly or unknowingly. FDT_ERR_EXISTS is harmless error in this context and we shouldn't cause boot failure because of that. That's why I add that exclusion. > > printf("failed to add reserved memory: %d\n", err); > > return err; > > } > > -- > > Regards, > Bin
Hi Atish, On Wed, Jun 24, 2020 at 2:17 AM Atish Patra <atishp at atishpatra.org> wrote: > > On Tue, Jun 23, 2020 at 12:24 AM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > Hi Atish, > > > > On Fri, Jun 19, 2020 at 9:52 AM Atish Patra <atish.patra at wdc.com> wrote: > > > > > > 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 at 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 6db48ad04a56..91524d9a5ae9 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) { > > > > This FDT_ERR_EXISTS error code is possibly returned by: > > > > node = fdt_add_subnode(blob, parent, name); > > if (node < 0) > > return node; > > > > But if it exists, I believe fdtdec_add_reserved_memory() already > > returns 0 because they are likely to have the same range of memory > > block start address and size, no? > > > > Currently, yes. As libfdt and fdtdec_add_reserved_memory external to > this code, functionality > can change without modifying fdt_fixup.c knowingly or unknowingly. > FDT_ERR_EXISTS is harmless error in this context and we shouldn't > cause boot failure because of that. > That's why I add that exclusion. Okay. But the error should be checked against -FDT_ERR_EXISTS. Regards, Bin
On Tue, Jun 23, 2020 at 5:51 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > Hi Atish, > > On Wed, Jun 24, 2020 at 2:17 AM Atish Patra <atishp at atishpatra.org> wrote: > > > > On Tue, Jun 23, 2020 at 12:24 AM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > > > Hi Atish, > > > > > > On Fri, Jun 19, 2020 at 9:52 AM Atish Patra <atish.patra at wdc.com> wrote: > > > > > > > > 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 at 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 6db48ad04a56..91524d9a5ae9 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) { > > > > > > This FDT_ERR_EXISTS error code is possibly returned by: > > > > > > node = fdt_add_subnode(blob, parent, name); > > > if (node < 0) > > > return node; > > > > > > But if it exists, I believe fdtdec_add_reserved_memory() already > > > returns 0 because they are likely to have the same range of memory > > > block start address and size, no? > > > > > > > Currently, yes. As libfdt and fdtdec_add_reserved_memory external to > > this code, functionality > > can change without modifying fdt_fixup.c knowingly or unknowingly. > > FDT_ERR_EXISTS is harmless error in this context and we shouldn't > > cause boot failure because of that. > > That's why I add that exclusion. > > Okay. But the error should be checked against -FDT_ERR_EXISTS. > My bad. I will fix it and send the next version. > Regards, > Bin
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index 6db48ad04a56..91524d9a5ae9 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; }
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 at wdc.com> --- arch/riscv/lib/fdt_fixup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)