[PATCH 1/1] riscv: simplify backtrace report

[PATCH 1/1] riscv: simplify backtrace report

[Heinrich Schuchardt]

* We already have a header 'backtrace', there is no need to repeat the
  word backtrace on every line.
* Add a blank line before the backtrace section of the crash report for
  improved readability.
* If U-Boot is compiled without backtrace, there is no need to write a
  message at all.
* Avoid #ifdef. We prefer functions to always be compiled and let
  the linker remove them if not needed.
* Foresee 3 digits for the backtrace index.

For testing the 'exception' command can be used.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 arch/riscv/lib/interrupts.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c
index 7350e2ced85..f9a1428a486 100644
--- a/arch/riscv/lib/interrupts.c
+++ b/arch/riscv/lib/interrupts.c
@@ -60,21 +60,20 @@ static void show_regs(struct pt_regs *regs)
 #endif
 }
 
-#if defined(CONFIG_FRAMEPOINTER) || defined(CONFIG_SPL_FRAMEPOINTER)
-static void show_backtrace(struct pt_regs *regs)
+static void __maybe_unused show_backtrace(struct pt_regs *regs)
 {
 	uintptr_t *fp = (uintptr_t *)regs->s0;
 	unsigned count = 0;
 	ulong ra;
 
-	printf("backtrace:\n");
+	printf("\nbacktrace:\n");
 
 	/* there are a few entry points where the s0 register is
 	 * set to gd, so to avoid changing those, just abort if
 	 * the value is the same */
 	while (fp != NULL && fp != (uintptr_t *)gd) {
 		ra = fp[-1];
-		printf("backtrace %2d: FP: " REG_FMT " RA: " REG_FMT,
+		printf("%3d: FP: " REG_FMT " RA: " REG_FMT,
 		       count, (ulong)fp, ra);
 
 		if (gd && gd->flags & GD_FLG_RELOC)
@@ -87,12 +86,6 @@ static void show_backtrace(struct pt_regs *regs)
 		count++;
 	}
 }
-#else
-static void show_backtrace(struct pt_regs *regs)
-{
-	printf("No backtrace support enabled\n");
-}
-#endif
 
 /**
  * instr_len() - get instruction length
@@ -165,7 +158,8 @@ static void _exit_trap(ulong code, ulong epc, ulong tval, struct pt_regs *regs)
 		       epc - gd->reloc_off, regs->ra - gd->reloc_off);
 
 	show_regs(regs);
-	show_backtrace(regs);
+	if (CONFIG_IS_ENABLED(FRAMEPOINTER))
+		show_backtrace(regs);
 	show_code(epc);
 	show_efi_loaded_images(epc);
 	panic("\n");
-- 
2.43.0

Re: [PATCH 1/1] riscv: simplify backtrace report

[Leo Liang]

On Tue, May 14, 2024 at 07:51:42AM +0200, Heinrich Schuchardt wrote:
> * We already have a header 'backtrace', there is no need to repeat the
>   word backtrace on every line.
> * Add a blank line before the backtrace section of the crash report for
>   improved readability.
> * If U-Boot is compiled without backtrace, there is no need to write a
>   message at all.
> * Avoid #ifdef. We prefer functions to always be compiled and let
>   the linker remove them if not needed.
> * Foresee 3 digits for the backtrace index.
> 
> For testing the 'exception' command can be used.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  arch/riscv/lib/interrupts.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>