[U-Boot] [PATCH 22/39] x86: Move Coreboot PCI into common cpu area

[Bin Meng]

Hi Simon,

On Fri, Nov 7, 2014 at 4:20 AM, Simon Glass <sjg at 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 at 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.

>  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.

> -       pci_register_hose(&coreboot_hose);
> +       pci_setup_type1(hose);
>
> -       pci_hose_scan(&coreboot_hose);
> +       pci_register_hose(hose);
> +
> +       pci_hose_scan(hose);
>  }

Regards,
Bin