Skip to contentSkip to conversation

MIPS: convert all boards to generic-board

PATCH New Daniel Schwierzeck 4 participants 21 messages Nov 15, 2014 – Nov 25, 2014 MIPS

lore
Share & tools
lore

Daniel Schwierzeck

author
#0 lore raw

cover 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
Signature · 1 line
2.1.3
author
#1 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

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

Signed-off-by Daniel Schwierzeck <[email protected]>

---

 common/board_f.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
common/board_f.c +7 -1
index b5bebc9..57e8a67 100644
@@ -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;
Signature · 1 line
2.1.3
#2 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

Hi Daniel,

Quoted 24 lines · Daniel Schwierzeck

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

Signed-off-by Daniel Schwierzeck <[email protected]>

---

 common/board_f.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
common/board_f.c +1 -1
index b5bebc9..57e8a67 100644
@@ -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

common/board_f.c static int reserve_stacks(void) · line 618

Quoted 10 lines
common/board_f.c +6 -0
@@ -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

author
#3 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

Hi Simon,

Quoted 55 lines · Simon Glass

Hi Daniel,

Quoted 24 lines · Daniel Schwierzeck

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

Signed-off-by Daniel Schwierzeck <[email protected]>

---

 common/board_f.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
common/board_f.c +1 -1
index b5bebc9..57e8a67 100644
@@ -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

common/board_f.c static int reserve_stacks(void) · line 618

Quoted 10 lines
common/board_f.c +6 -0
@@ -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.

Quoted 5 lines

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.

Signature · 1 line
- Daniel
#4 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

Hi Daniel,

Quoted 61 lines · Daniel Schwierzeck

Hi Simon,

Quoted 58 lines · Simon Glass

Hi Daniel,

Quoted 24 lines · Daniel Schwierzeck

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

Signed-off-by Daniel Schwierzeck <[email protected]>

---

 common/board_f.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
common/board_f.c +1 -1
index b5bebc9..57e8a67 100644
@@ -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

common/board_f.c static int reserve_stacks(void) · line 618

Quoted 10 lines
common/board_f.c +6 -0
@@ -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:

Quoted 7 lines
Quoted 2 lines

*--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

author
#5 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

Quoted 68 lines · Simon Glass

Hi Daniel,

Quoted 64 lines · Daniel Schwierzeck

Hi Simon,

Quoted 60 lines · Simon Glass

Hi Daniel,

Quoted 24 lines · Daniel Schwierzeck

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

Signed-off-by Daniel Schwierzeck <[email protected]>

---

 common/board_f.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
common/board_f.c +1 -1
index b5bebc9..57e8a67 100644
@@ -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

common/board_f.c static int reserve_stacks(void) · line 618

Quoted 10 lines
common/board_f.c +6 -0
@@ -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;

Quoted 8 lines
Quoted 4 lines

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.

Quoted 5 lines

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.

Signature · 1 line
- Daniel
#6 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

Hi Daniel,

Quoted 87 lines · Daniel Schwierzeck
Quoted 68 lines · Simon Glass

Hi Daniel,

Quoted 64 lines · Daniel Schwierzeck

Hi Simon,

Quoted 60 lines · Simon Glass

Hi Daniel,

Quoted 24 lines · Daniel Schwierzeck

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

Signed-off-by Daniel Schwierzeck <[email protected]>

---

 common/board_f.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
common/board_f.c +1 -1
index b5bebc9..57e8a67 100644
@@ -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

common/board_f.c static int reserve_stacks(void) · line 618

Quoted 10 lines
common/board_f.c +6 -0
@@ -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;

Quoted 8 lines
Quoted 4 lines

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.

Quoted 1 line
Quoted 6 lines
Quoted 4 lines

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

author
#7 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

Hi Simon,

Quoted 96 lines · Simon Glass

Hi Daniel,

Quoted 92 lines · Daniel Schwierzeck
Quoted 68 lines · Simon Glass

Hi Daniel,

Quoted 64 lines · Daniel Schwierzeck

Hi Simon,

Quoted 60 lines · Simon Glass

Hi Daniel,

Quoted 24 lines · Daniel Schwierzeck

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

Signed-off-by Daniel Schwierzeck <[email protected]>

---

 common/board_f.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
common/board_f.c +1 -1
index b5bebc9..57e8a67 100644
@@ -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

common/board_f.c static int reserve_stacks(void) · line 618

Quoted 10 lines
common/board_f.c +6 -0
@@ -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;

Quoted 8 lines
Quoted 4 lines

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.

Quoted 5 lines

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.

Quoted 5 lines
Quoted 1 line

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/

Signature · 1 line
- Daniel
#8 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

Hi Daniel,

Quoted 105 lines · Daniel Schwierzeck

Hi Simon,

Quoted 96 lines · Simon Glass

Hi Daniel,

Quoted 92 lines · Daniel Schwierzeck
Quoted 68 lines · Simon Glass

Hi Daniel,

Quoted 64 lines · Daniel Schwierzeck

Hi Simon,

Quoted 60 lines · Simon Glass

Hi Daniel,

Quoted 24 lines · Daniel Schwierzeck

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

Signed-off-by Daniel Schwierzeck <[email protected]>

---

 common/board_f.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
common/board_f.c +1 -1
index b5bebc9..57e8a67 100644
@@ -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

common/board_f.c static int reserve_stacks(void) · line 618

Quoted 10 lines
common/board_f.c +6 -0
@@ -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;

Quoted 8 lines
Quoted 4 lines

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.

Quoted 5 lines

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.

Quoted 5 lines
Quoted 1 line

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.

Quoted 8 lines

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

#9 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

In reply to Daniel Schwierzeck
Quoted 105 lines · Daniel Schwierzeck

Hi Simon,

Quoted 96 lines · Simon Glass

Hi Daniel,

Quoted 92 lines · Daniel Schwierzeck
Quoted 68 lines · Simon Glass

Hi Daniel,

Quoted 64 lines · Daniel Schwierzeck

Hi Simon,

Quoted 60 lines · Simon Glass

Hi Daniel,

Quoted 24 lines · Daniel Schwierzeck

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

Signed-off-by Daniel Schwierzeck <[email protected]>

---

 common/board_f.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
common/board_f.c +1 -1
index b5bebc9..57e8a67 100644
@@ -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

common/board_f.c static int reserve_stacks(void) · line 618

Quoted 10 lines
common/board_f.c +6 -0
@@ -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;

Quoted 8 lines
Quoted 4 lines

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.

Quoted 5 lines

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.

Quoted 5 lines
Quoted 1 line

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.

Quoted 8 lines

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.

Signature · 8 lines
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>
author
#10 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

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

Quoted 11 lines · Tom Rini
Quoted 11 lines
Quoted 6 lines
Quoted 2 lines

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;).

Quoted 9 lines
Quoted 4 lines

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-----

#11 lore raw

PATCH 1/7 common/board_f: add setup of initial stack frame for MIPS

In reply to Daniel Schwierzeck
Quoted 4 lines · Daniel Schwierzeck

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

Signed-off-by Daniel Schwierzeck <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
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>
author
#12 lore raw

PATCH 2/7 common/board_f: fix gcc warning on MIPS64

In reply to 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 <[email protected]>

---

 common/board_f.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
common/board_f.c +6 -4
index 57e8a67..d4d25d7 100644
@@ -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
Signature · 1 line
2.1.3
#13 lore raw

PATCH 2/7 common/board_f: fix gcc warning on MIPS64

Hi Daniel,

Quoted 8 lines · 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 <[email protected]>

Thanks, looks cleaner too.

Acked-by Simon Glass <[email protected]>

Regards,
Simon

#14 lore raw

PATCH 2/7 common/board_f: fix gcc warning on MIPS64

In reply to Daniel Schwierzeck
Quoted 9 lines · 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 <[email protected]>
Acked-by Simon Glass <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
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>
author
#15 lore raw

PATCH 3/7 MIPS: dbau1x00: switch to generic board

In reply to Daniel Schwierzeck
Signed-off-by Daniel Schwierzeck <[email protected]>

---

 include/configs/dbau1x00.h | 3 +++
 1 file changed, 3 insertions(+)
include/configs/dbau1x00.h +3 -0
index e0bf3dc..8a7447d 100644
@@ -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
Signature · 1 line
2.1.3
author
#16 lore raw

PATCH 4/7 MIPS: pb1x00: switch to generic board

In reply to Daniel Schwierzeck
Signed-off-by Daniel Schwierzeck <[email protected]>

---

 include/configs/pb1x00.h | 3 +++
 1 file changed, 3 insertions(+)
include/configs/pb1x00.h +3 -0
index 1c04a58..61e6af3 100644
@@ -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
Signature · 1 line
2.1.3
author
#17 lore raw

PATCH 5/7 MIPS: qemu_mips: switch to generic board

In reply to Daniel Schwierzeck
Signed-off-by Daniel Schwierzeck <[email protected]>

---

 include/configs/qemu-mips.h   | 3 +++
 include/configs/qemu-mips64.h | 3 +++
 2 files changed, 6 insertions(+)
include/configs/qemu-mips.h +3 -0
index 98ed8bc..1548d3e 100644
@@ -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 */
include/configs/qemu-mips64.h +3 -0
index e8f5a4c..61cafad 100644
@@ -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 */
Signature · 1 line
2.1.3
author
#18 lore raw

PATCH 6/7 MIPS: vct: switch to generic board

In reply to Daniel Schwierzeck
Signed-off-by Daniel Schwierzeck <[email protected]>

---

 include/configs/vct.h | 3 +++
 1 file changed, 3 insertions(+)
include/configs/vct.h +3 -0
index 217ba2f..83e4163 100644
@@ -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)
Signature · 1 line
2.1.3
author
#20 lore raw

PATCH 7/7 MIPS: remove board.c

In reply to Daniel Schwierzeck

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

Signed-off-by Daniel Schwierzeck <[email protected]>

---

 arch/mips/lib/Makefile |   3 -
 arch/mips/lib/board.c  | 320 -------------------------------------------------
 2 files changed, 323 deletions(-)
 delete mode 100644 arch/mips/lib/board.c
arch/mips/lib/Makefile +0 -3
index e483e86..7f9b653 100644
@@ -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
arch/mips/lib/board.c deleted +0 -320

320 lines not on this page · raw message

Signature · 1 line
2.1.3