Attention is currently required from: Hung-Te Lin, Arthur Heymans, Shelley Chen, Nico Huber, Furquan Shaikh, Paul Menzel, Angel Pons, Yu-Ping Wu. Jianjun Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56791 )
Change subject: soc/mediatek: Add PCIe support ......................................................................
Patch Set 11:
(8 comments)
File src/soc/mediatek/common/pcie.c:
https://review.coreboot.org/c/coreboot/+/56791/comment/ff07555b_43001940 PS9, Line 108: pcie_ctrl->base + PCIE_CFG_OFFSET_ADDR; : }
I think it's totally fine to move the read32p/write32p to device/mmio. […]
Done, moved to device/mmio.h: https://review.coreboot.org/c/coreboot/+/62561/1
File src/soc/mediatek/common/pcie.c:
https://review.coreboot.org/c/coreboot/+/56791/comment/eee44677_3c56e088 PS10, Line 120: val = read32(ctrl->base + PCIE_SETTING_REG); : val |= PCIE_RC_MODE; : write32(ctrl->base + PCIE_SETTING_REG, val);
`setbits32()` expects a pointer, so this would need to be: […]
I use write32p() instead, is that OK?
https://review.coreboot.org/c/coreboot/+/56791/comment/582366ac_a05dfdef PS10, Line 141: ctrl->base + PCIE_RST_CTRL_REG
After retyping `base` to `uintptr_t`, this needs a cast (or the parameter type of the function point […]
Done
https://review.coreboot.org/c/coreboot/+/56791/comment/867bd48c_7c96a03d PS10, Line 147: >
=
Done
https://review.coreboot.org/c/coreboot/+/56791/comment/e84bb85b_536b4bce PS10, Line 154: ms
This is incorrect. 'tries' is the number of tries, not elapsed time.
Done
https://review.coreboot.org/c/coreboot/+/56791/comment/e6cab86d_357c2928 PS10, Line 193: table + PCIE_ATR_SRC_ADDR_MSB_OFFSET
+1
Done
https://review.coreboot.org/c/coreboot/+/56791/comment/86a9fa50_2a500192 PS10, Line 204: }
Is it necessary to report the resource and configure the hardware […]
Done
https://review.coreboot.org/c/coreboot/+/56791/comment/4f5fdc89_9fbdb577 PS10, Line 231: return;
Why is this done here and not in `chip_ops->enable_dev()` or […]
Moved to ops->enable(), thanks for your review.
-
Jianjun Wang (Code Review)