Re: [U-Boot] [PATCH v2 15/33] x86: Refactor PCI to permit alternate init
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, 0x, - 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, 0x, + 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? } diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c new file mode 100644 index 000..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. +} 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 15/33] x86: Refactor PCI to permit alternate init
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, 0x, - 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, 0x, + 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 000..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
Re: [U-Boot] [PATCH v2 15/33] x86: Refactor PCI to permit alternate init
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, 0x, - 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, 0x, + 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(). } diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c new file mode 100644 index 000..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. Regards, Bin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 15/33] x86: Refactor PCI to permit alternate init
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, 0x, - 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, 0x, + 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 000..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
[U-Boot] [PATCH v2 15/33] x86: Refactor PCI to permit alternate init
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, 0x, - 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, 0x, + PCI_REGION_MEM); + hose-region_count = 1; - pci_hose_scan(coreboot_hose); + pci_setup_type1(hose); } diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c new file mode 100644 index 000..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); +} 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 -- 2.1.0.rc2.206.gedb03e5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot