kill prom_printf

Hi,

I was told by many people that prom_printf and friends should die and early_printk should be used instead. this patch (against 2.5) does first part of job. compiles and works on IP22 (SNI RM200 and IP32 don't compile anyway). Feedback appreciated, as always.

Index: arch/mips/Kconfig-shared
===================================================================
RCS file: /home/cvs/linux/arch/mips/Kconfig-shared,v retrieving revision 1.50

diff -u -r1.50 Kconfig-shared
@@ -679,11 +679,6 @@
 	depends on SNI_RM200_PCI || SGI_IP32
 	default y
 
-config ARC_PROMLIB
-	bool
-	depends on SNI_RM200_PCI || SGI_IP32 || SGI_IP22
-	default y
-
 config BOARD_SCACHE
 	bool
 	depends on MIPS_EV96100 || MOMENCO_OCELOT || SGI_IP22

Index: arch/mips/arc/Makefile
===================================================================
RCS file: /home/cvs/linux/arch/mips/arc/Makefile,v retrieving revision 1.9

diff -u -r1.9 Makefile
@@ -8,5 +8,4 @@
 				   misc.o time.o tree.o
 
 obj-$(CONFIG_ARC_MEMORY)	+= memory.o
-obj-$(CONFIG_ARC_CONSOLE)	+= arc_con.o
-obj-$(CONFIG_ARC_PROMLIB)	+= promlib.o
+obj-$(CONFIG_ARC_CONSOLE)	+= console.o

Index: arch/mips/arc/arc_con.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/arc/arc_con.c,v retrieving revision 1.10

Index: arch/mips/arc/console.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/arc/console.c,v retrieving revision 1.12

Index: arch/mips/arc/init.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/arc/init.c,v
retrieving revision 1.13

diff -u -r1.13 init.c
@@ -12,8 +12,6 @@
 
 #include <asm/sgialib.h>
 
-#undef DEBUG_PROM_INIT
-
 /* Master romvec interface. */
 struct linux_romvec *romvec;
 int prom_argc;

@@ -28,9 +26,9 @@
 	_prom_envp = (LONG *) envp;
 
 	if (pb->magic != 0x53435241) {
-		prom_printf("Aieee, bad prom vector magic %08lx\n", pb->magic);
-		while(1)
-			;
+		printk(KERN_EMERG "Aieee, bad prom vector magic %08lx\n",
+			pb->magic);
+		while(1) ;
 	}
 
 	prom_init_cmdline();

@@ -38,10 +36,4 @@
 	printk(KERN_INFO "PROMLIB: ARC firmware Version %d Revision %d\n",
 	       pb->ver, pb->rev);
 	prom_meminit();
-
-#ifdef DEBUG_PROM_INIT
-	prom_printf("Press a key to reboot\n");
-	prom_getchar();
-	ArcEnterInteractiveMode();
-#endif
 }

Index: arch/mips/arc/memory.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/arc/memory.c,v retrieving revision 1.24

diff -u -r1.24 memory.c
@@ -112,11 +112,11 @@
 #ifdef DEBUG
 	int i = 0;
 
-	prom_printf("ARCS MEMORY DESCRIPTOR dump:\n");
+	printk(KERN_DEBUG "ARCS MEMORY DESCRIPTOR dump:\n");
 	p = ArcGetMemoryDescriptor(PROM_NULL_MDESC);
 	while(p) {
-		prom_printf("[%d,%p]: base<%08lx> pages<%08lx> type<%s>\n",
-			    i, p, p->base, p->pages, mtypes(p->type));
+		printk("[%d,%p]: base<%08lx> pages<%08lx> type<%s>\n",
+			i, p, p->base, p->pages, mtypes(p->type));
 		p = ArcGetMemoryDescriptor(p);
 		i++;
 	}

Index: arch/mips/arc/promlib.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/arc/promlib.c,v retrieving revision 1.2

Index: arch/mips/arc/tree.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/arc/tree.c,v
retrieving revision 1.4

diff -u -r1.4 tree.c
@@ -93,11 +93,11 @@
 static void __init
 dump_component(pcomponent *p)
 {
-	prom_printf("[%p]:class<%s>type<%s>flags<%s>ver<%d>rev<%d>",
-		    p, classes[p->class], types[p->type],
-		    iflags[p->iflags], p->vers, p->rev);
-	prom_printf("key<%08lx>\n\tamask<%08lx>cdsize<%d>ilen<%d>iname<%s>\n",
-		    p->key, p->amask, (int)p->cdsize, (int)p->ilen, p->iname);
+	printk(KERN_DEBUG "[%p]:class<%s>type<%s>flags<%s>ver<%d>rev<%d>",
+		p, classes[p->class], types[p->type],
+		iflags[p->iflags], p->vers, p->rev);
+	printk("key<%08lx>\n\tamask<%08lx>cdsize<%d>ilen<%d>iname<%s>\n",
+		p->key, p->amask, (int)p->cdsize, (int)p->ilen, p->iname);
 }
 
 static void __init

Hmm, strange idea -- I guess that originates from systems that have no
suitable firmware to perform such an operation at the console. Currently only x86_64 implements early_printk() -- if we have an implementation for MIPS, we may consider removing the alternative. Also prom_printf() comes almost for free and works very early and as I see in the x86_64 version early_printk() requires initialization of a console driver, which may be unfortunate if debugging a problem within the driver.

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

maciej> Hmm, strange idea -- I guess that originates from systems that have no maciej> suitable firmware to perform such an operation at the console. Currently maciej> only x86_64 implements early_printk() -- if we have an implementation for maciej> MIPS, we may consider removing the alternative. Also prom_printf() comes maciej> almost for free and works very early and as I see in the x86_64 version maciej> early_printk() requires initialization of a console driver, which may be maciej> unfortunate if debugging a problem within the driver.

There is at least one implementation for x86 that uses the VGA directly (as the BIOS left it), getting it very soon.

But you are right, PC's speciality is not to have a nice console. Anyways, you can use early_printk() in MIPS. You only need to put the setup of the early console sooner, as for you the setup is basically a NOP.

Later, Juan.

In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

[snip]

(I'm only implementing it and I don't care how good idea it is :-) Anyway I still think that firmware should be used directly to print message before kernel halts in situations like arch/mips/arc/init.c at line 30)

Maciej, i'll try to make some conclusion from what I was told (and Juan is welcome to correct me if I missunderstood something)

Idea is to have only one way for printing kernel messages. In case of Indy, O2 and SNI RM200 "arc" console will do it. Here you can find where should be early console initialized:
http://marc.theaimsgroup.com/?l=linux-kernel&m=105519188505235&w=2
As Juan pointed out setup for such console is actually a nop and one is supposed to enable this feature only when debugging kernel. DEC prom console however needs some setup to determine REX entry points. early console is currently used on sh, alpha, x86_64 and ia64 architectures. Btw, see comment at the top of arch/sparc/prom/printf.c

Regards, ladis

That's a valid point, as long as enabling it does not require a
reconfiguration.

The DEC's entry points are a part of the problem only -- to support a
generic kernel, we need to move early_printk setup after setup_arch(), as the level of variation is huge then..

There is also that minor implementation problem -- how to pass varargs
from printk() to ROM's printf()? At least the firmware of the DECstation implements a full-featured printf() as in the C library.

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

yes, you'll have to do basic setup in your setup_console function...

you are implementing early console not printf (sorry again for confusion), so there is no need to pass varargs anywhere. btw, early_printk() as known from other archs is supposed to die in future. printk() should be used everywhere.

better to show patch...
(note that setup_early_printk() will be probably called before init_arch. how to choose right early console is still open question, but as we have not generic kernel yet, we can use same way as we are using for choosing proper prom_printf function. this patch was included in hope that it can show principle nothing more...)

Index: arch/mips/kernel/Makefile
===================================================================
RCS file: /home/cvs/linux/arch/mips/kernel/Makefile,v retrieving revision 1.69

diff -u -r1.69 Makefile
@@ -43,4 +43,6 @@
 
 obj-$(CONFIG_MODULES)		+= module.o
 
+obj-$(CONFIG_EARLY_PRINTK)	+= earlyprintk.o
+
 EXTRA_AFLAGS := $(CFLAGS)

Index: arch/mips/kernel/setup.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/kernel/setup.c,v retrieving revision 1.147

diff -u -r1.147 setup.c
@@ -113,6 +113,8 @@
 asmlinkage void __init
 init_arch(int argc, char **argv, char **envp, int *prom_vec)
 {
+	setup_early_printk();
+
 	/* Determine which MIPS variant we are running on. */
 	cpu_probe();
 

@@ -0,0 +1,35 @@
+#include <linux/console.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/string.h>
+#include <linux/config.h>
+
+struct console *early_console = NULL;
+
+int __init setup_early_printk(void)
+{  
+#ifdef CONFIG_ARC_CONSOLE
+	{
+		extern struct console arc_console;
+		early_console = &arc_console;
+	}
+#endif
+#ifdef CONFIG_PROM_CONSOLE
+	{
+		extern struct console prom_console;
+		early_console = &prom_console;
+	}
+#endif
+
+	register_console(early_console);
+	return 0;
+}
+
+void __init disable_early_printk(void)
+{ 
+	if (!early_console)
+		return;
+
+	printk(KERN_INFO "Disabling early console...\n");
+	unregister_console(early_console);
+} 

Hmm, calling the firmware for each character separately will certainly be
terribly slow, though it may be negligible as normally few messages will be output this way. And since the call to prom_printf() is so cheap for the DECstation, I'm going to retain the function for real low-level debugging, whether otherwise used or not.

BTW,I just realized console output via the firmware is mandatory for the
DECstation -- we have cases where the kernel is not going to be started far enough to have any console set up because of a misconfiguration. With current prom_printf() implementation the reason is output to the console and a user has a chance to know why. With an optional early printk, he'll just see the kernel return to the firmware for no apparent reason without any output.

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

kernel/printk.c doesn't call the low-level output routine for each character separately, but passes complete strings of characters.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

So I can just call prom_printf("%s", string), right? This would solve
this shortcoming.

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

More or less. The caveat is that console->write() is not called with a NULL-terminated string, but with a pointer and a length.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Well, I can't remember if the DEC's printf() supports "%<width>s", but
even if it doesn't, we can use a helper local buffer.

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

FYI, by the spec it does support the width modifier, but I can't verify
it at the run time right now.

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

Well, I would see early_printk() as advantageous if it was also capable
to leave messages in the kernel ring buffer for dmesg or klogd to fetch.

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

Ah, we probably don't understand each other. I should type EARLY_PRINTK instead of early_printk (sorry for my lazyness, I'm usually typing in lowercase). CONFIG_EARLY_PRINTK enables early console, you are supposed to use printk everywhere and that way you achieve such functionality.

ladis

So you need to explicitly configure it? That's very bad.

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

I think we can leave it enabled by default, since it doesn't hurt too much. Kernel cmdline argument could control usage of early console.

ladis

On Tue, 2003-06-17 13:18:59 +0100, Ladislav Michl <[email protected]> wrote in message <[email protected]>:

If we constantly add (new) kernel arguments, we may at some time face the problem that the calling PROM/firmware/whatever cannot handle a command line which is *that* long. IIRC DECstations have a quite limited prompt length. This hurts for "3/tftp():vmecoff root=/dev/ram nfsroot=/nfsroot/decxxxx ip=bootp console=ttyS2 console=tty0 early_printk=arc"

I'm just thinking about numerizing all __setup() calls so that you maybe can use something C99 style like: .15=/dev/ttyS0 (where .15 is the fiveteenth variable which is "console".

All that needs to be done are some lines at command line parsing and a small tool/script to extract correct values and their corresponding __setup() "name".

MfG, JBG

   Jan-Benedict Glaw       [email protected]    . +49-172-7608481
   "Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg
    fuer einen Freien Staat voll Freier Bürger" | im Internet! |   im Irak!
      ret = do_actions((curr | FREE_SPEECH) & ~(IRAQ_WAR_2 | DRM | TCPA));

On Tue, 2003-06-17 14:32:13 +0200, Jan-Benedict Glaw <[email protected]> wrote in message <[email protected]>:

Hmmm. That's almose trivial:) ./kernel/params.c:parse_one() needs to be tweaked a bit. The rest is a small script to tell us what's between __start___param and __stop___param.

MfG, JBG

   Jan-Benedict Glaw       [email protected]    . +49-172-7608481
   "Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg
    fuer einen Freien Staat voll Freier Bürger" | im Internet! |   im Irak!
      ret = do_actions((curr | FREE_SPEECH) & ~(IRAQ_WAR_2 | DRM | TCPA));

That has already been trained by the Alpha people. I think we simply
need a bootloader capable of handling network boots -- with the REX firmware it shouldn't be that difficult. For disk loads I suppose it's already handled by delo (I haven't checked). We don't have a tape loader, do we?

BTW, you may omit "nfsroot=" above if you send a root path in a BOOTP
reply and also "ip=bootp" shouldn't be necessary if the causing bug was fixed (I have a temporary patch). The limit is 37 characters for the "boot" variable (due to the limitation of the BBU RAM) and a bit higher for an explicitly typed in command line (I think it's 80 characters, but I can't remember for sure).

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

That's not the point -- someone will sooner or later disable it and
someone else will use that kernel and report bugs stating there is nothing output and the kernel hangs. And he might be unable to rebuild the kernel for whatever reason or the rebuilt kernel will "magically" work.

No problem -- it's like the "debug" option today, that I can ask someone
to add to get a more detailed report (my systems use it by default).

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

maciej> So you need to explicitly configure it? That's very bad.

You bet:
- you force everybody to use early_printk (you only want it for debugging).
- you configure early_printk for everybody (never have to configure it).

You can't have the cake and eat it :(

Why do you ever will want not to use early_printk?

when you are using a console that is not the prom console (why do you want to do that on MIPS is a misterios to me), but for other archs/machines it make sense:

think on a machine that you now it boots to use a network console.

Later, Juan.

In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

I'm not sure what you mean. Please elaborate.

I won't, but someone else certainly will.

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

maciej> On Tue, 17 Jun 2003, Juan Quintela wrote:
maciej> So you need to explicitly configure it? That's very bad.

maciej> I'm not sure what you mean. Please elaborate.

As it is used in the other platforms:

- you setup your console (there is a console by default).
- until that console is initilized, messages are buffered.

that is clearly bad if your system stops before initializing the
console: i.e. zero output.

- early_printk to the rescue, as soon as you can print, you initialize the console, and begin printing.

- so far so god.

- now it is time to initialize the real console (reading console=<blah> ....)
- output from now one goes to the real console.

Problems:
a - you want all your messages in your console, and your console is not
the console used by early_printk. Some meassages dissapear, why? because early_printk is the default -> you don't want early_printk by default.uu

b - you want at least some output if the kernel hangs early -> you want
early_printk by default.

I am not able to make happy people in the a) group or people in the b) group, but not both at the same time with the default early_console.

I hope that now explanation is better (for sure that it is larger).

maciej> I won't, but someone else certainly will.

I meaned here somebody in general, not you in particular :p

Later, Juan.

In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

No, if the `real' console has the CON_PRINTBUFFER flag set, all old buffered messages will be sent again to that console upon registration.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

geert> No, if the `real' console has the CON_PRINTBUFFER flag set, all old buffered geert> messages will be sent again to that console upon registration.

Didn't knew that.
Then it could be enabled by default without any adverse effect.

Later, Juan.

In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy