[PATCH] binman: Add option for pointing to external description

[Simon Glass]

Hi Michal,

On Wed, 9 Oct 2024 at 07:21, Michal Simek <michal.simek at amd.com> wrote:
>
> Hi,
>
> On 10/9/24 03:55, Simon Glass wrote:
> > Hi Michal,
> >
> > On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek at amd.com> wrote:
> >>
> >> Adding binman node with target images description can be unwanted feature
> >> but as of today there is no way to disable it.
> >> Also on size constrained systems it is not useful to add binman description
> >> to DTB.
> >> Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for
> >> target from DTB for binman itself.
> >>
> >> Signed-off-by: Michal Simek <michal.simek at amd.com>
> >> ---
> >>
> >>   Makefile    |  2 +-
> >>   lib/Kconfig | 10 ++++++++++
> >>   2 files changed, 11 insertions(+), 1 deletion(-)
> >>
> >
> > Doesn't this defeat one of the purposes of Binman, i.e. to document
> > images? We do want the .dts to include the image description. What
> > sort of problem is this causing?
>
> We have two boot flows.
> The first one (default one) is using Xilinx FSBL for SOM initialization with fit
> image (DTBS) + u-boot.elf + tfa.
>
> The second one is using U-Boot SPL instead of FSBL. This flow is used by
> buildroot for example.
>
> In perfect world I should describe both of these flows. I sent description for
> the second as RFC here.
> https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com

OK I'll take a look.

>
> but it is also reasonable to describe the first flow but I really don't want
> both descriptions ends up in the target image.

Why not? Knowing what is in the firmware is one of the goals of Binman.

>
> The second part is if you look at RFC and how fit-dtb.blob is composed. It is
> one DTB + DTBS which are composed from overlays.
>
> xilinx_zynqmp_kria_defconfig has
> CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA"
>
> That's why binman node should go to this DTB but because other images are
> composed with overlays binman node is spread to all DTBs inside FIT image.
>
> It means one binman description is in fit-dtb.blob 14 times which is far from
> ideal.

Yes, but I think what you are saying is that U-Boot doesn't need the
description, so you don't need it to appear in the dtbs in the FIT. Is
that right?

If so, then I think we should add a way to remove it, in Binman,
perhaps with a property in the top-level binman image.

>
> Third part is that I can't see binman node in DT schema or bindings that's why I
> expect this will be reported and I can't see any code which removes it before
> handing off to OS which is required for System Ready IR.
> And IIRC removing is also problematic for measured boot.

I did start this e.g. [1] but have not got back to it. Help would be
appreciated if it is important to you. I am not sure about System
Ready IR, but we shouldn't need to remove this.

Also, please add an fdtmap somewhere so the image can be listed.

Regards,
Simon

[1] Documentation/devicetree/bindings/mtd/partitions/binman.yaml