| Message ID | 1426493026-6821-1-git-send-email-abrodkin@synopsys.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Tom, Simon, On Mon, 2015-03-16 at 11:03 +0300, Alexey Brodkin wrote: > 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 <abrodkin@synopsys.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Tom Rini <trini@ti.com> Any comments on this one? This is a prerequisite for ARC updates so would be good to have it merged sometime soon. -Alexey
On 23 March 2015 at 10:40, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > Hi Tom, Simon, > > On Mon, 2015-03-16 at 11:03 +0300, Alexey Brodkin wrote: >> 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 <abrodkin@synopsys.com> >> Cc: Simon Glass <sjg@chromium.org> >> Cc: Tom Rini <trini@ti.com> > > 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
Hi Simon, On Mon, 2015-03-23 at 11:26 -0600, Simon Glass wrote: > On 23 March 2015 at 10:40, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > > Hi Tom, Simon, > > > > On Mon, 2015-03-16 at 11:03 +0300, Alexey Brodkin wrote: > >> 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 <abrodkin@synopsys.com> > >> Cc: Simon Glass <sjg@chromium.org> > >> Cc: Tom Rini <trini@ti.com> > > > > 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 :) > 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; > + ^ 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
Hi Alexey, On 23 March 2015 at 15:17, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > Hi Simon, > > On Mon, 2015-03-23 at 11:26 -0600, Simon Glass wrote: >> On 23 March 2015 at 10:40, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: >> > Hi Tom, Simon, >> > >> > On Mon, 2015-03-16 at 11:03 +0300, Alexey Brodkin wrote: >> >> 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 <abrodkin@synopsys.com> >> >> Cc: Simon Glass <sjg@chromium.org> >> >> Cc: Tom Rini <trini@ti.com> >> > >> > 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 :) > >> 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; >> + ^ > > 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. Yes that should be fine. Regards, Simon
diff --git a/common/board_f.c b/common/board_f.c index 731b539..f55550c 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -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 */
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 <abrodkin@synopsys.com> Cc: Simon Glass <sjg@chromium.org> Cc: Tom Rini <trini@ti.com> --- common/board_f.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)