[U-Boot] [PATCH] armv8: fix #if around spin-table code in start.S
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 */
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!
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
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!
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.
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.
participants (3)
-
Masahiro Yamada -
Oded Gabbay -
Tom Rini