Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35508 )
Change subject: trogdor: SoC makefile BLOB support ......................................................................
Patch Set 86:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35508/84/src/soc/qualcomm/sc7180/Ma... File src/soc/qualcomm/sc7180/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35508/84/src/soc/qualcomm/sc7180/Ma... PS84, Line 70: SC7180_BLOB := $(top)/3rdparty/blobs/soc/qualcomm/sc7180
Please fix this to point to 3rdparty/qc_blobs/sc7180 where the files now actually reside.
Done
https://review.coreboot.org/c/coreboot/+/35508/84/src/soc/qualcomm/sc7180/Ma... PS84, Line 78: none
Please make this $(CBFS_COMPRESS_FLAG)
All of these others should be okay using $(CBFS_COMPRESS_FLAG), not $(CBFS_PRERAM_COMPRESS_FLAG). Only QcLib should need the PRERAM because it's so big (the difference under the hood is that LZ4 can decompress in-place while LZMA requires a scratch buffer in the CBFS_CACHE region to load the file). It should theoretically make things slightly more efficient.
https://review.coreboot.org/c/coreboot/+/35508/84/src/soc/qualcomm/sc7180/Ma... PS84, Line 87: none
and this $(CBFS_PRERAM_COMPRESS_FLAG)
Done
https://review.coreboot.org/c/coreboot/+/35508/84/src/soc/qualcomm/sc7180/Ma... PS84, Line 107: BL31_FILE := $(SC7180_BLOB)/qtiseclib/bl31.elf
Please remove this whole file, this doesn't belong here. […]
Done
https://review.coreboot.org/c/coreboot/+/35508/86/src/soc/qualcomm/sc7180/Ma... File src/soc/qualcomm/sc7180/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35508/86/src/soc/qualcomm/sc7180/Ma... PS86, Line 68: BL31_MAKEARGS += PLAT=sc7180 Sorry we also need a
QTISECLIB_PATH=$(SC7180_BLOB)/qtiseclib/libqtisec.a
as part of BL31_MAKEARGS (and then reorder SC7180_BLOB definition above it, of course). Forgot that one.
-
Julius Werner (Code Review)