Hi Bin,
On 11 November 2014 07:14, Bin Meng <bmeng...@gmail.com> wrote:
> Hi Simon,
>
> On Tue, Nov 11, 2014 at 9:00 AM, Simon Glass <s...@chromium.org> wrote:
>> We want access PCI earlier in the init sequence, so refactor the code so
>> that it does not require use of a BSS variable to work. This will allow us
>> to use early malloc() to store information about a PCI hose.
>>
>> Common PCI code moves to arch/x86/cpu/pci.c and a new
>> board_pci_setup_hose() function is provided by boards to set up the (single)
>> hose used by that board.
>>
>> Signed-off-by: Simon Glass <s...@chromium.org>
>> ---
>>
>> Changes in v2: None
>>
>> arch/x86/cpu/Makefile | 1 +
>> arch/x86/cpu/coreboot/pci.c | 22 ++++++++--------------
>> arch/x86/cpu/pci.c | 26 ++++++++++++++++++++++++++
>> arch/x86/include/asm/pci.h | 11 +++++++++++
>> 4 files changed, 46 insertions(+), 14 deletions(-)
>> create mode 100644 arch/x86/cpu/pci.c
>>
>> 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/pci.c b/arch/x86/cpu/coreboot/pci.c
>> index 33f16a3..130fd88 100644
>> --- a/arch/x86/cpu/coreboot/pci.c
>> +++ b/arch/x86/cpu/coreboot/pci.c
>> @@ -13,8 +13,6 @@
>> #include <pci.h>
>> #include <asm/pci.h>
>>
>> -static struct pci_controller coreboot_hose;
>> -
>> static void config_pci_bridge(struct pci_controller *hose, pci_dev_t dev,
>> struct pci_config_table *table)
>> {
>> @@ -31,19 +29,15 @@ static struct pci_config_table
>> pci_coreboot_config_table[] = {
>> {}
>> };
>>
>> -void pci_init_board(void)
>> +void board_pci_setup_hose(struct pci_controller *hose)
>> {
>> - coreboot_hose.config_table = pci_coreboot_config_table;
>> - coreboot_hose.first_busno = 0;
>> - coreboot_hose.last_busno = 0;
>> -
>> - pci_set_region(coreboot_hose.regions + 0, 0x0, 0x0, 0xffffffff,
>> - PCI_REGION_MEM);
>> - coreboot_hose.region_count = 1;
>> -
>> - pci_setup_type1(&coreboot_hose);
>> + hose->config_table = pci_coreboot_config_table;
>> + hose->first_busno = 0;
>> + hose->last_busno = 0;
>>
>> - pci_register_hose(&coreboot_hose);
>> + pci_set_region(hose->regions + 0, 0x0, 0x0, 0xffffffff,
>> + PCI_REGION_MEM);
>> + hose->region_count = 1;
>>
>> - pci_hose_scan(&coreboot_hose);
>> + pci_setup_type1(hose);
>
> Should pci_setup_type1(hose) be moved to arch/x86/cpu/pci.c?
It seems happy enough in lib/ - why do you think it should move?
>
>> }
>> diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c
>> new file mode 100644
>> index 0000000..030cbbc
>> --- /dev/null
>> +++ b/arch/x86/cpu/pci.c
>> @@ -0,0 +1,26 @@
>> +/*
>> + * Copyright (c) 2011 The Chromium OS Authors.
>> + * (C) Copyright 2008,2009
>> + * Graeme Russ, <graeme.r...@gmail.com>
>> + *
>> + * (C) Copyright 2002
>> + * Daniel Engström, Omicron Ceti AB, <dan...@omicron.se>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <pci.h>
>> +#include <asm/pci.h>
>> +
>> +static struct pci_controller x86_hose;
>> +
>> +void pci_init_board(void)
>> +{
>> + struct pci_controller *hose = &x86_hose;
>> +
>> + board_pci_setup_hose(hose);
>> + pci_register_hose(hose);
>> +
>> + pci_hose_scan(hose);
>
> Should we save the return value of pci_hose_scan(hose) to
> hose->last_busno? I noticed that coreboot/pci.c is doing this in the
> config_device callback, but that callback causes infinite loop when I
> tested that logic on the Crown Bay board.
I expect so, but perhaps that is for you to decide out when you send
your patches? My objective is to get ivybridge into a reasonable
state, but I and certainly expecting that you will want to change some
things.
When you do that I will be able to test and may comments so we can
keep both working.
>
>> +}
>> diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
>> index 6b16188..c160707 100644
>> --- a/arch/x86/include/asm/pci.h
>> +++ b/arch/x86/include/asm/pci.h
>> @@ -12,5 +12,16 @@
>> #define DEFINE_PCI_DEVICE_TABLE(_table) \
>> const struct pci_device_id _table[]
>>
>> +struct pci_controller;
>> +
>> void pci_setup_type1(struct pci_controller *hose);
>> +
>> +/**
>> + * board_pci_setup_hose() - Set up the PCI hose
>> + *
>> + * This is called by the common x86 PCI code to set up the PCI controller
>> + * hose. It may be called when no memory/BSS is available so should just
>> + * store things in 'hose' and not in BSS variables.
>> + */
>> +void board_pci_setup_hose(struct pci_controller *hose);
>> #endif
>> --
>
> Regards,
> Bin
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
