[U-Boot-Users] [MIPS] Introduce machine_restart

[U-Boot-Users] [MIPS] Introduce machine_restart

[Shinya Kuribayashi]

Handles machine specific functions by using function pointers.

Signed-off-by: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
---

 board/incaip/incaip.c     |    8 ++++++++
 board/purple/purple.c     |   10 ++++++++++
 board/tb0229/tb0229.c     |   11 ++++++++++-
 cpu/mips/cpu.c            |   17 ++++++++++-------
 include/asm-mips/reboot.h |   15 +++++++++++++++
 5 files changed, 53 insertions(+), 8 deletions(-)
 create mode 100644 include/asm-mips/reboot.h


diff --git a/board/incaip/incaip.c b/board/incaip/incaip.c
index dbf0ecc..c2324bc 100644
--- a/board/incaip/incaip.c
+++ b/board/incaip/incaip.c
@@ -26,9 +26,15 @@
 #include <asm/addrspace.h>
 #include <asm/inca-ip.h>
 #include <asm/io.h>
+#include <asm/reboot.h>
 
 extern uint incaip_get_cpuclk(void);
 
+static void incaip_machine_restart(char *command)
+{
+	*INCA_IP_WDT_RST_REQ = 0x3f;
+}
+
 static ulong max_sdram_size(void)
 {
 	/* The only supported SDRAM data width is 16bit.
@@ -108,5 +114,7 @@ int checkboard (void)
 
 	set_io_port_base(0);
 
+	_machine_restart = incaip_machine_restart;
+
 	return 0;
 }
diff --git a/board/purple/purple.c b/board/purple/purple.c
index 74718af..2fa2e19 100644
--- a/board/purple/purple.c
+++ b/board/purple/purple.c
@@ -29,6 +29,7 @@
 #include <asm/io.h>
 #include <asm/addrspace.h>
 #include <asm/cacheops.h>
+#include <asm/reboot.h>
 
 #include "sconsole.h"
 
@@ -52,6 +53,13 @@ extern int	asc_serial_getc 	(void);
 extern int	asc_serial_tstc 	(void);
 extern void	asc_serial_setbrg 	(void);
 
+static void purple_machine_restart(char *command)
+{
+	void (*f)(void) = (void *) 0xbfc00000;
+
+	f();
+}
+
 static void sdram_timing_init (ulong size)
 {
 	register uint pass;
@@ -148,6 +156,8 @@ int checkboard (void)
 
 	set_io_port_base(0);
 
+	_machine_restart = purple_machine_restart;
+
 	return 0;
 }
 
diff --git a/board/tb0229/tb0229.c b/board/tb0229/tb0229.c
index 61c2e9b..c775069 100644
--- a/board/tb0229/tb0229.c
+++ b/board/tb0229/tb0229.c
@@ -12,9 +12,16 @@
 #include <common.h>
 #include <command.h>
 #include <asm/addrspace.h>
-#include <asm/inca-ip.h>
 #include <asm/io.h>
 #include <pci.h>
+#include <asm/reboot.h>
+
+static void tb0229_machine_restart(char *command)
+{
+	void (*f)(void) = (void *) 0xbfc00000;
+
+	f();
+}
 
 #if defined(CONFIG_PCI)
 static struct pci_controller hose;
@@ -37,5 +44,7 @@ int checkboard (void)
 
 	set_io_port_base(0);
 
+	_machine_restart = tb0229_machine_restart;
+
 	return 0;
 }
diff --git a/cpu/mips/cpu.c b/cpu/mips/cpu.c
index 71e5028..4c50829 100644
--- a/cpu/mips/cpu.c
+++ b/cpu/mips/cpu.c
@@ -23,9 +23,9 @@
 
 #include <common.h>
 #include <command.h>
-#include <asm/inca-ip.h>
 #include <asm/mipsregs.h>
 #include <asm/cacheops.h>
+#include <asm/reboot.h>
 
 #define cache_op(op,addr)						\
 	__asm__ __volatile__(						\
@@ -37,15 +37,18 @@
 	:								\
 	: "i" (op), "R" (*(unsigned char *)(addr)))
 
+/*
+ * Urgs ...  Too many MIPS machines to handle this in a generic way.
+ * So handle all using function pointers to machine specific
+ * functions.
+ */
+void (*_machine_restart)(char *command);
+
 int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
-#if defined(CONFIG_INCA_IP)
-	*INCA_IP_WDT_RST_REQ = 0x3f;
-#elif defined(CONFIG_PURPLE) || defined(CONFIG_TB0229)
-	void (*f)(void) = (void *) 0xbfc00000;
+	if (_machine_restart)
+		_machine_restart(NULL);
 
-	f();
-#endif
 	fprintf(stderr, "*** reset failed ***\n");
 	return 0;
 }
diff --git a/include/asm-mips/reboot.h b/include/asm-mips/reboot.h
new file mode 100644
index 0000000..e48c0bf
--- /dev/null
+++ b/include/asm-mips/reboot.h
@@ -0,0 +1,15 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 1997, 1999, 2001, 06 by Ralf Baechle
+ * Copyright (C) 2001 MIPS Technologies, Inc.
+ */
+#ifndef _ASM_REBOOT_H
+#define _ASM_REBOOT_H
+
+extern void (*_machine_restart)(char *command);
+extern void (*_machine_halt)(void);
+
+#endif /* _ASM_REBOOT_H */

[U-Boot-Users] [MIPS] Introduce machine_restart

[Jean-Christophe PLAGNIOL-VILLARD]

On 02:55 Wed 19 Mar     , Shinya Kuribayashi wrote:
> Handles machine specific functions by using function pointers.
> 
> Signed-off-by: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
If possible I will prefer a weak function

Best Regards,
J.

[U-Boot-Users] [MIPS] Introduce machine_restart

[Joakim Tjernlund]

> -----Original Message-----
> From: u-boot-users-bounces at lists.sourceforge.net [mailto:u-boot-users-bounces at lists.sourceforge.net]
> On Behalf Of Shinya Kuribayashi
> Sent: den 18 mars 2008 18:56
> To: u-boot-users at lists.sourceforge.net
> Subject: [U-Boot-Users] [MIPS] Introduce machine_restart
> 
> Handles machine specific functions by using function pointers.
> 
> Signed-off-by: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
> ---

I don't think it is good to use global fun ptrs. These are not available until
relocated to RAM.

 Jocke

[U-Boot-Users] [MIPS] Introduce machine_restart

[Wolfgang Denk]

In message <004701c88928$4f457bb0$edd07310$@Tjernlund@transmode.se>
Joakim Tjernlund wrote:
> 
> I don't think it is good to use global fun ptrs. These are not
> available until
> relocated to RAM.

Agreed!

In message <20080318184338.GA8728@game.jcrosoft.org>
Jean-Christophe Plagniol-Villar wrote:
>
> If possible I will prefer a weak function

...which raises the interesting question if, and how, weak function
pointers work before relocation.

Has anyone ever tested / analyzed this?

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
If something is different, it's either better or worse,  and  usually
both.                                                    - Larry Wall

[U-Boot-Users] [MIPS] Introduce machine_restart

[Shinya Kuribayashi]

Wolfgang Denk wrote:
> In message <004701c88928$4f457bb0$edd07310$@Tjernlund@transmode.se>
> Joakim Tjernlund wrote:
>> I don't think it is good to use global fun ptrs. These are not
>> available until
>> relocated to RAM.
> 
> Agreed!

I got it. Thanks for your comments.

> In message <20080318184338.GA8728@game.jcrosoft.org>
> Jean-Christophe Plagniol-Villar wrote:
>> If possible I will prefer a weak function
> 
> ...which raises the interesting question if, and how, weak function
> pointers work before relocation.
> 
> Has anyone ever tested / analyzed this?

I think I could, in a few days...

  Shinya

[U-Boot-Users] [MIPS] Introduce machine_restart

[Joakim Tjernlund]

> -----Original Message-----
> From: wd at denx.de [mailto:wd at denx.de]
> Sent: den 18 mars 2008 20:25
> To: Jean-Christophe PLAGNIOL-VILLARD
> Cc: Shinya Kuribayashi; Joakim Tjernlund; u-boot-users at lists.sourceforge.net
> Subject: Re: [U-Boot-Users] [MIPS] Introduce machine_restart
> 
> In message <004701c88928$4f457bb0$edd07310$@Tjernlund@transmode.se>
> Joakim Tjernlund wrote:
> >
> > I don't think it is good to use global fun ptrs. These are not
> > available until
> > relocated to RAM.
> 
> Agreed!
> 
> In message <20080318184338.GA8728@game.jcrosoft.org>
> Jean-Christophe Plagniol-Villar wrote:
> >
> > If possible I will prefer a weak function
> 
> ...which raises the interesting question if, and how, weak function
> pointers work before relocation.
> 
> Has anyone ever tested / analyzed this?

From memory during my work on uClibc ld.so:
It is no difference, the weak function makes it possible to have
several versions and select one at link time.
I THINK the linker will use the first it finds, weak or not. That
implies that the weak functions must be presented last on the
linker cmd line.

 Jocke

> 
> Best regards,
> 
> Wolfgang Denk
> 

[U-Boot-Users] [MIPS] Introduce machine_restart

[Vlad Lungu]

Wolfgang Denk wrote:
> In message <004701c88928$4f457bb0$edd07310$@Tjernlund@transmode.se>
> Joakim Tjernlund wrote:
>   
>> I don't think it is good to use global fun ptrs. These are not
>> available until
>> relocated to RAM.
>>     
>
> Agreed!
>
> In message <20080318184338.GA8728@game.jcrosoft.org>
> Jean-Christophe Plagniol-Villar wrote:
>   
>> If possible I will prefer a weak function
>>     
>
> ...which raises the interesting question if, and how, weak function
> pointers work before relocation.
>
> Has anyone ever tested / analyzed this?
>
>   

Weak symbols are a compile-time thing, not a run-time thing. The linker 
discards weak symbols
if a "regular" symbol with the same name is found, that's all. So it's 
actually link-time, rather than compile-time :-).

Vlad

[U-Boot-Users] [MIPS] Introduce machine_restart

[Shinya Kuribayashi]

Shinya Kuribayashi wrote:
> Wolfgang Denk wrote:
>> In message <004701c88928$4f457bb0$edd07310$@Tjernlund@transmode.se>
>> Joakim Tjernlund wrote:
>>> I don't think it is good to use global fun ptrs. These are not
>>> available until
>>> relocated to RAM.
>> Agreed!
> 
> I got it. Thanks for your comments.
> 
>> In message <20080318184338.GA8728@game.jcrosoft.org>
>> Jean-Christophe Plagniol-Villar wrote:
>>> If possible I will prefer a weak function
>> ...which raises the interesting question if, and how, weak function
>> pointers work before relocation.
>>
>> Has anyone ever tested / analyzed this?
> 
> I think I could, in a few days...

Revised patch is attached. This builds and works fine with me.

================>

[MIPS] Introduce _machine_restart

Handles machine specific functions by using weak functions.

Signed-off-by: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
---

 board/incaip/incaip.c     |    6 ++++++
 board/purple/purple.c     |    8 ++++++++
 board/tb0229/tb0229.c     |    9 ++++++++-
 cpu/mips/cpu.c            |   13 ++++++-------
 include/asm-mips/reboot.h |   14 ++++++++++++++
 5 files changed, 42 insertions(+), 8 deletions(-)
 create mode 100644 include/asm-mips/reboot.h


diff --git a/board/incaip/incaip.c b/board/incaip/incaip.c
index dbf0ecc..c624b3d 100644
--- a/board/incaip/incaip.c
+++ b/board/incaip/incaip.c
@@ -26,9 +26,15 @@
 #include <asm/addrspace.h>
 #include <asm/inca-ip.h>
 #include <asm/io.h>
+#include <asm/reboot.h>
 
 extern uint incaip_get_cpuclk(void);
 
+void _machine_restart(void)
+{
+	*INCA_IP_WDT_RST_REQ = 0x3f;
+}
+
 static ulong max_sdram_size(void)
 {
 	/* The only supported SDRAM data width is 16bit.
diff --git a/board/purple/purple.c b/board/purple/purple.c
index 74718af..13a1455 100644
--- a/board/purple/purple.c
+++ b/board/purple/purple.c
@@ -29,6 +29,7 @@
 #include <asm/io.h>
 #include <asm/addrspace.h>
 #include <asm/cacheops.h>
+#include <asm/reboot.h>
 
 #include "sconsole.h"
 
@@ -52,6 +53,13 @@ extern int	asc_serial_getc 	(void);
 extern int	asc_serial_tstc 	(void);
 extern void	asc_serial_setbrg 	(void);
 
+void _machine_restart(void)
+{
+	void (*f)(void) = (void *) 0xbfc00000;
+
+	f();
+}
+
 static void sdram_timing_init (ulong size)
 {
 	register uint pass;
diff --git a/board/tb0229/tb0229.c b/board/tb0229/tb0229.c
index 61c2e9b..d08b422 100644
--- a/board/tb0229/tb0229.c
+++ b/board/tb0229/tb0229.c
@@ -12,10 +12,17 @@
 #include <common.h>
 #include <command.h>
 #include <asm/addrspace.h>
-#include <asm/inca-ip.h>
 #include <asm/io.h>
+#include <asm/reboot.h>
 #include <pci.h>
 
+void _machine_restart(void)
+{
+	void (*f)(void) = (void *) 0xbfc00000;
+
+	f();
+}
+
 #if defined(CONFIG_PCI)
 static struct pci_controller hose;
 
diff --git a/cpu/mips/cpu.c b/cpu/mips/cpu.c
index de70c4d..8b43d8e 100644
--- a/cpu/mips/cpu.c
+++ b/cpu/mips/cpu.c
@@ -23,9 +23,9 @@
 
 #include <common.h>
 #include <command.h>
-#include <asm/inca-ip.h>
 #include <asm/mipsregs.h>
 #include <asm/cacheops.h>
+#include <asm/reboot.h>
 
 #define cache_op(op,addr)						\
 	__asm__ __volatile__(						\
@@ -37,15 +37,14 @@
 	:								\
 	: "i" (op), "R" (*(unsigned char *)(addr)))
 
+void __attribute__((weak)) _machine_restart(void)
+{
+}
+
 int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
-#if defined(CONFIG_INCA_IP)
-	*INCA_IP_WDT_RST_REQ = 0x3f;
-#elif defined(CONFIG_PURPLE) || defined(CONFIG_TB0229)
-	void (*f)(void) = (void *) 0xbfc00000;
+	_machine_restart();
 
-	f();
-#endif
 	fprintf(stderr, "*** reset failed ***\n");
 	return 0;
 }
diff --git a/include/asm-mips/reboot.h b/include/asm-mips/reboot.h
new file mode 100644
index 0000000..978d206
--- /dev/null
+++ b/include/asm-mips/reboot.h
@@ -0,0 +1,14 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 1997, 1999, 2001, 06 by Ralf Baechle
+ * Copyright (C) 2001 MIPS Technologies, Inc.
+ */
+#ifndef _ASM_REBOOT_H
+#define _ASM_REBOOT_H
+
+extern void _machine_restart(void);
+
+#endif /* _ASM_REBOOT_H */