Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32666 )
Change subject: mediatek/mt8183: Add md power off flow ......................................................................
Patch Set 6:
(8 comments)
https://review.coreboot.org/#/c/32666/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32666/6//COMMIT_MSG@9 PS6, Line 9: make suspend fail. fail suspend/resume.
https://review.coreboot.org/#/c/32666/6//COMMIT_MSG@9 PS6, Line 9: 26M clock will be hold by srcclkena SRCCLKENA holds 26M clock
https://review.coreboot.org/#/c/32666/6//COMMIT_MSG@10 PS6, Line 10: on by
https://review.coreboot.org/#/c/32666/6//COMMIT_MSG@10 PS6, Line 10: , so we release it. so we can simply release it for suspend/resume to work.
https://review.coreboot.org/#/c/32666/6//COMMIT_MSG@10 PS6, Line 10: srcclkena SRCCLKENA
https://review.coreboot.org/#/c/32666/6/src/soc/mediatek/mt8183/md_ctrl.c File src/soc/mediatek/mt8183/md_ctrl.c:
https://review.coreboot.org/#/c/32666/6/src/soc/mediatek/mt8183/md_ctrl.c@23 PS6, Line 23: (1 << 8) | (1 << 9) Do these have good reg constant names?
If possible, please define them as enum (can be in the .c file), and then use the named constant here to make it more meaningful.
https://review.coreboot.org/#/c/32666/6/src/soc/mediatek/mt8183/md_ctrl.c@25 PS6, Line 25: 0xFF also make a constant for this.
https://review.coreboot.org/#/c/32666/6/src/soc/mediatek/mt8183/soc.c File src/soc/mediatek/mt8183/soc.c:
https://review.coreboot.org/#/c/32666/6/src/soc/mediatek/mt8183/soc.c@20 PS6, Line 20: #include <soc/md_ctrl.h> order header files (i.e., move to before mmu_operations.h