From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Sun, 21 Dec 2014 16:15:39 -0700 Subject: [U-Boot] [PATCH] RFC: am35xx: Rearrange SPL on am35xx In-Reply-To: <20141221192939.GB10826@bill-the-cat> References: <1418948481-31124-1-git-send-email-sjg@chromium.org> <20141219144018.GT20704@bill-the-cat> <20141221123011.GA10826@bill-the-cat> <20141221192939.GB10826@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 Hi Tom, On 21 December 2014 at 12:29, Tom Rini wrote: > On Sun, Dec 21, 2014 at 11:53:21AM -0700, Simon Glass wrote: >> Hi Tom, >> >> On 21 December 2014 at 05:30, Tom Rini wrote: >> > On Fri, Dec 19, 2014 at 08:32:35AM -0700, Simon Glass wrote: >> >> Hi Tom, Albert, >> >> >> >> On 19 December 2014 at 07:40, Tom Rini wrote: >> >> > On Thu, Dec 18, 2014 at 05:21:21PM -0700, Simon Glass wrote: >> >> > >> >> >> This is an attempt to tidy up the early SPL code in an attempt to pave >> >> >> the way for driver model in SPL: >> >> >> >> >> >> - Avoid setting up SDRAM before board_init_f() >> >> >> - Avoid touching global_data before board_init_f() >> >> >> - Allow board_init_f() to set up a new stack (seems that the SRAM stack >> >> >> is not large enough on these boards) >> >> >> >> >> >> This needs more work but it does boot on Beaglebone Black. >> >> >> >> >> >> Signed-off-by: Simon Glass >> >> >> --- >> >> >> >> >> >> arch/arm/cpu/armv7/am33xx/board.c | 60 ++++++++++++++++++++++++++------------ >> >> >> arch/arm/cpu/armv7/lowlevel_init.S | 4 --- >> >> >> arch/arm/include/asm/spl.h | 3 ++ >> >> >> arch/arm/lib/crt0.S | 9 ++++++ >> >> >> include/configs/ti_armv7_common.h | 5 ++-- >> >> >> 5 files changed, 56 insertions(+), 25 deletions(-) >> >> > >> >> > This takes things in the wrong direction I think. Since omap3/4/5 have >> >> > the same problem we're going to have to duplicate a bunch of this code. >> >> > But we can do omap_save_boot_params a bit later I'm pretty sure we can >> >> > shove it into spl_board_init() in >> >> > arch/arm/cpu/armv7/omap-common/boot-common.c and I'm going to do my best >> >> > to do that today and test it on at least a few boards. >> >> >> >> I don't have a lot of background on SPL stuff as I only know one >> >> implementation in detail. So these comments may be a bit off. >> >> >> >> There seem to be two drivers causing this oddness: >> >> >> >> 1. The need to save boot params before global_data is available. I >> >> wonder if it is possible to avoid overwriting the boot params, and >> >> save them later, in board_init_f()? If not, then I don't think the >> >> global_data structure should be used. A static local variable in the >> >> data section, with just a few fields in it, could be used instead. >> >> That avoids the temptation to thing that we are creating a global_data >> >> structure before crt0.S does it officially. If the data had just been >> >> stored into the data section, without messing with global_data, then I >> >> don't think we would have this problem. >> > >> > This is actually covered today, really. We do "save_boot_params" first >> > thing and that function must save a magic register value from ROM (or >> > whatever) to a "stable" magic location in memory. >> >> Ah OK. So long as it doesn't save to global_data we are fine. It could >> just have a little structure somewhere in the data segment. >> >> > >> >> 2. Need for more stack that can be fitted into SRAM. I think the only >> >> sensible option here is to change the stack after board_init_f(). As >> >> Albert says this should be done in crt0.S (in fact that's where I put >> >> my code). Forcing the dram init to before board_init_f() in SPL seems >> >> broken to me. >> > >> > This is also covered. The flow is cpu_init_crit -> lowlevel_init (set >> > stack to CONFIG_SYS_INIT_SP_ADDR) -> s_init (init DDR) -> _main (set >> > stack to CONFIG_SPL_STACK, can be in DDR). >> >> Well s_init is a board-specific thing. What I mean is that crt0.S >> should set up an initial stack - there should be no stack before that. >> Then board_init_f() uses the initial stack, sets up DRAM, then we move >> to a second larger stack before board_init_r() is called. > > I'd like to see someone re-jigger all of the code here. It could work > but most armv7 SoCs have a lot of things going on in s_init and I'm not > sure how much of it we can delay all that much. We can (and do) use an > initial stack pointing elsewhere so maybe we could delay relocating the > main stack. Yes I've had a bit of a look around. I feel that s_init() should be for very limited purposes and is being abused. There is a tendency for everything to just keep moving earlier! > >> >> I think we should try to have the same flow as U-Boot proper: >> >> >> >> start.S >> >> lowlevel_init (no stack, no global_data, no dram) - can only use >> >> 'data' section to write stuff >> >> crt0.S (sets up stack and global_data, no dram) >> >> board_init_f (sets up dram) >> >> relocate stack if required (but not code) >> >> board_init_r (running with full stack in dram) >> > >> > I suppose since old school PowerPC needed i2c to read spd eeproms it's >> > possible this ordering could work still today where there's a good bit >> > of stuff neeed in order to bring DDR on some ARM platforms. >> >> (Note I am talking just about SPL here, as above) >> >> I suppose what I am saying is that there is no point in setting up >> DRAM before board_init_f() in SPL. There is no global_data available, >> nor is there any console, so any code that happens before >> board_init_f() should be absolutely essential for getting to >> board_init_f(). I can't think of much that fits in that category, >> maybe some clock things? > > Clock and pinmux. It could work... Now that you have omap sorted there are only a few that use gdata - I think iMX, sunxi and zynq. I'm having a bit of a look at sunxi. Regards, Simon