[U-Boot] [PATCH v4 8/9] spl: dm: Make it possible for the SPL to pick its own DTB from a FIT

[Simon Glass]

Hi Jean-Jacques,

On 24 August 2017 at 04:52, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
> Hi Simon,
>
> On 13/08/2017 23:35, Simon Glass wrote:
>>
>> Hi Jean-Jacques,
>>
>> On 7 August 2017 at 04:07, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
>>>
>>> u-boot can be embedded within a FIT image with multiple DTBs. It then
>>> selects at run-time  which one is best suited for the platform.
>>> Use the same principle here for the SPL: put the DTBs in a FIT image,
>>> compress it (LZO, GZIP, or no compression) and append it at the end of
>>> the
>>> SPL.
>>>
>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
>>> ---
>>>
>>>   change since v3: updated the help in Kconfig to take in account commit
>>> f1896c
>>>   ("spl: make SPL and normal u-boot stage use independent
>>> SYS_MALLOC_F_LEN")
>>>
>>>   doc/README.multi-dtb-fit | 44 +++++++++++++++++++++++--
>>>   dts/Kconfig              | 83
>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>   lib/fdtdec.c             | 85
>>> +++++++++++++++++++++++++++++++++++++++++++-----
>>>   scripts/Makefile.spl     | 35 +++++++++++++++++++-
>>>   4 files changed, 235 insertions(+), 12 deletions(-)
>>>
>> Unfortunately I have quite a few comments and a few nits below. I feel
>> this is an important feature and most of my comments relate to polish.
>> It's a nice implementation.
>>
> Thanks

[..]

>>> @@ -1243,7 +1292,25 @@ int fdtdec_setup(void)
>>>          gd->fdt_blob = (void *)getenv_ulong("fdtcontroladdr", 16,
>>>
>>> (uintptr_t)gd->fdt_blob);
>>>   # endif
>>> +
>>> +# if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
>>> +       /*
>>> +        *try and uncompress the blob.
>>> +        * max input size is set arbitrarily to 16MB (should more than
>>> enough)
>>
>> This seems ugly. Can you call fdt_totalsize() instead?
>
> Unfortunately we cannot. The data itself is not part of the fdt, it is
> appended after it. In the fdt, we find the descriptions, and for each binary
> the size and the offset relative to the end of the fdt.
>

So I think we need header so we can find out:

- the compression algorithm (since otherwise I'm not sure how you tell)
- the compressed size
 -the original uncompressed size

without having to decompress to find out. Perhaps add a magic value also?

> JJ
[..]