hsin-hsiung wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45398 )
Change subject: soc/mediatek/mt8192: add pmif driver ......................................................................
Patch Set 26:
(5 comments)
https://review.coreboot.org/c/coreboot/+/45398/14/src/soc/mediatek/mt8192/Ma... File src/soc/mediatek/mt8192/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45398/14/src/soc/mediatek/mt8192/Ma... PS14, Line 12: bootblock
Do we really need to enable PMIF in bootblock? Can we do that in romstage?
I think there are several modules, like rtc/clkbuf/srclken_rc, which depend on pmif module. It is OK for me to move pmif module in romstage if these modules can move, too.
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/pmif.h:
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/inc... PS7, Line 134: unsigned int
the address and value of pmic mt6359p is 32 bit, so I think u32 should be enough.
Done
https://review.coreboot.org/c/coreboot/+/45398/14/src/soc/mediatek/mt8192/pm... File src/soc/mediatek/mt8192/pmif_clk.c:
https://review.coreboot.org/c/coreboot/+/45398/14/src/soc/mediatek/mt8192/pm... PS14, Line 91: 50
Please explain why 50 is needed, or add a reference to which chapter this defined in which datasheet […]
We need the delay for stable clk, and it is described in ULPOSC_SW_Calibration.doc.
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... File src/soc/mediatek/mt8192/pmif_spi.c:
https://review.coreboot.org/c/coreboot/+/45398/7/src/soc/mediatek/mt8192/pmi... PS7, Line 171: };
In that pattern documented somewhere?
No. This part is to do the calibration to find out the correct timing of spi. The test pattern is to verify the bit 0~15 of the read-back data. 0x6996 = 2'b[0110100110010110] 0x9669 = 2'b[1001011001101001]
https://review.coreboot.org/c/coreboot/+/45398/14/src/soc/mediatek/mt8192/pm... File src/soc/mediatek/mt8192/pmif_spi.c:
https://review.coreboot.org/c/coreboot/+/45398/14/src/soc/mediatek/mt8192/pm... PS14, Line 321: 100
add a reference for where we can find this 100 being defined.
After checking, the project don't use modem feature, so we remove it.