* [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.