[PATCH v2 08/11] binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE

[Simon Glass]

Hi Bin,

On Sat, 8 May 2021 at 10:07, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Sun, May 9, 2021 at 12:48 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Sat, 8 May 2021 at 08:55, Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Sat, May 8, 2021 at 11:47 PM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > >
> > > > > For scenarios like OF_PRIOR_STAGE, no device tree blob is provided
> > > > > in the U-Boot build phase hence the binman node information is not
> > > > > available. In order to support such use case, a new Kconfig option
> > > > > BINMAN_DTB is introduced, to tell U-Boot build system that a device
> > > > > tree blob is explicitly required when using binman to package U-Boot.
> > > > >
> > > > > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> > > > >
> > > > > ---
> > > > >
> > > > > Changes in v2:
> > > > > - new patch: "binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE"
> > > > >
> > > > >  Makefile    |  3 ++-
> > > > >  dts/Kconfig | 18 ++++++++++++++++++
> > > > >  lib/Kconfig |  2 +-
> > > > >  3 files changed, 21 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Makefile b/Makefile
> > > > > index 3d88559b67..9043828675 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -918,6 +918,7 @@ endif
> > > > >  endif
> > > > >  INPUTS-$(CONFIG_TPL) += tpl/u-boot-tpl.bin
> > > > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot.dtb
> > > > > +INPUTS-$(CONFIG_BINMAN_DTB) += u-boot.dtb
> > > > >  ifeq ($(CONFIG_SPL_FRAMEWORK),y)
> > > > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img
> > > > >  endif
> > > > > @@ -1161,7 +1162,7 @@ u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> > > > >  u-boot.bin: u-boot-dtb.bin FORCE
> > > > >         $(call if_changed,copy)
> > > > >  else
> > > > > -u-boot.bin: u-boot-nodtb.bin FORCE
> > > > > +u-boot.bin: u-boot-nodtb.bin $(if $(CONFIG_BINMAN_DTB),dts/dt.dtb) FORCE
> >
> > What is this change for, specifically? Can you add a comment?
> >
> > It seems to be overriding the qemu thing of not actually building a DT?
> >
> > > > >         $(call if_changed,copy)
> > > > >  endif
> > > > >
> > > > > diff --git a/dts/Kconfig b/dts/Kconfig
> > > > > index 99ce75e1a2..84d43fc6a4 100644
> > > > > --- a/dts/Kconfig
> > > > > +++ b/dts/Kconfig
> > > > > @@ -19,6 +19,24 @@ config BINMAN
> > > > >         bool
> > > > >         select DTOC
> > > > >
> > > > > +config BINMAN_DTB
> > > > > +       bool
> > > > > +       depends on BINMAN
> > > > > +       default y if OF_PRIOR_STAGE
> > > > > +       help
> > > > > +         This option tells U-Boot build system that a device tree blob is
> > > > > +         explicitly required when using binman to package U-Boot.
> > > > > +
> > > > > +         This is not necessary in a common scenario where a device tree blob
> > > > > +         is provided in the arch/<arch>/dts directory for a specific board,
> > > > > +         where the binman node has been put in the board's device tree, aka
> > > > > +         an implicit way. However for a scenario like the board's device tree
> > > > > +         is not provided in the U-Boot source tree, but given to U-Boot in the
> > > > > +         runtime, e.g.: in the OF_PRIOR_STAGE case that the device tree blob
> > > > > +         is passed by a prior stage bootloader. For such scenario, we will
> > > > > +         need to provide a device tree blob containing binman node to describe
> > > > > +         how to package U-Boot.
> > > >
> > > > I don't fully understand the effect of this. Is binman (at build time)
> > > > using a binary DT provided by another build system? Or is it not
> > > > available at build time and binman cannot run?
> > >
> > > binman uses a DT provided by U-Boot, but that DT is only for binman if
> > > OF_PRIOR_STAGE. For OF_SEPARATE / EMBEDDED, we can include binman node
> > > there.
> > >
> > > You can check patch 10 for how it works.
> >
> > OK I think I get it.
> >
> > >
> > > >
> > > > The name is too vanilla I think. It should indicate that this is an
> > > > unusual situation.
> > >
> > > I cannot think of a good name :(
> >
> > How about BINMAN_FAKE_FDT ?
>
> Better than mine :)
>
> If FAKE confuses people, maybe BINMAN_STANDALONE_FDT, or BINMAN_PACKAGE_FDT ?

BINMAN_STANDALONE_FDT seems better actually

>
> >
> > Why do you need to turn off BINMAN_FDT?
>
> Because binman_init() fails on locating the image node in the device
> tree, where the binman node does not exist at all. The DT used only
> exists in the U-Boot build phase.

Oh yes I see. So confusing. Can you add some docs to binman about this?

>
> So here is what we have for different OF_CONTROL methods:
>
> OF_SEPARATE / OF_EMBEDED: binman node is available in the DT (exists

OF_EMBED

> along with other device nodes)
> OF_PRIOR_STAGE: binman node is not available in the DT (DT only
> contains the device nodes)

OK yes.

Regards,
Simon