From: Tom Rini <trini@ti.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v5 1/3] arm: spl: Fix SPL booting for OMAP3
Date: Wed, 3 Jul 2013 15:47:27 -0400 [thread overview]
Message-ID: <20130703194727.GD16630@bill-the-cat> (raw)
In-Reply-To: <20130627102726.1e1675bf@lilith>
On Thu, Jun 27, 2013 at 10:27:26AM +0200, Albert ARIBAUD wrote:
> Hi Stefan,
>
> On Tue, 25 Jun 2013 09:14:12 +0200, Stefan Roese <sr@denx.de> wrote:
>
> > Fix a problem with a re-assignment of r8 in the SPL version.
> >
> > This patch now moves the call to s_init() to a later stage, right before
> > calling board_init_f(). And makes sure that r8 is correctly initialized
> > before s_init() is called. r8 now is only written in crt0.S.
> >
> > This error was detected on the SPL port for the Compulab CM-T35 board
> > (OMAP3530).
> >
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > Cc: Tom Rini <trini@ti.com>
> > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> > ---
> > Albert, I'm not really happy with this patch as it evolves now. As you
> > will see, I had to make some further additions to crt0.S to fix a
> > problem for non-SPL builds and to fix compilation errors for non-OMAP
> > platforms. This gets quite ugly now. Looking back at my patch v1, this
> > looks much less intrusive.
> >
> > What do you think?
>
> I said the first patch was NAK, and the reasons I NAKed it remain.
>
> However, there might be another solution: instead of squeezing the
> call to s_init() in crt0.S right between the initial environment
> setting and the call to board_init_f(), we could simply move the
> s_init() call inside board_init_f().
>
> From a running conditions perspective, the only change would be that
> s_init() is going to run from a non-empty stack, but we know that there
> is free stack enough during board_init_f() to call functions.
>
> Moving the call to s_init() into board_init_f() removes any changes to
> crt0.S, which were my essential NAK reason and saves you some ugliness.
>
> I would even hazard that you could place s_init in init_sequence[], for
> instance as a first entry to be called (before arch_cpu_init). After
> all, the only difference in execution is that gdata is going to be
> initialized properly before s_init() kicks in.
>
> Also, a name change would be in order, because s_init() as a private
> OMAP function is ok, but as an init function invoked from board_init_f()
> it needs a more meaningful name.
So, this is one of the things that needs resolving for v2013.07. What
do you want to call the renamed s_init function? I think we need to go
the init_sequence route, and keep common/board_f.c in sync (I did a
trivial test the other week about moving am335x into the generic board
framework and it went fine, so I'll want to move all the TI boards I can
over soon). Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130703/5d1f8bfa/attachment.pgp>
next prev parent reply other threads:[~2013-07-03 19:47 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-14 8:54 [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Stefan Roese
2013-06-14 8:55 ` [U-Boot] [PATCH 2/3] arm: omap3: spl: Fix problem with 8bit NAND devices Stefan Roese
2013-07-30 13:25 ` [U-Boot] [U-Boot, " Tom Rini
2013-06-14 8:55 ` [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35 Stefan Roese
2013-06-17 11:53 ` Igor Grinberg
2013-06-17 12:38 ` Tom Rini
2013-06-17 13:38 ` Igor Grinberg
2013-06-17 13:52 ` Stefan Roese
2013-06-17 14:03 ` [U-Boot] [PATCH v2 " Stefan Roese
2013-06-18 6:14 ` Nikita Kiryanov
2013-07-30 10:52 ` [U-Boot] [PATCH v3 " Stefan Roese
2013-07-30 12:10 ` Albert ARIBAUD
2013-07-30 12:14 ` Stefan Roese
2013-11-15 7:51 ` [U-Boot] [PATCH v4] " Stefan Roese
[not found] ` <5288BBAC.2020307@compulab.co.il>
[not found] ` <5295BF6C.20902@compulab.co.il>
[not found] ` <5295C3F8.50101@denx.de>
[not found] ` <529E01B7.4070402@compulab.co.il>
2013-12-04 11:38 ` [U-Boot] Fwd: " Stefan Roese
2013-12-04 11:57 ` Tom Rini
2013-12-04 12:02 ` Stefan Roese
2013-12-04 12:15 ` Gupta, Pekon
2013-12-04 12:38 ` Stefan Roese
2013-06-20 16:42 ` [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Albert ARIBAUD
2013-06-20 17:01 ` Stefan Roese
2013-06-20 17:51 ` Albert ARIBAUD
2013-06-20 18:28 ` Stefan Roese
2013-06-20 19:18 ` Albert ARIBAUD
2013-06-21 2:13 ` [U-Boot] [PATCH v2 " Stefan Roese
2013-06-21 8:57 ` Albert ARIBAUD
2013-06-21 9:10 ` [U-Boot] [PATCH v3 " Stefan Roese
2013-06-21 10:30 ` Albert ARIBAUD
2013-06-21 10:39 ` Stefan Roese
2013-06-21 10:42 ` [U-Boot] [PATCH v4 " Stefan Roese
2013-06-21 11:02 ` Albert ARIBAUD
2013-06-25 7:14 ` [U-Boot] [PATCH v5 " Stefan Roese
2013-06-27 8:27 ` Albert ARIBAUD
2013-07-03 19:47 ` Tom Rini [this message]
2013-07-04 11:58 ` Albert ARIBAUD
2013-07-15 14:33 ` [U-Boot] [PATCH " Tom Rini
2013-07-16 6:24 ` Stefan Roese
2013-07-16 14:36 ` Tom Rini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130703194727.GD16630@bill-the-cat \
--to=trini@ti.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox