[U-Boot] [PATCH 22/39] x86: Move Coreboot PCI into common cpu area
This code is not really Coreboot-specific, so move it into the common area
and rename it.
Signed-off-by: Simon Glass s...@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[]
= {
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, 0x,
- 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, 0x,
+ PCI_REGION_MEM);
+ hose-region_count = 1;
- pci_register_hose(coreboot_hose);
+ pci_setup_type1(hose);
- pci_hose_scan(coreboot_hose);
+ pci_register_hose(hose);
+
+ pci_hose_scan(hose);
}
--
2.1.0.rc2.206.gedb03e5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 22/39] x86: Move Coreboot PCI into common cpu area
Hi Simon,
On Fri, Nov 7, 2014 at 4:20 AM, Simon Glass s...@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 s...@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, 0x,
- 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, 0x,
+ 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 22/39] x86: Move Coreboot PCI into common cpu area
Hi Bin,
On 6 November 2014 17:07, Bin Meng bmeng...@gmail.com wrote:
Hi Simon,
On Fri, Nov 7, 2014 at 4:20 AM, Simon Glass s...@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 s...@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, 0x,
- 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, 0x,
+ 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 22/39] x86: Move Coreboot PCI into common cpu area
Hi Simon,
On Fri, Nov 7, 2014 at 8:26 AM, Simon Glass s...@chromium.org wrote:
Hi Bin,
On 6 November 2014 17:07, Bin Meng bmeng...@gmail.com wrote:
Hi Simon,
-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?
Not really. But we can make this code really generic. So far it is broken.
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, 0x,
- 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, 0x,
+ 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.
OK, so if U-Boot boots from coreboot, the issue #1 and #2 disappear
because coreboot has already enumerated the buses and devices and have
memory/io allocation setup. That's fine. But this code is assumed to
be used in U-Boot without coreboot too, thus U-Boot has to do the same
thing as coreboot. And in the latter case, issue #1 and #2 do matter.
About the issue #3, even when U-Boot is booting from coreboot, the
system RAM region still needs to be set up otherwise you won't get any
PCI device driver in U-Boot work. You can try adding CONFIG_E1000 to
test an Intel Pro/1000 NIC in U-Boot and see what's happening.
Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 22/39] x86: Move Coreboot PCI into common cpu area
Hi Bin,
On 6 November 2014 18:39, Bin Meng bmeng...@gmail.com wrote:
Hi Simon,
On Fri, Nov 7, 2014 at 8:26 AM, Simon Glass s...@chromium.org wrote:
Hi Bin,
On 6 November 2014 17:07, Bin Meng bmeng...@gmail.com wrote:
Hi Simon,
-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?
Not really. But we can make this code really generic. So far it is broken.
I think the issue is that booting from Coreboot is quite a different
use case from everything else.
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, 0x,
- 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, 0x,
+ 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.
OK, so if U-Boot boots from coreboot, the issue #1 and #2 disappear
because coreboot has already enumerated the buses and devices and have
memory/io allocation setup. That's fine. But this code is assumed to
be used in U-Boot without coreboot too, thus U-Boot has to do the same
thing as coreboot. And in the latter case, issue #1 and #2 do matter.
About the issue #3, even when U-Boot is booting from coreboot, the
system RAM region still needs to be set up otherwise you won't get any
PCI device driver in U-Boot work. You can try adding CONFIG_E1000 to
test an Intel Pro/1000 NIC in U-Boot and see what's happening.
If I do that, then Coreboot will find the device and allocate space
for it, then U-Boot will juts use the allocated space.
Actually I think the region is completely misleading in the Coreboot
case and should juts be remove.
Issue 3 is not a problem either I think, since again Coreboot will allocate it.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 22/39] x86: Move Coreboot PCI into common cpu area
Hi Simon,
On Fri, Nov 7, 2014 at 9:53 AM, Simon Glass s...@chromium.org wrote:
Hi Bin,
On 6 November 2014 18:39, Bin Meng bmeng...@gmail.com wrote:
Hi Simon,
On Fri, Nov 7, 2014 at 8:26 AM, Simon Glass s...@chromium.org wrote:
Hi Bin,
On 6 November 2014 17:07, Bin Meng bmeng...@gmail.com wrote:
Hi Simon,
-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?
Not really. But we can make this code really generic. So far it is broken.
I think the issue is that booting from Coreboot is quite a different
use case from everything else.
We can use #ifdef CONFIG_SYS_COREBOOT in avoid re-allocating memory
resources in U-Boot.
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, 0x,
- 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, 0x,
+ 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.
OK, so if U-Boot boots from coreboot, the issue #1 and #2 disappear
because coreboot has already enumerated the buses and devices and have
memory/io allocation setup. That's fine. But this code is assumed to
be used in U-Boot without coreboot too, thus U-Boot has to do the same
thing as coreboot. And in the latter case, issue #1 and #2 do matter.
About the issue #3, even when U-Boot is booting from coreboot, the
system RAM region still needs to be set up otherwise you won't get any
PCI device driver in U-Boot work. You can try adding CONFIG_E1000 to
test an Intel Pro/1000 NIC in U-Boot and see what's happening.
If I do that, then Coreboot will find the device and allocate space
for it, then U-Boot will juts use the allocated space.
Yes, I understand this.
Actually I think the region is completely misleading in the Coreboot
case and should juts be remove.
I am not sure if memory and IO can be removed completely for the
coreboot case. I will need look into the pci.c/pci_auto.c and PCI
device driver to confirm.
Issue 3 is not a problem either I think, since again Coreboot will allocate
it.
No, it does matter for coreboot. If the pci hose does not have the
system RAM region setup, the pci_mem_to_phys will fail when PCI device
driver wants to do DMA to RAM.
Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 22/39] x86: Move Coreboot PCI into common cpu area
Hi Bin,
On 6 November 2014 19:12, Bin Meng bmeng...@gmail.com wrote:
Hi Simon,
On Fri, Nov 7, 2014 at 9:53 AM, Simon Glass s...@chromium.org wrote:
Hi Bin,
On 6 November 2014 18:39, Bin Meng bmeng...@gmail.com wrote:
Hi Simon,
On Fri, Nov 7, 2014 at 8:26 AM, Simon Glass s...@chromium.org wrote:
Hi Bin,
On 6 November 2014 17:07, Bin Meng bmeng...@gmail.com wrote:
Hi Simon,
-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?
Not really. But we can make this code really generic. So far it is broken.
I think the issue is that booting from Coreboot is quite a different
use case from everything else.
We can use #ifdef CONFIG_SYS_COREBOOT in avoid re-allocating memory
resources in U-Boot.
Hmm in that case I'd rather keep the files separate, since I'd like to
minimise the use of #ifdef.
Can we just leave off CONFIG_PCI_PNP?
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, 0x,
- 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, 0x,
+ 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.
OK, so if U-Boot boots from coreboot, the issue #1 and #2 disappear
because coreboot has already enumerated the buses and devices and have
memory/io allocation setup. That's fine. But this code is assumed to
be used in U-Boot without coreboot too, thus U-Boot has to do the same
thing as coreboot. And in the latter case, issue #1 and #2 do matter.
About the issue #3, even when U-Boot is booting from coreboot, the
system RAM region still needs to be set up otherwise you won't get any
PCI device driver in U-Boot work. You can try adding CONFIG_E1000 to
test an Intel Pro/1000 NIC in U-Boot and see what's happening.
If I do that, then Coreboot will find the device and allocate space
for it, then U-Boot will juts use the allocated space.
Yes, I understand this.
Actually I think the region is completely misleading in the Coreboot
case and should juts be remove.
I am not sure if memory and IO can be removed completely for the
coreboot case. I will need look into the pci.c/pci_auto.c and PCI
device driver to confirm.
Issue 3 is not a problem either I think, since again Coreboot will allocate
it.
No, it does matter for coreboot. If the pci hose does not have the
system RAM region setup, the pci_mem_to_phys will fail when PCI device
driver wants to do DMA to RAM.
Good point, that is not supported at present.
So perhaps we should always set up suitable memory/I/O/bus-master
regions and for Coreboot they will just not be used.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
