| Message ID | 4DF0B05B.10708@synertronixx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
On 06/09/2011 01:36 PM, Torsten Koschorrek wrote: > This patch fixed compiler errors due to missing definitions of > CONFIG_SYS_SDRAM_BASE and CONFIG_SYS_INIT_SP_ADDR. > > The board doesn't start, though. A v2 of this patch or a seperate patch > will fix the error when it was found. > > Signed-off-by: Torsten Koschorrek <koschorrek@synertronixx.de> > --- > board/scb9328/config.mk | 2 +- > board/scb9328/scb9328.c | 10 +++++++++- > include/configs/scb9328.h | 3 +++ > 3 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/board/scb9328/config.mk b/board/scb9328/config.mk > index 7c5e067..6e411de 100644 > --- a/board/scb9328/config.mk > +++ b/board/scb9328/config.mk > @@ -7,4 +7,4 @@ > # the code in RAM device only. > # > -CONFIG_SYS_TEXT_BASE = 0x08f00000 > +CONFIG_SYS_TEXT_BASE = 0x10000000 Try to get rid of config.mk, too. It is obsolete, and you can move CONFIG_SYS_TEXT_BASE inside scb9328.h. > diff --git a/board/scb9328/scb9328.c b/board/scb9328/scb9328.c > index 428e8c9..631f4e3 100644 > --- a/board/scb9328/scb9328.c > +++ b/board/scb9328/scb9328.c > @@ -39,6 +39,15 @@ int board_init (void) > int dram_init (void) > { > + /* dram_init must store complete ramsize in gd->ram_size */ > + gd->ram_size = get_ram_size((volatile void *)SCB9328_SDRAM_1, > + SCB9328_SDRAM_1_SIZE); > + > + return 0; Not understood. If you have more as one bank, the size is the sum of all banks. In dram_init_banksize() you setup more as one bank if CONFIG_NR_DRAM_BANKS > 1. Then you should set in dram_init: size1 = get_ram_size((volatile void *)SCB9328_SDRAM_1, SCB9328_SDRAM_1_SIZE); #if ( CONFIG_NR_DRAM_BANKS > 1 ) size2 = get_ram_size((volatile void *)SCB9328_SDRAM_2, SCB9328_SDRAM_2_SIZE); ..... and then: gd->ram_size = size1 + size2 + size3 + size4; > /** > diff --git a/include/configs/scb9328.h b/include/configs/scb9328.h > index 3da214e..c610ed1 100644 > --- a/include/configs/scb9328.h > +++ b/include/configs/scb9328.h > @@ -127,6 +127,9 @@ > #define SCB9328_SDRAM_1 0x08000000 /* SDRAM bank #1 */ > #define SCB9328_SDRAM_1_SIZE 0x01000000 /* 16 MB */ > +#define CONFIG_SYS_SDRAM_BASE SCB9328_SDRAM_1 > +#define CONFIG_SYS_INIT_SP_ADDR SCB9328_SDRAM_1 + 0xf00000 In most boards/processors the stack pointer is set to the internal RAM, that is available before the SDRAM controller is activated. When not available, cache can be used. Is there no internal memory available for this processor ? It could be that the stack is used before the SDRAM is initialized, and then it hangs. Best regards, Stefano Babic
On 06/10/2011 07:44 AM, Torsten Koschorrek wrote: > Hello, Hi Torsten, send your answer to the ML, too. Someone else can help you ;-) >> size1 = get_ram_size((volatile void *)SCB9328_SDRAM_1, >> SCB9328_SDRAM_1_SIZE); >> #if ( CONFIG_NR_DRAM_BANKS> 1 ) >> size2 = get_ram_size((volatile void *)SCB9328_SDRAM_2, >> SCB9328_SDRAM_2_SIZE); >> ..... >> >> and then: >> gd->ram_size = size1 + size2 + size3 + size4; >> > > Yes, I thought about it. The thing is, we only have one bank on the > system. So, dram_init_banksize() needs a cleanup, but that's a problem > for another cleanup-patch, which will be committed in a next step. > > (If it's needed I could do a minor cleanup first...) You decide. However, the code in the patch is wrong. If you have only one bank, you could directly simplify your code, I think. > >>> /** >>> diff --git a/include/configs/scb9328.h b/include/configs/scb9328.h >>> index 3da214e..c610ed1 100644 >>> --- a/include/configs/scb9328.h >>> +++ b/include/configs/scb9328.h >>> @@ -127,6 +127,9 @@ >>> #define SCB9328_SDRAM_1 0x08000000 /* SDRAM bank #1 */ >>> #define SCB9328_SDRAM_1_SIZE 0x01000000 /* 16 MB */ >>> +#define CONFIG_SYS_SDRAM_BASE SCB9328_SDRAM_1 >>> +#define CONFIG_SYS_INIT_SP_ADDR SCB9328_SDRAM_1 + 0xf00000 >> >> In most boards/processors the stack pointer is set to the internal RAM, >> that is available before the SDRAM controller is activated. When not >> available, cache can be used. Is there no internal memory available for >> this processor ? It could be that the stack is used before the SDRAM is >> initialized, and then it hangs. >> > > I suppose, here is the cause of the hanging problem, yes. While I was > trying to solve it I saw some board configs which use sdram, too. Take into account that a lot of new processors set automatically the RAM controller before u-boot is started. I am thinking to the newer i.MX processors, but this is true for other architectures, too. And in some cases u-boot runs as second or third bootloader, such as for at91sam, davinci and omap, without touching the RAM controller that is already programmed by the first-stage bootloader. In your case the SDRAM controller is probably not yet programmed when the stack pointer is set. > Internal RAM came to my mind, too, but I didn't had the time to > investigate further. > > Unfortunately I have to work on another project today and next week and > I think I'm not able to solve the hanging problem. Understood, I tried only to give you some hints where to check ;-) > Minor fixes (such as > config.mk) for the above patch should be possible, though. Ok, agree. Fix first the problem to make MAKEALL happy and build the board again. Best regards, Stefano Babic
Hello, Stefano Babic wrote: > On 06/10/2011 07:44 AM, Torsten Koschorrek wrote: > Hi Torsten, > > send your answer to the ML, too. Someone else can help you ;-) > Oh, yes, right. This little 'Reply All' Button, sorry :-) >>> size1 = get_ram_size((volatile void *)SCB9328_SDRAM_1, >>> SCB9328_SDRAM_1_SIZE); >>> #if ( CONFIG_NR_DRAM_BANKS> 1 ) >>> size2 = get_ram_size((volatile void *)SCB9328_SDRAM_2, >>> SCB9328_SDRAM_2_SIZE); >>> ..... >>> >>> and then: >>> gd->ram_size = size1 + size2 + size3 + size4; >>> >> >> Yes, I thought about it. The thing is, we only have one bank on the >> system. So, dram_init_banksize() needs a cleanup, but that's a problem >> for another cleanup-patch, which will be committed in a next step. >> >> (If it's needed I could do a minor cleanup first...) > > You decide. However, the code in the patch is wrong. If you have only > one bank, you could directly simplify your code, I think. > Good point. >> >> Unfortunately I have to work on another project today and next week and >> I think I'm not able to solve the hanging problem. > > Understood, I tried only to give you some hints where to check ;-) > ... and I appreciate that very much :-) Above all, your answers showed me, that I was looking in the right direction so far. >> Minor fixes (such as >> config.mk) for the above patch should be possible, though. > > Ok, agree. Fix first the problem to make MAKEALL happy and build the > board again. > OK, I just tested it again, MAKEALL is happy. 'include/configs/scb9328.h' is the only file that has to be patched with the patch already send to the ml. 'board/scb9328/config.mk' and 'board/scb9328/scb9328.c' definately need a cleanup, but compilation is ok. The cleanup of those two files 'll be done next week. And hopefully I find some time next week to work on the hangup problem, too. > Best regards, > Stefano Babic > Thanks Torsten
Torsten Koschorrek wrote: > Hello, > > Stefano Babic wrote: >> On 06/10/2011 07:44 AM, Torsten Koschorrek wrote: >> Hi Torsten, >> >> send your answer to the ML, too. Someone else can help you ;-) >> > > Oh, yes, right. This little 'Reply All' Button, sorry :-) > >>>> size1 = get_ram_size((volatile void *)SCB9328_SDRAM_1, >>>> SCB9328_SDRAM_1_SIZE); >>>> #if ( CONFIG_NR_DRAM_BANKS> 1 ) >>>> size2 = get_ram_size((volatile void *)SCB9328_SDRAM_2, >>>> SCB9328_SDRAM_2_SIZE); >>>> ..... >>>> >>>> and then: >>>> gd->ram_size = size1 + size2 + size3 + size4; >>>> >>> >>> Yes, I thought about it. The thing is, we only have one bank on the >>> system. So, dram_init_banksize() needs a cleanup, but that's a problem >>> for another cleanup-patch, which will be committed in a next step. >>> >>> (If it's needed I could do a minor cleanup first...) >> >> You decide. However, the code in the patch is wrong. If you have only >> one bank, you could directly simplify your code, I think. >> > > Good point. > >>> >>> Unfortunately I have to work on another project today and next week and >>> I think I'm not able to solve the hanging problem. >> >> Understood, I tried only to give you some hints where to check ;-) >> > > ... and I appreciate that very much :-) Above all, your answers showed > me, that I was looking in the right direction so far. > >>> Minor fixes (such as >>> config.mk) for the above patch should be possible, though. >> >> Ok, agree. Fix first the problem to make MAKEALL happy and build the >> board again. >> > > OK, I just tested it again, MAKEALL is happy. > > 'include/configs/scb9328.h' is the only file that has to be patched with > the patch already send to the ml. 'board/scb9328/config.mk' and > 'board/scb9328/scb9328.c' definately need a cleanup, but compilation is > ok. The cleanup of those two files 'll be done next week. And hopefully > I find some time next week to work on the hangup problem, too. > What's the status of this patch? Do I need to resend a modified version without config.mk and scb9328.c? Or I could do a small cleanup as discussed in this thread. But as I said, MAKEALL is happy with the modified scb9328.h thanks Torsten
On 07/04/2011 11:34 AM, Torsten Koschorrek wrote: Hi Torsten, > What's the status of this patch? Do I need to resend a modified version > without config.mk and scb9328.c? Or I could do a small cleanup as > discussed in this thread. But as I said, MAKEALL is happy with the > modified scb9328.h The patch is set to "Changed requested", that means, you have to fix the issues we discuss in the thread. It will not be merged as it is. You can alwasy check the status of the patches you submit on patchworks.ozlabs.org. Best regards, Stefano Babic
diff --git a/board/scb9328/config.mk b/board/scb9328/config.mk index 7c5e067..6e411de 100644 --- a/board/scb9328/config.mk +++ b/board/scb9328/config.mk @@ -7,4 +7,4 @@ # the code in RAM device only. # -CONFIG_SYS_TEXT_BASE = 0x08f00000 +CONFIG_SYS_TEXT_BASE = 0x10000000 diff --git a/board/scb9328/scb9328.c b/board/scb9328/scb9328.c index 428e8c9..631f4e3 100644 --- a/board/scb9328/scb9328.c +++ b/board/scb9328/scb9328.c @@ -39,6 +39,15 @@ int board_init (void) int dram_init (void) { + /* dram_init must store complete ramsize in gd->ram_size */ + gd->ram_size = get_ram_size((volatile void *)SCB9328_SDRAM_1, + SCB9328_SDRAM_1_SIZE); + + return 0; +} + +void dram_init_banksize (void) +{ #if ( CONFIG_NR_DRAM_BANKS > 0 ) gd->bd->bi_dram[0].start = SCB9328_SDRAM_1; gd->bd->bi_dram[0].size = SCB9328_SDRAM_1_SIZE; @@ -55,7 +64,6 @@ int dram_init (void) gd->bd->bi_dram[3].start = SCB9328_SDRAM_4; gd->bd->bi_dram[3].size = SCB9328_SDRAM_4_SIZE; #endif - return 0; } /** diff --git a/include/configs/scb9328.h b/include/configs/scb9328.h index 3da214e..c610ed1 100644 --- a/include/configs/scb9328.h +++ b/include/configs/scb9328.h @@ -127,6 +127,9 @@ #define SCB9328_SDRAM_1 0x08000000 /* SDRAM bank #1 */ #define SCB9328_SDRAM_1_SIZE 0x01000000 /* 16 MB */ +#define CONFIG_SYS_SDRAM_BASE SCB9328_SDRAM_1 +#define CONFIG_SYS_INIT_SP_ADDR SCB9328_SDRAM_1 + 0xf00000 + /* * Configuration for FLASH memory for the Synertronixx board */ -- 1.7.2.5
This patch fixed compiler errors due to missing definitions of CONFIG_SYS_SDRAM_BASE and CONFIG_SYS_INIT_SP_ADDR. The board doesn't start, though. A v2 of this patch or a seperate patch will fix the error when it was found. Signed-off-by: Torsten Koschorrek <koschorrek@synertronixx.de> --- board/scb9328/config.mk | 2 +- board/scb9328/scb9328.c | 10 +++++++++- include/configs/scb9328.h | 3 +++ 3 files changed, 13 insertions(+), 2 deletions(-)