Re: [PATCH v3 3/3] sparc: Implement arch_do_panic
| 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