diff mbox

[U-Boot] Revert "ARM: SPL: do not set gd again"

Message ID 1410913643-31238-1-git-send-email-trini@ti.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Tom Rini Sept. 17, 2014, 12:27 a.m. UTC
At the high level, the problem is that we set gd multiple times (and
still do, even after the commit we're reverting).  We set important
parts of gd to the copy which is not above stack but rather in the data
section.  For the release, we're going to revert this change and for the
next release we shall correct things to only, really, set gd once to an
appropriate location and ensure that comments about it are correct too.

This reverts commit f0c3a6c4ad09210d5d4aeafe87685ee75e5683d6.

Acked-by: Albert Aribaud <albert.u.boot@aribaud.net>
Signed-off-by: Tom Rini <trini@ti.com>
---
 arch/arm/lib/spl.c |    3 +++
 1 file changed, 3 insertions(+)

Comments

Tom Rini Sept. 17, 2014, 12:47 a.m. UTC | #1
On Tue, Sep 16, 2014 at 08:27:23PM -0400, Tom Rini wrote:

> At the high level, the problem is that we set gd multiple times (and
> still do, even after the commit we're reverting).  We set important
> parts of gd to the copy which is not above stack but rather in the data
> section.  For the release, we're going to revert this change and for the
> next release we shall correct things to only, really, set gd once to an
> appropriate location and ensure that comments about it are correct too.
> 
> This reverts commit f0c3a6c4ad09210d5d4aeafe87685ee75e5683d6.
> 
> Acked-by: Albert Aribaud <albert.u.boot@aribaud.net>
> Signed-off-by: Tom Rini <trini@ti.com>

Applied to u-boot/master, thanks!
Simon Glass Nov. 10, 2014, 10:13 p.m. UTC | #2
+Albert

Hi Tom,

On 16 September 2014 18:47, Tom Rini <trini@ti.com> wrote:
>
> On Tue, Sep 16, 2014 at 08:27:23PM -0400, Tom Rini wrote:
>
> > At the high level, the problem is that we set gd multiple times (and
> > still do, even after the commit we're reverting).  We set important
> > parts of gd to the copy which is not above stack but rather in the data
> > section.  For the release, we're going to revert this change and for the
> > next release we shall correct things to only, really, set gd once to an
> > appropriate location and ensure that comments about it are correct too.
> >
> > This reverts commit f0c3a6c4ad09210d5d4aeafe87685ee75e5683d6.
> >
> > Acked-by: Albert Aribaud <albert.u.boot@aribaud.net>
> > Signed-off-by: Tom Rini <trini@ti.com>
>
> Applied to u-boot/master, thanks!

Is this going to be un-reverted? I will need this done to apply the
driver model SPL series.

Regards,
Simon
Tom Rini Nov. 11, 2014, 9:57 p.m. UTC | #3
On Mon, Nov 10, 2014 at 03:13:44PM -0700, Simon Glass wrote:
> +Albert
> 
> Hi Tom,
> 
> On 16 September 2014 18:47, Tom Rini <trini@ti.com> wrote:
> >
> > On Tue, Sep 16, 2014 at 08:27:23PM -0400, Tom Rini wrote:
> >
> > > At the high level, the problem is that we set gd multiple times (and
> > > still do, even after the commit we're reverting).  We set important
> > > parts of gd to the copy which is not above stack but rather in the data
> > > section.  For the release, we're going to revert this change and for the
> > > next release we shall correct things to only, really, set gd once to an
> > > appropriate location and ensure that comments about it are correct too.
> > >
> > > This reverts commit f0c3a6c4ad09210d5d4aeafe87685ee75e5683d6.
> > >
> > > Acked-by: Albert Aribaud <albert.u.boot@aribaud.net>
> > > Signed-off-by: Tom Rini <trini@ti.com>
> >
> > Applied to u-boot/master, thanks!
> 
> Is this going to be un-reverted? I will need this done to apply the
> driver model SPL series.

So I've got am335x working again locally and now I'm trying to see if we
need to introduce a SoC-specific board_init_f for SPL here or not or if
I can shove save_omap_boot_params() into spl_board_init() and add
preloader_console_init rather generically to the ARM board_init_f SPL
function.  Once I've got this clean enough I need to dust off some
davinci and omap3 targets, do similar changes and then see if Hans was
right about why my olimex Allwinner board was behaving badly, and if so,
test the changes there too.  That'll cover most of the ARM boards that
re-set gd themselves when they can't with the above change
re-introduced.
Simon Glass Nov. 12, 2014, 12:59 a.m. UTC | #4
Hi Tom,

On 11 November 2014 14:57, Tom Rini <trini@ti.com> wrote:
> On Mon, Nov 10, 2014 at 03:13:44PM -0700, Simon Glass wrote:
>> +Albert
>>
>> Hi Tom,
>>
>> On 16 September 2014 18:47, Tom Rini <trini@ti.com> wrote:
>> >
>> > On Tue, Sep 16, 2014 at 08:27:23PM -0400, Tom Rini wrote:
>> >
>> > > At the high level, the problem is that we set gd multiple times (and
>> > > still do, even after the commit we're reverting).  We set important
>> > > parts of gd to the copy which is not above stack but rather in the data
>> > > section.  For the release, we're going to revert this change and for the
>> > > next release we shall correct things to only, really, set gd once to an
>> > > appropriate location and ensure that comments about it are correct too.
>> > >
>> > > This reverts commit f0c3a6c4ad09210d5d4aeafe87685ee75e5683d6.
>> > >
>> > > Acked-by: Albert Aribaud <albert.u.boot@aribaud.net>
>> > > Signed-off-by: Tom Rini <trini@ti.com>
>> >
>> > Applied to u-boot/master, thanks!
>>
>> Is this going to be un-reverted? I will need this done to apply the
>> driver model SPL series.
>
> So I've got am335x working again locally and now I'm trying to see if we
> need to introduce a SoC-specific board_init_f for SPL here or not or if
> I can shove save_omap_boot_params() into spl_board_init() and add
> preloader_console_init rather generically to the ARM board_init_f SPL
> function.  Once I've got this clean enough I need to dust off some
> davinci and omap3 targets, do similar changes and then see if Hans was
> right about why my olimex Allwinner board was behaving badly, and if so,
> test the changes there too.  That'll cover most of the ARM boards that
> re-set gd themselves when they can't with the above change
> re-introduced.

OK thanks. I might pull in some of the non-dependent patches in the meantime.

Regards,
Simon
Jeroen Hofstee Nov. 28, 2014, 9:48 p.m. UTC | #5
Hi all,

On 11-11-14 22:57, Tom Rini wrote:
> On Mon, Nov 10, 2014 at 03:13:44PM -0700, Simon Glass wrote:
>> +Albert
>>
>> Hi Tom,
>>
>> On 16 September 2014 18:47, Tom Rini <trini@ti.com> wrote:
>>> On Tue, Sep 16, 2014 at 08:27:23PM -0400, Tom Rini wrote:
>>>
>>>> At the high level, the problem is that we set gd multiple times (and
>>>> still do, even after the commit we're reverting).  We set important
>>>> parts of gd to the copy which is not above stack but rather in the data
>>>> section.  For the release, we're going to revert this change and for the
>>>> next release we shall correct things to only, really, set gd once to an
>>>> appropriate location and ensure that comments about it are correct too.
>>>>
>>>> This reverts commit f0c3a6c4ad09210d5d4aeafe87685ee75e5683d6.
>>>>
>>>> Acked-by: Albert Aribaud <albert.u.boot@aribaud.net>
>>>> Signed-off-by: Tom Rini <trini@ti.com>
>>> Applied to u-boot/master, thanks!
>> Is this going to be un-reverted? I will need this done to apply the
>> driver model SPL series.
> So I've got am335x working again locally and now I'm trying to see if we
> need to introduce a SoC-specific board_init_f for SPL here or not or if
> I can shove save_omap_boot_params() into spl_board_init() and add
> preloader_console_init rather generically to the ARM board_init_f SPL
> function.  Once I've got this clean enough I need to dust off some
> davinci and omap3 targets, do similar changes and then see if Hans was
> right about why my olimex Allwinner board was behaving badly, and if so,
> test the changes there too.  That'll cover most of the ARM boards that
> re-set gd themselves when they can't with the above change
> re-introduced.
>
>

Any progress on this?

Regards,
Jeroen
diff mbox

Patch

diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
index 75ab546..dfcc596 100644
--- a/arch/arm/lib/spl.c
+++ b/arch/arm/lib/spl.c
@@ -28,6 +28,9 @@  void __weak board_init_f(ulong dummy)
 	/* Clear the BSS. */
 	memset(__bss_start, 0, __bss_end - __bss_start);
 
+	/* Set global data pointer. */
+	gd = &gdata;
+
 	board_init_r(NULL, 0);
 }