From patchwork Fri Feb 28 23:09:04 2020
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
X-Patchwork-Id: 237022
List-Id: U-Boot discussion <u-boot.lists.denx.de>
From: rasmus.villemoes at prevas.dk (Rasmus Villemoes)
Date: Fri, 28 Feb 2020 23:09:04 +0000
Subject: caching BLOBLISTT_SPL_HANDOFF (was Re: [PATCH] common/board_f.c: use
 #ifdefs a little more consistently)
In-Reply-To: <20200228173524.GK18302@bill-the-cat>
References: <20200227081825.11039-1-rasmus.villemoes@prevas.dk>
 <CAPnjgZ2sZ=fvOp-3ZMMv_aRDp63cDXC9EbYBvYyc5fHmGjj1Jg@mail.gmail.com>
 <76d3bc74-0406-cd11-8793-b2192ffdb7ee@prevas.dk>
 <20200228154620.GJ18302@bill-the-cat>
 <1500fa23-9959-76cc-0b3e-148a22cd46da@prevas.dk>
 <20200228173524.GK18302@bill-the-cat>
Message-ID: <735f422a-c9fb-8cb3-5cc9-f2facec0d40c@prevas.dk>

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


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

--- 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,