Re: [PATCH v3 3/3] sparc: Implement arch_do_panic

[email protected]
Newsgroups org.kernel.vger.linux-s390
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Missing explicit `#include <linux/panic.h>` and `#include <linux/printk.h>` in `arch/sparc/kernel/setup.c`.
- [Low] Redundant local block-scoped `extern` declaration for `stop_a_enabled`.
--

--- Patch [3]: [PATCH v3 3/3] sparc: Implement arch_do_panic ---
commit 96c609160e8ec354a006cf513eb7fe2d614de6bc
Author: Mete Durlu <[email protected]>

sparc: Implement arch_do_panic

This commit implements a sparc specific arch_do_panic() instead of using
ifdef sections in vpanic() code. It also reorders arch specific panic handling.

> diff --git a/arch/sparc/kernel/setup.c b/arch/sparc/kernel/setup.c
> index 4975867d9001b..87d3569c520f8 100644
> --- a/arch/sparc/kernel/setup.c
> +++ b/arch/sparc/kernel/setup.c
[ ... ]
> @@ -36,6 +36,14 @@ static const struct ctl_table sparc_sysctl_table[] = {
>  #endif
>  };
>  
> +void arch_do_panic(void)
> +{

[Severity: Low]
Should this file explicitly include <linux/panic.h> and <linux/printk.h>?

Since arch_do_panic() is a globally exported function, missing the panic.h
header can cause a missing prototypes warning during compilation. It might
also be better to explicitly include printk.h for pr_emerg() rather than
relying on implicit header inclusions.

> +	extern int stop_a_enabled;

[Severity: Low]
Is this block-scoped extern declaration still needed?

While this declaration was copied from its old location in kernel/panic.c,
it is redundant here because arch/sparc/kernel/setup.c already includes
<asm/setup.h>, which provides the exact same global extern declaration for
stop_a_enabled. 

> +	/* Make sure the user can actually press Stop-A (L1-A) */
> +	stop_a_enabled = 1;
> +	pr_emerg("Press Stop-A (L1-A) from sun keyboard or send break\n"
> +		 "twice on console to return to the boot prom\n");
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.