[U-Boot] [PATCH v2 0/2] ARM: bug fixes of commit 41623c91 (arch/arm/lib/vectors.S)
Masahiro Yamada (2): arm: include config.h in arch/arm/lib/vectors.S arm: fix a double-definition error of _start symbol
arch/arm/lib/vectors.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Commit 41623c91 moved exception handling to arch/arm/lib/vectors.S, but it missed to include <config.h>.
This mistake causes two problems.
[1] "enbw_cmc", "da850evm_direct_nor", "calimain" boards are broken.
They expect CONFIG_SYS_DV_NOR_BOOT_CFG (= 0x00000011) at the beginning of the boot image.
But it is missing since commit 41623c91 because CONFIG_SYS_DV_NOR_BOOT_CFG is never defined in arch/arm/lib/vectors.S.
[2] Boards with CONFIG_USE_IRQ fail with undefined reference error of "IRQ_STACK_START" and "FIQ_STACK_START".
This is _potential_ problem because currently there is no board enabling CONFIG_USE_IRQ. But we cannot deny the possibility some board may enable it in future.
(If we are not willing to support interrupt feature, all references to CONFIG_USE_IRQ should be dropped immediately.)
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net ---
Changes in v2: - Rephrase commit subject and description. (No change in code-diff)
arch/arm/lib/vectors.S | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S index d68cc47..fad00da 100644 --- a/arch/arm/lib/vectors.S +++ b/arch/arm/lib/vectors.S @@ -13,6 +13,8 @@ * SPDX-License-Identifier: GPL-2.0+ */
+#include <config.h> + /* ************************************************************************* *
The symbol "_start" is defined twice in arch/arm/lib/vectors.S: around line 48 and line 54.
If CONFIG_SYS_DV_NOR_BOOT_CFG is defined (as on calimain board), build fails:
arch/arm/lib/vectors.S: Assembler messages: arch/arm/lib/vectors.S:54: Error: symbol `_start' is already defined make[1]: *** [arch/arm/lib/vectors.o] Error 1 make: *** [arch/arm/lib] Error 2
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net ---
Changes in v2: - No change (Resend as a series)
arch/arm/lib/vectors.S | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S index fad00da..493f337 100644 --- a/arch/arm/lib/vectors.S +++ b/arch/arm/lib/vectors.S @@ -45,8 +45,6 @@ ************************************************************************* */
-_start: - #ifdef CONFIG_SYS_DV_NOR_BOOT_CFG .word CONFIG_SYS_DV_NOR_BOOT_CFG #endif
Hi Masahiro,
On Mon, 9 Jun 2014 20:16:52 +0900, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Masahiro Yamada (2): arm: include config.h in arch/arm/lib/vectors.S arm: fix a double-definition error of _start symbol
arch/arm/lib/vectors.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
NAK on both patches.
1/2 commit message should drop any reference to CONFIG_USE_IRQ and limit itself to the visible and actual issue it solves, that is, the missing prefix word in the images of boards which define CONFIG_SYS_DV_NOR_BOOT_CFG.
2/2 is redundant as a standalone version of it has already been applied to u-boot-arm/master -- and I'm not going to roll back a legitimate fix.
Note BTW that the order in which the patches are listed here is wrong: it actively breaks the building of three boards. While I do understand that it is done on purpose for didactic reasons, I don't want a commit that breaks builds if I can avoid it, as such a commit makes things like git bisect (more) prone to failure (than they already are). In order to avoid breaking builds, patch 2/2 would have had to come before patch 1/2.
I do share your general concern that broken feature should be fixed, and an unused one should be removed, and as I said, I am working on removing CONFIG_USE_IRQ, all the more as no one seemd to notice it was broken. After all, you only noticed it incidentally.
I do understand too that you want this fixed quickly. It is a property of an open source project that it both thrives /and/ depends on people contributing. If you feel the CONFIG_USE_IRQ feature should be removed quicker than I (or someone else) can contribute a patch for it, please feel free to contribute such a patch yourself (possibly in V3 of this series in replacement of the current 2/2) -- I won't compete with you, since such a patch will /save/ me some time.
Amicalement,
participants (2)
-
Albert ARIBAUD -
Masahiro Yamada