[PATCH] m68k/mvme147: Don't unregister boot console needlessly

[PATCH] m68k/mvme147: Don't unregister boot console needlessly

[Finn Thain]

When MACH_IS_MVME147, the boot console calls mvme147_scc_write() to
generate console output. That will continue to work even after
debug_cons_nputs() becomes unavailable so there's no need to
unregister the boot console.

Cc: Daniel Palmer <daniel@0x0f.com>
Fixes: 077b33b9e283 ("m68k: mvme147: Reinstate early console")
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
 arch/m68k/kernel/early_printk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/m68k/kernel/early_printk.c b/arch/m68k/kernel/early_printk.c
index f11ef9f1f56f..2fb22d8395b5 100644
--- a/arch/m68k/kernel/early_printk.c
+++ b/arch/m68k/kernel/early_printk.c
@@ -60,7 +60,7 @@ early_param("earlyprintk", setup_early_printk);
 
 static int __init unregister_early_console(void)
 {
-	if (!early_console || MACH_IS_MVME16x)
+	if (!early_console || MACH_IS_MVME147 || MACH_IS_MVME16x)
 		return 0;
 
 	return unregister_console(early_console);
-- 
2.45.3

Re: [PATCH] m68k/mvme147: Don't unregister boot console needlessly

[Geert Uytterhoeven]

Hi Finn,

On Thu, 27 Mar 2025 at 23:39, Finn Thain <fthain@linux-m68k.org> wrote:
> When MACH_IS_MVME147, the boot console calls mvme147_scc_write() to
> generate console output. That will continue to work even after
> debug_cons_nputs() becomes unavailable so there's no need to
> unregister the boot console.
>
> Cc: Daniel Palmer <daniel@0x0f.com>
> Fixes: 077b33b9e283 ("m68k: mvme147: Reinstate early console")
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>

Thanks for your patch!

> --- a/arch/m68k/kernel/early_printk.c
> +++ b/arch/m68k/kernel/early_printk.c
> @@ -60,7 +60,7 @@ early_param("earlyprintk", setup_early_printk);
>
>  static int __init unregister_early_console(void)
>  {
> -       if (!early_console || MACH_IS_MVME16x)
> +       if (!early_console || MACH_IS_MVME147 || MACH_IS_MVME16x)
>                 return 0;
>
>         return unregister_console(early_console);

Perhaps the whole function and the late_initcall() can just be removed?

When the real console kicks in, it usually replaces the early console
(unless "keep_bootcon" is specified on the kernel command line, which
causes all kernel messages to be printed to both the early and normal
console (and thus may cause duplication, if they are the same device)).

Or does that apply only to earlycon early consoles, and not to
earlyprintk?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] m68k/mvme147: Don't unregister boot console needlessly

[Finn Thain]

On Fri, 28 Mar 2025, Geert Uytterhoeven wrote:

> > --- a/arch/m68k/kernel/early_printk.c
> > +++ b/arch/m68k/kernel/early_printk.c
> > @@ -60,7 +60,7 @@ early_param("earlyprintk", setup_early_printk);
> >
> >  static int __init unregister_early_console(void)
> >  {
> > -       if (!early_console || MACH_IS_MVME16x)
> > +       if (!early_console || MACH_IS_MVME147 || MACH_IS_MVME16x)
> >                 return 0;
> >
> >         return unregister_console(early_console);
> 
> Perhaps the whole function and the late_initcall() can just be removed?
> 

A comment in arch/m68k/kernel/early_printk.c gives the reason why that 
code exists: debug_cons_nputs() lives in .init.text. Platforms like MVME 
which don't use that function to implement earlyprintk don't have to worry 
about this.

I suppose MACH_IS_FOO is not a great way to encode that requirement. But 
it don't think it has to be self-documenting. It does have to be 
consistent with the conditionals in head.S.

Re: [PATCH] m68k/mvme147: Don't unregister boot console needlessly

[Geert Uytterhoeven]

Hi Finn,

On Fri, 28 Mar 2025 at 23:25, Finn Thain <fthain@linux-m68k.org> wrote:
> On Fri, 28 Mar 2025, Geert Uytterhoeven wrote:
> > > --- a/arch/m68k/kernel/early_printk.c
> > > +++ b/arch/m68k/kernel/early_printk.c
> > > @@ -60,7 +60,7 @@ early_param("earlyprintk", setup_early_printk);
> > >
> > >  static int __init unregister_early_console(void)
> > >  {
> > > -       if (!early_console || MACH_IS_MVME16x)
> > > +       if (!early_console || MACH_IS_MVME147 || MACH_IS_MVME16x)
> > >                 return 0;
> > >
> > >         return unregister_console(early_console);
> >
> > Perhaps the whole function and the late_initcall() can just be removed?
> >
>
> A comment in arch/m68k/kernel/early_printk.c gives the reason why that
> code exists: debug_cons_nputs() lives in .init.text. Platforms like MVME
> which don't use that function to implement earlyprintk don't have to worry
> about this.

Thanks, I had forgotten about that...

> I suppose MACH_IS_FOO is not a great way to encode that requirement. But
> it don't think it has to be self-documenting. It does have to be
> consistent with the conditionals in head.S.

Alternatively, check if early_console_instance.write() lies within
__init_begin..init_end?
That doesn't work with the current setup, as .write() always points
to the common debug_cons_write().  But it can be made to work
by setting up .write() with the proper function pointer in
setup_early_printk(), getting rid of repeated MACH_IS_*() checks in
the process.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds