From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Thu, 27 Jun 2013 10:27:26 +0200 Subject: [U-Boot] [PATCH v5 1/3] arm: spl: Fix SPL booting for OMAP3 In-Reply-To: <1372144452-14108-1-git-send-email-sr@denx.de> References: <1371200101-11510-1-git-send-email-sr@denx.de> <1372144452-14108-1-git-send-email-sr@denx.de> Message-ID: <20130627102726.1e1675bf@lilith> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Stefan, On Tue, 25 Jun 2013 09:14:12 +0200, Stefan Roese 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 > Cc: Tom Rini > Cc: Albert ARIBAUD > --- > 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. Amicalement, -- Albert.