Re: [PATCH] powerpc: fix regression in arch_initr_trap()

On Tue, 18 May 2021 01:50:56 -0400 "Stefan Roese" sr@denx.de wrote:
Hi Matt,
On 17.05.21 19:32, Matt Merhar wrote:
The assembly output of the arch_initr_trap() function differed by a single byte after common.h was removed from traps.c:
fff49a18 <arch_initr_trap>: fff49a18: 94 21 ff f0 stwu r1,-16(r1) fff49a1c: 7c 08 02 a6 mflr r0 fff49a20: 90 01 00 14 stw r0,20(r1) -fff49a24: 80 62 00 44 lwz r3,68(r2) +fff49a24: 80 62 00 38 lwz r3,56(r2) fff49a28: 4b ff 76 19 bl fff41040 <trap_init> fff49a2c: 80 01 00 14 lwz r0,20(r1) fff49a30: 38 60 00 00 li r3,0 fff49a34: 38 21 00 10 addi r1,r1,16 fff49a38: 7c 08 03 a6 mtlr r0
This was causing a consistent hard lockup during the MMC read / loading of the QoriQ FMan firmware on a P2041RDB board.
Re-adding the header causes identical assembly to be emitted and allows the firmware loading and subsequent boot to succeed.
Fixes: 401d1c4f5d ("common: Drop asm/global_data.h from common header") Signed-off-by: Matt Merhar mattmerhar@protonmail.com
Did you try to investigate what exactly causes this difference in the assembly code, when the header is not included? Re-adding this header seems like "papering over" the real problem to me.
As Rasmus helpfully pointed out, the global_data struct has a different layout depending on whether CONFIG_POST is defined.
In this case, CONFIG_POST is inconsistently defined depending on which headers are included by the various bits that reference that struct.
For the P2041RDB, the define now comes in like:
common.h -> config.h -> configs/P2041RDB.h -> #define CONFIG_POST CONFIG_SYS_POST_MEMORY
Would it help to clarify this in the commit message? Would it be better to include config.h instead of common.h? I could resend the patch with either or both changes. CONFIG_POST seems to only be defined in a handful of headers in include/configs/.
Thanks, Stefan
arch/powerpc/lib/traps.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/lib/traps.c b/arch/powerpc/lib/traps.c index c7bce82a44..ab8ca269a5 100644 --- a/arch/powerpc/lib/traps.c +++ b/arch/powerpc/lib/traps.c @@ -4,6 +4,7 @@
- Wolfgang Denk, DENX Software Engineering, wd@denx.de.
*/
+#include <common.h> #include <init.h> #include <asm/global_data.h>
Viele Grüße, Stefan
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
participants (1)
-
Matt Merhar