Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44703 )
Change subject: soc/mediatek/mt8192: Do EMI init before dram calibration ......................................................................
Patch Set 46:
(7 comments)
https://review.coreboot.org/c/coreboot/+/44703/36/src/soc/mediatek/mt8192/em... File src/soc/mediatek/mt8192/emi.c:
https://review.coreboot.org/c/coreboot/+/44703/36/src/soc/mediatek/mt8192/em... PS36, Line 389: write32((u32 *)0x40000000, read32((u32 *)0x40000000)); : write32((u32 *)0x40000100, read32((u32 *)0x40000100)); : write32((u32 *)0x40000200, read32((u32 *)0x40000200)); : write32((u32 *)0x40000300, read32((u32 *)0x40000300)); :
0x40000000 is the base addr of DRAM, EMI does basic memory R/W to adjust emi HW setting.
Ack
https://review.coreboot.org/c/coreboot/+/44703/46/src/soc/mediatek/mt8192/em... File src/soc/mediatek/mt8192/emi.c:
https://review.coreboot.org/c/coreboot/+/44703/46/src/soc/mediatek/mt8192/em... PS46, Line 13: write32 Please add a comment before these magic numbers for the source of them, for example the name of a datasheet (it is ok even if the data sheet is only for internal).
https://review.coreboot.org/c/coreboot/+/44703/46/src/soc/mediatek/mt8192/em... PS46, Line 262: write32 same, need a source for the magic numbers
https://review.coreboot.org/c/coreboot/+/44703/46/src/soc/mediatek/mt8192/em... PS46, Line 263: &ch[0].emi_ch if all regs are &ch[0].emi_chn, what about moving that to a parameter from caller, so we can always do
write32(&emi_chn->cona, ...)
https://review.coreboot.org/c/coreboot/+/44703/46/src/soc/mediatek/mt8192/em... PS46, Line 317: 0x05008305 some comments for where these magic numbers are defined, or a source/datasheet for the magic numbers
https://review.coreboot.org/c/coreboot/+/44703/46/src/soc/mediatek/mt8192/em... PS46, Line 325: write32 same, need source for the magic numbers
https://review.coreboot.org/c/coreboot/+/44703/46/src/soc/mediatek/mt8192/em... PS46, Line 389: write32((void *)0x40000000, read32((void *)0x40000000)); : write32((void *)0x40000100, read32((void *)0x40000100)); : write32((void *)0x40000200, read32((void *)0x40000200)); : write32((void *)0x40000300, read32((void *)0x40000300)); : add a comment here (from your words):
/* Do basic memory read/write for EMI to adjust its HW settings */ uintptr_t rw_ptr = _dram;
for (int i = 0; i < 4; i++) { rw_ptr += 0x100; write32((void *)rw_ptr, read32((void *)rw_ptr); }