Erin Lo has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31516 )
Change subject: google/kukui: boot up sspm ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/#/c/31516/1/src/mainboard/google/kukui/mainboard... File src/mainboard/google/kukui/mainboard.c:
https://review.coreboot.org/#/c/31516/1/src/mainboard/google/kukui/mainboard... PS1, Line 48: "sspm"
Don't directly use string literal as file name.
Could I use sspm.img?
https://review.coreboot.org/#/c/31516/1/src/soc/mediatek/mt8183/Makefile.inc File src/soc/mediatek/mt8183/Makefile.inc:
https://review.coreboot.org/#/c/31516/1/src/soc/mediatek/mt8183/Makefile.inc... PS1, Line 52: ramstage-y += sspm.c
move this line in between line 47~48
Why? I think it is in-order in line 52 ?
https://review.coreboot.org/#/c/31516/1/src/soc/mediatek/mt8183/include/soc/... File src/soc/mediatek/mt8183/include/soc/sspm.h:
https://review.coreboot.org/#/c/31516/1/src/soc/mediatek/mt8183/include/soc/... PS1, Line 26: #define CFG_SSPM_SRAM 0x10400000
How about we move it to addresmap. […]
I will move it to addresmap.h, and re-name to SSPM_SRAM_BASE
https://review.coreboot.org/#/c/31516/1/src/soc/mediatek/mt8183/sspm.c File src/soc/mediatek/mt8183/sspm.c:
https://review.coreboot.org/#/c/31516/1/src/soc/mediatek/mt8183/sspm.c@22 PS1, Line 22: #define SSPM_UART 1
this should be a Kconfig value
It is about pin selection depends on platform and just for debug. I will remove it in formal code.
https://review.coreboot.org/#/c/31516/1/src/soc/mediatek/mt8183/sspm.c@31 PS1, Line 31: write32
a command for what this triggers - resetting SSPM ? or also loads the CFG_SSPM_SRAM etc?
resetting SSPM