Hi Simon,
On Fri, Nov 7, 2014 at 9:53 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 6 November 2014 18:39, Bin Meng bmeng.cn@gmail.com wrote:
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.
I think the issue is that booting from Coreboot is quite a different use case from everything else.
We can use #ifdef CONFIG_SYS_COREBOOT in avoid re-allocating memory resources in U-Boot.
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.
If I do that, then Coreboot will find the device and allocate space for it, then U-Boot will juts use the allocated space.
Yes, I understand this.
Actually I think the region is completely misleading in the Coreboot case and should juts be remove.
I am not sure if memory and IO can be removed completely for the coreboot case. I will need look into the pci.c/pci_auto.c and PCI device driver to confirm.
Issue 3 is not a problem either I think, since again Coreboot will allocate it.
No, it does matter for coreboot. If the pci hose does not have the system RAM region setup, the pci_mem_to_phys will fail when PCI device driver wants to do DMA to RAM.
Regards, Bin