Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46930 )
Change subject: soc/mediatek/common: Add common API for loading firmwares ......................................................................
Patch Set 17:
(11 comments)
https://review.coreboot.org/c/coreboot/+/46930/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46930/14//COMMIT_MSG@7 PS14, Line 7: A common API for loading the blob file
Please use statements by adding a verb (in imperative mood): […]
Done
https://review.coreboot.org/c/coreboot/+/46930/14//COMMIT_MSG@9 PS14, Line 9: blog
blob
Done
https://review.coreboot.org/c/coreboot/+/46930/14//COMMIT_MSG@11 PS14, Line 11: load time and the blob size.
Please also mention, why the ramstage size is increased.
The change is no longer needed.
https://review.coreboot.org/c/coreboot/+/46930/14//COMMIT_MSG@12 PS14, Line 12:
Maybe paste the log output with one example from the new debug messages.
Done
https://review.coreboot.org/c/coreboot/+/46930/16/src/soc/mediatek/common/in... File src/soc/mediatek/common/include/soc/mtlib_common.h:
https://review.coreboot.org/c/coreboot/+/46930/16/src/soc/mediatek/common/in... PS16, Line 6: int load_blob_file(const char *name, unsigned long addr);
Hi Yu-Ping, […]
please review CB:48232 and CB:48233
https://review.coreboot.org/c/coreboot/+/46930/14/src/soc/mediatek/common/in... File src/soc/mediatek/common/include/soc/mtlib_common.h:
https://review.coreboot.org/c/coreboot/+/46930/14/src/soc/mediatek/common/in... PS14, Line 6: unsigned long
Should `addr` be `uintptr_t` or something similar? Or, as it’s passed to `memcpy()`, why not `void * […]
Done
https://review.coreboot.org/c/coreboot/+/46930/14/src/soc/mediatek/common/mt... File src/soc/mediatek/common/mtlib.c:
https://review.coreboot.org/c/coreboot/+/46930/14/src/soc/mediatek/common/mt... PS14, Line 9: #define BUF_SIZE (192 * KiB)
Is that the size of the biggest blob?
Done
https://review.coreboot.org/c/coreboot/+/46930/14/src/soc/mediatek/common/mt... PS14, Line 21: binary
"%s: Failed to load %s. […]
Done
https://review.coreboot.org/c/coreboot/+/46930/14/src/soc/mediatek/common/mt... PS14, Line 25: memcpy((void *)addr, blob_data, blob_bytes);
CBFS: Found 'dpm.dm' @0x1d980 size 0x1a […]
use _dram_dma as CBFS buffer and copy to target SRAM address.
https://review.coreboot.org/c/coreboot/+/46930/14/src/soc/mediatek/common/mt... PS14, Line 27: /* Memory barrier to ensure that all firmware code is loaded : * before we release the reset pin. : */
You can change the description, but I think the point is, from this function <load_blob_file> people […]
Done
https://review.coreboot.org/c/coreboot/+/46930/14/src/soc/mediatek/common/mt... PS14, Line 32: %ld
The length modifier for `size_t` is `z`.
Done