Hi Bin,
On 6 November 2014 17:07, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Nov 7, 2014 at 4:20 AM, Simon Glass sjg@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 sjg@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