Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29967 )
Change subject: qclib: Add qclib support with interface tables ......................................................................
Patch Set 36:
(4 comments)
LGTM other than nits and the blob issue.
https://review.coreboot.org/c/coreboot/+/29967/36/src/soc/qualcomm/common/qc... File src/soc/qualcomm/common/qclib.c:
https://review.coreboot.org/c/coreboot/+/29967/36/src/soc/qualcomm/common/qc... PS36, Line 147: _ddr_training, ssize, 0); This doesn't belong here
https://review.coreboot.org/c/coreboot/+/29967/36/src/soc/qualcomm/qcs405/Ma... File src/soc/qualcomm/qcs405/Makefile.inc:
PS36: off-topic, but this file should have a license header
https://review.coreboot.org/c/coreboot/+/29967/36/src/soc/qualcomm/qcs405/Ma... PS36, Line 67: ifneq (,$(findstring $(QCSDI_FILE),$(qcsdi_file))) I think I mentioned this elsewhere (or earlier on this patch?) already, this should not be submitted like this. Instead you need to properly upload these blobs (with valid distribution license) into the 3rdparty/blobs directory. Please remind your lawyers about this long-outstanding issue!
https://review.coreboot.org/c/coreboot/+/29967/36/src/soc/qualcomm/qcs405/mm... File src/soc/qualcomm/qcs405/mmu.c:
https://review.coreboot.org/c/coreboot/+/29967/36/src/soc/qualcomm/qcs405/mm... PS36, Line 35: void soc_mmu_dram_config_post_dram_init(void) You don't need to define this if it's empty.