common: bootm: reserve memory bank

Message ID 1528960045-6651-1-git-send-email-jason.zhu@rock-chips.com
State Superseded
Headers show
Series
  • common: bootm: reserve memory bank
Related show

Commit Message

jason.zhu@rock-chips.com June 14, 2018, 7:07 a.m.
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(-)

Comments

Tom Rini June 18, 2018, 7:02 p.m. | #1
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
jason.zhu@rock-chips.com June 19, 2018, 1:20 a.m. | #2
> 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
Kever Yang June 25, 2018, 2:31 a.m. | #3
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);
>  }

Patch

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);
 }