[U-Boot] [PATCH 1/3] ARM: don't panic if no flash is there

[U-Boot] [PATCH 1/3] ARM: don't panic if no flash is there

[Alessandro Rubini]

From: Alessandro Rubini <rubini@gnudd.com>

Commit f1d2b313c9eb6808d30c16a9eb5251240452a56c, which
introduced relocation, also added some ifdef to panic
for no flash is found but some is expected.  This is policy,
in my opinion, and should not be part of the boot loader
like it wasn't there earlier.

This is mandatory to have the versatile port work on qemu,
where flash has never been correctly identified without any
side effect on the usefulness of the emulation.

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
---
 arch/arm/lib/board.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index dc46e21..bc8b589 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -431,10 +431,6 @@ void board_init_f (ulong bootflag)
 	/* NOTREACHED - relocate_code() does not return */
 }
 
-#if !defined(CONFIG_SYS_NO_FLASH)
-static char *failed = "*** failed ***\n";
-#endif
-
 /************************************************************************
  *
  * This is the next part if the initialization sequence: we are now
@@ -501,9 +497,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 # else	/* !CONFIG_SYS_FLASH_CHECKSUM */
 		print_size (flash_size, "\n");
 # endif /* CONFIG_SYS_FLASH_CHECKSUM */
-	} else {
-		puts (failed);
-		hang ();
 	}
 #endif
 
-- 
1.5.6.5

[U-Boot] [PATCH 2/3] versatile: use boards.confg

[Alessandro Rubini]

From: Alessandro Rubini <rubini@gnudd.com>

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
---
 Makefile                                   |    9 ------
 board/armltd/versatile/split_by_variant.sh |   42 ----------------------------
 boards.cfg                                 |    5 ++-
 3 files changed, 4 insertions(+), 52 deletions(-)
 delete mode 100755 board/armltd/versatile/split_by_variant.sh

diff --git a/Makefile b/Makefile
index 93c33c7..bbd2e9c 100644
--- a/Makefile
+++ b/Makefile
@@ -998,15 +998,6 @@ edb9315a_config: unconfig
 	@$(MKCONFIG) -n $@ -t $(@:_config=) edb93xx arm arm920t edb93xx - ep93xx
 
 #########################################################################
-# ARM supplied Versatile development boards
-#########################################################################
-
-versatile_config	\
-versatileab_config	\
-versatilepb_config :	unconfig
-	@board/armltd/versatile/split_by_variant.sh $@
-
-#########################################################################
 ## XScale Systems
 #########################################################################
 
diff --git a/board/armltd/versatile/split_by_variant.sh b/board/armltd/versatile/split_by_variant.sh
deleted file mode 100755
index 475e5a3..0000000
--- a/board/armltd/versatile/split_by_variant.sh
+++ /dev/null
@@ -1,42 +0,0 @@
-#!/bin/sh
-# ---------------------------------------------------------
-#  Set the core module defines according to Core Module
-# ---------------------------------------------------------
-# ---------------------------------------------------------
-# Set up the Versatile type define
-# ---------------------------------------------------------
-
-mkdir -p ${obj}include
-variant=PB926EJ-S
-if [ "$1" = "" ]
-then
-	echo "$0:: No parameters - using versatilepb_config"
-	echo "#define CONFIG_ARCH_VERSATILE_PB" > ${obj}include/config.h
-	variant=PB926EJ-S
-else
-	case "$1" in
-	versatilepb_config	|	\
-	versatile_config)
-	echo "#define CONFIG_ARCH_VERSATILE_PB" > ${obj}include/config.h
-	;;
-
-	versatileab_config)
-	echo "#define CONFIG_ARCH_VERSATILE_AB" > ${obj}include/config.h
-	variant=AB926EJ-S
-	;;
-
-
-	*)
-	echo "$0:: Unrecognised config - using versatilepb_config"
-	echo "#define CONFIG_ARCH_VERSATILE_PB" > ${obj}include/config.h
-	variant=PB926EJ-S
-	;;
-
-	esac
-
-fi
-# ---------------------------------------------------------
-# Complete the configuration
-# ---------------------------------------------------------
-$MKCONFIG -a versatile arm arm926ejs versatile armltd versatile
-echo "Variant:: $variant"
diff --git a/boards.cfg b/boards.cfg
index 80488b8..39a023e 100644
--- a/boards.cfg
+++ b/boards.cfg
@@ -115,7 +115,10 @@ nhk8815                      arm         arm926ejs   nhk8815             st
 nhk8815_onenand              arm         arm926ejs   nhk8815             st             nomadik       nhk8815:BOOT_ONENAND
 omap5912osk                  arm         arm926ejs   -                   ti             omap
 edminiv2                     arm         arm926ejs   -                   LaCie          orion5x
-dkb			     arm         arm926ejs   -                   Marvell        pantheon
+dkb                          arm         arm926ejs   -                   Marvell        pantheon
+versatile                    arm         arm926ejs   -                   armltd         versatile
+versatileab                  arm         arm926ejs   versatile           armltd         versatile     versatile:ARCH_VERSATILE_AB
+versatilepb                  arm         arm926ejs   versatile           armltd         versatile     versatile:
 ca9x4_ct_vxp                 arm         armv7       vexpress            armltd
 efikamx                      arm         armv7       efikamx             -              mx5
 mx51evk                      arm         armv7       mx51evk             freescale      mx5
-- 
1.5.6.5

[U-Boot] [PATCH 3/3] versatile: support relocation

[Alessandro Rubini]

From: Alessandro Rubini <rubini@gnudd.com>

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
---
 board/armltd/versatile/versatile.c |    2 ++
 include/configs/versatile.h        |    3 +++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/board/armltd/versatile/versatile.c b/board/armltd/versatile/versatile.c
index 6e836dd..deca47f 100644
--- a/board/armltd/versatile/versatile.c
+++ b/board/armltd/versatile/versatile.c
@@ -88,6 +88,8 @@ int misc_init_r (void)
 ******************************/
 int dram_init (void)
 {
+	gd->ram_size = get_ram_size(CONFIG_SYS_SDRAM_BASE,
+				    CONFIG_SYS_SDRAM_SIZE);
 	return 0;
 }
 
diff --git a/include/configs/versatile.h b/include/configs/versatile.h
index 45d8434..d883725 100644
--- a/include/configs/versatile.h
+++ b/include/configs/versatile.h
@@ -168,6 +168,9 @@
 #define PHYS_SDRAM_1		0x00000000	/* SDRAM Bank #1 */
 #define PHYS_SDRAM_1_SIZE	0x08000000	/* 128 MB */
 #define PHYS_FLASH_SIZE		0x04000000	/* 64MB */
+#define CONFIG_SYS_SDRAM_BASE	0x00000000
+#define CONFIG_SYS_SDRAM_SIZE	PHYS_SDRAM_1_SIZE
+#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + 0x1000)
 
 /*-----------------------------------------------------------------------
  * FLASH and environment organization
-- 
1.5.6.5

[U-Boot] [PATCH 1/3] ARM: don't panic if no flash is there

[Wolfgang Denk]

Dear Alessandro Rubini,

In message <7edfdbf8df20140ad93f0a967fc526f8cef5360d.1303028702.git.rubini@gnudd.com> you wrote:
> From: Alessandro Rubini <rubini@gnudd.com>
> 
> Commit f1d2b313c9eb6808d30c16a9eb5251240452a56c, which
> introduced relocation, also added some ifdef to panic
> for no flash is found but some is expected.  This is policy,
> in my opinion, and should not be part of the boot loader
> like it wasn't there earlier.
> 
> This is mandatory to have the versatile port work on qemu,
> where flash has never been correctly identified without any
> side effect on the usefulness of the emulation.
> 
> Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
> ---
>  arch/arm/lib/board.c |    7 -------
>  1 files changed, 0 insertions(+), 7 deletions(-)

Sorry, but I reject this patch.  This is common code that has always
been there (at least in the reference implementation on Power; it was
a bug that got fixed that this was missing before in ARM).

If you do not have NOR flash, then you should not enable it (and
define CONFIG_SYS_NO_FLASH). Otherwise, you are supposed to have
flash memory.

If there are problems with the Qemu emulation of NOR flash these
should be fixed there.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You see but you do not observe.
Sir Arthur Conan Doyle, in "The Memoirs of Sherlock Holmes"

[U-Boot] [PATCH 1/3] ARM: don't panic if no flash is there

[Alessandro Rubini]

> Sorry, but I reject this patch.

Please let me restate the point once more. Maybe i've not been clear.

> If you do not have NOR flash, then you should not enable it (and
> define CONFIG_SYS_NO_FLASH). Otherwise, you are supposed to have
> flash memory.

I expect a serious boot loader to work in all possible situations and
to be a development tool, the thing that was called a monitor ages ago
(why do you have "mw" and "md" otherwise?). I've always loved u-boot
usefulness as opposed to the crappy "do nothing at all" wince or
freecale boot loaders.

Thus, I see three reasons to revert the older behaviour.

- even if flash got broken over time, the machine should be useable.
hang() here gains nothing, as I may still be able to boot though the net
and see what happens, without the need to recompile and reflesh a special
recovery boot loader

- during initial development it's common to have some parameter
misconfigured (and thus see no flash) while still using u-boot to
md/mw/whatever and see how's flash really mapepd.  It makes no sense
to be being forced to flash twice at each development loop (one trying
new code and hang() and another to get an interactive session).

- there may be board build both with and without NOR flash. On one
side you added relocation to support boards with different memory
sizes with a single u-boot binary, on the other you are now forbidding
a single binary to work on two similar boards with a different set of
peripherals.

This is policy to me, and I see no reason to force policy in a boot
loader.  I expect only simple and uniform behaviour: do we hang() if
no network is found?

> If there are problems with the Qemu emulation of NOR flash these
> should be fixed there.

Sure, if you want flash. I'm not asking non-compliant code the be added
in u-boot to deal with a flaky implementation elsewhere. I'd love a
perfectly working, though incomplete, emulation to be supported just
like real hardware prototypes with production bugs in the NOR mounting.

Amicalment (even though this message sounds harsh it isn't meant to be)
/alessandro

[U-Boot] [PATCH 1/3] ARM: don't panic if no flash is there

[Wolfgang Denk]

Dear Alessandro Rubini,

In message <20110417173205.GA28105@mail.gnudd.com> you wrote:
> 
> > If you do not have NOR flash, then you should not enable it (and
> > define CONFIG_SYS_NO_FLASH). Otherwise, you are supposed to have
> > flash memory.
> 
> I expect a serious boot loader to work in all possible situations and
> to be a development tool, the thing that was called a monitor ages ago
> (why do you have "mw" and "md" otherwise?). I've always loved u-boot
> usefulness as opposed to the crappy "do nothing at all" wince or
> freecale boot loaders.

You know that all this is just polemics, as in your special case none
of your arguments actually applies.

> Thus, I see three reasons to revert the older behaviour.

We want to unify the code across architectures.  The behavior we have
now is what we have in U-Boot (and PPCBoot) since day one, i. e. for
well over a decade.  The problem was just that some architectures
incorrectly implemented this, and such bugs got finally fixed.

> - even if flash got broken over time, the machine should be useable.
> hang() here gains nothing, as I may still be able to boot though the net
> and see what happens, without the need to recompile and reflesh a special
> recovery boot loader

If you have a system with NOR flash, you usually boot from NOR.  If
NOR is so broken that it fails the detection, you cannot make any
assumptions about which code might be running, and which not.  You
have fully undefined behaviour then.  In such a situation it is best
to shut the system down and halt in a safe mode, instead of running
any random code.  Many of these devices actually control hardware,
where uncontrolled misfunction might be dangerous.

I'm sorry, but my decision is made.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The number you have dialed is imaginary. Please divide by 0  and  try
again.

[U-Boot] [PATCH 1/3] ARM: don't panic if no flash is there

[Loïc Minier]

 Would it be possible to introduce a QEMU Versatile board in U-Boot
 derived from the regular Versatile board's config?  It would disable
 the things that aren't (yet) supported in QEMU or too painful to
 support.

-- 
Lo?c Minier

[U-Boot] [PATCH 1/3] ARM: don't panic if no flash is there

[Albert ARIBAUD]

Le 18/04/2011 11:40, Lo?c Minier a ?crit :
>   Would it be possible to introduce a QEMU Versatile board in U-Boot
>   derived from the regular Versatile board's config?  It would disable
>   the things that aren't (yet) supported in QEMU or too painful to
>   support.

Either that or introduce a config option for versatile that will allow 
the same code to support both the real and the QEMUed version -- 
although I tend to side with Wolfgang in saying that the interest of 
QEMU is its ability to emulate systems, so if the real versatile has 
NOR, it should have it too under QEMU.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] ARM: don't panic if no flash is there

[Wolfgang Denk]

Dear =?iso-8859-1?Q?Lo=EFc?= Minier,

In message <20110418094025.GC11828@bee.dooz.org> you wrote:
>  Would it be possible to introduce a QEMU Versatile board in U-Boot
>  derived from the regular Versatile board's config?  It would disable
>  the things that aren't (yet) supported in QEMU or too painful to
>  support.

Sure.  Please feel free to submit a patch.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It became apparent that one reason why the Ice Giants were  known  as
the  Ice  Giants  was  because they were, well, giants. The other was
that they were made of ice.              -Terry Pratchett, _Sourcery_