Attention is currently required from: Shelley Chen, Hung-Te Lin, Angel Pons, Jianjun Wang. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56789 )
Change subject: libpayload/pci: Add support for bus mapping ......................................................................
Patch Set 41:
(4 comments)
File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/56789/comment/cd675fad_1aa7addb PS36, Line 411: select PCI_COMMON : default n
In that case, PCI_IO_OPS should be set to "default n", I'm not sure if this can work properly on x86 […]
See my comment above.
https://review.coreboot.org/c/coreboot/+/56789/comment/16f0f265_72fece2a PS36, Line 417: select PCI_COMMON
PCI tag in libpayload only means that the common PCIe interface, I think we should select it instead […]
The value of PCI_IO_OPS doesn't matter when PCI is disabled, so PCI_IO_OPS should depend on "PCI=y". I tried
config PCI_IO_OPS depends on PCI default y if ARCH_X86 default n config PCIE_MEDIATEK depends on !PCI_IO_OPS default n
and it worked as expected:
1. MTK: Enable *both* PCI and PCIE_MEDIATEK from the overlay 2. x86: When PCI is enabled, PCI_IO_OPS will be selected automatically. 3. Others: When PCI is enabled but PCIE_MEDIATEK is not, pci_map_bus() will be undefined.
File payloads/libpayload/drivers/pci_map_bus_ops.c:
https://review.coreboot.org/c/coreboot/+/56789/comment/434aa632_6f3f75e4 PS35, Line 38: |
The cfg_base should be 4K alignment, and the range of reg should be 0 ~ 4K for each device's config […]
Okay, I think the current implementation is fine.
File payloads/libpayload/include/pci.h:
https://review.coreboot.org/c/coreboot/+/56789/comment/a9a072ac_6104afb3 PS40, Line 105: u32 pcidev_t
-
Yu-Ping Wu (Code Review)