[PATCH] common/board_f.c: use #ifdefs a little more consistently

[PATCH] common/board_f.c: use #ifdefs a little more consistently

[Rasmus Villemoes]

Some init functions, e.g. print_resetinfo(), are conditionally defined
depending on some config options, and are correspondingly
conditionally included in the init_sequence_f[] array.

Others are unconditionally defined and included in init_sequence_f[],
but have their entire body, sans a mandatory "return 0", conditionally
compiled.

For the simple cases, switch to the former model, making it a bit more
consistent. This also makes the U-Boot image very slightly smaller and
avoids a few useless calls to no-op functions during board_init_f.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 common/board_f.c | 54 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/common/board_f.c b/common/board_f.c
index 82a164752a..ed63fbcea2 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -285,16 +285,16 @@ static int setup_mon_len(void)
 	return 0;
 }
 
+#if CONFIG_IS_ENABLED(HANDOFF)
 static int setup_spl_handoff(void)
 {
-#if CONFIG_IS_ENABLED(HANDOFF)
 	gd->spl_handoff = bloblist_find(BLOBLISTT_SPL_HANDOFF,
 					sizeof(struct spl_handoff));
 	debug("Found SPL hand-off info %p\n", gd->spl_handoff);
-#endif
 
 	return 0;
 }
+#endif
 
 __weak int arch_cpu_init(void)
 {
@@ -437,17 +437,17 @@ static int reserve_video(void)
 	return 0;
 }
 
+#ifdef CONFIG_TRACE
 static int reserve_trace(void)
 {
-#ifdef CONFIG_TRACE
 	gd->relocaddr -= CONFIG_TRACE_BUFFER_SIZE;
 	gd->trace_buff = map_sysmem(gd->relocaddr, CONFIG_TRACE_BUFFER_SIZE);
 	debug("Reserving %luk for trace data at: %08lx\n",
 	      (unsigned long)CONFIG_TRACE_BUFFER_SIZE >> 10, gd->relocaddr);
-#endif
 
 	return 0;
 }
+#endif
 
 static int reserve_uboot(void)
 {
@@ -520,13 +520,13 @@ static int reserve_board(void)
 	return 0;
 }
 
+#ifdef CONFIG_MACH_TYPE
 static int setup_machine(void)
 {
-#ifdef CONFIG_MACH_TYPE
 	gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
-#endif
 	return 0;
 }
+#endif
 
 static int reserve_global_data(void)
 {
@@ -537,9 +537,9 @@ static int reserve_global_data(void)
 	return 0;
 }
 
+#ifndef CONFIG_OF_EMBED
 static int reserve_fdt(void)
 {
-#ifndef CONFIG_OF_EMBED
 	/*
 	 * If the device tree is sitting immediately above our image then we
 	 * must relocate it. If it is embedded in the data section, then it
@@ -553,24 +553,24 @@ static int reserve_fdt(void)
 		debug("Reserving %lu Bytes for FDT at: %08lx\n",
 		      gd->fdt_size, gd->start_addr_sp);
 	}
-#endif
 
 	return 0;
 }
+#endif
 
+#ifdef CONFIG_BOOTSTAGE
 static int reserve_bootstage(void)
 {
-#ifdef CONFIG_BOOTSTAGE
 	int size = bootstage_get_size();
 
 	gd->start_addr_sp -= size;
 	gd->new_bootstage = map_sysmem(gd->start_addr_sp, size);
 	debug("Reserving %#x Bytes for bootstage at: %08lx\n", size,
 	      gd->start_addr_sp);
-#endif
 
 	return 0;
 }
+#endif
 
 __weak int arch_reserve_stacks(void)
 {
@@ -590,16 +590,16 @@ static int reserve_stacks(void)
 	return arch_reserve_stacks();
 }
 
+#ifdef CONFIG_BLOBLIST
 static int reserve_bloblist(void)
 {
-#ifdef CONFIG_BLOBLIST
 	gd->start_addr_sp &= ~0xf;
 	gd->start_addr_sp -= CONFIG_BLOBLIST_SIZE;
 	gd->new_bloblist = map_sysmem(gd->start_addr_sp, CONFIG_BLOBLIST_SIZE);
-#endif
 
 	return 0;
 }
+#endif
 
 static int display_new_sp(void)
 {
@@ -675,23 +675,23 @@ static int init_post(void)
 }
 #endif
 
+#ifndef CONFIG_OF_EMBED
 static int reloc_fdt(void)
 {
-#ifndef CONFIG_OF_EMBED
 	if (gd->flags & GD_FLG_SKIP_RELOC)
 		return 0;
 	if (gd->new_fdt) {
 		memcpy(gd->new_fdt, gd->fdt_blob, gd->fdt_size);
 		gd->fdt_blob = gd->new_fdt;
 	}
-#endif
 
 	return 0;
 }
+#endif
 
+#ifdef CONFIG_BOOTSTAGE
 static int reloc_bootstage(void)
 {
-#ifdef CONFIG_BOOTSTAGE
 	if (gd->flags & GD_FLG_SKIP_RELOC)
 		return 0;
 	if (gd->new_bootstage) {
@@ -703,14 +703,14 @@ static int reloc_bootstage(void)
 		gd->bootstage = gd->new_bootstage;
 		bootstage_relocate();
 	}
-#endif
 
 	return 0;
 }
+#endif
 
+#ifdef CONFIG_BLOBLIST
 static int reloc_bloblist(void)
 {
-#ifdef CONFIG_BLOBLIST
 	if (gd->flags & GD_FLG_SKIP_RELOC)
 		return 0;
 	if (gd->new_bloblist) {
@@ -721,10 +721,10 @@ static int reloc_bloblist(void)
 		memcpy(gd->new_bloblist, gd->bloblist, size);
 		gd->bloblist = gd->new_bloblist;
 	}
-#endif
 
 	return 0;
 }
+#endif
 
 static int setup_reloc(void)
 {
@@ -886,7 +886,9 @@ static const init_fnc_t init_sequence_f[] = {
 #ifdef CONFIG_BLOBLIST
 	bloblist_init,
 #endif
+#if CONFIG_IS_ENABLED(HANDOFF)
 	setup_spl_handoff,
+#endif
 	initf_console_record,
 #if defined(CONFIG_HAVE_FSP)
 	arch_fsp_init,
@@ -974,15 +976,25 @@ static const init_fnc_t init_sequence_f[] = {
 	reserve_mmu,
 #endif
 	reserve_video,
+#ifdef CONFIG_TRACE
 	reserve_trace,
+#endif
 	reserve_uboot,
 	reserve_malloc,
 	reserve_board,
+#ifdef CONFIG_MACH_TYPE
 	setup_machine,
+#endif
 	reserve_global_data,
+#ifndef CONFIG_OF_EMBED
 	reserve_fdt,
+#endif
+#ifdef CONFIG_BOOTSTAGE
 	reserve_bootstage,
+#endif
+#ifdef CONFIG_BLOBLIST
 	reserve_bloblist,
+#endif
 	reserve_arch,
 	reserve_stacks,
 	dram_init_banksize,
@@ -1000,9 +1012,15 @@ static const init_fnc_t init_sequence_f[] = {
 	fix_fdt,
 #endif
 	INIT_FUNC_WATCHDOG_RESET
+#ifndef CONFIG_OF_EMBED
 	reloc_fdt,
+#endif
+#ifdef CONFIG_BOOTSTAGE
 	reloc_bootstage,
+#endif
+#ifdef CONFIG_BLOBLIST
 	reloc_bloblist,
+#endif
 	setup_reloc,
 #if defined(CONFIG_X86) || defined(CONFIG_ARC)
 	copy_uboot_to_ram,
-- 
2.23.0

[PATCH] common/board_f.c: use #ifdefs a little more consistently

[Simon Glass]

Hi Rasmus,

On Thu, 27 Feb 2020 at 00:18, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> Some init functions, e.g. print_resetinfo(), are conditionally defined
> depending on some config options, and are correspondingly
> conditionally included in the init_sequence_f[] array.
>
> Others are unconditionally defined and included in init_sequence_f[],
> but have their entire body, sans a mandatory "return 0", conditionally
> compiled.
>
> For the simple cases, switch to the former model, making it a bit more
> consistent. This also makes the U-Boot image very slightly smaller and
> avoids a few useless calls to no-op functions during board_init_f.

Can you check if it definitely does change the size? The reason I ask
is that it inlines those function calls in the normal case, at least
from my inspection.

Using if() is preferable to #if if there is no cost.

>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>  common/board_f.c | 54 ++++++++++++++++++++++++++++++++----------------
>  1 file changed, 36 insertions(+), 18 deletions(-)

Regards,
Simon

[PATCH] common/board_f.c: use #ifdefs a little more consistently

[Rasmus Villemoes]

On 28/02/2020 00.40, Simon Glass wrote:
> Hi Rasmus,
> 
> On Thu, 27 Feb 2020 at 00:18, Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> wrote:
>>
>> Some init functions, e.g. print_resetinfo(), are conditionally defined
>> depending on some config options, and are correspondingly
>> conditionally included in the init_sequence_f[] array.
>>
>> Others are unconditionally defined and included in init_sequence_f[],
>> but have their entire body, sans a mandatory "return 0", conditionally
>> compiled.
>>
>> For the simple cases, switch to the former model, making it a bit more
>> consistent. This also makes the U-Boot image very slightly smaller and
>> avoids a few useless calls to no-op functions during board_init_f.
> 
> Can you check if it definitely does change the size? 

It does, I did build to see how much. On ppc, it's 8 bytes per no-op
function (one "put 0 in r3", one blr instruction), plus 4 bytes for the
array entry, plus 4 bytes for a .fixup entry - in my case ending up with
7*16=112, because all but the #ifndef CONFIG_OF_EMBED applied.

The reason I ask
> is that it inlines those function calls in the normal case, at least
> from my inspection.

The compiler can't inline and thus eliminate these as they are not
called directly, but their addresses are used to populate the
init_sequence_f[] array and called through that.

> Using if() is preferable to #if if there is no cost.

Completely agree, and I also prefer to have the linker eliminate unused
functions rather than cluttering the C code with #ifdefs. But that can't
be used in this case.

Anyway, this wasn't primarily to save 112 bytes or whatnot from the
U-Boot image, just to use one style a little more consistently.

Rasmus

[PATCH] common/board_f.c: use #ifdefs a little more consistently

[Tom Rini]

On Fri, Feb 28, 2020 at 08:42:21AM +0000, Rasmus Villemoes wrote:
> On 28/02/2020 00.40, Simon Glass wrote:
> > Hi Rasmus,
> > 
> > On Thu, 27 Feb 2020 at 00:18, Rasmus Villemoes
> > <rasmus.villemoes@prevas.dk> wrote:
> >>
> >> Some init functions, e.g. print_resetinfo(), are conditionally defined
> >> depending on some config options, and are correspondingly
> >> conditionally included in the init_sequence_f[] array.
> >>
> >> Others are unconditionally defined and included in init_sequence_f[],
> >> but have their entire body, sans a mandatory "return 0", conditionally
> >> compiled.
> >>
> >> For the simple cases, switch to the former model, making it a bit more
> >> consistent. This also makes the U-Boot image very slightly smaller and
> >> avoids a few useless calls to no-op functions during board_init_f.
> > 
> > Can you check if it definitely does change the size? 
> 
> It does, I did build to see how much. On ppc, it's 8 bytes per no-op
> function (one "put 0 in r3", one blr instruction), plus 4 bytes for the
> array entry, plus 4 bytes for a .fixup entry - in my case ending up with
> 7*16=112, because all but the #ifndef CONFIG_OF_EMBED applied.
> 
> The reason I ask
> > is that it inlines those function calls in the normal case, at least
> > from my inspection.
> 
> The compiler can't inline and thus eliminate these as they are not
> called directly, but their addresses are used to populate the
> init_sequence_f[] array and called through that.
> 
> > Using if() is preferable to #if if there is no cost.
> 
> Completely agree, and I also prefer to have the linker eliminate unused
> functions rather than cluttering the C code with #ifdefs. But that can't
> be used in this case.
> 
> Anyway, this wasn't primarily to save 112 bytes or whatnot from the
> U-Boot image, just to use one style a little more consistently.

Perhaps we could come up with a little more macro-magic?  In
psuedo-code:
#define RESERVE_INIT_SEQ_F_ENTRY(fn) \
#if CONFIG_IS_ENABLED(toupper(fn))
reserve_##fn
#endif
#endif

And then a little harder thinking about negative-case (OF_EMBED) ?  Or
just have those be the few ifndef's?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200228/0e4c7bb6/attachment.sig>

[PATCH] common/board_f.c: use #ifdefs a little more consistently

[Rasmus Villemoes]

On 28/02/2020 16.46, Tom Rini wrote:
> On Fri, Feb 28, 2020 at 08:42:21AM +0000, Rasmus Villemoes wrote:
>> On 28/02/2020 00.40, Simon Glass wrote:

>>> Using if() is preferable to #if if there is no cost.
>>
>> Completely agree, and I also prefer to have the linker eliminate unused
>> functions rather than cluttering the C code with #ifdefs. But that can't
>> be used in this case.
>>
>> Anyway, this wasn't primarily to save 112 bytes or whatnot from the
>> U-Boot image, just to use one style a little more consistently.
> 
> Perhaps we could come up with a little more macro-magic?  In
> psuedo-code:
> #define RESERVE_INIT_SEQ_F_ENTRY(fn) \
> #if CONFIG_IS_ENABLED(toupper(fn))
> reserve_##fn
> #endif
> #endif

I'm afraid that's rather far from something that can be implemented;
macros cannot expand to other preprocessor directives, and toupper is
also pretty hard to do.

What we could do if we want to reduce #ifdefs while still eliminating
the no-op functions is to replace

#ifdef CONFIG_FOO
static int foo_init(void) {
  blabla;
  return 0;
}
#endif
...
init_sequence_f[] = { ...
#ifdef CONFIG_FOO
  foo_init,
#endif
...
}

by

static int foo_init(void) { /* always defined */
  blabla;
  return 0;
}

init_sequence_f[] = { ...
  IS_ENABLED(CONFIG_FOO) ? foo_init : NULL,
...
}

and teach the iterator to ignore NULL entries (I don't remember if we
use a NULL terminator or use ARRAY_SIZE; if the former one should switch
to the latter). It will still cost sizeof(void*) for the NULL entries,
but the function bodies (and on powerpc the .fixups) should be
eliminated, and there's not an #ifdef in sight.

If one also wants to get rid of those NULL entries, I did
https://lore.kernel.org/lkml/1444165543-2209-1-git-send-email-linux at rasmusvillemoes.dk/
a long time ago (see the COND_INITIALIZER()), but whether that can be
tweaked to still automatically avoid a "defined but not used" warning I
don't know.

> And then a little harder thinking about negative-case (OF_EMBED) ?  Or
> just have those be the few ifndef's?

That can be done in the same manner by just switching second and third
operand.

The problem is the function bodies which rely on symbols (for example
CONFIG_* values) that only exist if CONFIG_FOO.

Rasmus

[PATCH] common/board_f.c: use #ifdefs a little more consistently

[Tom Rini]

On Fri, Feb 28, 2020 at 05:24:58PM +0000, Rasmus Villemoes wrote:
> On 28/02/2020 16.46, Tom Rini wrote:
> > On Fri, Feb 28, 2020 at 08:42:21AM +0000, Rasmus Villemoes wrote:
> >> On 28/02/2020 00.40, Simon Glass wrote:
> 
> >>> Using if() is preferable to #if if there is no cost.
> >>
> >> Completely agree, and I also prefer to have the linker eliminate unused
> >> functions rather than cluttering the C code with #ifdefs. But that can't
> >> be used in this case.
> >>
> >> Anyway, this wasn't primarily to save 112 bytes or whatnot from the
> >> U-Boot image, just to use one style a little more consistently.
> > 
> > Perhaps we could come up with a little more macro-magic?  In
> > psuedo-code:
> > #define RESERVE_INIT_SEQ_F_ENTRY(fn) \
> > #if CONFIG_IS_ENABLED(toupper(fn))
> > reserve_##fn
> > #endif
> > #endif
> 
> I'm afraid that's rather far from something that can be implemented;
> macros cannot expand to other preprocessor directives, and toupper is
> also pretty hard to do.

Darn.  I would have been willing to move to reserve_FUNCTION if it would
have otherwise worked.

> What we could do if we want to reduce #ifdefs while still eliminating
> the no-op functions is to replace
> 
> #ifdef CONFIG_FOO
> static int foo_init(void) {
>   blabla;
>   return 0;
> }
> #endif
> ...
> init_sequence_f[] = { ...
> #ifdef CONFIG_FOO
>   foo_init,
> #endif
> ...
> }
> 
> by
> 
> static int foo_init(void) { /* always defined */
>   blabla;
>   return 0;
> }
> 
> init_sequence_f[] = { ...
>   IS_ENABLED(CONFIG_FOO) ? foo_init : NULL,
> ...
> }
> 
> and teach the iterator to ignore NULL entries (I don't remember if we
> use a NULL terminator or use ARRAY_SIZE; if the former one should switch
> to the latter). It will still cost sizeof(void*) for the NULL entries,
> but the function bodies (and on powerpc the .fixups) should be
> eliminated, and there's not an #ifdef in sight.

That sounds pretty nice actually.  If you're so inclined I'd like to see
it.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200228/144d46cb/attachment.sig>

caching BLOBLISTT_SPL_HANDOFF (was Re: [PATCH] common/board_f.c: use #ifdefs a little more consistently)

[Rasmus Villemoes]

On 28/02/2020 18.35, Tom Rini wrote:
> On Fri, Feb 28, 2020 at 05:24:58PM +0000, Rasmus Villemoes wrote:

>> eliminated, and there's not an #ifdef in sight.
> 
> That sounds pretty nice actually.  If you're so inclined I'd like to see
> it.
> 

So I started looking at that, and while it's mostly mechanical, one
quickly hits the case I was worried about, some of the functions
referring to symbols or struct members that are conditionally
defined/declared, so there's no way around guarding such accesses at the
preprocessor level.

Case in point: I'd like to do

--- a/common/board_f.c
+++ b/common/board_f.c
@@ -287,11 +287,9 @@ static int setup_mon_len(void)

 static int setup_spl_handoff(void)
 {
-#if CONFIG_IS_ENABLED(HANDOFF)
        gd->spl_handoff = bloblist_find(BLOBLISTT_SPL_HANDOFF,
                                        sizeof(struct spl_handoff));
        debug("Found SPL hand-off info %p\n", gd->spl_handoff);
-#endif

        return 0;
 }
@@ -886,7 +884,7 @@ static const init_fnc_t init_sequence_f[] = {
 #ifdef CONFIG_BLOBLIST
        bloblist_init,
 #endif
-       setup_spl_handoff,
+       CONFIG_IS_ENABLED(HANDOFF) ? setup_spl_handoff : NULL,
        initf_console_record,
 #if defined(CONFIG_HAVE_FSP)
        arch_fsp_init,

but that doesn't work because gd->spl_handoff only exists when
CONFIG_IS_ENABLED(BLOBLIST) && defined(CONFIG_SPL).

Now that particular one seems a bit fishy: Why is it ok to cache the
location of the BLOBLISTT_SPL_HANDOFF blob in gd->spl_handoff? Later in
the init sequence there's a call to reserve_bloblist, and later again
reloc_bloblist. Doesn't that leave gd->spl_handoff stale?

I'd expect that users would always have to look it up via
bloblist_find(). Simon?

Rasmus

caching BLOBLISTT_SPL_HANDOFF (was Re: [PATCH] common/board_f.c: use #ifdefs a little more consistently)

[Simon Glass]

Hi Rasmus,

On Fri, 28 Feb 2020 at 16:09, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 28/02/2020 18.35, Tom Rini wrote:
> > On Fri, Feb 28, 2020 at 05:24:58PM +0000, Rasmus Villemoes wrote:
>
> >> eliminated, and there's not an #ifdef in sight.
> >
> > That sounds pretty nice actually.  If you're so inclined I'd like to see
> > it.
> >
>
> So I started looking at that, and while it's mostly mechanical, one
> quickly hits the case I was worried about, some of the functions
> referring to symbols or struct members that are conditionally
> defined/declared, so there's no way around guarding such accesses at the
> preprocessor level.
>
> Case in point: I'd like to do
>
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -287,11 +287,9 @@ static int setup_mon_len(void)
>
>  static int setup_spl_handoff(void)
>  {
> -#if CONFIG_IS_ENABLED(HANDOFF)
>         gd->spl_handoff = bloblist_find(BLOBLISTT_SPL_HANDOFF,
>                                         sizeof(struct spl_handoff));
>         debug("Found SPL hand-off info %p\n", gd->spl_handoff);
> -#endif
>
>         return 0;
>  }
> @@ -886,7 +884,7 @@ static const init_fnc_t init_sequence_f[] = {
>  #ifdef CONFIG_BLOBLIST
>         bloblist_init,
>  #endif
> -       setup_spl_handoff,
> +       CONFIG_IS_ENABLED(HANDOFF) ? setup_spl_handoff : NULL,
>         initf_console_record,
>  #if defined(CONFIG_HAVE_FSP)
>         arch_fsp_init,
>
> but that doesn't work because gd->spl_handoff only exists when
> CONFIG_IS_ENABLED(BLOBLIST) && defined(CONFIG_SPL).
>
> Now that particular one seems a bit fishy: Why is it ok to cache the
> location of the BLOBLISTT_SPL_HANDOFF blob in gd->spl_handoff? Later in
> the init sequence there's a call to reserve_bloblist, and later again
> reloc_bloblist. Doesn't that leave gd->spl_handoff stale?

Yes it does. It is only supposed to be used in the early stages of
U-Boot (proper) init.

Actually I think that member could be dropped and we could search for
it each time:

./arch/x86/cpu/broadwell/cpu_from_spl.c

>
> I'd expect that users would always have to look it up via
> bloblist_find(). Simon?

Regards,
Simon

caching BLOBLISTT_SPL_HANDOFF

[Rasmus Villemoes]

On 02/03/2020 20.47, Simon Glass wrote:
> Hi Rasmus,
> 
> On Fri, 28 Feb 2020 at 16:09, Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> wrote:
>>

>> Now that particular one seems a bit fishy: Why is it ok to cache the
>> location of the BLOBLISTT_SPL_HANDOFF blob in gd->spl_handoff? Later in
>> the init sequence there's a call to reserve_bloblist, and later again
>> reloc_bloblist. Doesn't that leave gd->spl_handoff stale?
> 
> Yes it does. It is only supposed to be used in the early stages of
> U-Boot (proper) init.

Yes, that's what I thought - and if it's only actually used once or
twice during the early stages, there's not much point in caching it.

> Actually I think that member could be dropped and we could search for
> it each time:
> 
> ./arch/x86/cpu/broadwell/cpu_from_spl.c

Yes, there didn't seem to be many users, so it should not be that hard
to get rid of. I also think that sets a better precedent for future
bloblist users.

Rasmus

caching BLOBLISTT_SPL_HANDOFF

[Simon Glass]

Hi Rasmus,

On Mon, 2 Mar 2020 at 13:01, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 02/03/2020 20.47, Simon Glass wrote:
> > Hi Rasmus,
> >
> > On Fri, 28 Feb 2020 at 16:09, Rasmus Villemoes
> > <rasmus.villemoes@prevas.dk> wrote:
> >>
>
> >> Now that particular one seems a bit fishy: Why is it ok to cache the
> >> location of the BLOBLISTT_SPL_HANDOFF blob in gd->spl_handoff? Later in
> >> the init sequence there's a call to reserve_bloblist, and later again
> >> reloc_bloblist. Doesn't that leave gd->spl_handoff stale?
> >
> > Yes it does. It is only supposed to be used in the early stages of
> > U-Boot (proper) init.
>
> Yes, that's what I thought - and if it's only actually used once or
> twice during the early stages, there's not much point in caching it.
>
> > Actually I think that member could be dropped and we could search for
> > it each time:
> >
> > ./arch/x86/cpu/broadwell/cpu_from_spl.c
>
> Yes, there didn't seem to be many users, so it should not be that hard
> to get rid of. I also think that sets a better precedent for future
> bloblist users.

Sounds good, thanks.

I wonder if one day we will want to support multiple bloblists in
different memory locations.

Regards,
Simon