Nitheesh Sekar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29967 )
Change subject: qclib: Add qclib support with interface tables ......................................................................
Patch Set 30:
(6 comments)
https://review.coreboot.org/#/c/29967/25/src/soc/qualcomm/qcs405/include/soc... File src/soc/qualcomm/qcs405/include/soc/memlayout.ld:
https://review.coreboot.org/#/c/29967/25/src/soc/qualcomm/qcs405/include/soc... PS25, Line 43: 0x80000
nit: prefer giving sizes in decimal (e.g. […]
Done
https://review.coreboot.org/#/c/29967/25/src/soc/qualcomm/qcs405/include/soc... File src/soc/qualcomm/qcs405/include/soc/qclib.h:
https://review.coreboot.org/#/c/29967/25/src/soc/qualcomm/qcs405/include/soc... PS25, Line 19: void qclib_load_and_run(void);
Isn't this in some qualcomm/common header? This should be in some qualcomm/common header.
Done
https://review.coreboot.org/#/c/29967/25/src/soc/qualcomm/qcs405/mmu.c File src/soc/qualcomm/qcs405/mmu.c:
https://review.coreboot.org/#/c/29967/25/src/soc/qualcomm/qcs405/mmu.c@31 PS25, Line 31: mmu_config_range((void *)0x80000000, 0x40000000, MA_MEM | MA_S | MA_RW);
You can't do this here. This runs in the bootblock before DRAM is initialized. […]
Done
https://review.coreboot.org/#/c/29967/25/src/soc/qualcomm/qcs405/mmu.c@39 PS25, Line 39: MA_MEM | MA_NS | MA_RW);
This seems to be the same line as above? Again, you don't need it, common code already does that.
Done
https://review.coreboot.org/#/c/29967/25/src/soc/qualcomm/qcs405/soc_blob_lo... File src/soc/qualcomm/qcs405/soc_blob_load.c:
https://review.coreboot.org/#/c/29967/25/src/soc/qualcomm/qcs405/soc_blob_lo... PS25, Line 22: #define DCB_NAME "/dcb"
Strings should be inline for consistency with SDM845 code
Done
https://review.coreboot.org/#/c/29967/25/src/soc/qualcomm/qcs405/soc_blob_lo... PS25, Line 29: printk(BIOS_INFO, "%s:qcs405\n", __func__);
Unnecessary
Done
-
Nitheesh Sekar (Code Review)