Patchwork [U-Boot,2/2] edb93xx sdram: fix initialization

login
register
mail settings
Submitter Alessandro Rubini
Date Feb. 11, 2010, 8:46 p.m.
Message ID <9f7307fc105747976bae44df5dae51357f54d578.1265920498.git.rubini@unipv.it>
Download mbox | patch
Permalink /patch/71725/
State Not Applicable
Delegated to: Albert ARIBAUD
Headers show

Comments

Alessandro Rubini - Feb. 11, 2010, 8:46 p.m.
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>
---
 board/edb93xx/sdram_cfg.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
Matthias Kaehlcke - Feb. 11, 2010, 10:32 p.m.
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
Alessandro Rubini - Feb. 12, 2010, 7:01 a.m.
> 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
Matthias Kaehlcke - Feb. 12, 2010, 9:23 a.m.
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

Patch

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