[U-Boot] [PATCH] pci: Fix decode regions for memory banks

Since memory banks may not be located behind each other we need to add them separately.
Signed-off-by: Bernhard Messerklinger bernhard.messerklinger@br-automation.com ---
drivers/pci/pci-uclass.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 5a24eb6428..ad43e8a27c 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -815,7 +815,6 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, ofnode node) { int pci_addr_cells, addr_cells, size_cells; - phys_addr_t base = 0, size; int cells_per_record; const u32 *prop; int len; @@ -874,6 +873,21 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, }
/* Add a region for our local memory */ +#ifdef CONFIG_NR_DRAM_BANKS + bd_t *bd = gd->bd; + + for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) { + if (bd->bi_dram[i].size) { + pci_set_region(hose->regions + hose->region_count++, + bd->bi_dram[i].start, + bd->bi_dram[i].start, + bd->bi_dram[i].size, + PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); + } + } +#else + phys_addr_t base = 0, size; + size = gd->ram_size; #ifdef CONFIG_SYS_SDRAM_BASE base = CONFIG_SYS_SDRAM_BASE; @@ -882,6 +896,7 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, size = gd->pci_ram_top - base; pci_set_region(hose->regions + hose->region_count++, base, base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); +#endif
return 0; }

Bernhard Messerklinger bernhard.messerklinger@br-automation.com schrieb am 15.02.2018 08:59:53:
Von: Bernhard Messerklinger bernhard.messerklinger@br-automation.com An: u-boot@lists.denx.de Kopie: hannes.schmelzer@br-automation.com, Bernhard Messerklinger bernhard.messerklinger@br-automation.com, Simon Glass
Masahiro Yamada yamada.masahiro@socionext.com, Tuomas Tynkkynen tuomas.tynkkynen@iki.fi, Bin Meng bmeng.cn@gmail.com,
"xypron.glpk@gmx.de"
xypron.glpk@gmx.de, Hou Zhiqiang Zhiqiang.Hou@nxp.com Datum: 15.02.2018 09:00 Betreff: [PATCH] pci: Fix decode regions for memory banks
Since memory banks may not be located behind each other we need to add them separately.
Signed-off-by: Bernhard Messerklinger
bernhard.messerklinger@br-automation.com
drivers/pci/pci-uclass.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 5a24eb6428..ad43e8a27c 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -815,7 +815,6 @@ static int decode_regions(struct pci_controller
*hose,
ofnode parent_node, ofnode node) { int pci_addr_cells, addr_cells, size_cells;
- phys_addr_t base = 0, size; int cells_per_record; const u32 *prop; int len;
@@ -874,6 +873,21 @@ static int decode_regions(struct pci_controller
*hose,
ofnode parent_node, }
/* Add a region for our local memory */
+#ifdef CONFIG_NR_DRAM_BANKS
- bd_t *bd = gd->bd;
- for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
if (bd->bi_dram[i].size) {
pci_set_region(hose->regions + hose->region_count++,
bd->bi_dram[i].start,
bd->bi_dram[i].start,
bd->bi_dram[i].size,
PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
}
- }
+#else
- phys_addr_t base = 0, size;
- size = gd->ram_size;
#ifdef CONFIG_SYS_SDRAM_BASE base = CONFIG_SYS_SDRAM_BASE; @@ -882,6 +896,7 @@ static int decode_regions(struct pci_controller
*hose,
ofnode parent_node, size = gd->pci_ram_top - base; pci_set_region(hose->regions + hose->region_count++, base, base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); +#endif
return 0;
}
2.16.1
Reviewed-by: Hannes Schmelzer hannes.schmelzer@br-automation.com

On Thu, Feb 15, 2018 at 08:59:53AM +0100, Bernhard Messerklinger wrote:
Since memory banks may not be located behind each other we need to add them separately.
Signed-off-by: Bernhard Messerklinger bernhard.messerklinger@br-automation.com Reviewed-by: Hannes Schmelzer hannes.schmelzer@br-automation.com
Applied to u-boot/master, thanks!

Hi,
On Thu, Feb 15, 2018 at 3:59 PM, Bernhard Messerklinger bernhard.messerklinger@br-automation.com wrote:
Since memory banks may not be located behind each other we need to add them separately.
Signed-off-by: Bernhard Messerklinger bernhard.messerklinger@br-automation.com
drivers/pci/pci-uclass.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 5a24eb6428..ad43e8a27c 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -815,7 +815,6 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, ofnode node) { int pci_addr_cells, addr_cells, size_cells;
phys_addr_t base = 0, size; int cells_per_record; const u32 *prop; int len;
@@ -874,6 +873,21 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, }
/* Add a region for our local memory */
+#ifdef CONFIG_NR_DRAM_BANKS
bd_t *bd = gd->bd;
for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
if (bd->bi_dram[i].size) {
pci_set_region(hose->regions + hose->region_count++,
bd->bi_dram[i].start,
bd->bi_dram[i].start,
bd->bi_dram[i].size,
PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
}
}
+#else
Sorry for jumping out. With this commit, Intel Galileo board does not boot any more. x86 defines CONFIG_NR_DRAM_BANKS in x86-common.h, so this commit forces x86 to use the new logic instead of the old one, which breaks things. I have not debugged this on how to fix it. Any ideas?
phys_addr_t base = 0, size;
size = gd->ram_size;
#ifdef CONFIG_SYS_SDRAM_BASE base = CONFIG_SYS_SDRAM_BASE; @@ -882,6 +896,7 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, size = gd->pci_ram_top - base; pci_set_region(hose->regions + hose->region_count++, base, base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); +#endif
return 0;
}
Regards, Bin

Hi,
Thanks for the report. I am answering from my private email. At the moment I can't find any issue regarding my patch. It should make no difference since dram_init_banksize in quark/dram.c should set the dram bank. I will continue my investigation and contact you if I find out something.
Regards, Bernhard
On Thu, Mar 22, 2018 at 10:06 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi,
On Thu, Feb 15, 2018 at 3:59 PM, Bernhard Messerklinger bernhard.messerklinger@br-automation.com wrote:
Since memory banks may not be located behind each other we need to add them separately.
Signed-off-by: Bernhard Messerklinger bernhard.messerklinger@br-automation.com
drivers/pci/pci-uclass.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 5a24eb6428..ad43e8a27c 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -815,7 +815,6 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, ofnode node) { int pci_addr_cells, addr_cells, size_cells;
phys_addr_t base = 0, size; int cells_per_record; const u32 *prop; int len;
@@ -874,6 +873,21 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, }
/* Add a region for our local memory */
+#ifdef CONFIG_NR_DRAM_BANKS
bd_t *bd = gd->bd;
for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
if (bd->bi_dram[i].size) {
pci_set_region(hose->regions + hose->region_count++,
bd->bi_dram[i].start,
bd->bi_dram[i].start,
bd->bi_dram[i].size,
PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
}
}
+#else
Sorry for jumping out. With this commit, Intel Galileo board does not boot any more. x86 defines CONFIG_NR_DRAM_BANKS in x86-common.h, so this commit forces x86 to use the new logic instead of the old one, which breaks things. I have not debugged this on how to fix it. Any ideas?
phys_addr_t base = 0, size;
size = gd->ram_size;
#ifdef CONFIG_SYS_SDRAM_BASE base = CONFIG_SYS_SDRAM_BASE; @@ -882,6 +896,7 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, size = gd->pci_ram_top - base; pci_set_region(hose->regions + hose->region_count++, base, base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); +#endif
return 0;
}
Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

Hi Bernhard,
On Fri, Mar 23, 2018 at 12:27 AM, Bernhard Messerklinger messerklingerb@gmail.com wrote:
Hi,
Thanks for the report. I am answering from my private email. At the moment I can't find any issue regarding my patch. It should make no difference since dram_init_banksize in quark/dram.c should set the dram bank. I will continue my investigation and contact you if I find out something.
I've figured out where the bug is. Will send a patch soon.
Regards, Bin
participants (5)
-
Bernhard Messerklinger
-
Bernhard Messerklinger
-
Bin Meng
-
Hannes Schmelzer
-
Tom Rini