| Message ID | 9f7307fc105747976bae44df5dae51357f54d578.1265920498.git.rubini@unipv.it |
|---|---|
| State | Not Applicable |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
Hi Alessandro, El Thu, Feb 11, 2010 at 09:46:50PM +0100 Alessandro Rubini ha dit: > The configuration of SDRAM needs two more writel() operations, > otherwise some boards won't be able to boot. These additional writes > are present in vendor assembly code but were forgotten during the > rewriting in C. > > Signed-off-by: Alessandro Rubini <rubini@gnudd.com> > Acked-by: Matthias Kaehlcke <matthias@kaehlcke.net> i gave my ack after a visual review of the patch, without having tested it. i just installed a patched u-boot on one of my boards and it doesn't boot :( at a first glance the offender seems to be the write to the GlConfig register in force_precharge(), after commenting this line the board comes up. comparing with the initial SDRAM initialization written in assembly i noticed that you set the INIT bit in force_precharge(), while the assembly code doesn't. according to the user manual the INIT must be set to issue PRECHARGE ALL, but if i don't get it wrong this isn't our case, as we assume that PRECHARGE ALL doesn't work (see errata), thus we precharge manually by reading from the RAM. but even after clearing the INIT bit my board refuses to boot. i hope i'll find time this weekend to track this down. if you want to have a look at the initial sequence in assembly, it can be found here: http://lists.denx.de/pipermail/u-boot/2009-December/065133.html cu
> i gave my ack after a visual review of the patch, without having > tested it. i just installed a patched u-boot on one of my boards and > it doesn't boot :( Oh. The opposite of my board. Then, since I don't have a 9315A but only a similar one, it's better to drop the patch. I'll have a different sdram_cfg file for mine, then. Thanks for testing, Matthias /alessandro
El Fri, Feb 12, 2010 at 08:01:26AM +0100 Alessandro Rubini ha dit: > > i gave my ack after a visual review of the patch, without having > > tested it. i just installed a patched u-boot on one of my boards and > > it doesn't boot :( > > Oh. The opposite of my board. > > Then, since I don't have a 9315A but only a similar one, it's better > to drop the patch. I'll have a different sdram_cfg file for mine, > then. i'm pretty sure we'll be able to track this down, the purpose of your patch is correct, i actually wonder why the current code works ... if we can't solve the issue in the next days, you could still send a patch which only writes the GlConfig register in program_mode_registers(). this would fix at least on of the issues and make your code less different from the one in the tree
diff --git a/board/edb93xx/sdram_cfg.c b/board/edb93xx/sdram_cfg.c index 6155f0e..418959a 100644 --- a/board/edb93xx/sdram_cfg.c +++ b/board/edb93xx/sdram_cfg.c @@ -59,13 +59,15 @@ void sdram_cfg(void) static void force_precharge(void) { + struct sdram_regs *sdram = (struct sdram_regs *)SDRAM_BASE; + + writel(GLCONFIG_INIT | GLCONFIG_CKE, &sdram->glconfig); /* * Errata most EP93xx revisions say that PRECHARGE ALL isn't always * issued. * * Do a read from each bank to make sure they're precharged */ - PRECHARGE_BANK(0); PRECHARGE_BANK(1); PRECHARGE_BANK(2); @@ -101,6 +103,9 @@ static void setup_refresh_timer(void) static void program_mode_registers(void) { + struct sdram_regs *sdram = (struct sdram_regs *)SDRAM_BASE; + + writel(GLCONFIG_MRS | GLCONFIG_CKE, &sdram->glconfig); /* * The mode registers are programmed by performing a read from each * SDRAM bank. The value of the address that is read defines the value