Attention is currently required from: Shelley Chen, Hung-Te Lin, Nico Huber, Rex-BC Chen, Julius Werner, Arthur Heymans, Yu-Ping Wu. Jianjun Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63251 )
Change subject: coreboot_tables: Add PCIe info to coreboot table ......................................................................
Patch Set 18:
(1 comment)
Patchset:
PS9:
Oh, do we always need a platform driver on the libpayload side anyway? I thought that maybe they could all be handled the same...
If libpayload has individual platform drivers anyway, then yeah, only encode things that we expect may change between SoCs using the same driver (sounds like that's ctrl_base and maybe mmio_size?), and hardcode the rest in the driver (e.g. specific offsets from ctrl_base).
For PCIe part, we assume the PCIe buses has be scanned and setup successfully at coreboot stage, so we only need to provide the pci_read/write_config APIs [1] in libpayload for the payloads to access the configuration space of PCIe devices.
I think the difference of each non-x86 platform should be how to implement the pci_map_bus() API [2], it will access their hardware and return the base address of specific PCIe device's config space.
For MediaTek SoCs, we write the bdf (bus number, device number, function number) information to the register of 'ctrl_base + 0x140' to select the specific device, then the config space of this selected device will be mapped to the address of 'ctrl_base + 0x1000' [3].
Is that ok to pass the ctrl_base and mmio_size to libpayload? But I'm still curious why QC need the mmio_size, since the size of config space for each PCIe device or function should be 4KByte.
[1]https://review.coreboot.org/c/coreboot/+/56789/65/payloads/libpayload/driver... [2]https://review.coreboot.org/c/coreboot/+/56789/65/payloads/libpayload/driver... [3]https://review.coreboot.org/c/coreboot/+/56794/67/payloads/libpayload/driver...
-
Jianjun Wang (Code Review)