| Message ID | 1385577160-14826-1-git-send-email-abrodkin@synopsys.com |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
On 27 November 2013 11:32, Alexey Brodkin <Alexey.Brodkin@synopsys.com>wrote: > If U-Boot build with DEBUG enabled/defined the first call of "debug" > function (that dumps data to any available console) will happen before > zeroing of initial "gd" in init call "zero_global_data" in > "init_sequence_f". > > And if stack was not filled with zeros chances are high that > "gd->have_console" won't be 0. In its turn it will cause attempt to > output things to non-initialized yet serial console. > > So for safety and predictability we set "gd->have_console = 0". > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > Acked-by: Simon Glass <sjg@chromium.org> I have a similar patch locally, but it actually does memset() on the whole structure. Some archs handle this setup differently. For example both ARM and x86 now allocate it in low level code so there is no need for the board_f code to allocate a global data structure. Regards, Simon > > Cc: Mischa Jonker <mjonker@synopsys.com> > Cc: Wolfgang Denk <wd@denx.de> > Cc: Simon Glass <sjg@chromium.org> > --- > common/board_f.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/common/board_f.c b/common/board_f.c > index f0664bc..fcfd713 100644 > --- a/common/board_f.c > +++ b/common/board_f.c > @@ -1010,6 +1010,7 @@ void board_init_f(ulong boot_flags) > #endif > > gd->flags = boot_flags; > + gd->have_console = 0; > > if (initcall_run_list(init_sequence_f)) > hang(); > -- > 1.8.4.2 > >
On Wed, 2013-11-27 at 18:43 -0700, Simon Glass wrote: > I have a similar patch locally, but it actually does memset() on the > whole structure. Some archs handle this setup differently. For example > both ARM and x86 now allocate it in low level code so there is no need > for the board_f code to allocate a global data structure. > Another approach would be to move "zero_global_data" from "init_sequence_f" to the very beginning of "board_init_f" right after global data allocation. But IMHO it is a bit more significant modification so I decided to start from the simplest fix that resolves a particular issue I see. -Alexey
On Thu, 2013-11-28 at 13:55 +0400, Alexey Brodkin wrote: > On Wed, 2013-11-27 at 18:43 -0700, Simon Glass wrote: > > > > I have a similar patch locally, but it actually does memset() on the > > whole structure. Some archs handle this setup differently. For example > > both ARM and x86 now allocate it in low level code so there is no need > > for the board_f code to allocate a global data structure. > > > > Another approach would be to move "zero_global_data" from > "init_sequence_f" to the very beginning of "board_init_f" right after > global data allocation. > > But IMHO it is a bit more significant modification so I decided to start > from the simplest fix that resolves a particular issue I see. Any chance for this patch to be applied or described problem has to be resolved/worked-around in any other way? -Alexey
On 10 December 2013 14:47, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > On Thu, 2013-11-28 at 13:55 +0400, Alexey Brodkin wrote: >> On Wed, 2013-11-27 at 18:43 -0700, Simon Glass wrote: >> >> >> > I have a similar patch locally, but it actually does memset() on the >> > whole structure. Some archs handle this setup differently. For example >> > both ARM and x86 now allocate it in low level code so there is no need >> > for the board_f code to allocate a global data structure. >> > >> >> Another approach would be to move "zero_global_data" from >> "init_sequence_f" to the very beginning of "board_init_f" right after >> global data allocation. >> >> But IMHO it is a bit more significant modification so I decided to start >> from the simplest fix that resolves a particular issue I see. > > Any chance for this patch to be applied or described problem has to be > resolved/worked-around in any other way? I say apply it. Tom would you like me to do a pull request, or do you want to pick it up? Regards, Simon
On Wed, Nov 27, 2013 at 10:32:40PM +0400, Alexey Brodkin wrote: > If U-Boot build with DEBUG enabled/defined the first call of "debug" > function (that dumps data to any available console) will happen before > zeroing of initial "gd" in init call "zero_global_data" in > "init_sequence_f". > > And if stack was not filled with zeros chances are high that > "gd->have_console" won't be 0. In its turn it will cause attempt to > output things to non-initialized yet serial console. > > So for safety and predictability we set "gd->have_console = 0". > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > Cc: Mischa Jonker <mjonker@synopsys.com> > Cc: Wolfgang Denk <wd@denx.de> > Cc: Simon Glass <sjg@chromium.org> > Acked-by: Simon Glass <sjg@chromium.org> Applied to u-boot/master, thanks!
diff --git a/common/board_f.c b/common/board_f.c index f0664bc..fcfd713 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -1010,6 +1010,7 @@ void board_init_f(ulong boot_flags) #endif gd->flags = boot_flags; + gd->have_console = 0; if (initcall_run_list(init_sequence_f)) hang();
If U-Boot build with DEBUG enabled/defined the first call of "debug" function (that dumps data to any available console) will happen before zeroing of initial "gd" in init call "zero_global_data" in "init_sequence_f". And if stack was not filled with zeros chances are high that "gd->have_console" won't be 0. In its turn it will cause attempt to output things to non-initialized yet serial console. So for safety and predictability we set "gd->have_console = 0". Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Cc: Mischa Jonker <mjonker@synopsys.com> Cc: Wolfgang Denk <wd@denx.de> Cc: Simon Glass <sjg@chromium.org> --- common/board_f.c | 1 + 1 file changed, 1 insertion(+)