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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.