Because of the separation with util/cbfstool/amdcompress, there is a bunch of redundant parameters passed on commandline, while the information could be derived from ELF headers. IMHO, once we have created the final ELF it should be forbidden to reference the Kconfigs that were used inside the linker scripts.

View Change

2 comments:

• File src/soc/amd/picasso/Makefile.inc:

  • Patch Set #4, Line 213: PSP_BIOSBIN_DEST=$(shell printf "%x" $(call int-subtract, $(call int-add, $(CONFIG_X86_RESET_VECTOR) 0x10) $(PSP_BIOSBIN_SIZE)))

    I believe we would want to avoid new references to C_ENV_BOOTBLOCK_SIZE and take the actual uncompressed progsz from ELF headers.

    As for the reference to X86_RESET_VECTOR, I had a look at some (leaked to me via virtual directory listing) datasheets and it is more powerful in expression than it needs to be since hardware is wired at IP=fff0. You would avoid the reference and calculation here if you had made amdfwtool parse the ELF (and optionally compress too).

  • Patch Set #4, Line 301: $(PSP_BIOSBIN_FILE): $(PSP_ELF_FILE) $(AMDCOMPRESS)

    I am looking at CB:33401 where util/cbfstool/amdcompress.c was introduced and I am not so happy about it. AFAICS it has nothing to do with CBFS and the feature it implements should have been incorporated inside util/amdfwtool instead.

To view, visit change 37490. To unsubscribe, or for help writing mail filters, visit settings.