diff mbox

[U-Boot,v4,3/4] generic board patch of manual reloc and zero gd_t

Message ID 1376995682-51646-4-git-send-email-fenghua@phytium.com.cn
State Superseded
Delegated to: Albert ARIBAUD
Headers show

Commit Message

fenghua@phytium.com.cn Aug. 20, 2013, 10:48 a.m. UTC 
From: David Feng <fenghua@phytium.com.cn>

1. function board_init_f in board_f.c should firstly zero gd_t structure
   before it call initcall_run_list, otherwise the debug print will go run
   if DEBUG is defined. Because the printf function will use global data
   to determine whether serial port is initialized and could be written.
2. function board_init_r in board_r.c should firstly relocate init_sequence_r
   table before it call initcall_run_list. Command table also should be relocated.

Signed-off-by: David Feng <fenghua@phytium.com.cn>
---
 common/board_f.c |    6 ++++++
 common/board_r.c |   17 +++++++++++++++++
 2 files changed, 23 insertions(+)

Comments

Simon Glass Aug. 21, 2013, 5:27 a.m. UTC | #1 
Hi David,

On Tue, Aug 20, 2013 at 4:48 AM,  <fenghua@phytium.com.cn> wrote:
> From: David Feng <fenghua@phytium.com.cn>
>
> 1. function board_init_f in board_f.c should firstly zero gd_t structure
>    before it call initcall_run_list, otherwise the debug print will go run
>    if DEBUG is defined. Because the printf function will use global data
>    to determine whether serial port is initialized and could be written.
> 2. function board_init_r in board_r.c should firstly relocate init_sequence_r
>    table before it call initcall_run_list. Command table also should be relocated.
>
> Signed-off-by: David Feng <fenghua@phytium.com.cn>
> ---
>  common/board_f.c |    6 ++++++
>  common/board_r.c |   17 +++++++++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/common/board_f.c b/common/board_f.c
> index 5e738fb..f437bcb 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -1009,6 +1009,12 @@ void board_init_f(ulong boot_flags)
>         gd = &data;
>  #endif
>
> +       /*
> +        * Zero gd_t first, otherwise the debug print in initcall_run_list
> +        * function before zero_global_data is called will go wrong.
> +        */
> +       memset((void *)gd, 0, sizeof(gd_t));
> +

Yes, we need this. You should drop the zero_global_data() or whatever
currently does the memset()

>         gd->flags = boot_flags;
>
>         if (initcall_run_list(init_sequence_f))
> diff --git a/common/board_r.c b/common/board_r.c
> index 86ca1cb..1b4bdd2 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -157,6 +157,13 @@ static int initr_reloc_global_data(void)
>          */
>         gd->env_addr += gd->relocaddr - CONFIG_SYS_MONITOR_BASE;
>  #endif
> +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> +       /*
> +        * We have to relocate the command table manually
> +        */
> +       fixup_cmdtable(ll_entry_start(cmd_tbl_t, cmd),
> +                       ll_entry_count(cmd_tbl_t, cmd));
> +#endif /* CONFIG_NEEDS_MANUAL_RELOC */

Should this be done here or in main_loop()? How is this currently done
when not using generic board?

>         return 0;
>  }
>
> @@ -899,6 +906,7 @@ init_fnc_t init_sequence_r[] = {
>         initr_modem,
>  #endif
>         run_main_loop,
> +       NULL,
>  };
>
>  void board_init_r(gd_t *new_gd, ulong dest_addr)
> @@ -906,6 +914,15 @@ void board_init_r(gd_t *new_gd, ulong dest_addr)
>  #ifndef CONFIG_X86
>         gd = new_gd;
>  #endif
> +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> +       /*
> +        * We have to relocate the init_sequence_r table manually
> +        */
> +       init_fnc_t      *init_fnc_ptr;
> +       for (init_fnc_ptr = init_sequence_r; *init_fnc_ptr; ++init_fnc_ptr)
> +               *init_fnc_ptr = (init_fnc_t *)((unsigned long)(*init_fnc_ptr) + gd->reloc_off);
> +#endif /* CONFIG_NEEDS_MANUAL_RELOC */
> +
>         if (initcall_run_list(init_sequence_r))
>                 hang();
>

Regards,
Simon
fenghua@phytium.com.cn Aug. 21, 2013, 1:15 p.m. UTC | #2 
> -----原始邮件-----
> 发件人: "Simon Glass" <sjg@chromium.org>
> 发送时间: 2013年8月21日 星期三
> 收件人: FengHua <fenghua@phytium.com.cn>
> 抄送: "U-Boot Mailing List" <u-boot@lists.denx.de>, "trini@ti.com" <trini@ti.com>
> 主题: Re: [U-Boot] [PATCH v4 3/4] generic board patch of manual reloc and zero gd_t
> 
> Hi David,
> 
> On Tue, Aug 20, 2013 at 4:48 AM,  <fenghua@phytium.com.cn> wrote:
> > From: David Feng <fenghua@phytium.com.cn>
> >
> > 1. function board_init_f in board_f.c should firstly zero gd_t structure
> >    before it call initcall_run_list, otherwise the debug print will go run
> >    if DEBUG is defined. Because the printf function will use global data
> >    to determine whether serial port is initialized and could be written.
> > 2. function board_init_r in board_r.c should firstly relocate init_sequence_r
> >    table before it call initcall_run_list. Command table also should be relocated.
> >
> > Signed-off-by: David Feng <fenghua@phytium.com.cn>
> > ---
> >  common/board_f.c |    6 ++++++
> >  common/board_r.c |   17 +++++++++++++++++
> >  2 files changed, 23 insertions(+)
> >
> > diff --git a/common/board_f.c b/common/board_f.c
> > index 5e738fb..f437bcb 100644
> > --- a/common/board_f.c
> > +++ b/common/board_f.c
> > @@ -1009,6 +1009,12 @@ void board_init_f(ulong boot_flags)
> >         gd = &data;
> >  #endif
> >
> > +       /*
> > +        * Zero gd_t first, otherwise the debug print in initcall_run_list
> > +        * function before zero_global_data is called will go wrong.
> > +        */
> > +       memset((void *)gd, 0, sizeof(gd_t));
> > +
> 
> Yes, we need this. You should drop the zero_global_data() or whatever
> currently does the memset()
> 
> >         gd->flags = boot_flags;
> >
> >         if (initcall_run_list(init_sequence_f))
> > diff --git a/common/board_r.c b/common/board_r.c
> > index 86ca1cb..1b4bdd2 100644
> > --- a/common/board_r.c
> > +++ b/common/board_r.c
> > @@ -157,6 +157,13 @@ static int initr_reloc_global_data(void)
> >          */
> >         gd->env_addr += gd->relocaddr - CONFIG_SYS_MONITOR_BASE;
> >  #endif
> > +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> > +       /*
> > +        * We have to relocate the command table manually
> > +        */
> > +       fixup_cmdtable(ll_entry_start(cmd_tbl_t, cmd),
> > +                       ll_entry_count(cmd_tbl_t, cmd));
> > +#endif /* CONFIG_NEEDS_MANUAL_RELOC */
> 
> Should this be done here or in main_loop()? How is this currently done
> when not using generic board?

Most of these operations appear in board_init_r() function, so we can not do this in main_loop(), otherwise we must get rid of this from all related board_init_r() functions.

> 
> >         return 0;
> >  }
> >
> > @@ -899,6 +906,7 @@ init_fnc_t init_sequence_r[] = {
> >         initr_modem,
> >  #endif
> >         run_main_loop,
> > +       NULL,
> >  };
> >
> >  void board_init_r(gd_t *new_gd, ulong dest_addr)
> > @@ -906,6 +914,15 @@ void board_init_r(gd_t *new_gd, ulong dest_addr)
> >  #ifndef CONFIG_X86
> >         gd = new_gd;
> >  #endif
> > +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> > +       /*
> > +        * We have to relocate the init_sequence_r table manually
> > +        */
> > +       init_fnc_t      *init_fnc_ptr;
> > +       for (init_fnc_ptr = init_sequence_r; *init_fnc_ptr; ++init_fnc_ptr)
> > +               *init_fnc_ptr = (init_fnc_t *)((unsigned long)(*init_fnc_ptr) + gd->reloc_off);
> > +#endif /* CONFIG_NEEDS_MANUAL_RELOC */
> > +
> >         if (initcall_run_list(init_sequence_r))
> >                 hang();
> >
> 
> Regards,
> Simon
Scott Wood Aug. 22, 2013, 12:10 a.m. UTC | #3 
On Tue, 2013-08-20 at 23:27 -0600, Simon Glass wrote:
> Hi David,
> 
> On Tue, Aug 20, 2013 at 4:48 AM,  <fenghua@phytium.com.cn> wrote:
> > diff --git a/common/board_r.c b/common/board_r.c
> > index 86ca1cb..1b4bdd2 100644
> > --- a/common/board_r.c
> > +++ b/common/board_r.c
> > @@ -157,6 +157,13 @@ static int initr_reloc_global_data(void)
> >          */
> >         gd->env_addr += gd->relocaddr - CONFIG_SYS_MONITOR_BASE;
> >  #endif
> > +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> > +       /*
> > +        * We have to relocate the command table manually
> > +        */
> > +       fixup_cmdtable(ll_entry_start(cmd_tbl_t, cmd),
> > +                       ll_entry_count(cmd_tbl_t, cmd));
> > +#endif /* CONFIG_NEEDS_MANUAL_RELOC */
> 
> Should this be done here or in main_loop()? How is this currently done
> when not using generic board?

It shouldn't be done at all -- let's not revive manual relocations.
I'll try to get proper relocation working.

-Scott
fenghua@phytium.com.cn Aug. 22, 2013, 1:31 a.m. UTC | #4 
> -----原始邮件-----
> 发件人: "Scott Wood" <scottwood@freescale.com>
> 发送时间: 2013年8月22日 星期四
> 收件人: "Simon Glass" <sjg@chromium.org>
> 抄送: FengHua <fenghua@phytium.com.cn>, "trini@ti.com" <trini@ti.com>, "U-Boot
>  Mailing List" <u-boot@lists.denx.de>
> 主题: Re: [U-Boot] [PATCH v4 3/4] generic board patch of manual reloc and zero gd_t
> 
> On Tue, 2013-08-20 at 23:27 -0600, Simon Glass wrote:
> > Hi David,
> > 
> > On Tue, Aug 20, 2013 at 4:48 AM,  <fenghua@phytium.com.cn> wrote:
> > > diff --git a/common/board_r.c b/common/board_r.c
> > > index 86ca1cb..1b4bdd2 100644
> > > --- a/common/board_r.c
> > > +++ b/common/board_r.c
> > > @@ -157,6 +157,13 @@ static int initr_reloc_global_data(void)
> > >          */
> > >         gd->env_addr += gd->relocaddr - CONFIG_SYS_MONITOR_BASE;
> > >  #endif
> > > +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> > > +       /*
> > > +        * We have to relocate the command table manually
> > > +        */
> > > +       fixup_cmdtable(ll_entry_start(cmd_tbl_t, cmd),
> > > +                       ll_entry_count(cmd_tbl_t, cmd));
> > > +#endif /* CONFIG_NEEDS_MANUAL_RELOC */
> > 
> > Should this be done here or in main_loop()? How is this currently done
> > when not using generic board?
> 
> It shouldn't be done at all -- let's not revive manual relocations.
> I'll try to get proper relocation working.
> 
> -Scott
> 
> 

Even the rela relocation of aarch64 works we should keep this here. The generic board could be used by any other platform that need manual relocation.
>
Tom Rini Aug. 22, 2013, 12:58 p.m. UTC | #5 
On Thu, Aug 22, 2013 at 09:31:35AM +0800, FengHua wrote:
> 
> 
> 
> > -----????????????-----
> > ?????????: "Scott Wood" <scottwood@freescale.com>
> > ????????????: 2013???8???22??? ?????????
> > ?????????: "Simon Glass" <sjg@chromium.org>
> > ??????: FengHua <fenghua@phytium.com.cn>, "trini@ti.com" <trini@ti.com>, "U-Boot
> >  Mailing List" <u-boot@lists.denx.de>
> > ??????: Re: [U-Boot] [PATCH v4 3/4] generic board patch of manual reloc and zero gd_t
> > 
> > On Tue, 2013-08-20 at 23:27 -0600, Simon Glass wrote:
> > > Hi David,
> > > 
> > > On Tue, Aug 20, 2013 at 4:48 AM,  <fenghua@phytium.com.cn> wrote:
> > > > diff --git a/common/board_r.c b/common/board_r.c
> > > > index 86ca1cb..1b4bdd2 100644
> > > > --- a/common/board_r.c
> > > > +++ b/common/board_r.c
> > > > @@ -157,6 +157,13 @@ static int initr_reloc_global_data(void)
> > > >          */
> > > >         gd->env_addr += gd->relocaddr - CONFIG_SYS_MONITOR_BASE;
> > > >  #endif
> > > > +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> > > > +       /*
> > > > +        * We have to relocate the command table manually
> > > > +        */
> > > > +       fixup_cmdtable(ll_entry_start(cmd_tbl_t, cmd),
> > > > +                       ll_entry_count(cmd_tbl_t, cmd));
> > > > +#endif /* CONFIG_NEEDS_MANUAL_RELOC */
> > > 
> > > Should this be done here or in main_loop()? How is this currently done
> > > when not using generic board?
> > 
> > It shouldn't be done at all -- let's not revive manual relocations.
> > I'll try to get proper relocation working.
> 
> Even the rela relocation of aarch64 works we should keep this here.
> The generic board could be used by any other platform that need manual
> relocation.

No, the point is that manual relocation is legacy and new platforms
should be using proper relocation.
diff mbox

Patch

diff --git a/common/board_f.c b/common/board_f.c
index 5e738fb..f437bcb 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -1009,6 +1009,12 @@  void board_init_f(ulong boot_flags)
 	gd = &data;
 #endif
 
+	/*
+	 * Zero gd_t first, otherwise the debug print in initcall_run_list
+	 * function before zero_global_data is called will go wrong.
+	 */
+	memset((void *)gd, 0, sizeof(gd_t));
+
 	gd->flags = boot_flags;
 
 	if (initcall_run_list(init_sequence_f))
diff --git a/common/board_r.c b/common/board_r.c
index 86ca1cb..1b4bdd2 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -157,6 +157,13 @@  static int initr_reloc_global_data(void)
 	 */
 	gd->env_addr += gd->relocaddr - CONFIG_SYS_MONITOR_BASE;
 #endif
+#ifdef CONFIG_NEEDS_MANUAL_RELOC
+	/*
+	 * We have to relocate the command table manually
+	 */
+	fixup_cmdtable(ll_entry_start(cmd_tbl_t, cmd),
+			ll_entry_count(cmd_tbl_t, cmd));
+#endif /* CONFIG_NEEDS_MANUAL_RELOC */
 	return 0;
 }
 
@@ -899,6 +906,7 @@  init_fnc_t init_sequence_r[] = {
 	initr_modem,
 #endif
 	run_main_loop,
+	NULL,
 };
 
 void board_init_r(gd_t *new_gd, ulong dest_addr)
@@ -906,6 +914,15 @@  void board_init_r(gd_t *new_gd, ulong dest_addr)
 #ifndef CONFIG_X86
 	gd = new_gd;
 #endif
+#ifdef CONFIG_NEEDS_MANUAL_RELOC
+	/*
+	 * We have to relocate the init_sequence_r table manually
+	 */
+	init_fnc_t	*init_fnc_ptr;
+	for (init_fnc_ptr = init_sequence_r; *init_fnc_ptr; ++init_fnc_ptr)
+		*init_fnc_ptr = (init_fnc_t *)((unsigned long)(*init_fnc_ptr) + gd->reloc_off);
+#endif /* CONFIG_NEEDS_MANUAL_RELOC */
+
 	if (initcall_run_list(init_sequence_r))
 		hang();