common/board_f: make board_init_f_mem() independent on !CONFIG_X86

PATCH New Alexey Brodkin 2 participants 5 messages Mar 16, 2015 – Mar 23, 2015

lore

Share & tools

lore

Alexey Brodkin

author

2015-03-16 08:03 UTC #0 lore raw

PATCH 1/1 common/board_f: make board_init_f_mem() independent on !CONFIG_X86

Even though board_init_f_mem() is not used on x86 today there's no reason to not use it in the future.

Moreover board_init_f_mem() has nothing to do with any particular architecture so move it away from #else /* CONFIG_X86 */

Signed-off-by Alexey Brodkin <[email protected]>

Cc Simon Glass <[email protected]>

Cc Tom Rini <[email protected]>

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

common/board_f.c +2 -2

index 731b539..f55550c 100644
@@ -1079,7 +1079,8 @@ void board_init_f_r(void)
 	/* NOTREACHED - board_init_r() does not return */
 	hang();
 }
-#else
+#endif /* CONFIG_X86 */
+
 ulong board_init_f_mem(ulong top)
 {
 	/* Leave space for the stack we are running with now */

@@ -1097,4 +1098,3 @@ ulong board_init_f_mem(ulong top)
 
 	return top;
 }
-#endif /* CONFIG_X86 */

Signature · 1 line

2.1.0

Alexey Brodkin

author

2015-03-23 16:40 UTC #1 lore raw

PATCH 1/1 common/board_f: make board_init_f_mem() independent on !CONFIG_X86

Hi Tom, Simon,

Quoted 9 lines · Alexey Brodkin

Even though board_init_f_mem() is not used on x86 today there's no reason to not use it in the future.
Moreover board_init_f_mem() has nothing to do with any particular architecture so move it away from #else /* CONFIG_X86 */
Signed-off-by Alexey Brodkin <[email protected]>
Cc Simon Glass <[email protected]>
Cc Tom Rini <[email protected]>

Any comments on this one?
This is a prerequisite for ARC updates so would be good to have it merged sometime soon.

-Alexey

Simon Glass

2015-03-23 17:26 UTC #2 lore raw

PATCH 1/1 common/board_f: make board_init_f_mem() independent on !CONFIG_X86

Quoted 9 lines · Alexey Brodkin

Hi Tom, Simon,
Quoted 6 lines · Alexey Brodkin
Even though board_init_f_mem() is not used on x86 today there's no reason to not use it in the future.
Moreover board_init_f_mem() has nothing to do with any particular architecture so move it away from #else /* CONFIG_X86 */

Quoted 7 lines

Quoted 3 lines
Signed-off-by Alexey Brodkin <[email protected]>
Cc Simon Glass <[email protected]>
Cc Tom Rini <[email protected]>
Any comments on this one?
This is a prerequisite for ARC updates so would be good to have it merged sometime soon.

I must have missed something as it did not seem to change anything for ARC.

This breaks building on x86 though, so we can't take this patch as is. E.g.:

x86: + crownbay
+common/board_f.c: In function ?board_init_f_mem?:
+common/board_f.c:1092:5: error: lvalue required as left operand of assignment
+ gd = (struct global_data *)top;
+ ^

Regards,
Simon

Alexey Brodkin

author

2015-03-23 21:17 UTC #3 lore raw

PATCH 1/1 common/board_f: make board_init_f_mem() independent on !CONFIG_X86

Hi Simon,

Quoted 14 lines · Simon Glass

Quoted 13 lines · Alexey Brodkin
Hi Tom, Simon,
Quoted 9 lines · Alexey Brodkin
Even though board_init_f_mem() is not used on x86 today there's no reason to not use it in the future.
Moreover board_init_f_mem() has nothing to do with any particular architecture so move it away from #else /* CONFIG_X86 */
Signed-off-by Alexey Brodkin <[email protected]>
Cc Simon Glass <[email protected]>
Cc Tom Rini <[email protected]>

Quoted 5 lines

Quoted 3 lines
Any comments on this one?
This is a prerequisite for ARC updates so would be good to have it merged sometime soon.
I must have missed something as it did not seem to change anything for ARC.

I meant this series -
http://lists.denx.de/pipermail/u-boot/2015-March/208179.html

In particular here I wanted to use board_init_f_mem():
http://lists.denx.de/pipermail/u-boot/2015-March/208183.html

Note that in the previous patch
http://lists.denx.de/pipermail/u-boot/2015-March/208182.html I re-used former X86-only code sections in common/board_f.c - that's why I did need board_init_f_mem() to be separated from #ifdef CONFIG_X86 #else - I wanted to use both branches :)

Review

Quoted 7 lines

This breaks building on x86 though, so we can't take this patch as is. E.g.:
x86: + crownbay
(quoted diff) +4 -0
+common/board_f.c: In function ?board_init_f_mem?:
+common/board_f.c:1092:5: error: lvalue required as left operand of assignment
+  gd = (struct global_data *)top;
+     ^

That's why I wanted your opinion :)
Sandbox didn't show any problems and I didn't do makeall.

Because in case of X86 "gd" is an alias to get_fs_gd_ptr() we cannot do such assignments.

So then we'll need to keep board_init_f_mem() disabled for X86 like that:
--->8---
#ifndef CONFIG_X86
ulong board_init_f_mem(ulong top)

{
	/* Leave space for the stack we are running with now */
	top -= 0x40;

	top -= sizeof(struct global_data);
	top = ALIGN(top, 16);
	gd = (struct global_data *)top;
	memset((void *)gd, '\0', sizeof(*gd));

#ifdef CONFIG_SYS_MALLOC_F_LEN
	top -= CONFIG_SYS_MALLOC_F_LEN;
	gd->malloc_base = top;

#endif

	return top;
}

#endif
--->8---

Do you think that's OK? If so I'll send v2 shortly.

-Alexey

Simon Glass

2015-03-23 21:18 UTC #4 lore raw

PATCH 1/1 common/board_f: make board_init_f_mem() independent on !CONFIG_X86

Hi Alexey,

Quoted 66 lines · Alexey Brodkin

Hi Simon,
Quoted 14 lines · Simon Glass
Quoted 13 lines · Alexey Brodkin
Hi Tom, Simon,
Quoted 9 lines · Alexey Brodkin
Even though board_init_f_mem() is not used on x86 today there's no reason to not use it in the future.
Moreover board_init_f_mem() has nothing to do with any particular architecture so move it away from #else /* CONFIG_X86 */
Signed-off-by Alexey Brodkin <[email protected]>
Cc Simon Glass <[email protected]>
Cc Tom Rini <[email protected]>
Quoted 5 lines
Quoted 3 lines
Any comments on this one?
This is a prerequisite for ARC updates so would be good to have it merged sometime soon.
I must have missed something as it did not seem to change anything for ARC.
I meant this series -
http://lists.denx.de/pipermail/u-boot/2015-March/208179.html
In particular here I wanted to use board_init_f_mem():
http://lists.denx.de/pipermail/u-boot/2015-March/208183.html
Note that in the previous patch
http://lists.denx.de/pipermail/u-boot/2015-March/208182.html I re-used former X86-only code sections in common/board_f.c - that's why I did need board_init_f_mem() to be separated from #ifdef CONFIG_X86 #else - I wanted to use both branches :)
Review
Quoted 7 lines
This breaks building on x86 though, so we can't take this patch as is. E.g.:
x86: + crownbay
(quoted diff) +4 -0
+common/board_f.c: In function ?board_init_f_mem?:
+common/board_f.c:1092:5: error: lvalue required as left operand of assignment
+  gd = (struct global_data *)top;
+     ^
That's why I wanted your opinion :)
Sandbox didn't show any problems and I didn't do makeall.
Because in case of X86 "gd" is an alias to get_fs_gd_ptr() we cannot do such assignments.
So then we'll need to keep board_init_f_mem() disabled for X86 like that:
--->8---
#ifndef CONFIG_X86
ulong board_init_f_mem(ulong top)
{
        /* Leave space for the stack we are running with now */
        top -= 0x40;

        top -= sizeof(struct global_data);
        top = ALIGN(top, 16);
        gd = (struct global_data *)top;
        memset((void *)gd, '\0', sizeof(*gd));
#ifdef CONFIG_SYS_MALLOC_F_LEN
        top -= CONFIG_SYS_MALLOC_F_LEN;
        gd->malloc_base = top;

Quoted 8 lines

#endif
        return top;
}
#endif
--->8---
Do you think that's OK? If so I'll send v2 shortly.

Yes that should be fine.

Regards,
Simon