[U-Boot] [PATCH 0/7] MIPS: convert all boards to generic-board
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
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;
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@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
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@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.
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@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
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@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.
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@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
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@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/
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@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].
Excellent, thanks for digging into this.
Regards, Simon
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@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].
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.
-----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].
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
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!
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
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
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!
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
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
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 */
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)
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
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@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 */ -}
participants (4)
-
Daniel Schwierzeck -
Simon Glass -
Stefan Roese -
Tom Rini