powerpc: udbg based backend for hvc_console

powerpc: udbg based backend for hvc_console

[David Gibson]

This patch adds a new backend for the hvc console based on the
low-level udbg callbacks.  This effectively implements a working
runtime console in terms of the simple udbg primitives.  This is kind
of a hack - since udbg isn't something you really want to be using
routinely - but it's really useful during bringup.

This can be used to quickly implement a userspace usable console while
you're working on a proper driver for whatever console I/O device the
hardware has.  Or, it can be used to avoid writing a full blown
tty/console driver entirely for quick-and-dirty I/O hardware that will
later be replaced by something else.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Index: working-2.6/drivers/char/Kconfig
===================================================================
--- working-2.6.orig/drivers/char/Kconfig	2008-10-22 16:01:39.000000000 +1100
+++ working-2.6/drivers/char/Kconfig	2008-10-24 14:22:59.000000000 +1100
@@ -631,6 +631,12 @@ config HVC_XEN
 	help
 	  Xen virtual console device driver
 
+config HVC_UDBG
+       bool "udbg based fake hypervisor console"
+       depends on EXPERIMENTAL
+       select HVC_DRIVER
+       default n
+
 config VIRTIO_CONSOLE
 	tristate "Virtio console"
 	depends on VIRTIO
Index: working-2.6/drivers/char/Makefile
===================================================================
--- working-2.6.orig/drivers/char/Makefile	2008-10-22 15:50:59.000000000 +1100
+++ working-2.6/drivers/char/Makefile	2008-10-24 14:22:59.000000000 +1100
@@ -50,6 +50,7 @@ obj-$(CONFIG_HVC_BEAT)		+= hvc_beat.o
 obj-$(CONFIG_HVC_DRIVER)	+= hvc_console.o
 obj-$(CONFIG_HVC_IRQ)		+= hvc_irq.o
 obj-$(CONFIG_HVC_XEN)		+= hvc_xen.o
+obj-$(CONFIG_HVC_UDBG)		+= hvc_udbg.o
 obj-$(CONFIG_VIRTIO_CONSOLE)	+= virtio_console.o
 obj-$(CONFIG_RAW_DRIVER)	+= raw.o
 obj-$(CONFIG_SGI_SNSC)		+= snsc.o snsc_event.o
Index: working-2.6/drivers/char/hvc_udbg.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ working-2.6/drivers/char/hvc_udbg.c	2008-10-24 14:38:55.000000000 +1100
@@ -0,0 +1,96 @@
+/*
+ * udbg interface to hvc_console.c
+ *
+ * (C) Copyright David Gibson, IBM Corporation 2008.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ */
+
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/irq.h>
+
+#include <asm/udbg.h>
+
+#include "hvc_console.h"
+
+struct hvc_struct *hvc_udbg_dev;
+
+static int hvc_udbg_put(uint32_t vtermno, const char *buf, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++)
+		udbg_putc(buf[i]);
+
+	return i;
+}
+
+static int hvc_udbg_get(uint32_t vtermno, char *buf, int count)
+{
+	int i, c;
+
+	if (!udbg_getc_poll)
+		return 0;
+
+	for (i = 0; i < count; i++) {
+		if ((c = udbg_getc_poll()) == -1)
+			break;
+		buf[i] = c;
+	}
+
+	return i;
+}
+
+static struct hv_ops hvc_udbg_ops = {
+	.get_chars = hvc_udbg_get,
+	.put_chars = hvc_udbg_put,
+};
+
+static int __init hvc_udbg_init(void)
+{
+	struct hvc_struct *hp;
+
+	BUG_ON(hvc_udbg_dev);
+
+	hp = hvc_alloc(0, NO_IRQ, &hvc_udbg_ops, 16);
+	if (IS_ERR(hp))
+		return PTR_ERR(hp);
+
+	hvc_udbg_dev = hp;
+
+	return 0;
+}
+module_init(hvc_udbg_init);
+
+static void __exit hvc_udbg_exit(void)
+{
+	if (hvc_udbg_dev)
+		hvc_remove(hvc_udbg_dev);
+}
+module_exit(hvc_udbg_exit);
+
+static int __init hvc_udbg_console_init(void)
+{
+	hvc_instantiate(0, 0, &hvc_udbg_ops);
+	add_preferred_console("hvc", 0, NULL);
+
+	return 0;
+}
+console_initcall(hvc_udbg_console_init);


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: powerpc: udbg based backend for hvc_console

[Paul Mackerras]

David Gibson writes:

> This patch adds a new backend for the hvc console based on the
> low-level udbg callbacks.  This effectively implements a working
> runtime console in terms of the simple udbg primitives.  This is kind
> of a hack - since udbg isn't something you really want to be using
> routinely - but it's really useful during bringup.
> 
> This can be used to quickly implement a userspace usable console while
> you're working on a proper driver for whatever console I/O device the
> hardware has.  Or, it can be used to avoid writing a full blown
> tty/console driver entirely for quick-and-dirty I/O hardware that will
> later be replaced by something else.

Since only powerpc has udbg, shouldn't the Kconfig option depend on
PPC?

Paul.

Re: powerpc: udbg based backend for hvc_console

[David Gibson]

On Wed, Nov 05, 2008 at 03:53:55PM +1100, Paul Mackerras wrote:
> David Gibson writes:
> 
> > This patch adds a new backend for the hvc console based on the
> > low-level udbg callbacks.  This effectively implements a working
> > runtime console in terms of the simple udbg primitives.  This is kind
> > of a hack - since udbg isn't something you really want to be using
> > routinely - but it's really useful during bringup.
> > 
> > This can be used to quickly implement a userspace usable console while
> > you're working on a proper driver for whatever console I/O device the
> > hardware has.  Or, it can be used to avoid writing a full blown
> > tty/console driver entirely for quick-and-dirty I/O hardware that will
> > later be replaced by something else.
> 
> Since only powerpc has udbg, shouldn't the Kconfig option depend on
> PPC?

Ah.. yes, yes it should.  Forgot that hvconsole was used on
non-powerpc as well.  Corrected patch below.

powerpc: udbg based backend for hvc_console

This patch adds a new backend for the hvc console based on the
low-level udbg callbacks.  This effectively implements a working
runtime console in terms of the simple udbg primitives.  This is kind
of a hack - since udbg isn't something you really want to be using
routinely - but it's really useful during bringup.

This can be used to quickly implement a userspace usable console while
you're working on a proper driver for whatever console I/O device the
hardware has.  Or, it can be used to avoid writing a full blown
tty/console driver entirely for quick-and-dirty I/O hardware that will
later be replaced by something else.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Index: working-2.6/drivers/char/Kconfig
===================================================================
--- working-2.6.orig/drivers/char/Kconfig	2008-10-31 13:31:49.000000000 +1100
+++ working-2.6/drivers/char/Kconfig	2008-11-06 11:18:22.000000000 +1100
@@ -631,6 +631,12 @@ config HVC_XEN
 	help
 	  Xen virtual console device driver
 
+config HVC_UDBG
+       bool "udbg based fake hypervisor console"
+       depends on PPC && EXPERIMENTAL
+       select HVC_DRIVER
+       default n
+
 config VIRTIO_CONSOLE
 	tristate "Virtio console"
 	depends on VIRTIO
Index: working-2.6/drivers/char/Makefile
===================================================================
--- working-2.6.orig/drivers/char/Makefile	2008-10-31 13:31:49.000000000 +1100
+++ working-2.6/drivers/char/Makefile	2008-11-06 11:17:31.000000000 +1100
@@ -50,6 +50,7 @@ obj-$(CONFIG_HVC_BEAT)		+= hvc_beat.o
 obj-$(CONFIG_HVC_DRIVER)	+= hvc_console.o
 obj-$(CONFIG_HVC_IRQ)		+= hvc_irq.o
 obj-$(CONFIG_HVC_XEN)		+= hvc_xen.o
+obj-$(CONFIG_HVC_UDBG)		+= hvc_udbg.o
 obj-$(CONFIG_VIRTIO_CONSOLE)	+= virtio_console.o
 obj-$(CONFIG_RAW_DRIVER)	+= raw.o
 obj-$(CONFIG_SGI_SNSC)		+= snsc.o snsc_event.o
Index: working-2.6/drivers/char/hvc_udbg.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ working-2.6/drivers/char/hvc_udbg.c	2008-11-06 11:17:31.000000000 +1100
@@ -0,0 +1,96 @@
+/*
+ * udbg interface to hvc_console.c
+ *
+ * (C) Copyright David Gibson, IBM Corporation 2008.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ */
+
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/irq.h>
+
+#include <asm/udbg.h>
+
+#include "hvc_console.h"
+
+struct hvc_struct *hvc_udbg_dev;
+
+static int hvc_udbg_put(uint32_t vtermno, const char *buf, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++)
+		udbg_putc(buf[i]);
+
+	return i;
+}
+
+static int hvc_udbg_get(uint32_t vtermno, char *buf, int count)
+{
+	int i, c;
+
+	if (!udbg_getc_poll)
+		return 0;
+
+	for (i = 0; i < count; i++) {
+		if ((c = udbg_getc_poll()) == -1)
+			break;
+		buf[i] = c;
+	}
+
+	return i;
+}
+
+static struct hv_ops hvc_udbg_ops = {
+	.get_chars = hvc_udbg_get,
+	.put_chars = hvc_udbg_put,
+};
+
+static int __init hvc_udbg_init(void)
+{
+	struct hvc_struct *hp;
+
+	BUG_ON(hvc_udbg_dev);
+
+	hp = hvc_alloc(0, NO_IRQ, &hvc_udbg_ops, 16);
+	if (IS_ERR(hp))
+		return PTR_ERR(hp);
+
+	hvc_udbg_dev = hp;
+
+	return 0;
+}
+module_init(hvc_udbg_init);
+
+static void __exit hvc_udbg_exit(void)
+{
+	if (hvc_udbg_dev)
+		hvc_remove(hvc_udbg_dev);
+}
+module_exit(hvc_udbg_exit);
+
+static int __init hvc_udbg_console_init(void)
+{
+	hvc_instantiate(0, 0, &hvc_udbg_ops);
+	add_preferred_console("hvc", 0, NULL);
+
+	return 0;
+}
+console_initcall(hvc_udbg_console_init);


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: powerpc: udbg based backend for hvc_console

[Timur Tabi]

On Thu, Oct 23, 2008 at 9:54 PM, David Gibson
<david@gibson.dropbear.id.au> wrote:
> This patch adds a new backend for the hvc console based on the
> low-level udbg callbacks.  This effectively implements a working
> runtime console in terms of the simple udbg primitives.  This is kind
> of a hack - since udbg isn't something you really want to be using
> routinely - but it's really useful during bringup.

David, I have a stupid question.  I already have an HVC console driver
that works.  Do I need to do anything, other than enable HVC_UDBG, to
get my hvc console drive to use udbg?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: powerpc: udbg based backend for hvc_console

[David Gibson]

On Mon, Nov 17, 2008 at 01:41:24PM -0600, Timur Tabi wrote:
> On Thu, Oct 23, 2008 at 9:54 PM, David Gibson
> <david@gibson.dropbear.id.au> wrote:
> > This patch adds a new backend for the hvc console based on the
> > low-level udbg callbacks.  This effectively implements a working
> > runtime console in terms of the simple udbg primitives.  This is kind
> > of a hack - since udbg isn't something you really want to be using
> > routinely - but it's really useful during bringup.
> 
> David, I have a stupid question.  I already have an HVC console driver
> that works.  Do I need to do anything, other than enable HVC_UDBG, to
> get my hvc console drive to use udbg?

Um.. well.. if you have both activated, I think you can select which
HVC console backend will be used by using console=/dev/hvcNN on the
commandline, where values of NN correspond to different backends, in
order depending on link order in some complex fashion.

But if you already have a working HVC console driver, I don't see why
you'd want to use HVC_UDBG - it's essentially a bringup hack.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: powerpc: udbg based backend for hvc_console

[Timur Tabi]

On Thu, Oct 23, 2008 at 9:54 PM, David Gibson
<david@gibson.dropbear.id.au> wrote:

> This can be used to quickly implement a userspace usable console while
> you're working on a proper driver for whatever console I/O device the
> hardware has.  Or, it can be used to avoid writing a full blown
> tty/console driver entirely for quick-and-dirty I/O hardware that will
> later be replaced by something else.

Ok, I think I understand this better now.

Your approach seems backwards to me.  HVC console client drivers
already have simple put/get functions.  You've written an hvc driver
that makes udbg calls.  Wouldn't it have been better to make a udbg
driver that makes hvc calls?  That way, you effectively give udbg
support to all hvc drivers in one shot.

In order to support udbg in my hvc driver, I have to add udbg calls.
In fact, now that I've thought about it, I don't understand what your
driver does.  You take hvc callbacks and route them through udbg, but
this only works on drivers that have udbg callbacks in the first
place.  In that case, why would these drivers need an hvc middle-man?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: powerpc: udbg based backend for hvc_console

[David Gibson]

On Mon, Nov 17, 2008 at 02:04:38PM -0600, Timur Tabi wrote:
> On Thu, Oct 23, 2008 at 9:54 PM, David Gibson
> <david@gibson.dropbear.id.au> wrote:
> 
> > This can be used to quickly implement a userspace usable console while
> > you're working on a proper driver for whatever console I/O device the
> > hardware has.  Or, it can be used to avoid writing a full blown
> > tty/console driver entirely for quick-and-dirty I/O hardware that will
> > later be replaced by something else.
> 
> Ok, I think I understand this better now.
> 
> Your approach seems backwards to me.  HVC console client drivers
> already have simple put/get functions.  You've written an hvc driver
> that makes udbg calls.  Wouldn't it have been better to make a udbg
> driver that makes hvc calls?  That way, you effectively give udbg
> support to all hvc drivers in one shot.

Well, except that hvc calls aren't necessarily safe in the context
that udbg calls are made.  The idea here is that you want to implement
udbg during bringup anyway, to get really early debug info.  This
means that you can then use that udbg support to get a full console.

> In order to support udbg in my hvc driver, I have to add udbg calls.
> In fact, now that I've thought about it, I don't understand what your
> driver does.  You take hvc callbacks and route them through udbg, but
> this only works on drivers that have udbg callbacks in the first
> place.  In that case, why would these drivers need an hvc middle-man?

Because the udbg console works for kernel messages, but doesn't
support a full tty interface, so you can't run userspace with only a
udbg console.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: powerpc: udbg based backend for hvc_console

[Timur Tabi]

On Mon, Nov 17, 2008 at 6:40 PM, David Gibson
<david@gibson.dropbear.id.au> wrote:

> Because the udbg console works for kernel messages, but doesn't
> support a full tty interface, so you can't run userspace with only a
> udbg console.

I still don't understand the point of your driver.  You're just
routing hcalls to a udbg back-end.  So if you have a driver that
supports udbg but not standard console or TTY, then you can use your
driver to get console/tty I/O.  Is there a platform where such a setup
exists?  All HVC drivers support console and TTY, as do all serial
drivers.  So what platform would use your driver?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: powerpc: udbg based backend for hvc_console

[David Gibson]

On Mon, Nov 17, 2008 at 10:42:46PM -0600, Timur Tabi wrote:
> On Mon, Nov 17, 2008 at 6:40 PM, David Gibson
> <david@gibson.dropbear.id.au> wrote:
> 
> > Because the udbg console works for kernel messages, but doesn't
> > support a full tty interface, so you can't run userspace with only a
> > udbg console.
> 
> I still don't understand the point of your driver.  You're just
> routing hcalls to a udbg back-end.  So if you have a driver that
> supports udbg but not standard console or TTY, then you can use your
> driver to get console/tty I/O.  Is there a platform where such a setup
> exists?  All HVC drivers support console and TTY, as do all serial
> drivers.  So what platform would use your driver?

Like I said, it's a bringup hack.  If you have hardware that's
supported by an existing console driver, there's no reason you'd want
it.

For example, I've used it on a bringup prototype platform whose only
I/O is a weird FPGA-implemented UART (not 8250 compatible, or even
close).  I implemented udbg callbacks as one of the first things I
did, to get the early debugging.  But eventually I got userspace up,
and discovered that it wouldn't work on the udbg consoleuserspace
won't work with the udbg console.  I didn't really want to write
another, more complex, driver for this strange UART, so I glued
hvconsole to my existing udbg callbacks, giving me a userspace-capable
console.

Given the variety of strange I/O configurations in prototype and
embedded platforms, I can't imagine this was a unique situation.  So
I've pushed my patch out, so anyone else in a similar situation can
immediately turn their little udbg methods for whatever strange I/O
they have into a fully-functional console.  Maybe it's not something
you'd want to go to release with, but it certainly simplifies life
during bringup.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: powerpc: udbg based backend for hvc_console

[Timur Tabi]

David Gibson wrote:

> Given the variety of strange I/O configurations in prototype and
> embedded platforms, I can't imagine this was a unique situation.  So
> I've pushed my patch out, so anyone else in a similar situation can
> immediately turn their little udbg methods for whatever strange I/O
> they have into a fully-functional console.  Maybe it's not something
> you'd want to go to release with, but it certainly simplifies life
> during bringup.

Ok, I understand now.  However, I would like to see two changes:

1) Re-arrange the Makefile as I pointed out in another post.

2) Update the Kconfig help file to be very clear that this feature is only
meaningful if the platform has a udbg back-end but no other console or TTY driver.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: powerpc: udbg based backend for hvc_console

[David Gibson]

On Tue, Nov 18, 2008 at 09:06:17AM -0600, Timur Tabi wrote:
> David Gibson wrote:
> 
> > Given the variety of strange I/O configurations in prototype and
> > embedded platforms, I can't imagine this was a unique situation.  So
> > I've pushed my patch out, so anyone else in a similar situation can
> > immediately turn their little udbg methods for whatever strange I/O
> > they have into a fully-functional console.  Maybe it's not something
> > you'd want to go to release with, but it certainly simplifies life
> > during bringup.
> 
> Ok, I understand now.  However, I would like to see two changes:
> 
> 1) Re-arrange the Makefile as I pointed out in another post.

Um.. yeah.. I'm a bit baffled by this.. all the existing backends
are listed after hvc_console, I just added hvc_udbg to the end.  I
didn't really understand the rationale in that commit, but then I
haven't had time to look at it very much yet.

> 2) Update the Kconfig help file to be very clear that this feature
> is only meaningful if the platform has a udbg back-end but no other
> console or TTY driver.

Alrighty...

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: powerpc: udbg based backend for hvc_console

[Timur Tabi]

David Gibson wrote:

> Um.. yeah.. I'm a bit baffled by this.. all the existing backends
> are listed after hvc_console, I just added hvc_udbg to the end.  I
> didn't really understand the rationale in that commit, but then I
> haven't had time to look at it very much yet.

No, some are before:

obj-$(CONFIG_HVC_RTAS)          += hvc_rtas.o	<---
obj-$(CONFIG_HVC_BEAT)          += hvc_beat.o	<---
obj-$(CONFIG_HVC_DRIVER)        += hvc_console.o
obj-$(CONFIG_HVC_IRQ)           += hvc_irq.o
obj-$(CONFIG_HVC_XEN)           += hvc_xen.o
obj-$(CONFIG_HVC_UDBG)          += hvc_udbg.o

Until your patch, only Xen was wrong.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: powerpc: udbg based backend for hvc_console

[Milton Miller]

[I'm going to reply to several points in this thread in one reply.  I  
have restored context that was trimmed in later replys when I wanted to  
speak to.]

David Gibson wrote at 2008-11-18 00:28:28:
> On Mon, Nov 17, 2008 at 01:41:24PM -0600, Timur Tabi wrote:
>> On Thu, Oct 23, 2008 at 9:54 PM, David Gibson
>> <david@gibson.dropbear.id.au> wrote:
>>> This patch adds a new backend for the hvc console based on the
>>> low-level udbg callbacks.  This effectively implements a working
>>> runtime console in terms of the simple udbg primitives.  This is kind
>>> of a hack - since udbg isn't something you really want to be using
>>> routinely - but it's really useful during bringup.

Why is udbg hvc something you want to not use routinely?

Stated differently, if your routine (1) fundamently works one character  
at a time and (2) is not interrupt driven, and (3) only supports one  
channel, what avantage is there to an explicit hvc driver?

>>> This can be used to quickly implement a userspace usable console  
>>> while
>>> you're working on a proper driver for whatever console I/O device the
>>> hardware has.  Or, it can be used to avoid writing a full blown
>>> tty/console driver entirely for quick-and-dirty I/O hardware that  
>>> will
>>> later be replaced by something else.

Actually it looks remarkably similar to a cleaned up version of a patch  
i've been using since hvc_console was split to be a hookable shell.  Or  
was it the motivation for adding the hooks?  The code is different in  
how it structures the error checks and that it is implemented in  
drivers/char where it belongs (hence the cleaned up comment) but the  
fact that David recreated what I did does speak to its general utility.

>> David, I have a stupid question.  I already have an HVC console driver
>> that works.  Do I need to do anything, other than enable HVC_UDBG, to
>> get my hvc console drive to use udbg?

As was pointed out elsewhere, this is hvc calling udbg not udbg hooks  
calling hvc_cosole methods, which might need a different context.  But  
I wanted to comment on David's reply:

> Um.. well.. if you have both activated, I think you can select which  
> HVC console backend will be used by using console=/dev/hvcNN on the  
> commandline, where values of NN correspond to different backends, in  
> order depending on link order in some complex fashion.

It doesn't just work that way because of the way the hvc_console shell  
implements the console hooks.

If the drivers request different hvc channel numbers, then yes, you can  
select which hvc backend is used by choosing console=hvcNN.  If all  
request 0, then which ever one registers with hvc_console first wins  
the slot, and the others get a busy error return.   They can still  
register their tty later, at which point they get a dynamically  
determined /dev/hvcN slot, above all registered console slots.

There are multiple reasons for this design.  The original user, pseries  
vterm, wants to assign the channel number based on what the hypervisor  
tells it the channel should be.  There are two loops though the device  
tree, searching for different protocols (and consequently different  
backends), so they need to be able to specify the number and not just  
use the order they are found.  The platform describes not only the  
protocol, but which instance that is, and the backend currently  
registers with that number.  Another reason is its a bad idea to have  
console=/dev/hvc0 depend on link order instead of knowing which driver  
is selected.  If a user is specifing what the console is, it should not  
depend on other linked drivers.

Until this point, all mainline drivers have been exclusive in that only  
one will actually register with hvc_console midlayer on any given  
platform.  This is the first backend that is not exclusive, and  
therefore its coexistence needs special attention.

In my internal tree, I register (my version of) this not as hvc0 but as  
hvc4.  hvc0 is rtas, hvc1-3 are an internal backend, and this one is  
hvc4.  The order I chose is arbitrary, but the main point is it does't  
compete with hvc_rtas for slot 0.  (All of these drivers coexist, and I  
can choose which one I want to use based on my needs for that boot.)   
We could make it configurable via Kconfig, or just choose a slot.  (I  
think vterm can have 2 channels on some boxes where they drive real  
serial ports on the box.)

> But if you already have a working HVC console driver, I don't see why
> you'd want to use HVC_UDBG - it's essentially a bringup hack.

As I said, why should we need a fancier hvc_console backend if you are  
polling and gain no efficency processing  multiple characters at once.

The udbg drivers know if they are sharing the interface with a  
hvc_console some other driver.  If we trigger the registration of this  
backend on the udbg hook saying its useful (eg by setting a flag word  
in the function where we assign the udbg putc and getc pointers) then  
this driver will not need to be compiled out for "production" kernels.

David Gibson wrote at 2008-11-19 00:42:20:
> On Tue, Nov 18, 2008 at 09:06:17AM -0600, Timur Tabi wrote:
>> David Gibson wrote: [at 2008-11-18 05:14:08]
>>> Given the variety of strange I/O configurations in prototype and
>>> embedded platforms, I can't imagine this was a unique situation.  So
>>> I've pushed my patch out, so anyone else in a similar situation can
>>> immediately turn their little udbg methods for whatever strange I/O
>>> they have into a fully-functional console.  Maybe it's not something
>>> you'd want to go to release with, but it certainly simplifies life
>>> during bringup.
>>
>> Ok, I understand now.  However, I would like to see two changes:
>>
>> 1) Re-arrange the Makefile as I pointed out in another post.
[the other post]
Timur Tabi - 2008-11-17 20:18:31
>> One other thing ...
>> HVC console drivers must be compiled before hvc_console.o
>>
>> see  
>> http://git.kernel.org/?p=linux/kernel/git/paulus/powerpc.git; 
>> a=commit;h=938473b24636d77dc5e9c3f41090d071b6cf4389
[end other post]
>
> Um.. yeah.. I'm a bit baffled by this.. all the existing backends
> are listed after hvc_console, I just added hvc_udbg to the end.  I
> didn't really understand the rationale in that commit, but then I
> haven't had time to look at it very much yet.

I too challanged the necessity of that change.  I carefully designed  
the hvc_console layer to find the selected console weither it was  
registered before or after the console_initcall of hvc_console.

I am very disapointed that the changelog is so sparse.

However, I think the bottom line was that add_preferred_console is  
suppsoed to be called by architecture setup code.   Doing it the  
console_initcall is almost abuse.  But if the console_initcall is going  
to call add_preferred_console, then it must link before the hvc_console  
driver.   I would have to go back and find the discussion to remember  
the exact details.  (maybe it only needed to call  
add_preferreed_console before registering itself if its the only hvc  
backend).

Which does bring up the point of avoinding calling  
add_preferred_console to udbg just because it is linked in.  The udbg  
getc routines might work for xmon, but one would probably not like the  
result of the real driver and udbg trying to read the same device at  
the same time.  Perhaps the same flag that says "consider me or  
udbg_hvc" should apply, or we do add_preferred_console in arch code  
before console_initcall.  (its ok to preferr a console that never  
registers as long as another preferred console does register).

>> 2) Update the Kconfig help file to be very clear that this feature
>> is only meaningful if the platform has a udbg back-end but no other
>> console or TTY driver.
>
> Alrighty...

That does bring up the point, I don't see any kconfig help for this  
user-selectable option.

And if we don't have the udbg hooks say to enable it, then we really  
need to have text discouraging its use as it might break their console.

milton

Re: powerpc: udbg based backend for hvc_console

[Timur Tabi]

Milton Miller wrote:

> Stated differently, if your routine (1) fundamently works one character  
> at a time and (2) is not interrupt driven, and (3) only supports one  
> channel, what avantage is there to an explicit hvc driver?

I think it's because HVC has the ability to use HVC client drivers for console
and TTY, but udbg does not.  So if you have a udbg-only driver, it will cease to
function when the kernel switches to the normal console.

Perhaps it would make more sense to have the kernel continue to use udbg if
there is no normal console driver, instead of relying on David's driver.

> I too challanged the necessity of that change.  I carefully designed  
> the hvc_console layer to find the selected console weither it was  
> registered before or after the console_initcall of hvc_console.

I'm just pointing out the patch that was committed in 2006 to address this
issue.  I discovered the same bug that the patch mentioned, and only after a
long time searching did I find this patch.  And sure enough, re-arranging the
Makefile made the problem I was seeing go away.

Now, it's possible that this bug in hvc_console no longer exists.  However, I
have seen nothing to indicate it.  Unfortunately, I don't remember enough of the
details to reproduce the problem, so I can't verify it.

> However, I think the bottom line was that add_preferred_console is  
> suppsoed to be called by architecture setup code.   Doing it the  
> console_initcall is almost abuse.  But if the console_initcall is going  
> to call add_preferred_console, then it must link before the hvc_console  
> driver.   I would have to go back and find the discussion to remember  
> the exact details.  (maybe it only needed to call  
> add_preferreed_console before registering itself if its the only hvc  
> backend).

Well, I can't comment on your theory, but I do know that the
add_preferred_console() call should be made from the HVC client driver, or at
least HVC itself.  It should not be in the platform code, because then we have a
dependency between the driver and the platform code.  I prefer my drivers to
initialize everything on their own, without the help of the platform code.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: powerpc: udbg based backend for hvc_console

[David Gibson]

On Thu, Nov 20, 2008 at 01:16:27PM -0600, Timur Tabi wrote:
> Milton Miller wrote:
> 
> > Stated differently, if your routine (1) fundamently works one character  
> > at a time and (2) is not interrupt driven, and (3) only supports one  
> > channel, what avantage is there to an explicit hvc driver?
> 
> I think it's because HVC has the ability to use HVC client drivers
> for console and TTY, but udbg does not.  So if you have a udbg-only
> driver, it will cease to function when the kernel switches to the
> normal console.
> 
> Perhaps it would make more sense to have the kernel continue to use udbg if
> there is no normal console driver, instead of relying on David's
> driver.

This doesn't work - the udbg console can't support a full tty driver
which is needed by userspace.  Originally, I was going to extend the
udbg console so that it supported a tty backended onto the udbg
methods.  Then I realised that the code I'd need to do this would be
essentially identical to hvconsole, so why duplicate it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: powerpc: udbg based backend for hvc_console

[Milton Miller]

On Nov 20, 2008, at 1:16 PM, Timur Tabi wrote:

> Milton Miller wrote:
>
>> Stated differently, if your routine (1) fundamently works one 
>> character
>> at a time and (2) is not interrupt driven, and (3) only supports one
>> channel, what avantage is there to an explicit hvc driver?
>
> I think it's because HVC has the ability to use HVC client drivers for 
> console
> and TTY, but udbg does not.  So if you have a udbg-only driver, it 
> will cease to
> function when the kernel switches to the normal console.
>
> Perhaps it would make more sense to have the kernel continue to use 
> udbg if
> there is no normal console driver, instead of relying on David's 
> driver.

First, we have that option: just specify udbg-imortal on the command 
line, and the udbg console driver removes CON_BOOT from its flags.

Second, as David said in his reply, that doesn't let your userspace 
have a tty, which means no job control.

>> I too challanged the necessity of that change.  I carefully designed
>> the hvc_console layer to find the selected console weither it was
>> registered before or after the console_initcall of hvc_console.
>
> I'm just pointing out the patch that was committed in 2006 to address 
> this
> issue.  I discovered the same bug that the patch mentioned, and only 
> after a
> long time searching did I find this patch.  And sure enough, 
> re-arranging the
> Makefile made the problem I was seeing go away.
>
> Now, it's possible that this bug in hvc_console no longer exists.  
> However, I
> have seen nothing to indicate it.  Unfortunately, I don't remember 
> enough of the
> details to reproduce the problem, so I can't verify it.

As I said, I conceeded then the patch was required the way things 
worked.  There was another propsed patch a few months ago (3-9) that I 
pointed out they needed to be fixing the common console code; I don't 
remember seeing anything related after that but I could have missed it.

>> However, I think the bottom line was that add_preferred_console is
>> suppsoed to be called by architecture setup code.   Doing it the
>> console_initcall is almost abuse.  But if the console_initcall is 
>> going
>> to call add_preferred_console, then it must link before the 
>> hvc_console
>> driver.   I would have to go back and find the discussion to remember
>> the exact details.  (maybe it only needed to call
>> add_preferreed_console before registering itself if its the only hvc
>> backend).
>
> Well, I can't comment on your theory, but I do know that the
> add_preferred_console() call should be made from the HVC client 
> driver, or at
> least HVC itself.  It should not be in the platform code, because then 
> we have a
> dependency between the driver and the platform code.  I prefer my 
> drivers to
> initialize everything on their own, without the help of the platform 
> code.

Think about this for a second.

If every console driver said "prefer me", what point is there in having 
add_preferred_console?

The second reason is its too late.  We have already started scanning 
for drivers.  We are asking the console layer to adjust its choices 
after it has started scanning.

We want the last console= parameter on the command line to win.  So if 
that implys the last call to add_preferred_console wins, then you have 
code overriding the command line.


We have code that searches the device tree for the stdout that firmware 
used.  That code then tells the console layer which driver to choose 
when it recognises it.   The pSeries platform also has code that says 
when it finds vterm channels under lpar, then it should choose it.

I think we should have the code that sets the udbg method indicate that 
the console should be preferred.  While its not really arch code, it is 
architecture code in that its hooked from early discovrey based not 
cosole_initcall based (which runs before arch_initcall).  This is also 
the point where we know if the udbg method is sharing hardware with a 
full driver and therefore udbg_hvc should not be used.

milton

Re: powerpc: udbg based backend for hvc_console

[Timur Tabi]

Milton Miller wrote:

> We want the last console= parameter on the command line to win.  So if 
> that implys the last call to add_preferred_console wins, then you have 
> code overriding the command line.

Hmm, good point.  However, how likely is it that we'll have more than one
console driver?  Also, without calling add_preferred_console(), the kernel needs
to have a console= on the command line.

In my case, my driver only calls add_preferred_console() if the device tree
contains a specific property that it looks for.  In effect, this property
override the console= line.  However, the console= line goes to the HVC
subsystem, and not my driver, and I can't use it to send the configuration data
the driver needs.

Unfortunately, my driver hasn't been published yet, so it's hard to explain the
details.  I guess I need to think about this more.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: powerpc: udbg based backend for hvc_console

[Milton Miller]

On Nov 21, 2008, at 10:13 AM, Timur Tabi wrote:
> Milton Miller wrote:
>> We want the last console= parameter on the command line to win.  So if
>> that implys the last call to add_preferred_console wins, then you have
>> code overriding the command line.
>
> Hmm, good point.  However, how likely is it that we'll have more than 
> one
> console driver?

For anything than custom embedded configs, quite likely.   As I said, I 
have 3 hvc clients, but that is unusual.  But frame buffer, vterm, 
rtas, and serial console would be a typical mix for a distribution.

> Also, without calling add_preferred_console(), the kernel needs
> to have a console= on the command line.

I'm not arguing against the call, I'm arguing when/from where it should 
be made.

> In my case, my driver only calls add_preferred_console() if the device 
> tree
> contains a specific property that it looks for.  In effect, this 
> property
> override the console= line.  However, the console= line goes to the HVC
> subsystem, and not my driver, and I can't use it to send the 
> configuration data
> the driver needs.

Discovering the hardware from the device tree and triggering your 
regitration is the right approach.  I think if you discover from an 
early setup then you can go before the command line.

As far as getting parameters, you are talking like ttyS0,9600,8,n,1 ? 
If you go after hvc_console then you could add a patch for hvc_console 
to remember the setup and return it to possible clients.

> Unfortunately, my driver hasn't been published yet, so it's hard to 
> explain the
> details.  I guess I need to think about this more.

I dont' know what details you would want on the console= command line 
that you should not have in the device tree.   If its which hypervisor 
channel, then I would think just choosing your hvc number accordingly 
would work.  But I can only make wild guesses without details.

milton

Re: powerpc: udbg based backend for hvc_console

[David Gibson]

On Thu, Nov 20, 2008 at 01:07:11PM -0600, Milton Miller wrote:
> [I'm going to reply to several points in this thread in one reply.  I  
> have restored context that was trimmed in later replys when I wanted to  
> speak to.]
>
> David Gibson wrote at 2008-11-18 00:28:28:
>> On Mon, Nov 17, 2008 at 01:41:24PM -0600, Timur Tabi wrote:
>>> On Thu, Oct 23, 2008 at 9:54 PM, David Gibson
>>> <david@gibson.dropbear.id.au> wrote:
>>>> This patch adds a new backend for the hvc console based on the
>>>> low-level udbg callbacks.  This effectively implements a working
>>>> runtime console in terms of the simple udbg primitives.  This is kind
>>>> of a hack - since udbg isn't something you really want to be using
>>>> routinely - but it's really useful during bringup.
>
> Why is udbg hvc something you want to not use routinely?

Well.. because the udbg methods themselves are not sometimes (not
always, but reasonably often), something you don't really want to use
except during bringup.  A number of them use various hacks that lets
them work very early, and very reliably even when much of the kernel
is borked, but aren't the sort of thing you want to do except during
bringup.  e.g. being mapped by special bolted TLB entries established
very early, rather than using ioremap(); polled access for interrupt
capable devices; hard-coded device addresses, meaning the kernel will
only work on one precise type of system.

> Stated differently, if your routine (1) fundamently works one character  
> at a time and (2) is not interrupt driven, and (3) only supports one  
> channel, what avantage is there to an explicit hvc driver?

Using standard kernel interfaces, instead of random early debug hacks.

>>>> This can be used to quickly implement a userspace usable console  
>>>> while
>>>> you're working on a proper driver for whatever console I/O device the
>>>> hardware has.  Or, it can be used to avoid writing a full blown
>>>> tty/console driver entirely for quick-and-dirty I/O hardware that  
>>>> will
>>>> later be replaced by something else.
>
> Actually it looks remarkably similar to a cleaned up version of a patch  
> i've been using since hvc_console was split to be a hookable shell.  Or  
> was it the motivation for adding the hooks?  The code is different in  
> how it structures the error checks and that it is implemented in  
> drivers/char where it belongs (hence the cleaned up comment) but the  
> fact that David recreated what I did does speak to its general
> utility.

Heh, ok.

>>> David, I have a stupid question.  I already have an HVC console driver
>>> that works.  Do I need to do anything, other than enable HVC_UDBG, to
>>> get my hvc console drive to use udbg?
>
> As was pointed out elsewhere, this is hvc calling udbg not udbg hooks  
> calling hvc_cosole methods, which might need a different context.  But I 
> wanted to comment on David's reply:
>
>> Um.. well.. if you have both activated, I think you can select which  
>> HVC console backend will be used by using console=/dev/hvcNN on the  
>> commandline, where values of NN correspond to different backends, in  
>> order depending on link order in some complex fashion.
>
> It doesn't just work that way because of the way the hvc_console shell  
> implements the console hooks.
>
> If the drivers request different hvc channel numbers, then yes, you can  
> select which hvc backend is used by choosing console=hvcNN.  If all  
> request 0, then which ever one registers with hvc_console first wins the 
> slot, and the others get a busy error return.   They can still register 
> their tty later, at which point they get a dynamically determined 
> /dev/hvcN slot, above all registered console slots.

Ah, ok, my mistake.

> There are multiple reasons for this design.  The original user, pseries  
> vterm, wants to assign the channel number based on what the hypervisor  
> tells it the channel should be.  There are two loops though the device  
> tree, searching for different protocols (and consequently different  
> backends), so they need to be able to specify the number and not just  
> use the order they are found.  The platform describes not only the  
> protocol, but which instance that is, and the backend currently  
> registers with that number.  Another reason is its a bad idea to have  
> console=/dev/hvc0 depend on link order instead of knowing which driver  
> is selected.  If a user is specifing what the console is, it should not  
> depend on other linked drivers.
>
> Until this point, all mainline drivers have been exclusive in that only  
> one will actually register with hvc_console midlayer on any given  
> platform.  This is the first backend that is not exclusive, and  
> therefore its coexistence needs special attention.
>
> In my internal tree, I register (my version of) this not as hvc0 but as  
> hvc4.  hvc0 is rtas, hvc1-3 are an internal backend, and this one is  
> hvc4.  The order I chose is arbitrary, but the main point is it does't  
> compete with hvc_rtas for slot 0.  (All of these drivers coexist, and I  
> can choose which one I want to use based on my needs for that boot.)  We 
> could make it configurable via Kconfig, or just choose a slot.  (I think 
> vterm can have 2 channels on some boxes where they drive real serial 
> ports on the box.)

Hrm, ok.  Not sure what the sensible way to do this for my case would
be.

>> But if you already have a working HVC console driver, I don't see why
>> you'd want to use HVC_UDBG - it's essentially a bringup hack.
>
> As I said, why should we need a fancier hvc_console backend if you are  
> polling and gain no efficency processing  multiple characters at once.
>
> The udbg drivers know if they are sharing the interface with a  
> hvc_console some other driver.  If we trigger the registration of this  
> backend on the udbg hook saying its useful (eg by setting a flag word in 
> the function where we assign the udbg putc and getc pointers) then this 
> driver will not need to be compiled out for "production" kernels.
>
> David Gibson wrote at 2008-11-19 00:42:20:
>> On Tue, Nov 18, 2008 at 09:06:17AM -0600, Timur Tabi wrote:
>>> David Gibson wrote: [at 2008-11-18 05:14:08]
>>>> Given the variety of strange I/O configurations in prototype and
>>>> embedded platforms, I can't imagine this was a unique situation.  So
>>>> I've pushed my patch out, so anyone else in a similar situation can
>>>> immediately turn their little udbg methods for whatever strange I/O
>>>> they have into a fully-functional console.  Maybe it's not something
>>>> you'd want to go to release with, but it certainly simplifies life
>>>> during bringup.
>>>
>>> Ok, I understand now.  However, I would like to see two changes:
>>>
>>> 1) Re-arrange the Makefile as I pointed out in another post.
> [the other post]
> Timur Tabi - 2008-11-17 20:18:31
>>> One other thing ...
>>> HVC console drivers must be compiled before hvc_console.o
>>>
>>> see http://git.kernel.org/?p=linux/kernel/git/paulus/powerpc.git; 
>>> a=commit;h=938473b24636d77dc5e9c3f41090d071b6cf4389
> [end other post]
>>
>> Um.. yeah.. I'm a bit baffled by this.. all the existing backends
>> are listed after hvc_console, I just added hvc_udbg to the end.  I
>> didn't really understand the rationale in that commit, but then I
>> haven't had time to look at it very much yet.
>
> I too challanged the necessity of that change.  I carefully designed the 
> hvc_console layer to find the selected console weither it was registered 
> before or after the console_initcall of hvc_console.
>
> I am very disapointed that the changelog is so sparse.

Uh... but you're the author of that commit...?

> However, I think the bottom line was that add_preferred_console is  
> suppsoed to be called by architecture setup code.   Doing it the  
> console_initcall is almost abuse.  But if the console_initcall is going  
> to call add_preferred_console, then it must link before the hvc_console  
> driver.   I would have to go back and find the discussion to remember  
> the exact details.  (maybe it only needed to call add_preferreed_console 
> before registering itself if its the only hvc backend).
>
> Which does bring up the point of avoinding calling add_preferred_console 
> to udbg just because it is linked in.  The udbg getc routines might work 
> for xmon, but one would probably not like the result of the real driver 
> and udbg trying to read the same device at the same time.  Perhaps the 
> same flag that says "consider me or udbg_hvc" should apply, or we do 
> add_preferred_console in arch code before console_initcall.  (its ok to 
> preferr a console that never registers as long as another preferred 
> console does register).

Ah, yeah.  I'm not sure if my hvc_udbg really wants an
add_preferred_console.  It's there because it was in hvc_rtas or
whichever other hvc backend I based this on (forgotten which, now).
And the semantics of console selection makes my brain hurt, so I'm not
sure if it's actually needed for this case or not.

>>> 2) Update the Kconfig help file to be very clear that this feature
>>> is only meaningful if the platform has a udbg back-end but no other
>>> console or TTY driver.
>>
>> Alrighty...
>
> That does bring up the point, I don't see any kconfig help for this  
> user-selectable option.

Yeah, that's a good point.

> And if we don't have the udbg hooks say to enable it, then we really  
> need to have text discouraging its use as it might break their console.

Possibly it should just be dependent on CONFIG_PPC_EARLY_DEBUG.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: powerpc: udbg based backend for hvc_console

[Milton Miller]

On Nov 20, 2008, at 6:35 PM, David Gibson wrote:
> On Thu, Nov 20, 2008 at 01:07:11PM -0600, Milton Miller wrote:
>> David Gibson wrote at 2008-11-18 00:28:28:
>>> On Mon, Nov 17, 2008 at 01:41:24PM -0600, Timur Tabi wrote:
>>>> On Thu, Oct 23, 2008 at 9:54 PM, David Gibson
>>>> <david@gibson.dropbear.id.au> wrote:
>>>>> This patch adds a new backend for the hvc console based on the
>>>>> low-level udbg callbacks.  This effectively implements a working
>>>>> runtime console in terms of the simple udbg primitives.  This is 
>>>>> kind
>>>>> of a hack - since udbg isn't something you really want to be using
>>>>> routinely - but it's really useful during bringup.
>>
>> Why is udbg hvc something you want to not use routinely?
>
> Well.. because the udbg methods themselves are not sometimes (not
> always, but reasonably often), something you don't really want to use
> except during bringup.  A number of them use various hacks that lets
> them work very early, and very reliably even when much of the kernel
> is borked, but aren't the sort of thing you want to do except during
> bringup.  e.g. being mapped by special bolted TLB entries established
> very early, rather than using ioremap(); polled access for interrupt
> capable devices; hard-coded device addresses, meaning the kernel will
> only work on one precise type of system.

You are describing the CONFIG_PPC_DEBUG_EARLY udbg methods, but they 
are far from all of the available methods.  Several methods use the 
standard kernel apis, or do not need them (rtas, vterm).  If we had 
this, would we need hvc_rtas?

>> Stated differently, if your routine (1) fundamently works one 
>> character
>> at a time and (2) is not interrupt driven, and (3) only supports one
>> channel, what avantage is there to an explicit hvc driver?
>
> Using standard kernel interfaces, instead of random early debug hacks.

I was thinking of the less hackish udbg hooks that registsered in the 
platform setup timeframe.  Not requiring a pinned tlb that you would 
not want for io performance anyways I can see.

>>
>> Actually it looks remarkably similar to a cleaned up version of a 
>> patch
>> i've been using since hvc_console was split to be a hookable shell.  
>> Or
>> was it the motivation for adding the hooks?
>
> Heh, ok.

...
>>> Um.. well.. if you have both activated, I think you can select which
>>> HVC console backend will be used by using console=/dev/hvcNN on the
>>> commandline, where values of NN correspond to different backends, in
>>> order depending on link order in some complex fashion.
>>
>> It doesn't just work that way because of the way the hvc_console shell
>> implements the console hooks.
>>
>> If the drivers request different hvc channel numbers, then yes, you 
>> can
>> select which hvc backend is used by choosing console=hvcNN.  If all
>> request 0, then which ever one registers with hvc_console first wins 
>> the
>> slot, and the others get a busy error return.   They can still 
>> register
>> their tty later, at which point they get a dynamically determined
>> /dev/hvcN slot, above all registered console slots.
>
> Ah, ok, my mistake.
>
>> There are multiple reasons for this design.  The original user, 
>> pseries
>> vterm, wants to assign the channel number based on what the hypervisor
>> tells it the channel should be.  There are two loops though the device
>> tree, searching for different protocols (and consequently different
>> backends), so they need to be able to specify the number and not just
>> use the order they are found.  The platform describes not only the
>> protocol, but which instance that is, and the backend currently
>> registers with that number.  Another reason is its a bad idea to have
>> console=/dev/hvc0 depend on link order instead of knowing which driver
>> is selected.  If a user is specifing what the console is, it should 
>> not
>> depend on other linked drivers.
>>
>> Until this point, all mainline drivers have been exclusive in that 
>> only
>> one will actually register with hvc_console midlayer on any given
>> platform.  This is the first backend that is not exclusive, and
>> therefore its coexistence needs special attention.
>>
>> In my internal tree, I register (my version of) this not as hvc0 but 
>> as
>> hvc4.  hvc0 is rtas, hvc1-3 are an internal backend, and this one is
>> hvc4.  The order I chose is arbitrary, but the main point is it does't
>> compete with hvc_rtas for slot 0.  (All of these drivers coexist, and 
>> I
>> can choose which one I want to use based on my needs for that boot.)  
>> We
>> could make it configurable via Kconfig, or just choose a slot.  (I 
>> think
>> vterm can have 2 channels on some boxes where they drive real serial
>> ports on the box.)
>
> Hrm, ok.  Not sure what the sensible way to do this for my case would
> be.
>
>>> But if you already have a working HVC console driver, I don't see why
>>> you'd want to use HVC_UDBG - it's essentially a bringup hack.
>>
>> As I said, why should we need a fancier hvc_console backend if you are
>> polling and gain no efficency processing  multiple characters at once.
>>
>> The udbg drivers know if they are sharing the interface with a
>> hvc_console some other driver.  If we trigger the registration of this
>> backend on the udbg hook saying its useful (eg by setting a flag word 
>> in
>> the function where we assign the udbg putc and getc pointers) then 
>> this
>> driver will not need to be compiled out for "production" kernels.
>>
>> David Gibson wrote at 2008-11-19 00:42:20:
>>> On Tue, Nov 18, 2008 at 09:06:17AM -0600, Timur Tabi wrote:
>>>> David Gibson wrote: [at 2008-11-18 05:14:08]
>>>>> Given the variety of strange I/O configurations in prototype and
>>>>> embedded platforms, I can't imagine this was a unique situation.  
>>>>> So
>>>>> I've pushed my patch out, so anyone else in a similar situation can
>>>>> immediately turn their little udbg methods for whatever strange I/O
>>>>> they have into a fully-functional console.  Maybe it's not 
>>>>> something
>>>>> you'd want to go to release with, but it certainly simplifies life
>>>>> during bringup.
>>>>
>>>> Ok, I understand now.  However, I would like to see two changes:
>>>>
>>>> 1) Re-arrange the Makefile as I pointed out in another post.
>> [the other post]
>> Timur Tabi - 2008-11-17 20:18:31
>>>> One other thing ...
>>>> HVC console drivers must be compiled before hvc_console.o
>>>>
>>>> see http://git.kernel.org/?p=linux/kernel/git/paulus/powerpc.git;
>>>> a=commit;h=938473b24636d77dc5e9c3f41090d071b6cf4389
>> [end other post]
>>>
>>> Um.. yeah.. I'm a bit baffled by this.. all the existing backends
>>> are listed after hvc_console, I just added hvc_udbg to the end.  I
>>> didn't really understand the rationale in that commit, but then I
>>> haven't had time to look at it very much yet.
>>
>> I too challanged the necessity of that change.  I carefully designed 
>> the
>> hvc_console layer to find the selected console weither it was 
>> registered
>> before or after the console_initcall of hvc_console.
>>
>> I am very disapointed that the changelog is so sparse.
>
> Uh... but you're the author of that commit...?

Hmm, it would appear I was the author of the patch.  I'm still 
disapointed, maybe my standards are higher after a few years.  I was 
thinking that was externally triggered.

>> However, I think the bottom line was that add_preferred_console is
>> suppsoed to be called by architecture setup code.   Doing it the
>> console_initcall is almost abuse.  But if the console_initcall is 
>> going
>> to call add_preferred_console, then it must link before the 
>> hvc_console
>> driver.   I would have to go back and find the discussion to remember
>> the exact details.  (maybe it only needed to call 
>> add_preferreed_console
>> before registering itself if its the only hvc backend).
>>
>> Which does bring up the point of avoinding calling 
>> add_preferred_console
>> to udbg just because it is linked in.  The udbg getc routines might 
>> work
>> for xmon, but one would probably not like the result of the real 
>> driver
>> and udbg trying to read the same device at the same time.  Perhaps the
>> same flag that says "consider me or udbg_hvc" should apply, or we do
>> add_preferred_console in arch code before console_initcall.  (its ok 
>> to
>> preferr a console that never registers as long as another preferred
>> console does register).
>
> Ah, yeah.  I'm not sure if my hvc_udbg really wants an
> add_preferred_console.  It's there because it was in hvc_rtas or
> whichever other hvc backend I based this on (forgotten which, now).
> And the semantics of console selection makes my brain hurt, so I'm not
> sure if it's actually needed for this case or not.

I think part of the problem is dummy_console ends up as a default too 
often.  I'm not quite sure what its purpose really is.  I'm not 
actually sure it is the problem either.

>>>> 2) Update the Kconfig help file to be very clear that this feature
>>>> is only meaningful if the platform has a udbg back-end but no other
>>>> console or TTY driver.
>>>
>>> Alrighty...
>>
>> That does bring up the point, I don't see any kconfig help for this
>> user-selectable option.
>
> Yeah, that's a good point.
>
>> And if we don't have the udbg hooks say to enable it, then we really
>> need to have text discouraging its use as it might break their 
>> console.
>
> Possibly it should just be dependent on CONFIG_PPC_EARLY_DEBUG.

If you do that then I will have to patch it back out.  I choose which 
hvc based on
I think that is too restrictive.   I use udbg as one of my hvc 
channels, but I don't use PPC_EARLY_DEBUG, that requires too much 
platform knowledge.  A simple flag that says 0 = only udbg-console, 1 = 
allow hvc_udbg, 2 = prefer hvc_udbg that is set with the udbg hooks 
would suffice for me.

That's all I have time to write at the moment, I might reply to more 
later.

milton

Re: powerpc: udbg based backend for hvc_console

[Timur Tabi]

On Thu, Oct 23, 2008 at 9:54 PM, David Gibson
<david@gibson.dropbear.id.au> wrote:

One other thing ...

> --- working-2.6.orig/drivers/char/Makefile      2008-10-22 15:50:59.000000000 +1100
> +++ working-2.6/drivers/char/Makefile   2008-10-24 14:22:59.000000000 +1100
> @@ -50,6 +50,7 @@ obj-$(CONFIG_HVC_BEAT)                += hvc_beat.o
>  obj-$(CONFIG_HVC_DRIVER)       += hvc_console.o
>  obj-$(CONFIG_HVC_IRQ)          += hvc_irq.o
>  obj-$(CONFIG_HVC_XEN)          += hvc_xen.o
> +obj-$(CONFIG_HVC_UDBG)         += hvc_udbg.o

HVC console drivers must be compiled before hvc_console.o, so the
above should look like this:

obj-$(CONFIG_HVC_XEN)          += hvc_xen.o
obj-$(CONFIG_HVC_UDBG)         += hvc_udbg.o
obj-$(CONFIG_HVC_DRIVER)       += hvc_console.o
obj-$(CONFIG_HVC_IRQ)          += hvc_irq.o

see http://git.kernel.org/?p=linux/kernel/git/paulus/powerpc.git;a=commit;h=938473b24636d77dc5e9c3f41090d071b6cf4389

-- 
Timur Tabi
Linux kernel developer at Freescale