From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 00/27] spl: Use linker list and parameters for SPL image loading
Date: Mon, 19 Sep 2016 17:46:32 -0400 [thread overview]
Message-ID: <20160919214632.GM29602@bill-the-cat> (raw)
In-Reply-To: <CAPnjgZ1Qq15C2QEdJHT9wnXag3-Gq_OZNHKMcYTTM4t0Fqh7sw@mail.gmail.com>
On Sun, Sep 18, 2016 at 06:57:01PM -0600, Simon Glass wrote:
> Hi Tom,
>
> On 18 September 2016 at 16:14, Tom Rini <trini@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160919/d2e063f7/attachment.sig>
next prev parent reply other threads:[~2016-09-19 21:46 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-18 19:44 [U-Boot] [PATCH 00/27] spl: Use linker list and parameters for SPL image loading Simon Glass
2016-09-18 19:44 ` [U-Boot] [PATCH 01/27] spl: Move spl_board_load_image() into a generic header Simon Glass
2016-09-18 22:14 ` Tom Rini
2016-09-18 19:44 ` [U-Boot] [PATCH 02/27] spl: Add a parameter to spl_set_header_raw_uboot() Simon Glass
2016-09-18 22:14 ` Tom Rini
2016-09-18 19:44 ` [U-Boot] [PATCH 03/27] spl: Add a parameter to spl_parse_image_header() Simon Glass
2016-09-18 22:15 ` Tom Rini
2016-09-18 19:44 ` [U-Boot] [PATCH 04/27] spl: Add a parameter to jump_to_image_linux() Simon Glass
2016-09-18 22:15 ` Tom Rini
2016-09-18 19:44 ` [U-Boot] [PATCH 05/27] spl: Add function comments to spl_start_uboot() Simon Glass
2016-09-18 22:15 ` Tom Rini
2016-09-18 19:44 ` [U-Boot] [PATCH 06/27] spl: Kconfig: Move SPL_DISPLAY_PRINT to Kconfig Simon Glass
2016-09-18 22:15 ` Tom Rini
2016-09-18 19:44 ` [U-Boot] [PATCH 07/27] spl: Convert boot_device into a struct Simon Glass
2016-09-18 22:15 ` Tom Rini
2016-09-18 19:44 ` [U-Boot] [PATCH 08/27] spl: Add a way to declare an SPL image loader Simon Glass
2016-09-18 22:15 ` Tom Rini
2016-09-19 3:22 ` Simon Glass
2016-09-18 19:44 ` [U-Boot] [PATCH 09/27] spl: Convert spl_ram_load_image() to use linker list Simon Glass
2016-09-18 19:44 ` [U-Boot] [PATCH 10/27] spl: Convert spl_mmc_load_image() " Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 11/27] spl: Convert spl_ubi_load_image() " Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 12/27] spl: Convert spl_nand_load_image() " Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 13/27] spl: Convert spl_onenand_load_image() " Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 14/27] spl: Convert spl_nor_load_image() " Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 15/27] spl: Convert spl_ymodem_load_image() " Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 16/27] spl: Convert spl_usb_load_image() " Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 17/27] spl: Convert spl_sata_load_image() " Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 18/27] spl: spi: Move the generic SPI loader into common/spl Simon Glass
2016-09-19 0:30 ` Tom Rini
2016-09-18 19:45 ` [U-Boot] [PATCH 19/27] spl: Convert spl_spi_load_image() to use linker list Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 20/27] spi: Move freescale-specific code into a private header Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 21/27] spl: Convert spl_net_load_image() to use linker list Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 22/27] spl: Convert spl_board_load_image() " Simon Glass
2016-09-18 19:45 ` [U-Boot] [PATCH 23/27] spl: Pass spl_image as a parameter to load_image() methods Simon Glass
2016-09-19 0:30 ` Tom Rini
2016-09-18 19:45 ` [U-Boot] [PATCH 24/27] spl: Update ext functions to take an spl_image parameter Simon Glass
2016-09-19 0:30 ` Tom Rini
2016-09-18 19:45 ` [U-Boot] [PATCH 25/27] spl: Update fat " Simon Glass
2016-09-19 0:30 ` Tom Rini
2016-09-18 19:45 ` [U-Boot] [PATCH 26/27] spl: Update spl_load_simple_fit() to take an spl_image param Simon Glass
2016-09-19 0:31 ` Tom Rini
2016-09-18 19:45 ` [U-Boot] [PATCH 27/27] spl: Make spl_boot_list a local variable Simon Glass
2016-09-19 0:31 ` Tom Rini
2016-09-18 22:14 ` [U-Boot] [PATCH 00/27] spl: Use linker list and parameters for SPL image loading Tom Rini
2016-09-19 0:57 ` Simon Glass
2016-09-19 21:46 ` Tom Rini [this message]
2016-09-19 3:18 ` Simon Glass
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160919214632.GM29602@bill-the-cat \
--to=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox