Attention is currently required from: Hung-Te Lin, Paul Menzel, Yu-Ping Wu. Wenbin Mei has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51966 )
Change subject: soc/mediatek: add new driver 'msdc' for eMMC ......................................................................
Patch Set 5:
(9 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51966/comment/ed8ffa59_97aaf8c2 PS2, Line 7: mmc: Add MTK mmc driver support
You are also implementing (and enabling) early init. […]
Done
Commit Message:
https://review.coreboot.org/c/coreboot/+/51966/comment/3e8f0cfa_2fc9eebd PS3, Line 10:
- Did you write the driver from scratch? […]
Done
File src/mainboard/google/asurada/mainboard.c:
https://review.coreboot.org/c/coreboot/+/51966/comment/2c76040a_bb2e9068 PS1, Line 233: struct msdc_ctrlr msdc_host;
Where? I only see it in mtk_mmc_early_init? […]
Ack
https://review.coreboot.org/c/coreboot/+/51966/comment/96cebf11_0bbfac52 PS1, Line 243: (void *)MSDC0_BASE, (void *)MSDC0_TOP_BASE
this is the only place making it board-specific right? […]
Done
https://review.coreboot.org/c/coreboot/+/51966/comment/e6a638c3_1b9ea72d PS1, Line 247: 400*1000
P. […]
Yes, 400KHz is init clock, and all eMMCs will support it.
File src/soc/mediatek/common/msdc.c:
https://review.coreboot.org/c/coreboot/+/51966/comment/0216f304_e393adaf PS2, Line 70: udelay(1);
Sleep before read32(addr) to avoid 2 consecutive read ops (one in line #63 and the other from the fi […]
Ack
https://review.coreboot.org/c/coreboot/+/51966/comment/8b1d0a95_19267dac PS2, Line 72: MSDC_NOT_READY
"-MSDC_NOT_READY" is returned in msdc_poll_timeout(). Please be consistent.
Done
https://review.coreboot.org/c/coreboot/+/51966/comment/05491afe_6b672ba1 PS2, Line 349: host
Just "ctrlr"
We need "msdc_ctrlr *" pointer as first parameter for the "msdc_start_command" function, so it should be host.
https://review.coreboot.org/c/coreboot/+/51966/comment/c45f6871_07e9d94f PS2, Line 385: clrsetbits_le32
Just clrsetbits? Please double check.
Yes