Attention is currently required from: Xi Chen, Chien-Chih Tseng, Pi-Cheng Chen, flora.fu@mediatek.com. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48622 )
Change subject: soc/mediatek/mt8192: add apusys init flow ......................................................................
Patch Set 9:
(7 comments)
File src/soc/mediatek/mt8192/apusys.c:
https://review.coreboot.org/c/coreboot/+/48622/comment/7f4ac046_d1c63bb3 PS4, Line 11: : write32((void *)INFRA2APU_SRAM_PROT_EN, : read32((void *)INFRA2APU_SRAM_PROT_EN) & (~0xc0000000));
I'd encourage using SET32_BITFIELDS API for this, to provide better meaningful names
Ack
https://review.coreboot.org/c/coreboot/+/48622/comment/3572a81f_3cc086df PS4, Line 14: write32((void *)(APUSYS_MBOX + 0x0b0), 0x00010001); : write32((void *)(APUSYS_MBOX + 0x1b0), 0x00010001); : write32((void *)(APUSYS_MBOX + 0x2b0), 0x00010001); : write32((void *)(APUSYS_MBOX + 0x3b0), 0x00010001); : write32((void *)(APUSYS_MBOX + 0x4b0), 0x00010001); : write32((void *)(APUSYS_MBOX + 0x5b0), 0x00010001); : write32((void *)(APUSYS_MBOX + 0x6b0), 0x00010001); : write32((void *)(APUSYS_MBOX + 0x7b0), 0x00010001); :
int i; […]
Ack
File src/soc/mediatek/mt8192/apusys.c:
https://review.coreboot.org/c/coreboot/+/48622/comment/3a399ddb_35ef6997 PS5, Line 24: (u8 *)
no need to cast here if you will do (void *) later.
Ack
https://review.coreboot.org/c/coreboot/+/48622/comment/8b9a8a8a_da13a1c4 PS5, Line 24: 0x100
add a comment for name of the regsiter in MBOX + i*0x100 + 0xb0
Ack
https://review.coreboot.org/c/coreboot/+/48622/comment/65b5f7a7_7eac3d53 PS5, Line 26: SET32_BITFIELDS(addr, : NO_MPU, 1, LOCK, 1);
this can be in one line.
Ack
https://review.coreboot.org/c/coreboot/+/48622/comment/9461a90c_6bfa9fd0 PS5, Line 29: 0x%x
%p (do we support that in coreboot?) or %#x
Ack
https://review.coreboot.org/c/coreboot/+/48622/comment/0a127603_daf83ec5 PS5, Line 30: APUSYS_MBOX + i * 0x100 + 0xb0
(uintptr_t)addr
Ack