[PATCH v2 4/5] binman: Convert FIT entry type to a subclass of Section entry type

[Jan Kiszka]

On 15.02.22 13:27, Alper Nebi Yasak wrote:
> On 14/02/2022 12:09, Jan Kiszka wrote:
>> On 07.02.22 23:08, Alper Nebi Yasak wrote:
>>> The binman FIT entry type shares some code with the Section entry type.
>>> This shared code is bound to grow, since FIT entries are conceptually a
>>> variation of Section entries.
>>>
>>> Make FIT entry type a subclass of Section entry type, simplifying it a
>>> bit and providing us the features that Section implements. Also fix the
>>> subentry alignment test which now attempts to write symbols to a
>>> nonexistent SPL ELF test file by creating it first.
>>>
>>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak at gmail.com>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>> ---
>>
>> Only came to testing this now, and it causes a regression.
>>
>> Before this commit (I've added fdtmap to
>> arch/arm/dts/k3-am65-iot2050-boot-image.dtsi for this):
>>
>> $ source/tools/binman/binman -D ls -i flash.bin
>> Name                 Image-pos  Size     Entry-type         Offset   Uncomp-size
>> ----------------------------------------------------------------------------------
>> main-section                 0   8c0000  section                  0
>>   blob-ext at 0x000000          0    37373  blob-ext at 0x000000        0
>>   blob at 0x080000          80000    84484  blob at 0x080000        80000
>>   fit at 0x280000          280000    f445f  fit at 0x280000        280000
>>   fdtmap                37445f      cf9  fdtmap              37445f
>>   fill at 0x680000         680000    20000  fill at 0x680000       680000
>>   fill at 0x6a0000         6a0000    20000  fill at 0x6a0000       6a0000
>>   blob-ext at 0x6c0000     6c0000    415de  blob-ext at 0x6c0000   6c0000
>>   blob-ext at 0x740000     740000    43952  blob-ext at 0x740000   740000
>>   blob-ext at 0x7c0000     7c0000    415e2  blob-ext at 0x7c0000   7c0000
>>   blob-ext at 0x840000     840000    4395a  blob-ext at 0x840000   840000
>>
>> With this commit:
>>
>> $ source/tools/binman/binman -D ls -i flash.bin
>> Name                          Image-pos  Size     Entry-type         Offset   Uncomp-size
>> -------------------------------------------------------------------------------------------
>> main-section                          0   8c0000  section                  0
>>   blob-ext at 0x000000                   0    37373  blob-ext at 0x000000        0
>>   blob at 0x080000                   80000    84484  blob at 0x080000        80000
>>   fit at 0x280000                                    fit at 0x280000        280000
>>     u-boot                                        section
>>       u-boot-nodtb                                u-boot-nodtb
>>     fdt-iot2050-basic                             section
>>       blob                                        blob
>>     fdt-iot2050-basic-pg2                         section
>>       blob                                        blob
>>     fdt-iot2050-advanced                          section
>>       blob                                        blob
>>     fdt-iot2050-advanced-pg2                      section
>>       blob                                        blob
>>     k3-rti-wdt-firmware                           section
>>       blob-ext                                    blob-ext
>>   fdtmap                         37445f      cd9  fdtmap              37445f
>>   fill at 0x680000                  680000    20000  fill at 0x680000       680000
>>   fill at 0x6a0000                  6a0000    20000  fill at 0x6a0000       6a0000
>>   blob-ext at 0x6c0000              6c0000    415de  blob-ext at 0x6c0000   6c0000
>>   blob-ext at 0x740000              740000    43952  blob-ext at 0x740000   740000
>>   blob-ext at 0x7c0000              7c0000    415e2  blob-ext at 0x7c0000   7c0000
>>   blob-ext at 0x840000              840000    4395a  blob-ext at 0x840000   840000
>>
> 
> The AddMissingProperties() and SetCalculatedProperties() methods were
> disabled for FIT as a fixup to this patch, that's why image-pos etc.
> aren't available. See comments at [1] for some context.
> 
> Hopefully my two patches [2][3] fix things, can you test with them?
> 
> [1] "binman: Correct the error message for a bad hash algorithm"
> https://patchwork.ozlabs.org/project/uboot/patch/20220208105941.1.I8f212a1150defebaf8b7b15a79f7a2fc62c276b2@changeid/
> 
> [2] "binman: Skip processing "hash" subnodes of FIT subsections"
> https://patchwork.ozlabs.org/project/uboot/patch/20220209190236.26479-1-alpernebiyasak@gmail.com/
> 

This one helped, indeed.

Thanks,
Jan

> [3] "binman: Update image positions of FIT subentries"
> https://patchwork.ozlabs.org/project/uboot/patch/20220207220809.4497-6-alpernebiyasak@gmail.com/
> 
>> The fit is nicely shown but its Image-pos column is now empty. And that 
>> leads to:
>>
>> $ source/tools/binman/binman -D extract -i flash.bin -f extr.bin fit at 0x280000
>> binman: unsupported operand type(s) for +: 'int' and 'NoneType'
>>
>> Traceback (most recent call last):
>>   File "source/tools/binman/binman", line 141, in RunBinman
>>     ret_code = control.Binman(args)
>>   File "/data/u-boot/tools/binman/control.py", line 639, in Binman
>>     not args.uncompressed, args.format)
>>   File "/data/u-boot/tools/binman/control.py", line 260, in ExtractEntries
>>     data = entry.ReadData(decomp, alt_format)
>>   File "/data/u-boot/tools/binman/etype/section.py", line 763, in ReadData
>>     data = parent_data[offset:offset + self.size]
>> TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'
>>
>> Similar breakages for "replace" and likely more sub-commands.
> 
> OTOH, Entry_section docstring does say subentry offsets can be None, so
> it might be a good idea to fix these TypeErrors regardless.
> 
>>
>> I have to revert this locally for now - no time to debug.
>>
>> Jan
>>

-- 
Siemens AG, Technology
Competence Center Embedded Linux