Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46930 )
Change subject: soc/mediatek/common: A common API for loading the blob file ......................................................................
Patch Set 14:
(8 comments)
Very nice.
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):
Add common API to load blob file
https://review.coreboot.org/c/coreboot/+/46930/14//COMMIT_MSG@9 PS14, Line 9: blog blob
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.
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.
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 *`?
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?
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. : */ Please use format the comment according to the [coding style][1].
[1]: https://doc.coreboot.org/coding_style.html#commenting
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`.