Erin Lo has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31516 )
Change subject: mediatek/mt8183: Add SSPM init support ......................................................................
Patch Set 12:
(7 comments)
Patch Set 10:
(1 comment)
https://review.coreboot.org/#/c/31516/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31516/10//COMMIT_MSG@9 PS10, Line 9: SSPM is a Secure System Power Manager for : power control in secure domain.
Please use the full text width.
Done
https://review.coreboot.org/#/c/31516/10//COMMIT_MSG@11 PS10, Line 11: is load
is to load
Done
https://review.coreboot.org/#/c/31516/10//COMMIT_MSG@11 PS10, Line 11: SSPM firmware
Is that already in the 3rdparty repository?
https://review.coreboot.org/c/blobs/+/32698
https://review.coreboot.org/#/c/31516/10/src/soc/mediatek/mt8183/sspm.c File src/soc/mediatek/mt8183/sspm.c:
https://review.coreboot.org/#/c/31516/10/src/soc/mediatek/mt8183/sspm.c@19 PS10, Line 19: <arch/io.h>
This header file has been renamed. Use "<arch/mmio.h>" instead.
Done
https://review.coreboot.org/#/c/31516/10/src/soc/mediatek/mt8183/sspm.c@35 PS10, Line 35: die("SSPM file not found.");
As the name is in a variable, please print the name in the error message too.
Done
https://review.coreboot.org/#/c/31516/10/src/soc/mediatek/mt8183/sspm.c@38 PS10, Line 38: are
is
Done
https://review.coreboot.org/#/c/31516/10/src/soc/mediatek/mt8183/sspm.c@38 PS10, Line 38: /* Memory barrier to ensure that all fw code are loaded : * before we release the reset pin. : */
Please follow the coding style. […]
Done