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

[Michal Simek <michal.simek@amd.com>]

Hi,

On 10/9/24 03:55, Simon Glass wrote:
> Hi Michal,
> 
> On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@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@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

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

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.

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.

Thanks,
Michal