Rishabh Sharma has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33425 )
Change subject: qcs405: Add support for specifying mbn_version ......................................................................
Patch Set 7:
(5 comments)
https://review.coreboot.org/c/coreboot/+/33425/5/util/qualcomm/mbn_tools.py File util/qualcomm/mbn_tools.py:
https://review.coreboot.org/c/coreboot/+/33425/5/util/qualcomm/mbn_tools.py@... PS5, Line 78: FLASH_PARTI_VERSION = 3 # Flash Partition Version Number
Should remove this and use header_version everywhere.
Done
https://review.coreboot.org/c/coreboot/+/33425/5/util/qualcomm/mbn_tools.py@... PS5, Line 1006:
Should we maybe also assert that header version is one of the supported values (3, 5 or 6) somewhere […]
Done
https://review.coreboot.org/c/coreboot/+/33425/5/util/qualcomm/mbn_tools.py@... PS5, Line 1007: if is_ext_mbn_v5 == True:
This should check header_version == VERSION_5 instead.
If I modify this, mbn_tools will not remain backward compatible where some board is using is_ext_mbn_v5 to add mbn header version 5. So its better to keep it as unchanged so that backward compatibility is maintained.
https://review.coreboot.org/c/coreboot/+/33425/5/util/qualcomm/mbn_tools.py@... PS5, Line 1009: boot_header.flash_parti_ver = VERSION_5 # version
Just do flash_parti_ver = header_version once outside the conditional.
Again same comment as above. We have not enforced createxbl to provide header_version. Header_version is optional to mbn_tools. So this type of assignment is better to maintain backward compatibility where createxbl is invoking mbn_tools with is_ext_mbn_v5 as True without header_version.
https://review.coreboot.org/c/coreboot/+/33425/5/util/qualcomm/mbn_tools.py@... PS5, Line 1050: is_sha384_algo = False
Unused?
Done