Hi Michal, On Wed, 16 Oct 2024 at 00:00, Michal Simek <[email protected]> wrote: > > Hi Simon, > > On 10/15/24 14:48, Simon Glass wrote: > > Hi Michal, > > > > On Thu, 10 Oct 2024 at 07:03, Michal Simek <[email protected]> wrote: > >> > >> > >> > >> On 10/9/24 23:14, Simon Glass wrote: > >>> Hi Michal, > >>> > >>> On Wed, 9 Oct 2024 at 07:21, Michal Simek <[email protected]> wrote: > >>>> > >>>> Hi, > >>>> > >>>> On 10/9/24 03:55, Simon Glass wrote: > >>>>> Hi Michal, > >>>>> > >>>>> On Mon, 7 Oct 2024 at 07:05, Michal Simek <[email protected]> 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 <[email protected]> > >>>>>> --- > >>>>>> > >>>>>> 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.si...@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. > >> > >> If this is single binary composition with clear layout then likely fine. > >> In our case where we target evaluation boards which can boot out of > >> different > >> boot devices it will be more confusing. > >> For these I want to generated all images also for testing purpose not only > >> images which you will burn to qspi. > >> > >>>> > >>>> 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? > >> > >> Yes. > >> I know that there is a code around it but as of now I don't want to use > >> any of > >> this feature. > >> > >>> 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. > >> > >> Works for me but keep in your mind that for SOM this should be removed > >> from all > >> combinations and for me it is easier not to add that description there > >> instead > >> of adding it and removing it. > > > > OK, I think you are saying that the description is repeated in each > > .dtb since each is built by U-Boot's build system and then they are > > added to the FIT. > > yep
OK, got it. I think we should add an way to make the binman node optional. > > > > > But what is to stop people from not bothering to fill in the binman > > description in U-Boot? I worry that vendors will have instructions > > like 'build U-Boot with the in-tree devicetree, which has no binman > > node, but pass this option to use this other file (not in mainline, > > just our special vendor branch), just for Binman's use', > > > > Where do you plan to keep this other file? > > In u-boot repo of course. And all configurations which makes sense. > And pretty much if vendors wants to hide it they can no matter of this patch. > I understand your concern but vendors can do it today. So what value are you going to use for BINMAN_EXTERNAL_DTB ? Is there a patch for that? Perhaps it should be renamed, since it suggests that the file is out of tree. Regards, Simon
