| Message ID | 20180215075953.5015-1-bernhard.messerklinger@br-automation.com |
|---|---|
| State | Accepted |
| Commit | 664758c3dd1cf9c892ce98112e629cb032ac64aa |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] pci: Fix decode regions for memory banks | expand |
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 <sjg@chromium.org>, > 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
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; }
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(-)