[PATCH 1/3] Provide console_device()

[PATCH 1/3] Provide console_device()

[Russell King]

[This patch series has also been separately sent to the architecture
 maintainers]

Add console_device() to return the console tty driver structure and the
index.  Acquire the console lock while scanning the list of console
drivers to protect us against console driver list manipulations.

Signed-off-by: Russell King <rmk@arm.linux.org.uk>

diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/drivers/char/tty_io.c linux/drivers/char/tty_io.c
--- orig/drivers/char/tty_io.c	Wed Jun  9 00:51:02 2004
+++ linux/drivers/char/tty_io.c	Mon Jun 14 10:55:28 2004
@@ -1344,13 +1344,8 @@ retry_open:
 	}
 #endif
 	if (device == MKDEV(TTYAUX_MAJOR,1)) {
-		struct console *c = console_drivers;
-		for (c = console_drivers; c; c = c->next) {
-			if (!c->device)
-				continue;
-			driver = c->device(c, &index);
-			if (!driver)
-				continue;
+		driver = console_device(&index);
+		if (driver) {
 			/* Don't let /dev/console block */
 			filp->f_flags |= O_NONBLOCK;
 			noctty = 1;
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/kernel/printk.c linux/kernel/printk.c
--- orig/kernel/printk.c	Sun May 30 10:33:00 2004
+++ linux/kernel/printk.c	Mon Jun 14 10:56:18 2004
@@ -683,6 +683,23 @@ void console_unblank(void)
 }
 EXPORT_SYMBOL(console_unblank);
 
+struct tty_driver *console_device(int *index)
+{
+	struct console *c;
+	struct tty_driver *driver = NULL;
+
+	acquire_console_sem();
+	for (c = console_drivers; c != NULL; c = c->next) {
+		if (!c->device)
+			continue;
+		driver = c->device(c, index);
+		if (driver)
+			break;
+	}
+	release_console_sem();
+	return driver;
+}
+
 /*
  * The console driver calls this routine during kernel initialization
  * to register the console printing procedure with printk() and to
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/include/linux/console.h linux/include/linux/console.h
--- orig/include/linux/console.h	Mon May 24 11:26:21 2004
+++ linux/include/linux/console.h	Mon Jun 14 10:55:28 2004
@@ -104,6 +104,7 @@ extern void acquire_console_sem(void);
 extern void release_console_sem(void);
 extern void console_conditional_schedule(void);
 extern void console_unblank(void);
+extern struct tty_driver *console_device(int *);
 extern int is_console_locked(void);
 
 /* Some debug stub to catch some of the obvious races in the VT code */

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH 2/3] Provide console_suspend() and console_resume()

[Russell King]

Add console_suspend() and console_resume() methods so the serial drivers
can disable console output before suspending a port, and re-enable output
afterwards.

We also add locking to ensure that we synchronise with any in-progress
printk.

Signed-off-by: Russell King <rmk@arm.linux.org.uk>

diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/drivers/serial/serial_core.c linux/drivers/serial/serial_core.c
--- orig/drivers/serial/serial_core.c	Sun May 30 10:32:45 2004
+++ linux/drivers/serial/serial_core.c	Mon Jun 14 10:59:49 2004
@@ -1923,7 +1923,7 @@ int uart_suspend_port(struct uart_driver
 	 * Disable the console device before suspending.
 	 */
 	if (uart_console(port))
-		port->cons->flags &= ~CON_ENABLED;
+		console_suspend(port->cons);
 
 	uart_change_pm(state, 3);
 
@@ -1945,7 +1945,7 @@ int uart_resume_port(struct uart_driver 
 	 */
 	if (uart_console(port)) {
 		uart_change_speed(state, NULL);
-		port->cons->flags |= CON_ENABLED;
+		console_resume(port->cons);
 	}
 
 	if (state->info && state->info->flags & UIF_INITIALIZED) {
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/kernel/printk.c linux/kernel/printk.c
--- orig/kernel/printk.c	Mon Jun 14 11:01:49 2004
+++ linux/kernel/printk.c	Mon Jun 14 11:01:37 2004
@@ -700,6 +700,22 @@ struct tty_driver *console_device(int *i
 	return driver;
 }
 
+void console_suspend(struct console *console)
+{
+	acquire_console_sem();
+	console->flags &= ~CON_ENABLED;
+	release_console_sem();
+}
+EXPORT_SYMBOL(console_suspend);
+
+void console_resume(struct console *console)
+{
+	acquire_console_sem();
+	console->flags |= CON_ENABLED;
+	release_console_sem();
+}
+EXPORT_SYMBOL(console_resume);
+
 /*
  * The console driver calls this routine during kernel initialization
  * to register the console printing procedure with printk() and to
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej orig/include/linux/console.h linux/include/linux/console.h
--- orig/include/linux/console.h	Mon Jun 14 11:01:49 2004
+++ linux/include/linux/console.h	Mon Jun 14 11:00:21 2004
@@ -105,6 +105,8 @@ extern void release_console_sem(void);
 extern void console_conditional_schedule(void);
 extern void console_unblank(void);
 extern struct tty_driver *console_device(int *);
+extern void console_suspend(struct console *);
+extern void console_resume(struct console *);
 extern int is_console_locked(void);
 
 /* Some debug stub to catch some of the obvious races in the VT code */

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [RFC][PATCH 3/3] Convert console list to list_head

[Russell King]

Convert console drivers to use list_head rather than its own private
list implementation.  Unfortunately, several other places reference
"console_drivers" directly, all of them without locking.  PPC/PPC64
even seems to export the list for no apparant reason.

arch/ia64/hp/sim/simserial.c:extern struct console *console_drivers; /* from kernel/printk.c */
arch/ia64/hp/sim/simserial.c:   console = console_drivers;

  This is interesting - it appears to search the console driver list
  for the first console, hold a reference to it, and call its write
  method.  If a console were to be unregistered via unregister_console()
  this would potentially oops.

arch/parisc/kernel/pdc_cons.c:  while ((console = console_drivers) != NULL)
arch/parisc/kernel/pdc_cons.c:          unregister_console(console_drivers);
arch/parisc/kernel/traps.c:     if (!console_drivers)
arch/parisc/kernel/traps.c:     if (!console_drivers)

  PARISC seems to like to unregister all consoles, and then re-
  registers its own console driver.  IOW, it assumes it can safely
  override the state of the console subsystem independent of whatever
  drivers may be registered.

arch/ppc/kernel/ppc_ksyms.c:EXPORT_SYMBOL(console_drivers);
arch/ppc64/kernel/ppc_ksyms.c:EXPORT_SYMBOL(console_drivers);

  Random exports of this kernel symbol...

arch/sparc64/kernel/setup.c:    cons = console_drivers;
arch/sparc64/kernel/setup.c:            cons = console_drivers;

  Sparc64 appears to want to temporarily override the console
  subsystem for debugging purposes.  Maybe there's a better way?

Since "console_drivers" vanishes as a result of this, all the above
break, so this needs further work before it can be merged into 2.6.
Can people consider the above and come up with both a cleaner and
saner solution?

Signed-Off-By: David Woodhouse <dwmw2@infradead.org>
Signed-Off-By: Russell King <rmk@arm.linux.org.uk>

--- linux/kernel/printk.c.console	Mon Jun 14 11:01:37 2004
+++ linux/kernel/printk.c	Mon Jun 14 11:06:07 2004
@@ -61,7 +61,8 @@
  * driver system.
  */
 static DECLARE_MUTEX(console_sem);
-struct console *console_drivers;
+static LIST_HEAD(console_driver_list);
+
 /*
  * This is used for debugging the mess that is the VT code by
  * keeping track if we have the console semaphore held. It's
@@ -383,7 +384,7 @@
 {
 	struct console *con;
 
-	for (con = console_drivers; con; con = con->next) {
+	list_for_each_entry(con, &console_driver_list, list) {
 		if ((con->flags & CON_ENABLED) && con->write)
 			con->write(con, &LOG_BUF(start), end - start);
 	}
@@ -396,7 +397,7 @@
 				unsigned long end, int msg_log_level)
 {
 	if (msg_log_level < console_loglevel &&
-			console_drivers && start != end) {
+	    !list_empty(&console_driver_list) && start != end) {
 		if ((start & LOG_BUF_MASK) > (end & LOG_BUF_MASK)) {
 			/* wrapped write */
 			__call_console_drivers(start & LOG_BUF_MASK,
@@ -580,7 +581,7 @@
  * acquire_console_sem - lock the console system for exclusive use.
  *
  * Acquires a semaphore which guarantees that the caller has
- * exclusive access to the console system and the console_drivers list.
+ * exclusive access to the console system and the console_driver_list.
  *
  * Can sleep, returns nothing.
  */
@@ -676,7 +677,7 @@
 		return;
 	console_locked = 1;
 	console_may_schedule = 0;
-	for (c = console_drivers; c != NULL; c = c->next)
+	list_for_each_entry(c, &console_driver_list, list)
 		if ((c->flags & CON_ENABLED) && c->unblank)
 			c->unblank();
 	release_console_sem();
@@ -689,7 +690,7 @@
 	struct tty_driver *driver = NULL;
 
 	acquire_console_sem();
-	for (c = console_drivers; c != NULL; c = c->next) {
+	list_for_each_entry(c, &console_driver_list, list) {
 		if (!c->device)
 			continue;
 		driver = c->device(c, index);
@@ -772,13 +773,11 @@
 	 *	preferred driver at the head of the list.
 	 */
 	acquire_console_sem();
-	if ((console->flags & CON_CONSDEV) || console_drivers == NULL) {
-		console->next = console_drivers;
-		console_drivers = console;
-	} else {
-		console->next = console_drivers->next;
-		console_drivers->next = console;
-	}
+	if ((console->flags & CON_CONSDEV) || list_empty(&console_driver_list)) 
+		list_add(&console->list, &console_driver_list);
+	else
+		list_add(&console->list, console_driver_list.next);
+
 	if (console->flags & CON_PRINTBUFFER) {
 		/*
 		 * release_console_sem() will print out the buffered messages
@@ -794,31 +793,17 @@
 
 int unregister_console(struct console * console)
 {
-        struct console *a,*b;
 	int res = 1;
 
 	acquire_console_sem();
-	if (console_drivers == console) {
-		console_drivers=console->next;
-		res = 0;
-	} else {
-		for (a=console_drivers->next, b=console_drivers ;
-		     a; b=a, a=b->next) {
-			if (a == console) {
-				b->next = a->next;
-				res = 0;
-				break;
-			}  
-		}
-	}
+	list_del(&console->list);
 	
 	/* If last console is removed, we re-enable picking the first
 	 * one that gets registered. Without that, pmac early boot console
 	 * would prevent fbcon from taking over.
 	 */
-	if (console_drivers == NULL)
+	if (list_empty(&console_driver_list))
 		preferred_console = -1;
-		
 
 	release_console_sem();
 	return res;
--- linux/include/linux/console.h.console	Mon Jun 14 11:00:21 2004
+++ linux/include/linux/console.h	Mon Jun 14 11:05:37 2004
@@ -16,6 +16,7 @@
 
 #include <linux/types.h>
 #include <linux/spinlock.h>
+#include <linux/list.h>
 
 struct vc_data;
 struct console_font_op;
@@ -93,13 +94,12 @@
 	short	index;
 	int	cflag;
 	void	*data;
-	struct	 console *next;
+	struct list_head list;
 };
 
 extern int add_preferred_console(char *name, int idx, char *options);
 extern void register_console(struct console *);
 extern int unregister_console(struct console *);
-extern struct console *console_drivers;
 extern void acquire_console_sem(void);
 extern void release_console_sem(void);
 extern void console_conditional_schedule(void);

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH 2/3] Provide console_suspend() and console_resume()

[Pavel Machek]

Hi!

> Add console_suspend() and console_resume() methods so the serial drivers
> can disable console output before suspending a port, and re-enable output
> afterwards.

Could it be called console_stop()/console_start()? suspend/resume
sounds like power managment, and it is unrelated....
								Pavel

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: [PATCH 2/3] Provide console_suspend() and console_resume()

[Andrew Morton]

Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
> 
> > Add console_suspend() and console_resume() methods so the serial drivers
> > can disable console output before suspending a port, and re-enable output
> > afterwards.
> 
> Could it be called console_stop()/console_start()? suspend/resume
> sounds like power managment, and it is unrelated....

That's exactly what we ended up doing.

Re: [PATCH 2/3] Provide console_suspend() and console_resume()

[Russell King]

On Wed, Jun 30, 2004 at 02:37:29PM +0200, Pavel Machek wrote:
> Hi!
> 
> > Add console_suspend() and console_resume() methods so the serial drivers
> > can disable console output before suspending a port, and re-enable output
> > afterwards.
> 
> Could it be called console_stop()/console_start()? suspend/resume
> sounds like power managment, and it is unrelated....

It is power management related!  However, we decided that console_start/
console_stop was a better name for it.

That said, it is _NOT_ intended for general use since there is NO
counting of how many times console_stop() is called.  It is merely
used by the serial driver to prevent further serial console output
before shutting the console port down for power management.

This avoids the problem where, on resume, we haven't even enabled
the UART - if we were to attempt to write to the UART in this
circumstance, it would not be setup in any way - no baud rate,
and in fact, since it is disabled, we'd spin for ever waiting for
the port to say its completed transmission of the first byte.

So yes, it most definitely _is_ power management related.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core