Attention is currently required from: Shelley Chen, Hung-Te Lin, Furquan Shaikh, Paul Menzel, Jianjun Wang. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56791 )
Change subject: soc/mediatek: Add PCIe support ......................................................................
Patch Set 4:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56791/comment/de2fca35_10609295 PS3, Line 11:
Since this file is not platform related, I'm not sure if this is a good idea to add this comment in pcie.c like src/soc/mediatek/mt8186/pll.c does.
Fine. Then please explain it in the commit message.
File src/soc/mediatek/common/pcie.c:
https://review.coreboot.org/c/coreboot/+/56791/comment/832a1669_d1fed5fd PS2, Line 329: mtk_pcie_gen3_probe
Thanks for your suggestion, I removed most of these redundant functions, but there are still some re […]
Furquan is no longer in the team. Shelly, do you mind reviewing this?
File src/soc/mediatek/common/pcie.c:
https://review.coreboot.org/c/coreboot/+/56791/comment/672841bb_b391445b PS4, Line 116: i size_t for i
https://review.coreboot.org/c/coreboot/+/56791/comment/ed5b4858_813e6dcb PS4, Line 194: - 1 There's already "(size) - 1" in PCIE_ATR_SIZE(). Please confirm if this extra "- 1" is needed.
https://review.coreboot.org/c/coreboot/+/56791/comment/1251b875_7c424693 PS4, Line 200: printk(BIOS_INFO, "%s: set %s trans window[%d]: cpu_addr = %#llx, pci_addr = %#llx, size = %#llx\n", Line too long.
https://review.coreboot.org/c/coreboot/+/56791/comment/14215fac_f442ebe8 PS4, Line 202: (unsigned long long) Why do we need to cast it? Can't we just print with %#x?
https://review.coreboot.org/c/coreboot/+/56791/comment/9e8d767d_93864785 PS4, Line 216: if (!res->size) : continue; When will this happen?
https://review.coreboot.org/c/coreboot/+/56791/comment/bc9fdf0d_af8968cb PS4, Line 326: (struct bus *) No need to cast.