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:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56791/comment/a8b170f0_4f1069cb PS3, Line 10: support
supporting
Ack
https://review.coreboot.org/c/coreboot/+/56791/comment/8f4b73d9_55be4537 PS3, Line 11:
MT8195 Register Map V0.3-2, Chapter 3.18 PCIe controller (Page 1250).
Please follow the style of https://review.coreboot.org/c/coreboot/+/58936/7/src/soc/mediatek/mt8186/pll... and add a comment at the beginning of pcie.c.
Should I add this information in commit message?
Yes, please.
Build pass and boot up to kernel successfully via SSD.
Please add
TEST=Build pass and boot up to kernel successfully via SSD BUG=b:123
See CB:59569 as an example.
File src/soc/mediatek/common/pcie.c:
https://review.coreboot.org/c/coreboot/+/56791/comment/f79a4100_5af95f92 PS3, Line 85: static inline int mtk_fls(int x)
Agree, do you have any suggestions on which file this function should be added to?
Maybe commonlib/bsd/include/commonlib/bsd/helpers.h? Note that there're other definitions you will need to handle to avoid breaking existing platforms:
1. commonlib/storage/sdhci.c: Change to use the definition in helpers.h 2. include/cpu/x86/mtrr.h: This is x86-specific, so you may need to either add an #ifdef for the common version, or merge that into the common version.
Also, please do it in a separate patch.
-
Yu-Ping Wu (Code Review)