RE: 8260/8xx Embedded Boot patch

RE: 8260/8xx Embedded Boot patch

[Curtis, Allen]

>I'll just check it in right from my tree if it is what you need.  The
>part inside the '#if 0' to hardcode the values still needs to
>be fixed if
>someone actually uses that.  In fact, I'll just fix it and push out the
>change since it will work on all boards depending upon what you want.

I looked at the patch and I have a couple comments:

1. The loop that overwrites the b_enetaddr[] should check that it does not
contain any information before assigning the default. This could have been
passed in by the bios.

2. If you assume that any platform that uses CONFIG_EMBEDDEDBOOT may want to
pass in a bd_info structure, then the platform specific patch in head.S is
wrong. Based on the current architecture I believe this is a safe
assumption. You may totally ignore this parameter in embed_config.c where
the platform dependent code is located.

diff -aru devel_old/arch/ppc/boot/simple/head.S
linuxppc_2_4_devel/arch/ppc/boot/simple/head.S
--- devel_old/arch/ppc/boot/simple/head.S	Wed Aug 28 00:59:37 2002
+++ linuxppc_2_4_devel/arch/ppc/boot/simple/head.S	Wed Aug 28 01:00:12
2002
@@ -59,7 +59,7 @@
 	isync
 #endif

-#if defined(CONFIG_MBX) || defined(CONFIG_RPX6)
+#if defined(CONFIG_EMBEDDEDBOOT)
 	mr	r29,r3	/* On the MBX860, r3 is the board info pointer.
...


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: 8260/8xx Embedded Boot patch

[Dan Malek]

Curtis, Allen wrote:


> 1. The loop that overwrites the b_enetaddr[] should check that it does not
> contain any information before assigning the default. This could have been
> passed in by the bios.

If you have a board/rom configuration where this isn't the right thing
to do, then create a new embed_config() function for it.  These are not
intended to be generic functions, and if you someday have experience with
lots of different board/rom combinations, you'll understand why this is
the case.

This code is intended to be a quick and easy update for a new board without
having to worry about how you may affect someone else.  There are LOTS of
boards using this code, and I don't like to be changing something that is
currently working without the ability to test everything else that may
be affected.  You definitely have to take the "if it works, don't touch it"
attitude here and add something new that suits your purpose.


> 2. If you assume that any platform that uses CONFIG_EMBEDDEDBOOT may want to
> pass in a bd_info structure,

We don't assume that at all.  In fact there are only three boards now where
we make that assumption and it is clear in the code which ones they are.
There is certainly no standard way of linking information between a boot rom
and this code, and tomorrow we may find something completely different.  If
you write code in embed_config() that expects something to be passed from a
boot rom (which in some cases is not a bd_t), then you must ensure it gets
passed from head.S properly.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: 8260/8xx Embedded Boot patch

[Curtis, Allen]

>currently working without the ability to test everything else that may
>be affected.  You definitely have to take the "if it works,

They are not using this code, it was not working.

>> 2. If you assume that any platform that uses
>CONFIG_EMBEDDEDBOOT may want to
>> pass in a bd_info structure,
>
>We don't assume that at all.  In fact there are only three
>boards now where
>we make that assumption and it is clear in the code which ones
>they are.

You seem to have missed the part about putting the platform specific stuff
in embed_config.c. When you are starting up and you use documentation such
as Documentation/powerpc/SBC8260_memory_mapping.txt, which specifies that
the BIOS passes in bd_info structures, you assume that is what you should do
and then find out that it only works for specific boards.

This brings me back to my original suggestion, create kernel features with
specific behaviors that platform specific header files can enable. Keep the
kernel source free of platform conditional compiles with the exception of
files contained in the platform directory. In this case CONFIG_EMBEDDED
indicates that simple boot should be used and the bios may provide a bd_info
structure. It does not specify the structure or that a structure is required
but the provision has been made for it.


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

8260/8xx Embedded Boot patch

[Allen Curtis]

[-- Attachment #1: Type: text/plain, Size: 114 bytes --]

Fix/cleanup to embedded boot
--
All things come to those who wait. Some of us just have to wait a little
longer...

[-- Attachment #2: embedded.patch --]
[-- Type: text/x-diff, Size: 4232 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux 2.4 for PowerPC development tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1114.1.1 -> 1.1114.1.2
#	  arch/ppc/config.in	1.150   -> 1.151
#	arch/ppc/boot/simple/head.S	1.3     -> 1.4
#	arch/ppc/boot/simple/embed_config.c	1.13    -> 1.14
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/08/26	acurtis@ws01.onz.com	1.1114.1.2
# CONFIG_EMBEDDEDBOOT and CONFIG_PPC_ALL cleanup
# --------------------------------------------
#
diff -Nru a/arch/ppc/boot/simple/embed_config.c b/arch/ppc/boot/simple/embed_config.c
--- a/arch/ppc/boot/simple/embed_config.c	Tue Aug 27 16:29:02 2002
+++ b/arch/ppc/boot/simple/embed_config.c	Tue Aug 27 16:29:02 2002
@@ -27,7 +27,7 @@

 /* For those boards that don't provide one.
 */
-#if !defined(CONFIG_MBX)
+#ifndef(CONFIG_EMBEDDEDBOOT)
 static	bd_t	bdinfo;
 #endif

@@ -444,6 +444,10 @@
 	bd_t	*bd;

 	bd = *bdp;
+
+#if !defined(CONFIG_EMBEDDEDBOOT)
+	bd = &bdinfo;
+	*bdp = bd;
 #if 0
 	/* This is actually provided by my boot rom.  I have it
 	 * here for those people that may load the kernel with
@@ -469,6 +473,17 @@
 	for (i=0; i<6; i++) {
 		bd->bi_enetaddr[i] = *cp++;
 	}
+#else
+	/* The boot rom passes these to us in MHz.  Linux now expects
+	 * them to be in Hz.
+	 */
+	bd->bi_intfreq *= 1000000;
+	bd->bi_busfreq *= 1000000;
+	bd->bi_cpmfreq *= 1000000;
+	bd->bi_brgfreq *= 1000000;
+
+	clk_8260(bd);
+#endif
 }
 #endif /* EST8260 */

diff -Nru a/arch/ppc/boot/simple/head.S b/arch/ppc/boot/simple/head.S
--- a/arch/ppc/boot/simple/head.S	Tue Aug 27 16:29:02 2002
+++ b/arch/ppc/boot/simple/head.S	Tue Aug 27 16:29:02 2002
@@ -59,7 +59,7 @@
 	isync
 #endif

-#if defined(CONFIG_MBX) || defined(CONFIG_RPX6)
+#ifdef CONFIG_EMBEDDEDBOOT
 	mr	r29,r3	/* On the MBX860, r3 is the board info pointer.
 			 * On the RPXSUPER, r3 points to the
 			 * NVRAM configuration keys.
@@ -103,7 +103,7 @@
 	mr	r3, r29
 #endif

-#if defined(CONFIG_MBX) || defined(CONFIG_RPX6)
+#ifdef CONFIG_EMBEDDEDBOOT
 	mr	r4,r29	/* put the board info pointer where the relocate
 			 * routine will find it
 			 */
diff -Nru a/arch/ppc/config.in b/arch/ppc/config.in
--- a/arch/ppc/config.in	Tue Aug 27 16:29:02 2002
+++ b/arch/ppc/config.in	Tue Aug 27 16:29:02 2002
@@ -62,12 +62,18 @@

 if [ "$CONFIG_8260" = "y" ]; then
   define_bool CONFIG_SERIAL_CONSOLE y
+  define_bool CONFIG_PPC_ALL    n
+
   choice 'Machine Type'	\
 	"EST8260	CONFIG_EST8260	\
 	 SBS8260	CONFIG_SBS8260	\
 	 RPXSUPER	CONFIG_RPX6	\
  	 TQM8260	CONFIG_TQM8260	\
 	 Willow		CONFIG_WILLOW"	Willow
+
+	if [ "$CONFIG_EST8260" = "y" -o "$CONFIG_RPX6" = "y" ]; then
+	  define_bool CONFIG_EMBEDDEDBOOT y
+	fi
 fi

 if [ "$CONFIG_40x" = "y" ]; then
@@ -131,6 +137,17 @@
 	 MBX		CONFIG_MBX		\
 	 WinCept	CONFIG_WINCEPT"		RPX-Lite

+  if [ "$CONFIG_MBX"     = "y" -o \
+       "$CONFIG_RPXLITE" = "y" -o \
+       "$CONFIG_IVMS8"   = "y" -o \
+       "$CONFIG_SM850"   = "y" -o \
+       "$CONFIG_TQM823L" = "y" -o \
+       "$CONFIG_TQM850L" = "y" -o \
+       "$CONFIG_TQM860L" = "y" -o \
+       "$CONFIG_BSEIP"   = "y" ]; then
+    define_bool CONFIG_EMBEDDEDBOOT y
+  fi
+
   if [ "$CONFIG_TQM823L" = "y" -o \
        "$CONFIG_TQM850L" = "y" -o \
        "$CONFIG_FPS850L" = "y" -o \
@@ -343,10 +360,6 @@
     fi
   fi
   dep_bool 'OCP Device proc fs support (experimental)' CONFIG_OCP_PROC $CONFIG_IBM_OCP
-fi
-
-if [ "$CONFIG_8xx" = "y" -o "$CONFIG_8260" = "y" ]; then
-    define_bool CONFIG_EMBEDDEDBOOT y
 fi
 endmenu

diff -Nru a/arch/ppc/boot/simple/embed_config.c b/arch/ppc/boot/simple/embed_config.c
--- a/arch/ppc/boot/simple/embed_config.c	Tue Aug 27 16:29:27 2002
+++ b/arch/ppc/boot/simple/embed_config.c	Tue Aug 27 16:29:27 2002
@@ -27,7 +27,7 @@

 /* For those boards that don't provide one.
 */
-#ifndef(CONFIG_EMBEDDEDBOOT)
+#ifndef CONFIG_EMBEDDEDBOOT
 static	bd_t	bdinfo;
 #endif

@@ -402,7 +402,7 @@
 clk_8260(bd_t *bd)
 {
 	uint	scmr, vco_out, clkin;
-	uint	plldf, pllmf, busdf, brgdf, cpmdf;
+	uint	plldf, pllmf, busdf, cpmdf;
 	volatile immap_t	*ip;

 	ip = (immap_t *)IMAP_ADDR;

Re: 8260/8xx Embedded Boot patch

[Tom Rini]

On Tue, Aug 27, 2002 at 05:06:51PM -0700, Allen Curtis wrote:

> Fix/cleanup to embedded boot

The first thing I saw was the define_bool CONFIG_ALL_PPC n.
If this was actually needed someplace, there's a bug there that needs to
be fixed.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: 8260/8xx Embedded Boot patch

[acurtis]

> The first thing I saw was the define_bool CONFIG_ALL_PPC n.
> If this was actually needed someplace, there's a bug there that needs to
> be fixed.

This is not required. All of the xxxx8260_defconfig files define this so you do
not get all of the generic Mac stuff like IDE and video. If it is not appropriate
take it out. It just serves to weed out a lot of options you are just going to
turn-off anyway.

------------------------------------------------------------
Allen Curtis              |  All good things come to those
Ones and Zeros, Inc.      |  who wait. Some of us have to
mailto:acurtis@onz.com    |  wait a little longer.

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: 8260/8xx Embedded Boot patch

[Tom Rini]

On Tue, Aug 27, 2002 at 05:28:29PM -0700, acurtis@onz.com wrote:
>
> > The first thing I saw was the define_bool CONFIG_ALL_PPC n.
> > If this was actually needed someplace, there's a bug there that needs to
> > be fixed.
>
> This is not required. All of the xxxx8260_defconfig files define this so you do
> not get all of the generic Mac stuff like IDE and video. If it is not appropriate
> take it out. It just serves to weed out a lot of options you are just going to
> turn-off anyway.

They shouldn't show up on 8260, since they don't show up on other,
non-CONFIG_ALL_PPC platforms.  Again, if they show up it's a bug.  The
bug is doing:
dep_{tristate,bool} 'Some pmac-specific driver' CONFIG_PMACSPEC $CONFIG_ALL_PPC
It should be:
if [ "$CONFIG_ALL_PPC" = "y" ]; then
   {bool,tristate} 'Some pmac-specific driver' CONFIG_PMACSPEC
fi

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: 8260/8xx Embedded Boot patch

[Dan Malek]

Allen Curtis wrote:

> Fix/cleanup to embedded boot

Please explain these changes.  You shouldn't be using embed_config.c if
you don't have EMBEDDEDBOOT defined.  The EMBEDDEDBOOT is a generic
bootloader that should only have unique functions in embed_config.c
You should be definining a board configuration for whatever you are
doing and use that.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: 8260/8xx Embedded Boot patch

[Allen Curtis]

> Please explain these changes.  You shouldn't be using embed_config.c if
> you don't have EMBEDDEDBOOT defined.  The EMBEDDEDBOOT is a generic
> bootloader that should only have unique functions in embed_config.c
> You should be definining a board configuration for whatever you are
> doing and use that.

I am using a modified EST8260 configuration. However the EST8260 embedded
stuff was broke. The bd_info structure was not passed up from head.S. The
call to configure the clocks was missing at the end of embed_config(). You
will also see that I translate the CONFIG_RPX (and other platform
conditionals) into CONFIG_EMBEDDED and use the single conditional compile.


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: 8260/8xx Embedded Boot patch

[Dan Malek]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

Allen Curtis wrote:

> I am using a modified EST8260 configuration. However the EST8260 embedded
> stuff was broke.

So, if you have an EST configuration that passes a bd pointer from the
boot rom in r3, the attached patch should be all that is necessary, correct?

I'll just check it in right from my tree if it is what you need.  The
part inside the '#if 0' to hardcode the values still needs to be fixed if
someone actually uses that.  In fact, I'll just fix it and push out the
change since it will work on all boards depending upon what you want.

Thanks.


	-- Dan

[-- Attachment #2: estembed.patch --]
[-- Type: text/plain, Size: 1229 bytes --]

diff -aru devel_old/arch/ppc/boot/simple/embed_config.c linuxppc_2_4_devel/arch/ppc/boot/simple/embed_config.c
--- devel_old/arch/ppc/boot/simple/embed_config.c	Wed Aug 28 00:59:37 2002
+++ linuxppc_2_4_devel/arch/ppc/boot/simple/embed_config.c	Wed Aug 28 01:01:04 2002
@@ -465,6 +465,8 @@
 	bd->bi_brgfreq *= 1000000;
 #endif

+	clk_8260(bd);
+
 	cp = (u_char *)def_enet_addr;
 	for (i=0; i<6; i++) {
 		bd->bi_enetaddr[i] = *cp++;
diff -aru devel_old/arch/ppc/boot/simple/head.S linuxppc_2_4_devel/arch/ppc/boot/simple/head.S
--- devel_old/arch/ppc/boot/simple/head.S	Wed Aug 28 00:59:37 2002
+++ linuxppc_2_4_devel/arch/ppc/boot/simple/head.S	Wed Aug 28 01:00:12 2002
@@ -59,7 +59,7 @@
 	isync
 #endif

-#if defined(CONFIG_MBX) || defined(CONFIG_RPX6)
+#if defined(CONFIG_MBX) || defined(CONFIG_RPX6) || defined(CONFIG_EST8260)
 	mr	r29,r3	/* On the MBX860, r3 is the board info pointer.
 			 * On the RPXSUPER, r3 points to the
 			 * NVRAM configuration keys.
@@ -103,7 +103,7 @@
 	mr	r3, r29
 #endif

-#if defined(CONFIG_MBX) || defined(CONFIG_RPX6)
+#if defined(CONFIG_MBX) || defined(CONFIG_RPX6) || defined(CONFIG_EST8260)
 	mr	r4,r29	/* put the board info pointer where the relocate
 			 * routine will find it
 			 */