lkml.org 
[lkml]   [2021]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] powerpc: Make the code in __show_regs nice-looking
From
Date


Le 22/04/2021 à 17:10, Xiongwei Song a écrit :
> From: Xiongwei Song <sxwjean@gmail.com>
>
> 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.

And the name is not nice either. All other functions testing anything on the trap type are called
trap_is_something()

Here your function would be better called something like trap_sets_dsisr()

>
> Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
> ---
> arch/powerpc/kernel/process.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 89e34aa273e2..5c3830837f3a 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1453,9 +1453,23 @@ static void print_msr_bits(unsigned long val)
> #define REGS_PER_LINE 8
> #endif
>
> +static bool interrupt_detail_printable(int trap)
> +{
> + switch (trap) {
> + case INTERRUPT_MACHINE_CHECK:
> + case INTERRUPT_DATA_STORAGE:
> + case INTERRUPT_ALIGNMENT:
> + return true;
> + default:
> + return false;
> + }
> +
> + return false;

That's redundant with the default case inside the switch.

> +}
> +
> static void __show_regs(struct pt_regs *regs)
> {
> - int i, trap;
> + int i;
>
> printk("NIP: "REG" LR: "REG" CTR: "REG"\n",
> regs->nip, regs->link, regs->ctr);
> @@ -1464,12 +1478,9 @@ static void __show_regs(struct pt_regs *regs)
> printk("MSR: "REG" ", regs->msr);
> print_msr_bits(regs->msr);
> pr_cont(" CR: %08lx XER: %08lx\n", regs->ccr, regs->xer);
> - 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) {
> + if (interrupt_detail_printable(TRAP(regs))) {
> if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->dsisr);
> else
>

\
 
 \ /
  Last update: 2021-04-22 17:28    [W:0.163 / U:0.996 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site