* [U-Boot] [PATCH] armv8: fix #if around spin-table code in start.S
@ 2016-12-27 9:19 Oded Gabbay
2017-01-10 23:14 ` Tom Rini
2017-01-15 18:29 ` [U-Boot] " Tom Rini
0 siblings, 2 replies; 6+ messages in thread
From: Oded Gabbay @ 2016-12-27 9:19 UTC (permalink / raw)
To: u-boot
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@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 */
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] armv8: fix #if around spin-table code in start.S
2016-12-27 9:19 [U-Boot] [PATCH] armv8: fix #if around spin-table code in start.S Oded Gabbay
@ 2017-01-10 23:14 ` Tom Rini
2017-01-11 8:11 ` Oded Gabbay
2017-01-15 18:29 ` [U-Boot] " Tom Rini
1 sibling, 1 reply; 6+ messages in thread
From: Tom Rini @ 2017-01-10 23:14 UTC (permalink / raw)
To: u-boot
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@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.
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!
--
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/20170110/6c24dfb7/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] armv8: fix #if around spin-table code in start.S
2017-01-10 23:14 ` Tom Rini
@ 2017-01-11 8:11 ` Oded Gabbay
0 siblings, 0 replies; 6+ messages in thread
From: Oded Gabbay @ 2017-01-11 8:11 UTC (permalink / raw)
To: u-boot
On Wed, Jan 11, 2017 at 1:14 AM, Tom Rini <trini@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@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] armv8: fix #if around spin-table code in start.S
2016-12-27 9:19 [U-Boot] [PATCH] armv8: fix #if around spin-table code in start.S Oded Gabbay
2017-01-10 23:14 ` Tom Rini
@ 2017-01-15 18:29 ` Tom Rini
2017-01-16 1:19 ` Masahiro Yamada
1 sibling, 1 reply; 6+ messages in thread
From: Tom Rini @ 2017-01-15 18:29 UTC (permalink / raw)
To: u-boot
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@gmail.com>
Applied to u-boot/master, 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/20170115/760bc578/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] armv8: fix #if around spin-table code in start.S
2017-01-15 18:29 ` [U-Boot] " Tom Rini
@ 2017-01-16 1:19 ` Masahiro Yamada
2017-01-17 1:22 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2017-01-16 1:19 UTC (permalink / raw)
To: u-boot
Hi Tom.
2017-01-16 3:29 GMT+09:00 Tom Rini <trini@konsulko.com>:
> 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@gmail.com>
>
> Applied to u-boot/master, thanks!
>
> --
> Tom
>
I had not noticed this patch until it was applied.
At least, the statement in the git-log
"Using CONFIG_IS_ENABLED() doesn't work in SPL" is wrong.
So, when I saw the git history today, I wondered what was going on.
Then, I found this discussion in the ML.
It does not matter to either apply or discard this patch
because it is a matter of taste.
If you decide to apply it,
the git-log should have been replaced with Oded's comment:
--------
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.
----------
It is too late this time, but please take care of it next time.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] armv8: fix #if around spin-table code in start.S
2017-01-16 1:19 ` Masahiro Yamada
@ 2017-01-17 1:22 ` Tom Rini
0 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2017-01-17 1:22 UTC (permalink / raw)
To: u-boot
On Mon, Jan 16, 2017 at 10:19:46AM +0900, Masahiro Yamada wrote:
> Hi Tom.
>
>
> 2017-01-16 3:29 GMT+09:00 Tom Rini <trini@konsulko.com>:
> > 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@gmail.com>
> >
> > Applied to u-boot/master, thanks!
> >
> > --
> > Tom
> >
>
>
> I had not noticed this patch until it was applied.
>
> At least, the statement in the git-log
> "Using CONFIG_IS_ENABLED() doesn't work in SPL" is wrong.
> So, when I saw the git history today, I wondered what was going on.
> Then, I found this discussion in the ML.
>
> It does not matter to either apply or discard this patch
> because it is a matter of taste.
>
>
> If you decide to apply it,
> the git-log should have been replaced with Oded's comment:
>
> --------
> 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.
> ----------
>
>
> It is too late this time, but please take care of it next time.
Yeah, I should have asked for a v2 with a different commit message.
--
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/20170116/db6284f1/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-17 1:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-27 9:19 [U-Boot] [PATCH] armv8: fix #if around spin-table code in start.S Oded Gabbay
2017-01-10 23:14 ` Tom Rini
2017-01-11 8:11 ` Oded Gabbay
2017-01-15 18:29 ` [U-Boot] " Tom Rini
2017-01-16 1:19 ` Masahiro Yamada
2017-01-17 1:22 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.