| Message ID | 1528960045-6651-1-git-send-email-jason.zhu@rock-chips.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | common: bootm: reserve memory bank | expand |
On Thu, Jun 14, 2018 at 03:07:25PM +0800, Jason Zhu wrote: > Actually the DRAM is not only seperated in one > bank. The DRAM bank information is stored in > gd->bd->bi_dram, so reserve lmb according to > gd->bd->bi_dram. > > Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com> > --- > common/bootm.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/common/bootm.c b/common/bootm.c > index e789f68..46689fa 100644 > --- a/common/bootm.c > +++ b/common/bootm.c > @@ -53,16 +53,23 @@ __weak void board_quiesce_devices(void) > #ifdef CONFIG_LMB > static void boot_start_lmb(bootm_headers_t *images) > { > + lmb_init(&images->lmb); > +#ifdef CONFIG_NR_DRAM_BANKS > + int i; > + > + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { > + lmb_add(&images->lmb, gd->bd->bi_dram[i].start, > + gd->bd->bi_dram[i].size); > + } > +#else > ulong mem_start; > phys_size_t mem_size; > > - lmb_init(&images->lmb); > - > mem_start = env_get_bootm_low(); > mem_size = env_get_bootm_size(); > > lmb_add(&images->lmb, (phys_addr_t)mem_start, mem_size); > - > +#endif > arch_lmb_reserve(&images->lmb); > board_lmb_reserve(&images->lmb); > } Can you describe a bit how you ran into this problem? Thanks! -- Tom
> Actually the DRAM is not only seperated in one > bank. The DRAM bank information is stored in > gd->bd->bi_dram, so reserve lmb according to > gd->bd->bi_dram. > > Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com> > --- > common/bootm.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/common/bootm.c b/common/bootm.c > index e789f68..46689fa 100644 > --- a/common/bootm.c > +++ b/common/bootm.c > @@ -53,16 +53,23 @@ __weak void board_quiesce_devices(void) > #ifdef CONFIG_LMB > static void boot_start_lmb(bootm_headers_t *images) > { > + lmb_init(&images->lmb); > +#ifdef CONFIG_NR_DRAM_BANKS > + int i; > + > + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { > + lmb_add(&images->lmb, gd->bd->bi_dram[i].start, > + gd->bd->bi_dram[i].size); > + } > +#else > ulong mem_start; > phys_size_t mem_size; > > - lmb_init(&images->lmb); > - > mem_start = env_get_bootm_low(); > mem_size = env_get_bootm_size(); > > lmb_add(&images->lmb, (phys_addr_t)mem_start, mem_size); > - > +#endif > arch_lmb_reserve(&images->lmb); > board_lmb_reserve(&images->lmb); > } Can you describe a bit how you ran into this problem? Thanks! > Run the function dram_init_banksize, then the dram is seperated in several > banks. Sometimes the bank0's size maybe zero. If we call boot_start_lmb > to reserve lmb which just only reserve bank0, the lmb's memory size is zero. > This can cause allocate memory error when we want to reserve lmb for fdt. > So reserve lmb depend on CONFIG_NR_DRAM_BANKS(function dram_init_banksize > seperates the dram by CONFIG_NR_DRAM_BANKS) but not just bank0. > Thanks! jason.zhu@rock-chips.com From: Tom Rini Date: 2018-06-19 03:02 To: Jason Zhu CC: Simon Glass; u-boot; Alexander Graf Subject: Re: [U-Boot] [PATCH] common: bootm: reserve memory bank On Thu, Jun 14, 2018 at 03:07:25PM +0800, Jason Zhu wrote: > Actually the DRAM is not only seperated in one > bank. The DRAM bank information is stored in > gd->bd->bi_dram, so reserve lmb according to > gd->bd->bi_dram. > > Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com> > --- > common/bootm.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/common/bootm.c b/common/bootm.c > index e789f68..46689fa 100644 > --- a/common/bootm.c > +++ b/common/bootm.c > @@ -53,16 +53,23 @@ __weak void board_quiesce_devices(void) > #ifdef CONFIG_LMB > static void boot_start_lmb(bootm_headers_t *images) > { > + lmb_init(&images->lmb); > +#ifdef CONFIG_NR_DRAM_BANKS > + int i; > + > + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { > + lmb_add(&images->lmb, gd->bd->bi_dram[i].start, > + gd->bd->bi_dram[i].size); > + } > +#else > ulong mem_start; > phys_size_t mem_size; > > - lmb_init(&images->lmb); > - > mem_start = env_get_bootm_low(); > mem_size = env_get_bootm_size(); > > lmb_add(&images->lmb, (phys_addr_t)mem_start, mem_size); > - > +#endif > arch_lmb_reserve(&images->lmb); > board_lmb_reserve(&images->lmb); > } Can you describe a bit how you ran into this problem? Thanks! -- Tom
Hi Jason, On 06/14/2018 03:07 PM, Jason Zhu wrote: > Actually the DRAM is not only seperated in one > bank. The DRAM bank information is stored in > gd->bd->bi_dram, so reserve lmb according to > gd->bd->bi_dram. This commit message is not clear enough for what you have done in this patch, you should mention the LMB is not correctly mapped when DRAM has more than 1 bank. > > Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com> > --- > common/bootm.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/common/bootm.c b/common/bootm.c > index e789f68..46689fa 100644 > --- a/common/bootm.c > +++ b/common/bootm.c > @@ -53,16 +53,23 @@ __weak void board_quiesce_devices(void) > #ifdef CONFIG_LMB > static void boot_start_lmb(bootm_headers_t *images) > { > + lmb_init(&images->lmb); > +#ifdef CONFIG_NR_DRAM_BANKS > + int i; > + > + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { > + lmb_add(&images->lmb, gd->bd->bi_dram[i].start, > + gd->bd->bi_dram[i].size); > + } > +#else > ulong mem_start; > phys_size_t mem_size; For the config without CONFIG_NR_DRAM_BANKS enable, there will be a warning. Thanks, - Kever > > - lmb_init(&images->lmb); > - > mem_start = env_get_bootm_low(); > mem_size = env_get_bootm_size(); > > lmb_add(&images->lmb, (phys_addr_t)mem_start, mem_size); > - > +#endif > arch_lmb_reserve(&images->lmb); > board_lmb_reserve(&images->lmb); > }
diff --git a/common/bootm.c b/common/bootm.c index e789f68..46689fa 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -53,16 +53,23 @@ __weak void board_quiesce_devices(void) #ifdef CONFIG_LMB static void boot_start_lmb(bootm_headers_t *images) { + lmb_init(&images->lmb); +#ifdef CONFIG_NR_DRAM_BANKS + int i; + + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { + lmb_add(&images->lmb, gd->bd->bi_dram[i].start, + gd->bd->bi_dram[i].size); + } +#else ulong mem_start; phys_size_t mem_size; - lmb_init(&images->lmb); - mem_start = env_get_bootm_low(); mem_size = env_get_bootm_size(); lmb_add(&images->lmb, (phys_addr_t)mem_start, mem_size); - +#endif arch_lmb_reserve(&images->lmb); board_lmb_reserve(&images->lmb); }
Actually the DRAM is not only seperated in one bank. The DRAM bank information is stored in gd->bd->bi_dram, so reserve lmb according to gd->bd->bi_dram. Signed-off-by: Jason Zhu <jason.zhu@rock-chips.com> --- common/bootm.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)