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 16:
(8 comments)
https://review.coreboot.org/c/coreboot/+/34545/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34545/2//COMMIT_MSG@7 PS2, Line 7: SPM: mt8183: support spm suspend in coreboot
Please use the common prefix (see `git log --oneline`).
Ack
https://review.coreboot.org/c/coreboot/+/34545/2//COMMIT_MSG@10 PS2, Line 10:
Test by powerd_dbus_suspend (it would call linux suspend to ram) […]
Ack
https://review.coreboot.org/c/coreboot/+/34545/2//COMMIT_MSG@11 PS2, Line 11: Change-Id: I3393a772f025b0912a5a25a63a87512454fbc86e
ok, I would update on next version.
Ack
https://review.coreboot.org/c/coreboot/+/34545/2/src/soc/mediatek/mt8183/spm... File src/soc/mediatek/mt8183/spm.c:
https://review.coreboot.org/c/coreboot/+/34545/2/src/soc/mediatek/mt8183/spm... PS2, Line 225: CBFS_TYPE_RAW);
ok, I would update on next version.
Ack
https://review.coreboot.org/c/coreboot/+/34545/2/src/soc/mediatek/mt8183/spm... PS2, Line 243: BIOS_INFO
ok, I would update on next version.
not updated yet?
https://review.coreboot.org/c/coreboot/+/34545/2/src/soc/mediatek/mt8183/spm... PS2, Line 244: (unsigned int)(unsigned long)
This is for print dram address of pcmdesc->base […]
this seems not fixed yet. please replace by %p without casting.
https://review.coreboot.org/c/coreboot/+/34545/2/src/soc/mediatek/mt8183/spm... PS2, Line 305: spm_load_firmware(index);
I would try to add on next version.
Ack
https://review.coreboot.org/c/coreboot/+/34545/2/src/soc/mediatek/mt8183/spm... PS2, Line 310: printk(BIOS_ERR, "SPM: firmware is not ready!!!\n");
User need check dram type and if this driver could get right firmware. […]
Ack