Hi Bin, On 11 November 2014 18:50, Bin Meng <bmeng...@gmail.com> wrote: > Hi Simon, > > On Wed, Nov 12, 2014 at 9:02 AM, Simon Glass <s...@chromium.org> wrote: >> 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? > > Sorry I should have said it more clearly. I mean the call to > pci_setup_type1() should be moved to > arch/x86/cpu/pci.c::pci_init_board(), right after the call to > board_pci_setup_hose().
OK I see. But I think this might be board-specific code. Some boards might have different functions for access. > >>> >>>> } >>>> 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. > > Yep, I will send patches after your patches are in the mainline. OK. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot