On Thu, Apr 22, 2021 at 11:27 PM Christophe Leroy
[off-list ref] wrote:

Le 22/04/2021 à 17:10, Xiongwei Song a écrit :

quoted

From: Xiongwei Song <redacted>

Create a new function named interrupt_detail_printable to judge which
interrupts can print esr/dsisr register.

What is the benefit of that function ? It may be interesting if the test was done at several places,
but as it is done at only one place, I don't thing it is an improvement.

Until know, you new immediately what was the traps that would print it. Now you have to go and look
into a sub-function.

How about replace if statement with switch statement directly, like
the changes below:

@@ -1467,13 +1481,17 @@ static void __show_regs(struct pt_regs *regs)
        trap = TRAP(regs);
        if (!trap_is_syscall(regs) && cpu_has_feature(CPU_FTR_CFAR))
                pr_cont("CFAR: "REG" ", regs->orig_gpr3);
-       if (trap == INTERRUPT_MACHINE_CHECK ||
-           trap == INTERRUPT_DATA_STORAGE ||
-           trap == INTERRUPT_ALIGNMENT) {
+       switch(trap){
+       case INTERRUPT_MACHINE_CHECK:
+       case INTERRUPT_DATA_STORAGE:
+       case INTERRUPT_ALIGNMENT:
                if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
                        pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar,

regs->dsisr);
                else
                        pr_cont("DAR: "REG" DSISR: %08lx ", regs->dar,
regs->dsisr);
+               break;
+       default:
+               break;
        }

Thanks,
Xiongwei