Hi Simon,
On Fri, Nov 7, 2014 at 8:26 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 6 November 2014 17:07, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
-static struct pci_config_table pci_coreboot_config_table[] = { +static struct pci_config_table pci_x86_config_table[] = { /* vendor, device, class, bus, dev, func */ { PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, &config_pci_bridge }, @@ -33,17 +33,19 @@ static struct pci_config_table pci_coreboot_config_table[] = {
Use this config_table will cause infinite loop when doing pci_hose_scan later. I do not use config_table on my Crown Bay port, instead using CONFIG_PCI_PNP which works very well.
OK, so are you saying that we should leave this separate for each board?
Not really. But we can make this code really generic. So far it is broken.
void pci_init_board(void) {
coreboot_hose.config_table = pci_coreboot_config_table;coreboot_hose.first_busno = 0;coreboot_hose.last_busno = 0;
struct pci_controller *hose = &x86_hose;
pci_set_region(coreboot_hose.regions + 0, 0x0, 0x0, 0xffffffff,PCI_REGION_MEM);coreboot_hose.region_count = 1;
hose->config_table = pci_x86_config_table;hose->first_busno = 0;hose->last_busno = 0;
pci_setup_type1(&coreboot_hose);
pci_set_region(hose->regions + 0, 0x0, 0x0, 0xffffffff,PCI_REGION_MEM);hose->region_count = 1;There are 3 issues with the pci_set_region here: 1). The whole 4GiB PCI memory region creats conflicts with the systeam RAM memory map. This is a programming effor and normally causes undefined behaviour from chipset perspective. 2). There is no IO region configured, that means any device behind the PCI/PCIe bridge with only IO bar will fail to work. 3). There is no systeam RAM region configured, that means any device driver behind the PCI/PCIe bridge will fail to create pci addr <-> cpu physical addr mappings.
Actually this is not real setup - for the coreboot case it is scan-only, there is no memory or I/O allocation going on.
OK, so if U-Boot boots from coreboot, the issue #1 and #2 disappear because coreboot has already enumerated the buses and devices and have memory/io allocation setup. That's fine. But this code is assumed to be used in U-Boot without coreboot too, thus U-Boot has to do the same thing as coreboot. And in the latter case, issue #1 and #2 do matter. About the issue #3, even when U-Boot is booting from coreboot, the system RAM region still needs to be set up otherwise you won't get any PCI device driver in U-Boot work. You can try adding CONFIG_E1000 to test an Intel Pro/1000 NIC in U-Boot and see what's happening.
Regards, Bin