[SPAM] Re: Replace make-fit-atf.py with binman. Was: migrate u-boot-rockchip.bin to binman and generate an image for SPI

[Simon Glass]

Hi Xavier,

On Tue, 2 Aug 2022 at 11:19, Xavier Drudis Ferran <xdrudis at tinet.cat> wrote:
>
> El Tue, Aug 02, 2022 at 06:41:40AM -0600, Simon Glass deia:
> >
> > It seems we need a lot more guidance here. I can help write more into
> > the packaging docs, perhaps:
> >
> > https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#motivation
> >
> > Can you please suggest a few questions to answer?
> >
>
> I can try. But let me start by saying thank you for the guidance that was already there.
> I had read it and I still don't understand it together with some of what you said on the list.
> So how about...:
>
> - Exact criteria for what is a binary you take from external sources
>    and and what is a packaged image that's binman responsability. So,
>    TF-A is compiled externally, and the U-Boot build system just gets
>    a BL31 env variable pointing at an bl31.elf or so. But u-boot.itb,
>    despite being a FIT that includes parts of TF-A, maybe parts of
>    tee.bin, some dtbs and binaries built by U-Boot itself and who
>    knows if public keys or extra dtb overlays provided directly by the
>    user, or something I can't think of now, it's not a binary from
>    external sources but a U-Boot image that needs to be packaged by
>    binman? For me there's some grey zone. Since make_fit_atf.py is in
>    U-Boot then u-boot.itb is a binary, but since part of the contents
>    are external, and mixed with U-Boot's own code then it's a packaged
>    image. But then it's just part of the final packaged image that
>    one can store in SPI, SD or wherever.
>
> -  Scope ? how customizable needs the description of the image to be ?
>
>    If a board meeds different images for booting from sd, spi and nand,
>    should the *-u-boot.dts* provide the 3 images or one or ...?
>
>    Must the image description recognize all possible kconfig options ?
>    So should it look for splash source = spi and put a copy of some
>    bmp or bmp.gz at some offset if the user opted for splash image in spi?
>
>    Or that's a stretch and it's basically it should work for the
>    default config for each board and users who change things should
>    change the dts or package manually following the various READMEs ?
>    (taking into account that any manual packaging risks the image
>    description in dts no longer matching the image in case some soft is
>    reading that)
>
> - Use ? Visibility ?
>
>   From the motivation text one would think that users run
>      make boardX_defconfig
>      make
>      BL31=bl31.elf binman
>
>   But in fact currently it is make who calls binman. Is it just a temporary
>   situation and the intent is for make just to build binaries and then
>   binman will be called afterwards to package images ? Because binman not
>   only packages what make built, it changes some properties of the dtb that
>   was built by make, if I've understood it. In that sense it seems to be
>   part of the build system not just a packager that comes afterwards.
>
>   And the different parameters of binman are supposed to be set by make,
>   and then more technical or is binman supposed to be used by the user
>   to build some custom image too?
>
> - What about reuse ?
>
>   the binman recipes go into u-boot.dts* files . But into which ?  The
>   SOC .dtsi ? the mach .dtsi ? the board .dtsi ? For example many
>   boards include rk3399-u-boot.dtsi. Rk3399 can boot from spi, sd, or
>   emmc (the same image can be used for emmc and sd). That's the
>   bootrom. In theory SPL (a fatter SPL) could load U-Boot from nvme, I
>   think, or USB, or serial, or network but I don't think that's
>   implemented?
>
>   So if you put the binman node in rk3399-u-boot.dtsi you could have
>   it generate two images, one for sd/emmc and one for spi (plus
>   intermediate images that people asked for to customize things). If
>   you put it in rockchip-u-boot.dtsi then it can be more reused, but
>   maybe then you need a format for nand too? And what if a board
>   doesn't have emmc nor SD ? or doesn't have spi NOR? Should it modify
>   the inherited binman node to disable one of the images, or just generate
>   some unused image and call it a day ? Or you should put your binman
>   subnodes in independent .dtsi files and carefully include the needed
>   ones in each board -u-boot.dts ? Or you put it all in the most
>   generic u-boot.dtsi you can and #ifdef away the parts that apply to
>   each board/configuration ?  So far we seem to prefer this last
>   approach, but I don't know whether it was a plan or what.
>
>
> >
> > Because we have lots of scripts with no tests and it will proliferate
> > even more. Binman is data-driven, has tests and allows us to build
> > common functionality used by all SoCs.
> >
>
> Makes sense, tests are necessary, but some functionality may be common
> and some may be very specific for some arch or SOC or vendor. If all
> specific kirks also go into binman it can get a little complex. It can
> still be worth it, it's just that when some external entity decides
> things work some way (because they design a SOC or write a bootrom, or
> whatever) it'll be easier for them to give a shell script that
> implements what's needed than to understand and adapt binman. So
> that's a job for others to do possibly. So until someone does (like
> Quentin just did for rkspi) then we'll still have binman and whatever
> the external vendor came up with.
>
> One option would be to use binman just for generic things (padding,
> alignment, extraction, concatenation, calling external tools). And
> resign ourselves to keep having some external tools that make calls
> before calling binman, or that binman itself calls. I don't know.
>
> >
> > Have you looked at osfc binman talk from a few years ago?
> >
>
> No, or maybe so long ago that I forgot. I read what I could from
> doc/. I generally prefer text than video. But I may take a look, now
> that you mention it.
>
> > Binman is not to replace make. The dependency we are talking about is
> > between different images generated by Binman. Part of the problem is
> > that you know what you want, but it is a bit foggy to me.
> >
>
> We are currently building images that include images, so some files
> are both binaries and images, and that is not supported. If you
> support that in a general way (because binman seems to be intended to
> generalize a bit packaging) then it can become a little complex
> because new dependencies between images or sections arise. If you want
> to keep binman simpler, you could offload that to make. But you don't
> want it, I still don't know why.
>
> > To move forward, can I suggest you send a patch with a binman
> > description file containing what you want. Obviously it won't work,
> > due to the dependencies, but I can then figure out how to enable that
> > in binman. Can you try that?
> >
>
> I can help explaining my doubts because they're big and candid, and I
> could try to explain what is needed for Rock Pi 4, but I don't think I
> could explain it better than what Quentin and Jerome already did.
>
> I sent a dtsi with a binman description file
>
> https://lists.denx.de/pipermail/u-boot/2022-July/490068.html
>
> (you replied to that message, but maybe didn't read until the end ?)
>
> Your suggestion was to add ordering properties to binman. I guess we
> could, but I'm not sure that's very scalable or even very data driven.
> Its simpler than managing dependencies, I guess, so that's good.
>
> But I'm no longer sure that's what I want. Well I'm sure that's not
> what I want since Jerome pointed out that that would mistreat
> tee.bin. Maybe what I want is to keep using make_fit_atf.py to build
> u-boot.itb and then consider u-boot.itb a binary for binman, that's
> what I think Quentin already sent, before I messed it up by mentioning
> make_fit_atf.py at all. Maybe just remove the message make outputs
> saying CONFIG_USE_SPL_FIT_GENERATOR looks ugly ? Because that's what
> lead me to the rabbit hole...
>

I think there is a bit much agonising over things here. I'll see if I
can create a patch that addresses these points and send it out. Thank
you for writing it all up.

Regards,
Simon