From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Mon, 2 Feb 2015 12:26:31 -0700 Subject: [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S In-Reply-To: <20150202185657.GH10826@bill-the-cat> References: <20150122162058.GF10826@bill-the-cat> <54C14B0E.9000308@redhat.com> <20150122210357.GG10826@bill-the-cat> <54C20C34.1090908@redhat.com> <20150126151802.GB10826@bill-the-cat> <54C69659.3060500@redhat.com> <20150127142347.GK10826@bill-the-cat> <20150131222550.72782f77@lilith> <20150131214956.GC10826@bill-the-cat> <20150202185657.GH10826@bill-the-cat> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de +Bin Hi Tom, On 2 February 2015 at 11:56, Tom Rini wrote: > > On Sat, Jan 31, 2015 at 03:14:35PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On 31 January 2015 at 14:49, Tom Rini wrote: > > > > > > On Sat, Jan 31, 2015 at 10:25:50PM +0100, Albert ARIBAUD wrote: > > > > Hello Tom, > > > > > > > > On Tue, 27 Jan 2015 09:23:47 -0500, Tom Rini wrote: > > > > > On Mon, Jan 26, 2015 at 08:32:41PM +0100, Hans de Goede wrote: > > > > > > Hi, > > > > > > > > > > > > On 26-01-15 16:18, Tom Rini wrote: > > > > > > >On Fri, Jan 23, 2015 at 09:54:12AM +0100, Hans de Goede wrote: > > > > > > >>Hi, > > > > > > >> > > > > > > >>On 22-01-15 22:03, Tom Rini wrote: > > > > > > >>>On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote: > > > > > > >>>>Hi, > > > > > > >>>> > > > > > > >>>>On 22-01-15 17:20, Tom Rini wrote: > > > > > > >>>>>On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote: > > > > > > >>>>> > > > > > > >>>>>>On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the > > > > > > >>>>>>icache, etc. Add a soc_init hook with a weak default which just calls > > > > > > >>>>>>cpu_init_cp15. > > > > > > >>>>>> > > > > > > >>>>>>This way different implementations can be provided to do some extra work > > > > > > >>>>>>before or after cpu_init_cp15, or completely replacing cpu_init_cp15. > > > > > > >>>>>> > > > > > > >>>>>>Signed-off-by: Hans de Goede > > > > > > >>>>>>--- > > > > > > >>>>>> arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++- > > > > > > >>>>>> 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > >>>>>> > > > > > > >>>>>>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S > > > > > > >>>>>>index fdc05b9..9882b20 100644 > > > > > > >>>>>>--- a/arch/arm/cpu/armv7/start.S > > > > > > >>>>>>+++ b/arch/arm/cpu/armv7/start.S > > > > > > >>>>>>@@ -64,7 +64,7 @@ reset: > > > > > > >>>>>> > > > > > > >>>>>> /* the mask ROM code should have PLL and others stable */ > > > > > > >>>>>> #ifndef CONFIG_SKIP_LOWLEVEL_INIT > > > > > > >>>>>>- bl cpu_init_cp15 > > > > > > >>>>>>+ bl soc_init > > > > > > >>>>>> bl cpu_init_crit > > > > > > >>>>>> #endif > > > > > > >>>>> > > > > > > >>>>>I like the direction here. And I want to make sure I get the sunxi > > > > > > >>>>>direction right here too (as I agree with the need / desire for boot0 + > > > > > > >>>>>U-Boot to be a valid combination). I think we can take this a step > > > > > > >>>>>farther. cpu_init_crit (on armv7) is basically a call to s_init(). > > > > > > >>>>> > > > > > > >>>>>For am33xx (and I bet but need to do and test omap3+) we can, with > > > > > > >>>>>Simon's patch to let us move stack to DDR a tiny bit later, in the SPL > > > > > > >>>>>case make s_init empty, which just leaves us with (with your patch) > > > > > > >>>>>soc_init. Is there some way we can put all of this together in a > > > > > > >>>>>function? > > > > > > >>>> > > > > > > >>>>You mean essentially call s_init here and have s_init call cpu_init_cp15 > > > > > > >>>>I guess we could do that, but it would require auditing all existing armv7 > > > > > > >>>>users of s_init. This may require me to rethink how / when I do timer & > > > > > > >>>>gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) > > > > > > >>>>problem. > > > > > > >>> > > > > > > >>>Basically. From my first pass audit of s_init, it's either empty > > > > > > >>>(Kona), sunxi, or omap/etc so I get to deal with it. And the default > > > > > > >>>soc_init would just be the call to cpu_init_cp15 as you have it and we > > > > > > >>>drop the lowlevel_init hurdles. > > > > > > >> > > > > > > >>Ok, so what you're suggesting is a patch which: > > > > > > >> > > > > > > >>1) Changes: > > > > > > >> > > > > > > >>#ifndef CONFIG_SKIP_LOWLEVEL_INIT > > > > > > >> bl cpu_init_cp15 > > > > > > >> bl cpu_init_crit > > > > > > >>#endif > > > > > > >> > > > > > > >>Into: > > > > > > >> > > > > > > >>#ifndef CONFIG_SKIP_LOWLEVEL_INIT > > > > > > >> bl lowlevel_init > > > > > > >>#endif > > > > > > >> > > > > > > >>Which will setup the stack and then call the s_init C function > > > > > > >> > > > > > > >>2) Adds a weak default s_init which calls cpu_init_cp15 > > > > > > >> > > > > > > >>3) Patch all existing s_init functions to call cpu_init_cp15 > > > > > > >>before doing anything else. > > > > > > > > > > > > > >Pretty close. Simon's SPL DM series and related clean-ups got me > > > > > > >thinking that yes, seemingly too much got shoved into "s_init" that > > > > > > >really could have been done using an existing hook done slightly later. > > > > > > > > > > > > > >>And then in follow up patches we can: > > > > > > >> > > > > > > >>4) Drop cpu_init_crit > > > > > > >> > > > > > > >>5) Cleanup some s_init functions (this will be left to the individual > > > > > > >>SoC maintainers) > > > > > > >> > > > > > > >>I think that is a good idea, Albert what do you think about this ? > > > > > > > > > > > > > >So I'd like to see 5 done "soon" afterwards as it's me (omap*) and > > > > > > >sunxi. I think we can simplfy the call sequence too, to roughly: > > > > > > >#ifndef CONFIG_SKIP_LOWLEVEL_INIT > > > > > > > ... Set up stack for C, it's just a few instrs > > > > > > > bl lowlevel_init > > > > > > >#endif > > > > > > > bl _main > > > > > > > > > > > > > >__weak asm > > > > > > >lowlevel_init: > > > > > > > bl cpu_init_cp15 > > > > > > > return to caller > > > > > > > > > > > > > >And comment that anything called via lowlevel_init must be C-callable. > > > > > > >I hope that once #5 is done no one actually has a lowlevel_init that's > > > > > > >done in C but we've kept the door open should it be needed down the > > > > > > >road (as I _think_ we can shuffle both the omap* and sunxi stuff to do > > > > > > >their inits as needed in both SPL and full U-Boot from an early hook in > > > > > > >board_init_r, top of my head is board_init calls some_other_func() in > > > > > > >full U-Boot to ensure GPIOs, etc, on sunxi and spl_board_init() calls > > > > > > >same func in SPL, and we can consolidate again further down the road as > > > > > > >we get SPL and full U-Boot more in sync on the call chain). > > > > > > > > > > > > Sounds good to me, and I'm fine with working the sunxi side of things. > > > > > > > > > > > > Since you seem to have this all in your head can you do a patch for this > > > > > > replacing my patchset ? > > > > > > > > > > I suppose that's what happens when you have a detailed plan, will do ;) > > > > > > > > I'll comment if the patch is out (going through my mail one at a time) > > > > but I just want to chime in on C contexts: I'd rather have this done > > > > only once and from crt0.S. If this means crt0.S must do the call to > > > > lowlevel_init, then so be it IMO. > > > > > > This might be workable, yeah. What I've decided after doing some of the > > > work is that the rest of arm has cpu_init_crit and some callouts and I'm > > > thinking that rather than make armv7 much different we should re-adjust > > > things to fit back into the regular mold which I think is possible. > > > > > > Simon, do you think we could move arch_cpu_init up in the call sequence > > > in common/board_f.c ? > > > > On x86 we set up early PCI then, since without that we cannot init the CPU. > > > > I suppose we could split it into two calls, but why do you want to > > move it earlier? > > Wait, since we need it early, why would moving it up earlier in > board_init_r() be a problem for x86? And (and this is being split into > different email threads, sigh), it would be good, possibly, if we have > something that means "very early init things, but we can be written in > C". Do you mean board_init_f() or board_init_r()? I was talking about board_init_f(). PCI gets inited twice on x86 (for some boards) - once before relocation and one after. Regards, Simon