Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34545 )
Change subject: mediatek/mt8183: Init SPM driver ......................................................................
Patch Set 17:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34545/17/src/mainboard/google/kukui... File src/mainboard/google/kukui/mainboard.c:
https://review.coreboot.org/c/coreboot/+/34545/17/src/mainboard/google/kukui... PS17, Line 161: SPM initial fail!!! SPM initialization failed, suspend/resume may fail.
https://review.coreboot.org/c/coreboot/+/34545/17/src/soc/mediatek/mt8183/Ma... File src/soc/mediatek/mt8183/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34545/17/src/soc/mediatek/mt8183/Ma... PS17, Line 72: 3rdparty/blobs/soc/mediatek/mt8183/pcm_allinone_lp4_3200.bin let's try to refactor this with the sspm one.
MT8183_BLOB_DIR := 3rdparty/blobs/src/mediatek/mt8183
... sspm.bin-file $(MT8183_BLOB_DIR)/sspm.bin ...
pcm_allinone_... := $(MT8183_BLOB_DIR)/pcm_allinone_lp4_3733.bin
https://review.coreboot.org/c/coreboot/+/34545/17/src/soc/mediatek/mt8183/sp... File src/soc/mediatek/mt8183/spm.c:
https://review.coreboot.org/c/coreboot/+/34545/17/src/soc/mediatek/mt8183/sp... PS17, Line 80: write32(&mtk_spm->spare_ack_mask, : (read32(&mtk_spm->spare_ack_mask) & ~SPARE_ACK_MASK_B_BIT1) | : SPARE_ACK_MASK_B_BIT0); Is this equivalent to:
clrsetbits32(read32(&mtk_spm->spare_ack_mask), SPARE_ACK_MASK_B_BIT1, SPARE_ACK_MASK_B_BIT0);
https://review.coreboot.org/c/coreboot/+/34545/17/src/soc/mediatek/mt8183/sp... PS17, Line 88: write32 setbits32 ?
https://review.coreboot.org/c/coreboot/+/34545/17/src/soc/mediatek/mt8183/sp... PS17, Line 130: read32(&mtk_spm->pcm_reg0_data) wrong indent - this should be aligned to (
https://review.coreboot.org/c/coreboot/+/34545/17/src/soc/mediatek/mt8183/sp... PS17, Line 135: write32 clrsetbits32?
https://review.coreboot.org/c/coreboot/+/34545/17/src/soc/mediatek/mt8183/sp... PS17, Line 309: while should we do a wait_us or wait_ms here?