On Sun, Sep 18, 2016 at 06:57:01PM -0600, Simon Glass wrote: > Hi Tom, > > On 18 September 2016 at 16:14, Tom Rini <tr...@konsulko.com> wrote: > > On Sun, Sep 18, 2016 at 01:44:49PM -0600, Simon Glass wrote: > > > >> At present the SPL code uses a global spl_image variable which is shared > >> amongst lots of files, some in common/spl and some elsewhere. There is no > >> need for this to be global, and in fact a parameter makes it easier to > >> understand what information the functions act on. It also reduces the BSS > >> use in the SPL (at the expense of stack) which is useful on boards which > >> don't have BSS available early on. > > > > Thanks for taking a stab at cleaning this up. > > I ran into it with the 64-bit x86 stuff and decided to take a detour :-) > > > > > [snip] > >> There is a priorty value attached to each loader which should allow the > >> existing ordering to be maintained. > > > > I worry here about some of the corner cases, but it's probably no more > > or less fragile than currently at least. I'm actually not sure how > > that's working in this version of the series, everyone gets priority 0 > > filled in so it seems like we're on linker order. But I'm also not > > convinced that it's a problem here. As far as I can recall, the use > > cases are that a given binary will support say NAND and MMC, and at run > > time knows that it came from NAND or MMC, and thus we need to look at > > the same device for U-Boot itself. So order doesn't matter there. So > > we can just drop that part I think. > > I see in the code that if Ubifs is enabled, it uses that instead of > NAND and ONENAND. There is another case too - SPI might have a > board-specific loader which takes precedence (although I suspect in > the sunxi case the generic loader is not present). > > If these are not needed then we can drop it. > > At present priority is handled by adding the priority value into the > linker list element name. Since they are sorted in order within the > image, this gives the required result.
Looking over the code, UBI is setup as mutually exclusive. It also looks like that SPL_SPI_BOOT, SPL_SPI_SUNXI and SPL_SPI_LOAD are all exclusive as well. So we should clean that up so that we select the right back end if SPL_SPI is set, and we can drop the order part. > >> Code size is about 20 bytes larger on average which I think is acceptable. > >> The BSS size drops by about 64 bytes, but really this just transfers to > >> the stack. > > > > How about the worst case growths in what we have today? > > Ah well that's something buildman cannot tell me! There is a small > overhead for the lookup code (about 20-30 bytes) and 8-12 bytes for > each loader method. Here's the buildman summary. > > $ buildman -b spl -s --step 0 -S > boards.cfg is up to date. Nothing to do. > Summary of 2 commits for 1196 boards (32 threads, 1 job per thread) > 01: Makefile: Give a build error if ad-hoc CONFIG options are added > blackfin: + cm-bf527 bf609-ezkit bf537-stamp > sparc: + grsim grsim_leon2 gr_cpci_ax2000 gr_xc3s_1500 gr_ep2s60 > nios2: + 10m50 3c120 > microblaze: + microblaze-generic > openrisc: + openrisc-generic > 28: spl: Make spl_boot_list a local variable > aarch64: (for 51/51 boards) spl/u-boot-spl:all +36.8 > spl/u-boot-spl:bss -6.3 spl/u-boot-spl:data +5.6 > spl/u-boot-spl:rodata +7.5 spl/u-boot-spl:text +29.9 > powerpc: (for 415/415 boards) spl/u-boot-spl:all -24.7 > spl/u-boot-spl:bss -0.1 spl/u-boot-spl:data -0.1 > spl/u-boot-spl:rodata -15.9 spl/u-boot-spl:text -8.6 > sandbox: (for 3/3 boards) spl/u-boot-spl:all +37.3 > spl/u-boot-spl:bss -10.7 spl/u-boot-spl:rodata +10.7 > spl/u-boot-spl:text +37.3 > arm: (for 552/552 boards) all -0.8 bss -0.8 data -0.1 > spl/u-boot-spl:all -66.3 spl/u-boot-spl:bss -22.3 > spl/u-boot-spl:data -5.1 spl/u-boot-spl:rodata +0.5 > spl/u-boot-spl:text -39.3 You should have a bit more output :) Adding in -d will list every board and then what I do (since I do -SCdvel and -Ssdel after to log it) is read over the list and manually look at the deviations. > >> There is an obvious follow-on from this, to move boot_name_table[] into the > >> same linker list struct (i.e. add a name field to struct spl_image_loader). > >> The complication here is that we don't want naming if > >> CONFIG_SPL_LIBCOMMON_SUPPORT is not enabled, since it bloats the code. In > >> addition I think that common/spl/spl.c can be tidied up a little. > > > > Yeah, I worry about size growth in doing that too. But maybe we can be > > a little clever when declaring the ll and just not have the strings if > > !LIBCOMMON ? > > Yes I think so...perhaps someone else might take a look, or I'll wait > for a rainy day! Sounds good enough for now, thanks. -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot