From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Sun, 7 Oct 2012 19:19:37 +0200 Subject: [U-Boot] [PATCH] arm: rmobile: bugfix: wrong register saving in lowlevel_init In-Reply-To: <50719D43.7010502@myspectrum.nl> References: <20121004183155.GA1809@bill-the-cat> <1349411962-29202-1-git-send-email-koba@kmckk.co.jp> <20121005182308.475b3406@lilith> <50719D43.7010502@myspectrum.nl> Message-ID: <20121007191937.4b39d734@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 Jeroen, On Sun, 07 Oct 2012 17:18:27 +0200, Jeroen Hofstee wrote: > Hello All, > > On 10/07/2012 01:34 PM, Enric Balletb? i Serra wrote: > > Hi Albert, > > > > 2012/10/5 Albert ARIBAUD : > >> Hi Tetsuyuki, > >> > >> On Fri, 5 Oct 2012 13:39:22 +0900, Tetsuyuki Kobayashi > >> wrote: > >> > >>> lowlevel_init() of rmobile badly assumed that ip register holds return address. > >>> The commit "63ee53a7 armv7 cpu_init_crit: Simplify code" breaks this assumption. > >>> This patch removes this bad assumption and simplify code. > >>> > >>> Signed-off-by: Tetsuyuki Kobayashi > >>> --- > >>> > >> ... > > Note that the patch that Tetsuyuki says also breaks SPL support for > > OMAP3 boards, at least my IGEP boards doesn't boot and hangs at SPL > > level. > > > > U-Boot SPL 2012.10-rc1-00244-g28e5ac2 (Oct 07 2012 - 13:11:29) > > > > Bisecting the problem I encountered the problem is the commit > > "63ee53a7 armv7 cpu_init_crit: Simplify code". > > > > Cheers, > > Enric > > > I can confirm above. Also the tam3517 som (omap3) fails to boot due to > mentioned commit. The patch from Tetsuyuki is arch specific (rmobile) so > that won't fix the omap case. Reverting the patch, 63ee53a, does help. > > Is there anything against reverting the patch (at least for the release...)? Here is my opinion: 1) I think patch 63ee53a7 is right in considering there is no need for cpu_init_crit to save lr in ip before calling lowlevel_init especially considering this is a tail call. Only lowlevel_init can tell if it uses ip or lr for its own purposes, thus any saving of ip and/or lr due to the workings of lowlevel_init should be performed in lowlevel_init. 2) I am not sure that the patch in this discussion depends on 63ee53a7, because IIUC, the patch simply saves ip "on a stack" then lr into ip, and after running s_init, restores from ip and ip from the stack; it never assumes ip contains a return address. I know we're that close to the release, but I want to be sure we understand what needs fixing. Kobayashi, Jeroen, can you indicate precisely how the issues you encounter are related to 63ee53a7? > Regards, > Jeroen Amicalement, -- Albert.