diff mbox series

[v5,08/13] lib: kconfig: Limit BINMAN_FDT for OF_SEPARATE or OF_EMBED

Message ID 20210510122341.13798-9-bmeng.cn@gmail.com
State Accepted
Commit 1621d3c43414ea7b4307ef521e8ef574774af33f
Delegated to: Andes
Headers show
Series riscv: Switch to use binman to generate u-boot.itb | expand

Commit Message

Bin Meng May 10, 2021, 12:23 p.m. UTC
Generally speaking BINMAN_FDT makes sense for OF_SEPARATE or OF_EMBED.
For the other OF_CONTROL methods, it's quite possible binman node is
not available as binman is invoked during the build phase instead of
runtime. Let's only turn it on for OF_SEPARATE or OF_EMBED by default.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

(no changes since v3)

Changes in v3:
- new patch: "lib: kconfig: Limit BINMAN_FDT for OF_SEPARATE or OF_EMBED"

 lib/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Simon Glass May 10, 2021, 4:28 p.m. UTC | #1
Hi Bin,

On Mon, 10 May 2021 at 06:24, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Generally speaking BINMAN_FDT makes sense for OF_SEPARATE or OF_EMBED.
> For the other OF_CONTROL methods, it's quite possible binman node is
> not available as binman is invoked during the build phase instead of
> runtime. Let's only turn it on for OF_SEPARATE or OF_EMBED by default.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - new patch: "lib: kconfig: Limit BINMAN_FDT for OF_SEPARATE or OF_EMBED"
>
>  lib/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I think you need to handle the sandbox case too - OF_HOSTFILE

Perhaps we don't have tests for this? I need to add some.

Regards,
SImon
Bin Meng May 10, 2021, 4:34 p.m. UTC | #2
Hi Simon,

On Tue, May 11, 2021 at 12:28 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Mon, 10 May 2021 at 06:24, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Generally speaking BINMAN_FDT makes sense for OF_SEPARATE or OF_EMBED.
> > For the other OF_CONTROL methods, it's quite possible binman node is
> > not available as binman is invoked during the build phase instead of
> > runtime. Let's only turn it on for OF_SEPARATE or OF_EMBED by default.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >
> > ---
> >
> > (no changes since v3)
> >
> > Changes in v3:
> > - new patch: "lib: kconfig: Limit BINMAN_FDT for OF_SEPARATE or OF_EMBED"
> >
> >  lib/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> I think you need to handle the sandbox case too - OF_HOSTFILE

Ah, that's odd :)

Or maybe we should just ignore the -ENOENT error and return 0 in
binman_init(), instead of changing the Kconfig?

    ret = find_image_node(&binman->image);
    if (ret)
        return 0;

What do you think?

>
> Perhaps we don't have tests for this? I need to add some.

Regards,
Bin
Simon Glass May 10, 2021, 4:46 p.m. UTC | #3
Hi Bin,

On Mon, 10 May 2021 at 10:34, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, May 11, 2021 at 12:28 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Mon, 10 May 2021 at 06:24, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Generally speaking BINMAN_FDT makes sense for OF_SEPARATE or OF_EMBED.
> > > For the other OF_CONTROL methods, it's quite possible binman node is
> > > not available as binman is invoked during the build phase instead of
> > > runtime. Let's only turn it on for OF_SEPARATE or OF_EMBED by default.
> > >
> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > >
> > > ---
> > >
> > > (no changes since v3)
> > >
> > > Changes in v3:
> > > - new patch: "lib: kconfig: Limit BINMAN_FDT for OF_SEPARATE or OF_EMBED"
> > >
> > >  lib/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > I think you need to handle the sandbox case too - OF_HOSTFILE
>
> Ah, that's odd :)
>
> Or maybe we should just ignore the -ENOENT error and return 0 in
> binman_init(), instead of changing the Kconfig?
>
>     ret = find_image_node(&binman->image);
>     if (ret)
>         return 0;
>
> What do you think?

Not too keen since I'd like to know if it works, i.e. keep it deterministic.

So if sandbox still runs OK with this change, then we can figure it out later.

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon
diff mbox series

Patch

diff --git a/lib/Kconfig b/lib/Kconfig
index 6d2d41de30..7d5990c940 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -25,7 +25,7 @@  config BCH
 config BINMAN_FDT
 	bool "Allow access to binman information in the device tree"
 	depends on BINMAN && DM && OF_CONTROL
-	default y
+	default y if OF_SEPARATE || OF_EMBED
 	help
 	  This enables U-Boot to access information about binman entries,
 	  stored in the device tree in a binman node. Typical uses are to