Patchwork [U-Boot,v3,2/5] arm bootm: Do not append zero ATAG_MEM

login
register
mail settings
Submitter Pali Rohár
Date Oct. 13, 2012, 7:31 p.m.
Message ID <1350156720-13387-3-git-send-email-pali.rohar@gmail.com>
Download mbox | patch
Permalink /patch/191308/
State Rejected
Delegated to: Tom Rini
Headers show

Comments

Pali Rohár - Oct. 13, 2012, 7:31 p.m.
If dram bank size is calculated at runtime, it can be zero on some boards.
This patch added code which ignore these zero bank size in ATAG_MEM.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 arch/arm/lib/bootm.c |    3 +++
 1 file changed, 3 insertions(+)
Marek Vasut - Oct. 13, 2012, 11:45 p.m.
Dear Pali Rohár,

> If dram bank size is calculated at runtime, it can be zero on some boards.
> This patch added code which ignore these zero bank size in ATAG_MEM.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  arch/arm/lib/bootm.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index c092bfa..925925d 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -132,6 +132,9 @@ static void setup_memory_tags(bd_t *bd)
>  	int i;
> 
>  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> +		if (bd->bi_dram[i].size == 0)
> +			continue;

This doesn't look right at all, your board is misconfigured if bd->bi_dram[i] == 
0. Did you misconfigure CONFIG_NR_DRAM_BANKS ?

>  		params->hdr.tag = ATAG_MEM;
>  		params->hdr.size = tag_size (tag_mem32);

Best regards,
Marek Vasut
Pali Rohár - Oct. 14, 2012, 12:08 a.m.
On Sunday 14 October 2012 01:45:06 Marek Vasut wrote:
> >  	
> >  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> > 
> > +		if (bd->bi_dram[i].size == 0)
> > +			continue;
> 
> This doesn't look right at all, your board is misconfigured if
> bd->bi_dram[i] == 0. Did you misconfigure CONFIG_NR_DRAM_BANKS
> ?

On some N900 devices there are two banks and on some only one. If 
there is only one it has size 256 MB and if there are two both 
has 128 MB. CONFIG_NR_DRAM_BANKS must be specified at compile 
time, but for N900 I need runtime detection.
Marek Vasut - Oct. 14, 2012, 12:17 a.m.
Dear Pali Rohár,

> On Sunday 14 October 2012 01:45:06 Marek Vasut wrote:
> > >  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> > > 
> > > +		if (bd->bi_dram[i].size == 0)
> > > +			continue;
> > 
> > This doesn't look right at all, your board is misconfigured if
> > bd->bi_dram[i] == 0. Did you misconfigure CONFIG_NR_DRAM_BANKS
> > ?
> 
> On some N900 devices there are two banks and on some only one. If
> there is only one it has size 256 MB and if there are two both
> has 128 MB. CONFIG_NR_DRAM_BANKS must be specified at compile
> time, but for N900 I need runtime detection.

How can that be? Are these two banks contiguous?

Best regards,
Marek Vasut
Pali Rohár - Oct. 14, 2012, 12:23 a.m.
On Sunday 14 October 2012 02:17:01 Marek Vasut wrote:
> Dear Pali Rohár,
> 
> > On Sunday 14 October 2012 01:45:06 Marek Vasut wrote:
> > > >  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> > > > 
> > > > +		if (bd->bi_dram[i].size == 0)
> > > > +			continue;
> > > 
> > > This doesn't look right at all, your board is misconfigured
> > > if
> > > bd->bi_dram[i] == 0. Did you misconfigure
> > > CONFIG_NR_DRAM_BANKS
> > > ?
> > 
> > On some N900 devices there are two banks and on some only
> > one. If there is only one it has size 256 MB and if there
> > are two both has 128 MB. CONFIG_NR_DRAM_BANKS must be
> > specified at compile time, but for N900 I need runtime
> > detection.
> 
> How can that be? Are these two banks contiguous?
> 

Yes.
Marek Vasut - Oct. 14, 2012, 12:27 a.m.
Dear Pali Rohár,

> On Sunday 14 October 2012 02:17:01 Marek Vasut wrote:
> > Dear Pali Rohár,
> > 
> > > On Sunday 14 October 2012 01:45:06 Marek Vasut wrote:
> > > > >  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> > > > > 
> > > > > +		if (bd->bi_dram[i].size == 0)
> > > > > +			continue;
> > > > 
> > > > This doesn't look right at all, your board is misconfigured
> > > > if
> > > > bd->bi_dram[i] == 0. Did you misconfigure
> > > > CONFIG_NR_DRAM_BANKS
> > > > ?
> > > 
> > > On some N900 devices there are two banks and on some only
> > > one. If there is only one it has size 256 MB and if there
> > > are two both has 128 MB. CONFIG_NR_DRAM_BANKS must be
> > > specified at compile time, but for N900 I need runtime
> > > detection.
> > 
> > How can that be? Are these two banks contiguous?
> 
> Yes.

And how does the memory map differ in case of device with one 256MB block then ?

Best regards,
Marek Vasut
Pali Rohár - Oct. 14, 2012, 12:35 a.m.
On Sunday 14 October 2012 02:27:05 Marek Vasut wrote:
> Dear Pali Rohár,
> 
> > On Sunday 14 October 2012 02:17:01 Marek Vasut wrote:
> > > Dear Pali Rohár,
> > > 
> > > > On Sunday 14 October 2012 01:45:06 Marek Vasut wrote:
> > > > > >  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> > > > > > 
> > > > > > +		if (bd->bi_dram[i].size == 0)
> > > > > > +			continue;
> > > > > 
> > > > > This doesn't look right at all, your board is
> > > > > misconfigured
> > > > > if
> > > > > bd->bi_dram[i] == 0. Did you misconfigure
> > > > > CONFIG_NR_DRAM_BANKS
> > > > > ?
> > > > 
> > > > On some N900 devices there are two banks and on some only
> > > > one. If there is only one it has size 256 MB and if there
> > > > are two both has 128 MB. CONFIG_NR_DRAM_BANKS must be
> > > > specified at compile time, but for N900 I need runtime
> > > > detection.
> > > 
> > > How can that be? Are these two banks contiguous?
> > 
> > Yes.
> 
> And how does the memory map differ in case of device with one
> 256MB block then ?
> 

Memory map is same, from 0x80000000 to 0x90000000.

For two bank devices uboot reports:
size=08000000 start=80000000
size=08000000 start=88000000

And for one:
size=10000000 start=80000000
size=00000000 start=90000000
Marek Vasut - Oct. 14, 2012, 1:08 a.m.
Dear Pali Rohár,

> On Sunday 14 October 2012 02:27:05 Marek Vasut wrote:
> > Dear Pali Rohár,
> > 
> > > On Sunday 14 October 2012 02:17:01 Marek Vasut wrote:
> > > > Dear Pali Rohár,
> > > > 
> > > > > On Sunday 14 October 2012 01:45:06 Marek Vasut wrote:
> > > > > > >  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> > > > > > > 
> > > > > > > +		if (bd->bi_dram[i].size == 0)
> > > > > > > +			continue;
> > > > > > 
> > > > > > This doesn't look right at all, your board is
> > > > > > misconfigured
> > > > > > if
> > > > > > bd->bi_dram[i] == 0. Did you misconfigure
> > > > > > CONFIG_NR_DRAM_BANKS
> > > > > > ?
> > > > > 
> > > > > On some N900 devices there are two banks and on some only
> > > > > one. If there is only one it has size 256 MB and if there
> > > > > are two both has 128 MB. CONFIG_NR_DRAM_BANKS must be
> > > > > specified at compile time, but for N900 I need runtime
> > > > > detection.
> > > > 
> > > > How can that be? Are these two banks contiguous?
> > > 
> > > Yes.
> > 
> > And how does the memory map differ in case of device with one
> > 256MB block then ?
> 
> Memory map is same, from 0x80000000 to 0x90000000.
> 
> For two bank devices uboot reports:
> size=08000000 start=80000000
> size=08000000 start=88000000
> 
> And for one:
> size=10000000 start=80000000
> size=00000000 start=90000000

Tom, can this not be handled as a single area? How does the omap memory layout 
look?

Best regards,
Marek Vasut
Tom Rini - Oct. 16, 2012, 3:56 p.m.
On Sun, Oct 14, 2012 at 03:08:20AM +0200, Marek Vasut wrote:
> Dear Pali Roh?r,
> 
> > On Sunday 14 October 2012 02:27:05 Marek Vasut wrote:
> > > Dear Pali Roh?r,
> > > 
> > > > On Sunday 14 October 2012 02:17:01 Marek Vasut wrote:
> > > > > Dear Pali Roh?r,
> > > > > 
> > > > > > On Sunday 14 October 2012 01:45:06 Marek Vasut wrote:
> > > > > > > >  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> > > > > > > > 
> > > > > > > > +		if (bd->bi_dram[i].size == 0)
> > > > > > > > +			continue;
> > > > > > > 
> > > > > > > This doesn't look right at all, your board is
> > > > > > > misconfigured
> > > > > > > if
> > > > > > > bd->bi_dram[i] == 0. Did you misconfigure
> > > > > > > CONFIG_NR_DRAM_BANKS
> > > > > > > ?
> > > > > > 
> > > > > > On some N900 devices there are two banks and on some only
> > > > > > one. If there is only one it has size 256 MB and if there
> > > > > > are two both has 128 MB. CONFIG_NR_DRAM_BANKS must be
> > > > > > specified at compile time, but for N900 I need runtime
> > > > > > detection.
> > > > > 
> > > > > How can that be? Are these two banks contiguous?
> > > > 
> > > > Yes.
> > > 
> > > And how does the memory map differ in case of device with one
> > > 256MB block then ?
> > 
> > Memory map is same, from 0x80000000 to 0x90000000.
> > 
> > For two bank devices uboot reports:
> > size=08000000 start=80000000
> > size=08000000 start=88000000
> > 
> > And for one:
> > size=10000000 start=80000000
> > size=00000000 start=90000000
> 
> Tom, can this not be handled as a single area? How does the omap
> memory layout look?

On other platforms we simply lie about the number of banks as at least
today nothing is re-using the number of banks and re-reconfiguring DDR.
Making n900 also just claim one bank and then reporting the correct
total memory size as the size of that one bank will put it into good
company at least.

Patch

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index c092bfa..925925d 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -132,6 +132,9 @@  static void setup_memory_tags(bd_t *bd)
 	int i;
 
 	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+		if (bd->bi_dram[i].size == 0)
+			continue;
+
 		params->hdr.tag = ATAG_MEM;
 		params->hdr.size = tag_size (tag_mem32);