[U-Boot] [PATCH 0/7] MIPS: convert all boards to generic-board

[U-Boot] [PATCH 0/7] MIPS: convert all boards to generic-board

[Daniel Schwierzeck]

Daniel Schwierzeck (7):
  common/board_f: add setup of initial stack frame for MIPS
  common/board_f: fix gcc warning on MIPS64
  MIPS: dbau1x00: switch to generic board
  MIPS: pb1x00: switch to generic board
  MIPS: qemu_mips: switch to generic board
  MIPS: vct: switch to generic board
  MIPS: remove board.c

 arch/mips/lib/Makefile        |   3 -
 arch/mips/lib/board.c         | 320 ------------------------------------------
 common/board_f.c              |  18 ++-
 include/configs/dbau1x00.h    |   3 +
 include/configs/pb1x00.h      |   3 +
 include/configs/qemu-mips.h   |   3 +
 include/configs/qemu-mips64.h |   3 +
 include/configs/vct.h         |   3 +
 8 files changed, 28 insertions(+), 328 deletions(-)
 delete mode 100644 arch/mips/lib/board.c

-- 
2.1.3

[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Daniel Schwierzeck]

The MIPS specific setup of the initial stack frame was not
ported to generic board_f.

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>

---

 common/board_f.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/common/board_f.c b/common/board_f.c
index b5bebc9..57e8a67 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -579,7 +579,7 @@ static int reserve_stacks(void)
 	gd->irq_sp = gd->start_addr_sp;
 # endif
 #else
-# ifdef CONFIG_PPC
+# if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
 	ulong *s;
 # endif
 
@@ -609,6 +609,12 @@ static int reserve_stacks(void)
 	s = (ulong *) gd->start_addr_sp;
 	*s = 0; /* Terminate back chain */
 	*++s = 0; /* NULL return address */
+# elif defined(CONFIG_MIPS)
+	/* Clear initial stack frame */
+	s = (ulong *) gd->start_addr_sp;
+	*s-- = 0;
+	*s-- = 0;
+	gd->start_addr_sp = (ulong) s;
 # endif /* Architecture specific code */
 
 	return 0;
-- 
2.1.3

[U-Boot] [PATCH 2/7] common/board_f: fix gcc warning on MIPS64

[Daniel Schwierzeck]

This fixes following warning when compiled with MIPS64

common/board_f.c: In function 'display_text_info':
common/board_f.c:150:2: warning: format '%X' expects argument i
  of type 'unsigned int', but argument 2 has type 'long unsigned int' [-Wformat=]
  debug("U-Boot code: %08X -> %08lX  BSS: -> %08lX\n",

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>

---

 common/board_f.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/common/board_f.c b/common/board_f.c
index 57e8a67..d4d25d7 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -142,17 +142,19 @@ static int init_baud_rate(void)
 static int display_text_info(void)
 {
 #ifndef CONFIG_SANDBOX
-	ulong bss_start, bss_end;
+	ulong bss_start, bss_end, text_base;
 
 	bss_start = (ulong)&__bss_start;
 	bss_end = (ulong)&__bss_end;
 
-	debug("U-Boot code: %08X -> %08lX  BSS: -> %08lX\n",
 #ifdef CONFIG_SYS_TEXT_BASE
-	      CONFIG_SYS_TEXT_BASE, bss_start, bss_end);
+	text_base = CONFIG_SYS_TEXT_BASE;
 #else
-	      CONFIG_SYS_MONITOR_BASE, bss_start, bss_end);
+	text_base = CONFIG_SYS_MONITOR_BASE;
 #endif
+
+	debug("U-Boot code: %08lX -> %08lX  BSS: -> %08lX\n",
+		text_base, bss_start, bss_end);
 #endif
 
 #ifdef CONFIG_MODEM_SUPPORT
-- 
2.1.3

[U-Boot] [PATCH 3/7] MIPS: dbau1x00: switch to generic board

[Daniel Schwierzeck]

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---

 include/configs/dbau1x00.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/configs/dbau1x00.h b/include/configs/dbau1x00.h
index e0bf3dc..8a7447d 100644
--- a/include/configs/dbau1x00.h
+++ b/include/configs/dbau1x00.h
@@ -15,6 +15,9 @@
 #define CONFIG_DBAU1X00		1
 #define CONFIG_SOC_AU1X00	1  /* alchemy series cpu */
 
+#define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DISPLAY_BOARDINFO
+
 #ifdef CONFIG_DBAU1000
 /* Also known as Merlot */
 #define CONFIG_SOC_AU1000	1
-- 
2.1.3

[U-Boot] [PATCH 4/7] MIPS: pb1x00: switch to generic board

[Daniel Schwierzeck]

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---

 include/configs/pb1x00.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/configs/pb1x00.h b/include/configs/pb1x00.h
index 1c04a58..61e6af3 100644
--- a/include/configs/pb1x00.h
+++ b/include/configs/pb1x00.h
@@ -15,6 +15,9 @@
 #define CONFIG_PB1X00		1
 #define CONFIG_SOC_AU1X00	1  /* alchemy series cpu */
 
+#define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DISPLAY_BOARDINFO
+
 #ifdef CONFIG_PB1000
 #define CONFIG_SOC_AU1000	1
 #else
-- 
2.1.3

[U-Boot] [PATCH 5/7] MIPS: qemu_mips: switch to generic board

[Daniel Schwierzeck]

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---

 include/configs/qemu-mips.h   | 3 +++
 include/configs/qemu-mips64.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/include/configs/qemu-mips.h b/include/configs/qemu-mips.h
index 98ed8bc..1548d3e 100644
--- a/include/configs/qemu-mips.h
+++ b/include/configs/qemu-mips.h
@@ -13,6 +13,9 @@
 #define __CONFIG_H
 
 #define CONFIG_QEMU_MIPS
+
+#define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DISPLAY_BOARDINFO
 #define CONFIG_MISC_INIT_R
 
 #define CONFIG_BOOTDELAY	10	/* autoboot after 10 seconds */
diff --git a/include/configs/qemu-mips64.h b/include/configs/qemu-mips64.h
index e8f5a4c..61cafad 100644
--- a/include/configs/qemu-mips64.h
+++ b/include/configs/qemu-mips64.h
@@ -13,6 +13,9 @@
 #define __CONFIG_H
 
 #define CONFIG_QEMU_MIPS
+
+#define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DISPLAY_BOARDINFO
 #define CONFIG_MISC_INIT_R
 
 #define CONFIG_BOOTDELAY	10	/* autoboot after 10 seconds */
-- 
2.1.3

[U-Boot] [PATCH 6/7] MIPS: vct: switch to generic board

[Daniel Schwierzeck]

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>

---

 include/configs/vct.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/configs/vct.h b/include/configs/vct.h
index 217ba2f..83e4163 100644
--- a/include/configs/vct.h
+++ b/include/configs/vct.h
@@ -25,6 +25,9 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DISPLAY_BOARDINFO
+
 #define CPU_CLOCK_RATE			324000000 /* Clock for the MIPS core */
 #define CONFIG_SYS_MIPS_TIMER_FREQ	(CPU_CLOCK_RATE / 2)
 
-- 
2.1.3

[U-Boot] [PATCH 7/7] MIPS: remove board.c

[Daniel Schwierzeck]

After all MIPS boards are switched to generic-board, the
MIPS specific board.c can be removed.

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>

---

 arch/mips/lib/Makefile |   3 -
 arch/mips/lib/board.c  | 320 -------------------------------------------------
 2 files changed, 323 deletions(-)
 delete mode 100644 arch/mips/lib/board.c

diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
index e483e86..7f9b653 100644
--- a/arch/mips/lib/Makefile
+++ b/arch/mips/lib/Makefile
@@ -5,9 +5,6 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
-ifndef CONFIG_SYS_GENERIC_BOARD
-obj-y	+= board.o
-endif
 obj-y	+= io.o
 
 obj-$(CONFIG_CMD_BOOTM) += bootm.o
diff --git a/arch/mips/lib/board.c b/arch/mips/lib/board.c
deleted file mode 100644
index 3feb020..0000000
--- a/arch/mips/lib/board.c
+++ /dev/null
@@ -1,320 +0,0 @@
-/*
- * (C) Copyright 2003
- * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#include <common.h>
-#include <command.h>
-#include <malloc.h>
-#include <serial.h>
-#include <stdio_dev.h>
-#include <version.h>
-#include <net.h>
-#include <environment.h>
-#include <nand.h>
-#include <onenand_uboot.h>
-#include <spi.h>
-
-#ifdef CONFIG_BITBANGMII
-#include <miiphy.h>
-#endif
-
-DECLARE_GLOBAL_DATA_PTR;
-
-ulong monitor_flash_len;
-
-static char *failed = "*** failed ***\n";
-
-int __board_early_init_f(void)
-{
-	/*
-	 * Nothing to do in this dummy implementation
-	 */
-	return 0;
-}
-int board_early_init_f(void)
-	__attribute__((weak, alias("__board_early_init_f")));
-
-static int init_func_ram(void)
-{
-#ifdef	CONFIG_BOARD_TYPES
-	int board_type = gd->board_type;
-#else
-	int board_type = 0;	/* use dummy arg */
-#endif
-	puts("DRAM:  ");
-
-	gd->ram_size = initdram(board_type);
-	if (gd->ram_size > 0) {
-		print_size(gd->ram_size, "\n");
-		return 0;
-	}
-	puts(failed);
-	return 1;
-}
-
-static int display_banner(void)
-{
-
-	printf("\n\n%s\n\n", version_string);
-	return 0;
-}
-
-#ifndef CONFIG_SYS_NO_FLASH
-static void display_flash_config(ulong size)
-{
-	puts("Flash: ");
-	print_size(size, "\n");
-}
-#endif
-
-static int init_baudrate(void)
-{
-	gd->baudrate = getenv_ulong("baudrate", 10, CONFIG_BAUDRATE);
-	return 0;
-}
-
-
-/*
- * Breath some life into the board...
- *
- * The first part of initialization is running from Flash memory;
- * its main purpose is to initialize the RAM so that we
- * can relocate the monitor code to RAM.
- */
-
-/*
- * All attempts to come up with a "common" initialization sequence
- * that works for all boards and architectures failed: some of the
- * requirements are just _too_ different. To get rid of the resulting
- * mess of board dependend #ifdef'ed code we now make the whole
- * initialization sequence configurable to the user.
- *
- * The requirements for any new initalization function is simple: it
- * receives a pointer to the "global data" structure as it's only
- * argument, and returns an integer return code, where 0 means
- * "continue" and != 0 means "fatal error, hang the system".
- */
-typedef int (init_fnc_t)(void);
-
-init_fnc_t *init_sequence[] = {
-	board_early_init_f,
-	timer_init,
-	env_init,		/* initialize environment */
-	init_baudrate,		/* initialize baudrate settings */
-	serial_init,		/* serial communications setup */
-	console_init_f,
-	display_banner,		/* say that we are here */
-	checkboard,
-	init_func_ram,
-	NULL,
-};
-
-
-void board_init_f(ulong bootflag)
-{
-	gd_t gd_data, *id;
-	bd_t *bd;
-	init_fnc_t **init_fnc_ptr;
-	ulong addr, addr_sp, len;
-	ulong *s;
-
-	/* Pointer is writable since we allocated a register for it.
-	 */
-	gd = &gd_data;
-	/* compiler optimization barrier needed for GCC >= 3.4 */
-	__asm__ __volatile__("" : : : "memory");
-
-	memset((void *)gd, 0, sizeof(gd_t));
-
-	for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
-		if ((*init_fnc_ptr)() != 0)
-			hang();
-	}
-
-	/*
-	 * Now that we have DRAM mapped and working, we can
-	 * relocate the code and continue running from DRAM.
-	 */
-	addr = CONFIG_SYS_SDRAM_BASE + gd->ram_size;
-
-	/* We can reserve some RAM "on top" here.
-	 */
-
-	/* round down to next 4 kB limit.
-	 */
-	addr &= ~(4096 - 1);
-	debug("Top of RAM usable for U-Boot at: %08lx\n", addr);
-
-	/* Reserve memory for U-Boot code, data & bss
-	 * round down to next 16 kB limit
-	 */
-	len = bss_end() - CONFIG_SYS_MONITOR_BASE;
-	addr -= len;
-	addr &= ~(16 * 1024 - 1);
-
-	debug("Reserving %ldk for U-Boot at: %08lx\n", len >> 10, addr);
-
-	 /* Reserve memory for malloc() arena.
-	 */
-	addr_sp = addr - TOTAL_MALLOC_LEN;
-	debug("Reserving %dk for malloc() at: %08lx\n",
-			TOTAL_MALLOC_LEN >> 10, addr_sp);
-
-	/*
-	 * (permanently) allocate a Board Info struct
-	 * and a permanent copy of the "global" data
-	 */
-	addr_sp -= sizeof(bd_t);
-	bd = (bd_t *)addr_sp;
-	gd->bd = bd;
-	debug("Reserving %zu Bytes for Board Info at: %08lx\n",
-			sizeof(bd_t), addr_sp);
-
-	addr_sp -= sizeof(gd_t);
-	id = (gd_t *)addr_sp;
-	debug("Reserving %zu Bytes for Global Data at: %08lx\n",
-			sizeof(gd_t), addr_sp);
-
-	/* Reserve memory for boot params.
-	 */
-	addr_sp -= CONFIG_SYS_BOOTPARAMS_LEN;
-	bd->bi_boot_params = addr_sp;
-	debug("Reserving %dk for boot params() at: %08lx\n",
-			CONFIG_SYS_BOOTPARAMS_LEN >> 10, addr_sp);
-
-	/*
-	 * Finally, we set up a new (bigger) stack.
-	 *
-	 * Leave some safety gap for SP, force alignment on 16 byte boundary
-	 * Clear initial stack frame
-	 */
-	addr_sp -= 16;
-	addr_sp &= ~0xF;
-	s = (ulong *)addr_sp;
-	*s-- = 0;
-	*s-- = 0;
-	addr_sp = (ulong)s;
-	debug("Stack Pointer at: %08lx\n", addr_sp);
-
-	/*
-	 * Save local variables to board info struct
-	 */
-	bd->bi_memstart	= CONFIG_SYS_SDRAM_BASE;	/* start of DRAM */
-	bd->bi_memsize	= gd->ram_size;		/* size of DRAM in bytes */
-
-	memcpy(id, (void *)gd, sizeof(gd_t));
-
-	relocate_code(addr_sp, id, addr);
-
-	/* NOTREACHED - relocate_code() does not return */
-}
-
-/*
- * This is the next part if the initialization sequence: we are now
- * running from RAM and have a "normal" C environment, i. e. global
- * data can be written, BSS has been cleared, the stack size in not
- * that critical any more, etc.
- */
-
-void board_init_r(gd_t *id, ulong dest_addr)
-{
-#ifndef CONFIG_SYS_NO_FLASH
-	ulong size;
-#endif
-	bd_t *bd;
-
-	gd = id;
-	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
-
-	debug("Now running in RAM - U-Boot at: %08lx\n", dest_addr);
-
-	gd->reloc_off = dest_addr - CONFIG_SYS_MONITOR_BASE;
-
-	monitor_flash_len = image_copy_end() - dest_addr;
-
-	serial_initialize();
-
-	bd = gd->bd;
-
-	/* The Malloc area is immediately below the monitor copy in DRAM */
-	mem_malloc_init(CONFIG_SYS_MONITOR_BASE + gd->reloc_off -
-			TOTAL_MALLOC_LEN, TOTAL_MALLOC_LEN);
-
-#ifndef CONFIG_SYS_NO_FLASH
-	/* configure available FLASH banks */
-	size = flash_init();
-	display_flash_config(size);
-	bd->bi_flashstart = CONFIG_SYS_FLASH_BASE;
-	bd->bi_flashsize = size;
-
-#if CONFIG_SYS_MONITOR_BASE == CONFIG_SYS_FLASH_BASE
-	bd->bi_flashoffset = monitor_flash_len;	/* reserved area for U-Boot */
-#else
-	bd->bi_flashoffset = 0;
-#endif
-#else
-	bd->bi_flashstart = 0;
-	bd->bi_flashsize = 0;
-	bd->bi_flashoffset = 0;
-#endif
-
-#ifdef CONFIG_CMD_NAND
-	puts("NAND:  ");
-	nand_init();		/* go init the NAND */
-#endif
-
-#if defined(CONFIG_CMD_ONENAND)
-	onenand_init();
-#endif
-
-	/* relocate environment function pointers etc. */
-	env_relocate();
-
-#if defined(CONFIG_PCI)
-	/*
-	 * Do pci configuration
-	 */
-	pci_init();
-#endif
-
-/** leave this here (after malloc(), environment and PCI are working) **/
-	/* Initialize stdio devices */
-	stdio_init();
-
-	jumptable_init();
-
-	/* Initialize the console (after the relocation and devices init) */
-	console_init_r();
-/** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** **/
-
-	/* Initialize from environment */
-	load_addr = getenv_ulong("loadaddr", 16, load_addr);
-
-#ifdef CONFIG_CMD_SPI
-	puts("SPI:   ");
-	spi_init();		/* go init the SPI */
-	puts("ready\n");
-#endif
-
-#if defined(CONFIG_MISC_INIT_R)
-	/* miscellaneous platform dependent initialisations */
-	misc_init_r();
-#endif
-
-#ifdef CONFIG_BITBANGMII
-	bb_miiphy_init();
-#endif
-#if defined(CONFIG_CMD_NET)
-	puts("Net:   ");
-	eth_initialize(gd->bd);
-#endif
-
-	/* main_loop() can return to retry autoboot, if so just run it again. */
-	for (;;)
-		main_loop();
-
-	/* NOTREACHED - no way out of command loop except booting */
-}
-- 
2.1.3

[U-Boot] [PATCH 6/7] MIPS: vct: switch to generic board

[Stefan Roese]

On 15.11.2014 23:46, Daniel Schwierzeck wrote:
> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>

Acked-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Simon Glass]

Hi Daniel,

On 15 November 2014 22:46, Daniel Schwierzeck
<daniel.schwierzeck@gmail.com> wrote:
> The MIPS specific setup of the initial stack frame was not
> ported to generic board_f.
>
> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>
> ---
>
>  common/board_f.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/common/board_f.c b/common/board_f.c
> index b5bebc9..57e8a67 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -579,7 +579,7 @@ static int reserve_stacks(void)
>         gd->irq_sp = gd->start_addr_sp;
>  # endif
>  #else
> -# ifdef CONFIG_PPC
> +# if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
>         ulong *s;
>  # endif
>
> @@ -609,6 +609,12 @@ static int reserve_stacks(void)
>         s = (ulong *) gd->start_addr_sp;
>         *s = 0; /* Terminate back chain */
>         *++s = 0; /* NULL return address */
> +# elif defined(CONFIG_MIPS)
> +       /* Clear initial stack frame */
> +       s = (ulong *) gd->start_addr_sp;
> +       *s-- = 0;
> +       *s-- = 0;
> +       gd->start_addr_sp = (ulong) s;
>  # endif /* Architecture specific code */

Great to see this happening.

There is a comment in the code here:

/*
* Handle architecture-specific things here
* TODO(sjg at chromium.org): Perhaps create arch_reserve_stack()
* to handle this and put in arch/xxx/lib/stack.c
*/

Perhaps we should do this. You could create a weak function which is
called for all archs, and implement it just for MIPS at present. I'm
not sure about a good prototype. Perhaps pass it gd and comment that
it is allowed to change memory to set up the stack, and adjust
gd->start_addr_sp and other stack-related values.

Also while I see that PPC writes above the stack pointer, I'm not sure
why it is valid. Should you in fact use:

*--s = 0;
*--s = 0;

Regards,
Simon

[U-Boot] [PATCH 2/7] common/board_f: fix gcc warning on MIPS64

[Simon Glass]

Hi Daniel,

On 15 November 2014 22:46, Daniel Schwierzeck
<daniel.schwierzeck@gmail.com> wrote:
> This fixes following warning when compiled with MIPS64
>
> common/board_f.c: In function 'display_text_info':
> common/board_f.c:150:2: warning: format '%X' expects argument i
>   of type 'unsigned int', but argument 2 has type 'long unsigned int' [-Wformat=]
>   debug("U-Boot code: %08X -> %08lX  BSS: -> %08lX\n",
>
> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>

Thanks, looks cleaner too.

Acked-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Daniel Schwierzeck]

Hi Simon,

On 17.11.2014 07:24, Simon Glass wrote:
> Hi Daniel,
> 
> On 15 November 2014 22:46, Daniel Schwierzeck
> <daniel.schwierzeck@gmail.com> wrote:
>> The MIPS specific setup of the initial stack frame was not
>> ported to generic board_f.
>>
>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>
>> ---
>>
>>  common/board_f.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/board_f.c b/common/board_f.c
>> index b5bebc9..57e8a67 100644
>> --- a/common/board_f.c
>> +++ b/common/board_f.c
>> @@ -579,7 +579,7 @@ static int reserve_stacks(void)
>>         gd->irq_sp = gd->start_addr_sp;
>>  # endif
>>  #else
>> -# ifdef CONFIG_PPC
>> +# if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
>>         ulong *s;
>>  # endif
>>
>> @@ -609,6 +609,12 @@ static int reserve_stacks(void)
>>         s = (ulong *) gd->start_addr_sp;
>>         *s = 0; /* Terminate back chain */
>>         *++s = 0; /* NULL return address */
>> +# elif defined(CONFIG_MIPS)
>> +       /* Clear initial stack frame */
>> +       s = (ulong *) gd->start_addr_sp;
>> +       *s-- = 0;
>> +       *s-- = 0;
>> +       gd->start_addr_sp = (ulong) s;
>>  # endif /* Architecture specific code */
> 
> Great to see this happening.
> 
> There is a comment in the code here:
> 
> /*
> * Handle architecture-specific things here
> * TODO(sjg at chromium.org): Perhaps create arch_reserve_stack()
> * to handle this and put in arch/xxx/lib/stack.c
> */
> 
> Perhaps we should do this. You could create a weak function which is
> called for all archs, and implement it just for MIPS at present. I'm
> not sure about a good prototype. Perhaps pass it gd and comment that
> it is allowed to change memory to set up the stack, and adjust
> gd->start_addr_sp and other stack-related values.
> 
> Also while I see that PPC writes above the stack pointer, I'm not sure
> why it is valid. Should you in fact use:
> 
> *--s = 0;
> *--s = 0;

I'd like to have those patches merged for 2015.01. So I want to keep the
current code to not break anything. Maybe this is not necessary at all.
The MIPS Malta board already uses generic board and does not seem to
have any problems.

-- 
- Daniel

[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Simon Glass]

Hi Daniel,

On 19 November 2014 16:59, Daniel Schwierzeck
<daniel.schwierzeck@gmail.com> wrote:
> Hi Simon,
>
> On 17.11.2014 07:24, Simon Glass wrote:
>> Hi Daniel,
>>
>> On 15 November 2014 22:46, Daniel Schwierzeck
>> <daniel.schwierzeck@gmail.com> wrote:
>>> The MIPS specific setup of the initial stack frame was not
>>> ported to generic board_f.
>>>
>>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>
>>> ---
>>>
>>>  common/board_f.c | 8 +++++++-
>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/common/board_f.c b/common/board_f.c
>>> index b5bebc9..57e8a67 100644
>>> --- a/common/board_f.c
>>> +++ b/common/board_f.c
>>> @@ -579,7 +579,7 @@ static int reserve_stacks(void)
>>>         gd->irq_sp = gd->start_addr_sp;
>>>  # endif
>>>  #else
>>> -# ifdef CONFIG_PPC
>>> +# if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
>>>         ulong *s;
>>>  # endif
>>>
>>> @@ -609,6 +609,12 @@ static int reserve_stacks(void)
>>>         s = (ulong *) gd->start_addr_sp;
>>>         *s = 0; /* Terminate back chain */
>>>         *++s = 0; /* NULL return address */
>>> +# elif defined(CONFIG_MIPS)
>>> +       /* Clear initial stack frame */
>>> +       s = (ulong *) gd->start_addr_sp;
>>> +       *s-- = 0;
>>> +       *s-- = 0;
>>> +       gd->start_addr_sp = (ulong) s;
>>>  # endif /* Architecture specific code */
>>
>> Great to see this happening.
>>
>> There is a comment in the code here:
>>
>> /*
>> * Handle architecture-specific things here
>> * TODO(sjg at chromium.org): Perhaps create arch_reserve_stack()
>> * to handle this and put in arch/xxx/lib/stack.c
>> */
>>
>> Perhaps we should do this. You could create a weak function which is
>> called for all archs, and implement it just for MIPS at present. I'm
>> not sure about a good prototype. Perhaps pass it gd and comment that
>> it is allowed to change memory to set up the stack, and adjust
>> gd->start_addr_sp and other stack-related values.
>>
>> Also while I see that PPC writes above the stack pointer, I'm not sure
>> why it is valid. Should you in fact use:
>>
>> *--s = 0;
>> *--s = 0;
>
> I'd like to have those patches merged for 2015.01. So I want to keep the
> current code to not break anything. Maybe this is not necessary at all.
> The MIPS Malta board already uses generic board and does not seem to
> have any problems.

I don't see a problem with merging this for 2015.01. Are you saying
you don't think it is needed but can't be sure? So you want to merge
it and see what people report?

In that case I think you should add a comment to that effect, but also
do the function as I mentioned above. We are trying to remove the
arch-specific code in this file and certainly don't want to add more.

Regards,
Simon

[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Daniel Schwierzeck]

On 19.11.2014 23:22, Simon Glass wrote:
> Hi Daniel,
> 
> On 19 November 2014 16:59, Daniel Schwierzeck
> <daniel.schwierzeck@gmail.com> wrote:
>> Hi Simon,
>>
>> On 17.11.2014 07:24, Simon Glass wrote:
>>> Hi Daniel,
>>>
>>> On 15 November 2014 22:46, Daniel Schwierzeck
>>> <daniel.schwierzeck@gmail.com> wrote:
>>>> The MIPS specific setup of the initial stack frame was not
>>>> ported to generic board_f.
>>>>
>>>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>>
>>>> ---
>>>>
>>>>  common/board_f.c | 8 +++++++-
>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/common/board_f.c b/common/board_f.c
>>>> index b5bebc9..57e8a67 100644
>>>> --- a/common/board_f.c
>>>> +++ b/common/board_f.c
>>>> @@ -579,7 +579,7 @@ static int reserve_stacks(void)
>>>>         gd->irq_sp = gd->start_addr_sp;
>>>>  # endif
>>>>  #else
>>>> -# ifdef CONFIG_PPC
>>>> +# if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
>>>>         ulong *s;
>>>>  # endif
>>>>
>>>> @@ -609,6 +609,12 @@ static int reserve_stacks(void)
>>>>         s = (ulong *) gd->start_addr_sp;
>>>>         *s = 0; /* Terminate back chain */
>>>>         *++s = 0; /* NULL return address */
>>>> +# elif defined(CONFIG_MIPS)
>>>> +       /* Clear initial stack frame */
>>>> +       s = (ulong *) gd->start_addr_sp;
>>>> +       *s-- = 0;
>>>> +       *s-- = 0;
>>>> +       gd->start_addr_sp = (ulong) s;
>>>>  # endif /* Architecture specific code */
>>>
>>> Great to see this happening.
>>>
>>> There is a comment in the code here:
>>>
>>> /*
>>> * Handle architecture-specific things here
>>> * TODO(sjg at chromium.org): Perhaps create arch_reserve_stack()
>>> * to handle this and put in arch/xxx/lib/stack.c
>>> */
>>>
>>> Perhaps we should do this. You could create a weak function which is
>>> called for all archs, and implement it just for MIPS at present. I'm
>>> not sure about a good prototype. Perhaps pass it gd and comment that
>>> it is allowed to change memory to set up the stack, and adjust
>>> gd->start_addr_sp and other stack-related values.
>>>
>>> Also while I see that PPC writes above the stack pointer, I'm not sure
>>> why it is valid. Should you in fact use:
>>>
>>> *--s = 0;
>>> *--s = 0;
>>
>> I'd like to have those patches merged for 2015.01. So I want to keep the
>> current code to not break anything. Maybe this is not necessary at all.
>> The MIPS Malta board already uses generic board and does not seem to
>> have any problems.
> 
> I don't see a problem with merging this for 2015.01. Are you saying
> you don't think it is needed but can't be sure? So you want to merge
> it and see what people report?

that code is taken unmodified from arch/mips/lib/board.c to not change
or break anything. But that code is old and maybe copied from PowerPC in
the early phases of U-Boot. I'm only saying that I need to investigate
if that code could be dropped or not. But that is a task for the next
merge window.

> 
> In that case I think you should add a comment to that effect, but also
> do the function as I mentioned above. We are trying to remove the
> arch-specific code in this file and certainly don't want to add more.
> 

ok I'll send an updated patch.

-- 
- Daniel

[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Simon Glass]

Hi Daniel,

On 20 November 2014 16:54, Daniel Schwierzeck
<daniel.schwierzeck@gmail.com> wrote:
>
>
> On 19.11.2014 23:22, Simon Glass wrote:
>> Hi Daniel,
>>
>> On 19 November 2014 16:59, Daniel Schwierzeck
>> <daniel.schwierzeck@gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On 17.11.2014 07:24, Simon Glass wrote:
>>>> Hi Daniel,
>>>>
>>>> On 15 November 2014 22:46, Daniel Schwierzeck
>>>> <daniel.schwierzeck@gmail.com> wrote:
>>>>> The MIPS specific setup of the initial stack frame was not
>>>>> ported to generic board_f.
>>>>>
>>>>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>>>
>>>>> ---
>>>>>
>>>>>  common/board_f.c | 8 +++++++-
>>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/common/board_f.c b/common/board_f.c
>>>>> index b5bebc9..57e8a67 100644
>>>>> --- a/common/board_f.c
>>>>> +++ b/common/board_f.c
>>>>> @@ -579,7 +579,7 @@ static int reserve_stacks(void)
>>>>>         gd->irq_sp = gd->start_addr_sp;
>>>>>  # endif
>>>>>  #else
>>>>> -# ifdef CONFIG_PPC
>>>>> +# if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
>>>>>         ulong *s;
>>>>>  # endif
>>>>>
>>>>> @@ -609,6 +609,12 @@ static int reserve_stacks(void)
>>>>>         s = (ulong *) gd->start_addr_sp;
>>>>>         *s = 0; /* Terminate back chain */
>>>>>         *++s = 0; /* NULL return address */
>>>>> +# elif defined(CONFIG_MIPS)
>>>>> +       /* Clear initial stack frame */
>>>>> +       s = (ulong *) gd->start_addr_sp;
>>>>> +       *s-- = 0;
>>>>> +       *s-- = 0;
>>>>> +       gd->start_addr_sp = (ulong) s;
>>>>>  # endif /* Architecture specific code */
>>>>
>>>> Great to see this happening.
>>>>
>>>> There is a comment in the code here:
>>>>
>>>> /*
>>>> * Handle architecture-specific things here
>>>> * TODO(sjg at chromium.org): Perhaps create arch_reserve_stack()
>>>> * to handle this and put in arch/xxx/lib/stack.c
>>>> */
>>>>
>>>> Perhaps we should do this. You could create a weak function which is
>>>> called for all archs, and implement it just for MIPS at present. I'm
>>>> not sure about a good prototype. Perhaps pass it gd and comment that
>>>> it is allowed to change memory to set up the stack, and adjust
>>>> gd->start_addr_sp and other stack-related values.
>>>>
>>>> Also while I see that PPC writes above the stack pointer, I'm not sure
>>>> why it is valid. Should you in fact use:
>>>>
>>>> *--s = 0;
>>>> *--s = 0;
>>>
>>> I'd like to have those patches merged for 2015.01. So I want to keep the
>>> current code to not break anything. Maybe this is not necessary at all.
>>> The MIPS Malta board already uses generic board and does not seem to
>>> have any problems.
>>
>> I don't see a problem with merging this for 2015.01. Are you saying
>> you don't think it is needed but can't be sure? So you want to merge
>> it and see what people report?
>
> that code is taken unmodified from arch/mips/lib/board.c to not change
> or break anything. But that code is old and maybe copied from PowerPC in
> the early phases of U-Boot. I'm only saying that I need to investigate
> if that code could be dropped or not. But that is a task for the next
> merge window.
>
>>
>> In that case I think you should add a comment to that effect, but also
>> do the function as I mentioned above. We are trying to remove the
>> arch-specific code in this file and certainly don't want to add more.
>>
>
> ok I'll send an updated patch.

Thanks - and as I mentioned it seems wrong to write to a word above
the top of the stack.

Regards,
Simon

[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Daniel Schwierzeck]

Hi Simon,

On 20.11.2014 18:22, Simon Glass wrote:
> Hi Daniel,
> 
> On 20 November 2014 16:54, Daniel Schwierzeck
> <daniel.schwierzeck@gmail.com> wrote:
>>
>>
>> On 19.11.2014 23:22, Simon Glass wrote:
>>> Hi Daniel,
>>>
>>> On 19 November 2014 16:59, Daniel Schwierzeck
>>> <daniel.schwierzeck@gmail.com> wrote:
>>>> Hi Simon,
>>>>
>>>> On 17.11.2014 07:24, Simon Glass wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> On 15 November 2014 22:46, Daniel Schwierzeck
>>>>> <daniel.schwierzeck@gmail.com> wrote:
>>>>>> The MIPS specific setup of the initial stack frame was not
>>>>>> ported to generic board_f.
>>>>>>
>>>>>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>>  common/board_f.c | 8 +++++++-
>>>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/common/board_f.c b/common/board_f.c
>>>>>> index b5bebc9..57e8a67 100644
>>>>>> --- a/common/board_f.c
>>>>>> +++ b/common/board_f.c
>>>>>> @@ -579,7 +579,7 @@ static int reserve_stacks(void)
>>>>>>         gd->irq_sp = gd->start_addr_sp;
>>>>>>  # endif
>>>>>>  #else
>>>>>> -# ifdef CONFIG_PPC
>>>>>> +# if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
>>>>>>         ulong *s;
>>>>>>  # endif
>>>>>>
>>>>>> @@ -609,6 +609,12 @@ static int reserve_stacks(void)
>>>>>>         s = (ulong *) gd->start_addr_sp;
>>>>>>         *s = 0; /* Terminate back chain */
>>>>>>         *++s = 0; /* NULL return address */
>>>>>> +# elif defined(CONFIG_MIPS)
>>>>>> +       /* Clear initial stack frame */
>>>>>> +       s = (ulong *) gd->start_addr_sp;
>>>>>> +       *s-- = 0;
>>>>>> +       *s-- = 0;
>>>>>> +       gd->start_addr_sp = (ulong) s;
>>>>>>  # endif /* Architecture specific code */
>>>>>
>>>>> Great to see this happening.
>>>>>
>>>>> There is a comment in the code here:
>>>>>
>>>>> /*
>>>>> * Handle architecture-specific things here
>>>>> * TODO(sjg at chromium.org): Perhaps create arch_reserve_stack()
>>>>> * to handle this and put in arch/xxx/lib/stack.c
>>>>> */
>>>>>
>>>>> Perhaps we should do this. You could create a weak function which is
>>>>> called for all archs, and implement it just for MIPS at present. I'm
>>>>> not sure about a good prototype. Perhaps pass it gd and comment that
>>>>> it is allowed to change memory to set up the stack, and adjust
>>>>> gd->start_addr_sp and other stack-related values.
>>>>>
>>>>> Also while I see that PPC writes above the stack pointer, I'm not sure
>>>>> why it is valid. Should you in fact use:
>>>>>
>>>>> *--s = 0;
>>>>> *--s = 0;
>>>>
>>>> I'd like to have those patches merged for 2015.01. So I want to keep the
>>>> current code to not break anything. Maybe this is not necessary at all.
>>>> The MIPS Malta board already uses generic board and does not seem to
>>>> have any problems.
>>>
>>> I don't see a problem with merging this for 2015.01. Are you saying
>>> you don't think it is needed but can't be sure? So you want to merge
>>> it and see what people report?
>>
>> that code is taken unmodified from arch/mips/lib/board.c to not change
>> or break anything. But that code is old and maybe copied from PowerPC in
>> the early phases of U-Boot. I'm only saying that I need to investigate
>> if that code could be dropped or not. But that is a task for the next
>> merge window.
>>
>>>
>>> In that case I think you should add a comment to that effect, but also
>>> do the function as I mentioned above. We are trying to remove the
>>> arch-specific code in this file and certainly don't want to add more.
>>>
>>
>> ok I'll send an updated patch.
> 
> Thanks - and as I mentioned it seems wrong to write to a word above
> the top of the stack.
> 

I discard this patch. The only requirement for the stack pointer on MIPS
is alignment on a 8 Byte boundary. reserve_stacks already aligns it to
16 Byte (gd->start_addr_sp &= ~0xf;).

To make stack walking and backtraces working flawlessy in gdb, I found
another solution [1].

[1] http://patchwork.ozlabs.org/patch/413182/

-- 
- Daniel

[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Simon Glass]

Hi Daniel,

On 21 November 2014 21:46, Daniel Schwierzeck
<daniel.schwierzeck@gmail.com> wrote:
> Hi Simon,
>
> On 20.11.2014 18:22, Simon Glass wrote:
>> Hi Daniel,
>>
>> On 20 November 2014 16:54, Daniel Schwierzeck
>> <daniel.schwierzeck@gmail.com> wrote:
>>>
>>>
>>> On 19.11.2014 23:22, Simon Glass wrote:
>>>> Hi Daniel,
>>>>
>>>> On 19 November 2014 16:59, Daniel Schwierzeck
>>>> <daniel.schwierzeck@gmail.com> wrote:
>>>>> Hi Simon,
>>>>>
>>>>> On 17.11.2014 07:24, Simon Glass wrote:
>>>>>> Hi Daniel,
>>>>>>
>>>>>> On 15 November 2014 22:46, Daniel Schwierzeck
>>>>>> <daniel.schwierzeck@gmail.com> wrote:
>>>>>>> The MIPS specific setup of the initial stack frame was not
>>>>>>> ported to generic board_f.
>>>>>>>
>>>>>>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>>  common/board_f.c | 8 +++++++-
>>>>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/common/board_f.c b/common/board_f.c
>>>>>>> index b5bebc9..57e8a67 100644
>>>>>>> --- a/common/board_f.c
>>>>>>> +++ b/common/board_f.c
>>>>>>> @@ -579,7 +579,7 @@ static int reserve_stacks(void)
>>>>>>>         gd->irq_sp = gd->start_addr_sp;
>>>>>>>  # endif
>>>>>>>  #else
>>>>>>> -# ifdef CONFIG_PPC
>>>>>>> +# if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
>>>>>>>         ulong *s;
>>>>>>>  # endif
>>>>>>>
>>>>>>> @@ -609,6 +609,12 @@ static int reserve_stacks(void)
>>>>>>>         s = (ulong *) gd->start_addr_sp;
>>>>>>>         *s = 0; /* Terminate back chain */
>>>>>>>         *++s = 0; /* NULL return address */
>>>>>>> +# elif defined(CONFIG_MIPS)
>>>>>>> +       /* Clear initial stack frame */
>>>>>>> +       s = (ulong *) gd->start_addr_sp;
>>>>>>> +       *s-- = 0;
>>>>>>> +       *s-- = 0;
>>>>>>> +       gd->start_addr_sp = (ulong) s;
>>>>>>>  # endif /* Architecture specific code */
>>>>>>
>>>>>> Great to see this happening.
>>>>>>
>>>>>> There is a comment in the code here:
>>>>>>
>>>>>> /*
>>>>>> * Handle architecture-specific things here
>>>>>> * TODO(sjg at chromium.org): Perhaps create arch_reserve_stack()
>>>>>> * to handle this and put in arch/xxx/lib/stack.c
>>>>>> */
>>>>>>
>>>>>> Perhaps we should do this. You could create a weak function which is
>>>>>> called for all archs, and implement it just for MIPS at present. I'm
>>>>>> not sure about a good prototype. Perhaps pass it gd and comment that
>>>>>> it is allowed to change memory to set up the stack, and adjust
>>>>>> gd->start_addr_sp and other stack-related values.
>>>>>>
>>>>>> Also while I see that PPC writes above the stack pointer, I'm not sure
>>>>>> why it is valid. Should you in fact use:
>>>>>>
>>>>>> *--s = 0;
>>>>>> *--s = 0;
>>>>>
>>>>> I'd like to have those patches merged for 2015.01. So I want to keep the
>>>>> current code to not break anything. Maybe this is not necessary at all.
>>>>> The MIPS Malta board already uses generic board and does not seem to
>>>>> have any problems.
>>>>
>>>> I don't see a problem with merging this for 2015.01. Are you saying
>>>> you don't think it is needed but can't be sure? So you want to merge
>>>> it and see what people report?
>>>
>>> that code is taken unmodified from arch/mips/lib/board.c to not change
>>> or break anything. But that code is old and maybe copied from PowerPC in
>>> the early phases of U-Boot. I'm only saying that I need to investigate
>>> if that code could be dropped or not. But that is a task for the next
>>> merge window.
>>>
>>>>
>>>> In that case I think you should add a comment to that effect, but also
>>>> do the function as I mentioned above. We are trying to remove the
>>>> arch-specific code in this file and certainly don't want to add more.
>>>>
>>>
>>> ok I'll send an updated patch.
>>
>> Thanks - and as I mentioned it seems wrong to write to a word above
>> the top of the stack.
>>
>
> I discard this patch. The only requirement for the stack pointer on MIPS
> is alignment on a 8 Byte boundary. reserve_stacks already aligns it to
> 16 Byte (gd->start_addr_sp &= ~0xf;).
>
> To make stack walking and backtraces working flawlessy in gdb, I found
> another solution [1].
>
> [1] http://patchwork.ozlabs.org/patch/413182/

Excellent, thanks for digging into this.

Regards,
Simon

[U-Boot] [U-Boot, 1/7] common/board_f: add setup of initial stack frame for MIPS

[Tom Rini]

On Sat, Nov 15, 2014 at 11:46:52PM +0100, Daniel Schwierzeck wrote:

> The MIPS specific setup of the initial stack frame was not
> ported to generic board_f.
> 
> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141124/2c89722b/attachment.pgp>

[U-Boot] [U-Boot, 2/7] common/board_f: fix gcc warning on MIPS64

[Tom Rini]

On Sat, Nov 15, 2014 at 11:46:53PM +0100, Daniel Schwierzeck wrote:

> This fixes following warning when compiled with MIPS64
> 
> common/board_f.c: In function 'display_text_info':
> common/board_f.c:150:2: warning: format '%X' expects argument i
>   of type 'unsigned int', but argument 2 has type 'long unsigned int' [-Wformat=]
>   debug("U-Boot code: %08X -> %08lX  BSS: -> %08lX\n",
> 
> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141124/acad67f9/attachment.pgp>

[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Tom Rini]

On Fri, Nov 21, 2014 at 09:46:09PM +0100, Daniel Schwierzeck wrote:
> Hi Simon,
> 
> On 20.11.2014 18:22, Simon Glass wrote:
> > Hi Daniel,
> > 
> > On 20 November 2014 16:54, Daniel Schwierzeck
> > <daniel.schwierzeck@gmail.com> wrote:
> >>
> >>
> >> On 19.11.2014 23:22, Simon Glass wrote:
> >>> Hi Daniel,
> >>>
> >>> On 19 November 2014 16:59, Daniel Schwierzeck
> >>> <daniel.schwierzeck@gmail.com> wrote:
> >>>> Hi Simon,
> >>>>
> >>>> On 17.11.2014 07:24, Simon Glass wrote:
> >>>>> Hi Daniel,
> >>>>>
> >>>>> On 15 November 2014 22:46, Daniel Schwierzeck
> >>>>> <daniel.schwierzeck@gmail.com> wrote:
> >>>>>> The MIPS specific setup of the initial stack frame was not
> >>>>>> ported to generic board_f.
> >>>>>>
> >>>>>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> >>>>>>
> >>>>>> ---
> >>>>>>
> >>>>>>  common/board_f.c | 8 +++++++-
> >>>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/common/board_f.c b/common/board_f.c
> >>>>>> index b5bebc9..57e8a67 100644
> >>>>>> --- a/common/board_f.c
> >>>>>> +++ b/common/board_f.c
> >>>>>> @@ -579,7 +579,7 @@ static int reserve_stacks(void)
> >>>>>>         gd->irq_sp = gd->start_addr_sp;
> >>>>>>  # endif
> >>>>>>  #else
> >>>>>> -# ifdef CONFIG_PPC
> >>>>>> +# if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
> >>>>>>         ulong *s;
> >>>>>>  # endif
> >>>>>>
> >>>>>> @@ -609,6 +609,12 @@ static int reserve_stacks(void)
> >>>>>>         s = (ulong *) gd->start_addr_sp;
> >>>>>>         *s = 0; /* Terminate back chain */
> >>>>>>         *++s = 0; /* NULL return address */
> >>>>>> +# elif defined(CONFIG_MIPS)
> >>>>>> +       /* Clear initial stack frame */
> >>>>>> +       s = (ulong *) gd->start_addr_sp;
> >>>>>> +       *s-- = 0;
> >>>>>> +       *s-- = 0;
> >>>>>> +       gd->start_addr_sp = (ulong) s;
> >>>>>>  # endif /* Architecture specific code */
> >>>>>
> >>>>> Great to see this happening.
> >>>>>
> >>>>> There is a comment in the code here:
> >>>>>
> >>>>> /*
> >>>>> * Handle architecture-specific things here
> >>>>> * TODO(sjg at chromium.org): Perhaps create arch_reserve_stack()
> >>>>> * to handle this and put in arch/xxx/lib/stack.c
> >>>>> */
> >>>>>
> >>>>> Perhaps we should do this. You could create a weak function which is
> >>>>> called for all archs, and implement it just for MIPS at present. I'm
> >>>>> not sure about a good prototype. Perhaps pass it gd and comment that
> >>>>> it is allowed to change memory to set up the stack, and adjust
> >>>>> gd->start_addr_sp and other stack-related values.
> >>>>>
> >>>>> Also while I see that PPC writes above the stack pointer, I'm not sure
> >>>>> why it is valid. Should you in fact use:
> >>>>>
> >>>>> *--s = 0;
> >>>>> *--s = 0;
> >>>>
> >>>> I'd like to have those patches merged for 2015.01. So I want to keep the
> >>>> current code to not break anything. Maybe this is not necessary at all.
> >>>> The MIPS Malta board already uses generic board and does not seem to
> >>>> have any problems.
> >>>
> >>> I don't see a problem with merging this for 2015.01. Are you saying
> >>> you don't think it is needed but can't be sure? So you want to merge
> >>> it and see what people report?
> >>
> >> that code is taken unmodified from arch/mips/lib/board.c to not change
> >> or break anything. But that code is old and maybe copied from PowerPC in
> >> the early phases of U-Boot. I'm only saying that I need to investigate
> >> if that code could be dropped or not. But that is a task for the next
> >> merge window.
> >>
> >>>
> >>> In that case I think you should add a comment to that effect, but also
> >>> do the function as I mentioned above. We are trying to remove the
> >>> arch-specific code in this file and certainly don't want to add more.
> >>>
> >>
> >> ok I'll send an updated patch.
> > 
> > Thanks - and as I mentioned it seems wrong to write to a word above
> > the top of the stack.
> > 
> 
> I discard this patch. The only requirement for the stack pointer on MIPS
> is alignment on a 8 Byte boundary. reserve_stacks already aligns it to
> 16 Byte (gd->start_addr_sp &= ~0xf;).
> 
> To make stack walking and backtraces working flawlessy in gdb, I found
> another solution [1].
> 
> [1] http://patchwork.ozlabs.org/patch/413182/

And I of course missed this email while sorting out other problems with
stuff I was applying.  I'll just back this out and take the better fix.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141124/ee1fa20c/attachment.pgp>

[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Daniel Schwierzeck]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 24.11.2014 23:20, Tom Rini wrote:
>>>> 
>>>> ok I'll send an updated patch.
>>> 
>>> Thanks - and as I mentioned it seems wrong to write to a word
>>> above the top of the stack.
>>> 
>> 
>> I discard this patch. The only requirement for the stack pointer
>> on MIPS is alignment on a 8 Byte boundary. reserve_stacks already
>> aligns it to 16 Byte (gd->start_addr_sp &= ~0xf;).
>> 
>> To make stack walking and backtraces working flawlessy in gdb, I
>> found another solution [1].
>> 
>> [1] http://patchwork.ozlabs.org/patch/413182/
> 
> And I of course missed this email while sorting out other problems
> with stuff I was applying.  I'll just back this out and take the
> better fix.
> 

hm, the patch landed in v2015.01-rc2. Do you want to create a revert
patch or shall I include one in my next pull request?

- -- 
- - Daniel
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJUdITqAAoJEPf1RifdDuszTuIIAJQzh2/EU16Hn+l/JC+TvJ3h
802FJMuKWeZRoF6NPrvZQ8x2SUloc2PfEOyQnMgUtChKOjzuba64Gk08ymzLQQ2Z
r7Iv4W3so/9jv41PLeuFpVCAzypRuOi01cXGOxJYLgj1U9QRxAmGXxyIM/v4IFP+
13ykcJLzxsp4qM12dEyBMQ+8uxysO3yjOwqEdk/q8agqttgnNd1ZjQ1Qzifs9udd
ITuxfuii4iwzM+XWg06Q01kC60hEHis4P/HHgHile/Wd0yKrX1rGPYozpvZO+mIr
r9w8Bnsdm/wgJOLX604MHMMjy6fkivrI7ZlCishEwMmOpnwElfnVwubjPFRQH0I=
=r3h1
-----END PGP SIGNATURE-----