Hi Bin,
On 6 November 2014 17:07, Bin Meng <bmeng...@gmail.com> wrote: > Hi Simon, > > On Fri, Nov 7, 2014 at 4:20 AM, Simon Glass <s...@chromium.org> wrote: >> This code is not really Coreboot-specific, so move it into the common area >> and rename it. > > OK, but current coreboot pci codes are broken, thus need to be fixed > before making it common. > >> Signed-off-by: Simon Glass <s...@chromium.org> >> --- >> >> arch/x86/cpu/Makefile | 1 + >> arch/x86/cpu/coreboot/Makefile | 1 - >> arch/x86/cpu/{coreboot => }/pci.c | 24 +++++++++++++----------- >> 3 files changed, 14 insertions(+), 12 deletions(-) >> rename arch/x86/cpu/{coreboot => }/pci.c (63%) >> >> diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile >> index 9d38ef7..97f36d5 100644 >> --- a/arch/x86/cpu/Makefile >> +++ b/arch/x86/cpu/Makefile >> @@ -11,3 +11,4 @@ >> extra-y = start.o >> obj-$(CONFIG_X86_RESET_VECTOR) += resetvec.o start16.o >> obj-y += interrupts.o cpu.o call64.o >> +obj-$(CONFIG_PCI) += pci.o >> diff --git a/arch/x86/cpu/coreboot/Makefile b/arch/x86/cpu/coreboot/Makefile >> index cd0bf4e..65cf2cb 100644 >> --- a/arch/x86/cpu/coreboot/Makefile >> +++ b/arch/x86/cpu/coreboot/Makefile >> @@ -19,4 +19,3 @@ obj-$(CONFIG_SYS_COREBOOT) += tables.o >> obj-$(CONFIG_SYS_COREBOOT) += ipchecksum.o >> obj-$(CONFIG_SYS_COREBOOT) += sdram.o >> obj-$(CONFIG_SYS_COREBOOT) += timestamp.o >> -obj-$(CONFIG_PCI) += pci.o >> diff --git a/arch/x86/cpu/coreboot/pci.c b/arch/x86/cpu/pci.c >> similarity index 63% >> rename from arch/x86/cpu/coreboot/pci.c >> rename to arch/x86/cpu/pci.c >> index 33f16a3..f35c8b3 100644 >> --- a/arch/x86/cpu/coreboot/pci.c >> +++ b/arch/x86/cpu/pci.c >> @@ -13,7 +13,7 @@ >> #include <pci.h> >> #include <asm/pci.h> >> >> -static struct pci_controller coreboot_hose; >> +static struct pci_controller x86_hose; >> >> static void config_pci_bridge(struct pci_controller *hose, pci_dev_t dev, >> struct pci_config_table *table) >> @@ -24,7 +24,7 @@ static void config_pci_bridge(struct pci_controller *hose, >> pci_dev_t dev, >> pci_hose_scan_bus(hose, secondary); >> } >> >> -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? > >> 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. > >> - pci_register_hose(&coreboot_hose); >> + pci_setup_type1(hose); >> >> - pci_hose_scan(&coreboot_hose); >> + pci_register_hose(hose); >> + >> + pci_hose_scan(hose); >> } > > Regards, > Bin Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot