[U-Boot] [PATCH] armv8: fix #if around spin-table code in start.S

[Oded Gabbay]

On Wed, Jan 11, 2017 at 1:14 AM, Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, Dec 27, 2016 at 11:19:43AM +0200, Oded Gabbay wrote:
>
> > Using CONFIG_IS_ENABLED() doesn't work in SPL. This patch replaces the only
> > occurrence of CONFIG_IS_ENABLED() in start.S to a regular #if defined().
> > It also adds "&& !defined(CONFIG_SPL_BUILD)" to that #if statement because
> > the spin-table code can't currently work in SPL, and the spin-table file
> > isn't even compiled in SPL.
> >
> > Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
> > ---
> >  arch/arm/cpu/armv8/start.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
> > index 4f5f6d8..2f975a0 100644
> > --- a/arch/arm/cpu/armv8/start.S
> > +++ b/arch/arm/cpu/armv8/start.S
> > @@ -99,7 +99,7 @@ save_boot_params_ret:
> >       /* Processor specific initialization */
> >       bl      lowlevel_init
> >
> > -#if CONFIG_IS_ENABLED(ARMV8_SPIN_TABLE)
> > +#if defined(CONFIG_ARMV8_SPIN_TABLE) && !defined(CONFIG_SPL_BUILD)
> >       branch_if_master x0, x1, master_cpu
> >       b       spin_table_secondary_jump
> >       /* never return */
>
> I don't get it, sorry.  Looking at include/linux/kconfig.h:
> CONFIG_IS_ENABLED(ARMV8_SPIN_TABLE) is 1 if CONFIG_ARMV8_SPIN_TABLE and
> CONFIG_SPL_BUILD is undefined.
> CONFIG_IS_ENABLED(ARMV8_SPIN_TABLE) is always 0 since
> CONFIG_SPL_ARMV8_SPIN_TABLE is not a symbol.  And since we don't link
> the spin table objects in outside of SPL, we wouldn't want to try and
> call those functions.
hmm, I'm really confused. For some reason, I can't recreate the
original problem that caused me to create this patch. I also agree
with your analysis.  I must conclude that my original problem was due
to a different reason.
Sorry for the waste of time.

>
> So, what's the bug exactly?  Just a lack-of-clarity by using
> CONFIG_IS_ENABLED(ARMV8_SPIN_TABLE) vs 'defined(CONFIG_ARMV8_SPIN_TABLE)
> && !defined(CONFIG_SPL_BUILD)' ?  Thanks!

Although my original purpose was to fix an actual bug (which doesn't
exists), I do think there is a clarity issue here, which this patch
addresses. You need to go to kconfig.h, read the comments there to
understand how CONFIG_IS_ENABLED is working with SPL, which is more
tiresome than just doing straight #ifdef. It is definitely more
confusing for a newbee.

In addition, this patch makes the code more consistent, because all
other configuration checks in start.S use a straight #ifdef and not
CONFIG_IS_ENABLED.

But since this patch doesn't fix an actual bug, I guess you can drop
it if you don't want it.
Thanks,
Oded

>
> --
> Tom