LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* kgdb 2.6.28 merge plans
@ 2008-10-21 19:14 Jason Wessel
  2008-10-21 19:14 ` [PATCH 1/7] kgdb: Add the ability to schedule a breakpoint via a tasklet Jason Wessel
  2008-10-21 22:49 ` kgdb 2.6.28 merge plans Alan Cox
  0 siblings, 2 replies; 16+ messages in thread
From: Jason Wessel @ 2008-10-21 19:14 UTC (permalink / raw)
  To: kgdb-bugreport; +Cc: linux-kernel

It is late in the merge window, but these are plans for the kgdb
merges for 2.6.28.

* The blackfin folks needed some changes to the kgdb core
  to allow some more flexibility
* Another kgdboc I/O drivers is ready for merge
* The CONSOLE_POLL API has been extended to allow for
  an RX poll call back from a low level serial driver
* kgdboc will request/allocate a tty so as to sync with
  the user space with respect to having only one
  invocation of the low level start up code.

To an end user there are a couple key differences with the enhanced
kgdboc.  There were numerous complaints that folks just wanted kgdboc
to work like the 8250_kgdb module where gdb could just connect and it
worked.  This is the new default behavior of kgdboc.  It will
intercept a control-c or the start of a gdb serial packet and
automatically jump into the debugger, much like the 8250_kgdb module.
The other difference is that with the tty allocation, kgdboc can also
now be configured on a non-console or inactive serial port.

After this patch set is applied the only thing that kgdboc cannot
currently do that the 8250_kgdb module could is perform debugging
before the tty and console kernel subsystems have been initialized in
the kernel.  It is likely that this can be accomplished a different
way in the future and the 8250_kgdb module will be deprecated and
never merged to the mainline kernel.

Final regression testing for these features is being completed in the
next day. Find the code at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git for_linus

Short log follows:

--- 
Atsushi Nemoto (1):
      kgdb: kgdboc console poll hooks for serial_txx9 uart

Jason Wessel (5):
      kgdb: Add the ability to schedule a breakpoint via a tasklet
      tty: Fix sparse static warning for tty_driver_lookup_tty
      kgdboc, tty: Add the rx polling call back capability
      kgdboc, 8250: Add the rx polling hook to 8250 driver
      kgdboc, tty: use tty open to start low level drivers

Sonic Zhang (1):
      kgdb: Make mem access function weak in kgdb.c andkgdb.h

 Documentation/DocBook/kgdb.tmpl |   49 ++++++++++++++---
 drivers/char/tty_io.c           |  105 +++++++++++++++++++++++++++++++-----
 drivers/serial/8250.c           |    4 ++
 drivers/serial/kgdboc.c         |   82 +++++++++++++++++++++++++++-
 drivers/serial/serial_core.c    |   24 ++++++++-
 drivers/serial/serial_txx9.c    |  113 +++++++++++++++++++++++++++++++-------
 include/linux/kgdb.h            |   43 ++++++++++++++-
 include/linux/serial_core.h     |    3 +
 include/linux/tty_driver.h      |    8 +++-
 kernel/kgdb.c                   |   33 ++++++++++-
 10 files changed, 413 insertions(+), 51 deletions(-)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/7] kgdb: Add the ability to schedule a breakpoint via a tasklet
  2008-10-21 19:14 kgdb 2.6.28 merge plans Jason Wessel
@ 2008-10-21 19:14 ` Jason Wessel
  2008-10-21 19:14   ` [PATCH 2/7] tty: Fix sparse static warning for tty_driver_lookup_tty Jason Wessel
  2008-10-21 22:49 ` kgdb 2.6.28 merge plans Alan Cox
  1 sibling, 1 reply; 16+ messages in thread
From: Jason Wessel @ 2008-10-21 19:14 UTC (permalink / raw)
  To: kgdb-bugreport; +Cc: linux-kernel, Jason Wessel

Some kgdb I/O modules require the ability to create a breakpoint
tasklet, such as kgdboc and external modules such as kgdboe.  The
breakpoint tasklet is used as an asynchronous entry point into the
debugger which will have a different function scope than the current
execution path where it might not be safe to have an inline breakpoint
inside the kgdb I/O driver.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 include/linux/kgdb.h |    1 +
 kernel/kgdb.c        |   26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 6adcc29..8fdd3cb 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -279,5 +279,6 @@ extern int kgdb_nmicallback(int cpu, void *regs);
 
 extern int			kgdb_single_step;
 extern atomic_t			kgdb_active;
+extern void kgdb_schedule_breakpoint(void);
 
 #endif /* _KGDB_H_ */
diff --git a/kernel/kgdb.c b/kernel/kgdb.c
index e4dcfb2..5365d46 100644
--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -123,6 +123,7 @@ atomic_t			kgdb_active = ATOMIC_INIT(-1);
  */
 static atomic_t			passive_cpu_wait[NR_CPUS];
 static atomic_t			cpu_in_kgdb[NR_CPUS];
+static atomic_t			kgdb_break_tasklet_var;
 atomic_t			kgdb_setting_breakpoint;
 
 struct task_struct		*kgdb_usethread;
@@ -1623,6 +1624,31 @@ static void kgdb_unregister_callbacks(void)
 	}
 }
 
+/*
+ * There are times a tasklet needs to be used vs a compiled in in
+ * break point so as to cause an exception outside a kgdb I/O module,
+ * such as is the case with kgdboe, where calling a breakpoint in the
+ * I/O driver itself would be fatal.
+ */
+static void kgdb_tasklet_bpt(unsigned long ing)
+{
+	kgdb_breakpoint();
+	atomic_set(&kgdb_break_tasklet_var, 0);
+}
+
+static DECLARE_TASKLET(kgdb_tasklet_breakpoint, kgdb_tasklet_bpt, 0);
+
+void kgdb_schedule_breakpoint(void)
+{
+	if (atomic_read(&kgdb_break_tasklet_var) ||
+		atomic_read(&kgdb_active) != -1 ||
+		atomic_read(&kgdb_setting_breakpoint))
+		return;
+	atomic_inc(&kgdb_break_tasklet_var);
+	tasklet_schedule(&kgdb_tasklet_breakpoint);
+}
+EXPORT_SYMBOL_GPL(kgdb_schedule_breakpoint);
+
 static void kgdb_initial_breakpoint(void)
 {
 	kgdb_break_asap = 0;
-- 
1.6.0.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/7] tty: Fix sparse static warning for tty_driver_lookup_tty
  2008-10-21 19:14 ` [PATCH 1/7] kgdb: Add the ability to schedule a breakpoint via a tasklet Jason Wessel
@ 2008-10-21 19:14   ` Jason Wessel
  2008-10-21 19:14     ` [PATCH 3/7] kgdboc, tty: Add the rx polling call back capability Jason Wessel
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Wessel @ 2008-10-21 19:14 UTC (permalink / raw)
  To: kgdb-bugreport; +Cc: linux-kernel, Jason Wessel, alan

Fixed sparse warning:
drivers/char/tty_io.c:1216:19: warning: symbol 'tty_driver_lookup_tty' was not declared. Should it be static?

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/char/tty_io.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 59f4721..6fb2cbf 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -1213,7 +1213,7 @@ static void tty_line_name(struct tty_driver *driver, int index, char *p)
  *	be held until the 'fast-open' is also done. Will change once we
  *	have refcounting in the driver and per driver locking
  */
-struct tty_struct *tty_driver_lookup_tty(struct tty_driver *driver,
+static struct tty_struct *tty_driver_lookup_tty(struct tty_driver *driver,
 		struct inode *inode, int idx)
 {
 	struct tty_struct *tty;
-- 
1.6.0.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 3/7] kgdboc, tty: Add the rx polling call back capability
  2008-10-21 19:14   ` [PATCH 2/7] tty: Fix sparse static warning for tty_driver_lookup_tty Jason Wessel
@ 2008-10-21 19:14     ` Jason Wessel
  2008-10-21 19:14       ` [PATCH 4/7] kgdboc, 8250: Add the rx polling hook to 8250 driver Jason Wessel
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Wessel @ 2008-10-21 19:14 UTC (permalink / raw)
  To: kgdb-bugreport; +Cc: linux-kernel, Jason Wessel, alan

The idea is to allow kgdboc to intercept a <contorol-c> or any other
character of preference to cause breakpoint interrupt which will start
the kgdb interface running on the controlling terminal where the
character was typed.

The default behavior of kgdboc changes such that the control-c will
always generate an entry to kgdb unless the "n" option is used in the
kgdb configuration line. IE: kgdboc=ttyS0,n,115200

In order to make use of the new API, a low level serial driver must
check to see if it should execute the callback function for each
character that it processes.  This is similar to the approach used
with the NET_POLL API's rx_hook.

The only changes to the tty layer introduced by this patch are:
  * Add poll_rx_cb() call back for the low level driver
  * Move the poll_init() into kgdboc and out of tty_find_polling_driv()
  * change poll_init() to accept the rx callback parameter

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 Documentation/DocBook/kgdb.tmpl |   49 ++++++++++++++++++++++----
 drivers/char/tty_io.c           |    7 +---
 drivers/serial/kgdboc.c         |   72 +++++++++++++++++++++++++++++++++++++-
 drivers/serial/serial_core.c    |   24 ++++++++++++-
 include/linux/serial_core.h     |    3 ++
 include/linux/tty_driver.h      |    3 +-
 kernel/kgdb.c                   |    1 +
 7 files changed, 141 insertions(+), 18 deletions(-)

diff --git a/Documentation/DocBook/kgdb.tmpl b/Documentation/DocBook/kgdb.tmpl
index 372dec2..67c17c8 100644
--- a/Documentation/DocBook/kgdb.tmpl
+++ b/Documentation/DocBook/kgdb.tmpl
@@ -184,7 +184,14 @@
   or built-in.
   <orderedlist>
   <listitem><para>From the module load or build-in</para>
-  <para><constant>kgdboc=&lt;tty-device&gt;,[baud]</constant></para>
+  <para><constant>kgdboc=&lt;tty-device&gt;[,n][,B][,c###][,baud]</constant></para>
+   <itemizedlist>
+   <listitem><para>,n = No monitoring the port for a break char.</para>
+     <para>You would consider using this option if you would like to be able to type the control-c character on your console device.  In which case, to enter the debugger, you need to use sysrq-g</para></listitem>
+   <listitem><para>,B = Monitor the port for a break char and issue a breakpoint in line</para></listitem>
+   <listitem><para>,c### = Use an alternate break character 1-255 instead of ^C (3), example to use ^D as the break char kgdboc=ttyS0,c4,115200</para></listitem>
+   <listitem><para>,baud = A baud rate parameter IE: 115200n81</para></listitem>
+   </itemizedlist>
   <para>
   The example here would be if your console port was typically ttyS0, you would use something like <constant>kgdboc=ttyS0,115200</constant> or on the ARM Versatile AB you would likely use <constant>kgdboc=ttyAMA0,115200</constant>
   </para>
@@ -195,8 +202,9 @@
   </orderedlist>
   </para>
   <para>
-  NOTE: Kgdboc does not support interrupting the target via the
-  gdb remote protocol.  You must manually send a sysrq-g unless you
+  NOTE: By default kgdboc tries to use the mode of operation where the
+  low level serial driver will intercept control-c.  If you elect not
+  to use this mode, you must manually send a sysrq-g unless you
   have a proxy that splits console output to a terminal problem and
   has a separate port for the debugger to connect to that sends the
   sysrq-g for you.
@@ -253,9 +261,14 @@
     attach before you can connect gdb.
     </para>
     <para>
-    If you are not using different kgdb I/O driver other than kgdboc,
-    you should be able to connect and the target will automatically
-    respond.
+    The kgdboc driver has two modes of operation depending on if the
+    low level serial driver supports the rx polling call back and how
+    the arguments you passed to kgdboc to configure it.  By default
+    gdb expects to be able to connect to kgdb and start issuing gdb
+    serial commands.  If you specificed the ",n" (IE:
+    kgdboc=ttyS0,n,115200) or your serial driver does not implement
+    the rx poll hook, you must enter the debugger by using the sysrq-g
+    sequence prior to connecting gdb.
     </para>
     <para>
     Example (using a serial port):
@@ -415,7 +428,7 @@
   The kgdboc driver is actually a very thin driver that relies on the
   underlying low level to the hardware driver having "polling hooks"
   which the to which the tty driver is attached.  In the initial
-  implementation of kgdboc it the serial_core was changed to expose a
+  implementation of kgdboc the serial_core was changed to expose a
   low level uart hook for doing polled mode reading and writing of a
   single character while in an atomic context.  When kgdb makes an I/O
   request to the debugger, kgdboc invokes a call back in the serial
@@ -424,7 +437,14 @@
   consoles in the future.
   </para>
   <para>
-  When using kgdboc with a uart, the uart driver must implement two callbacks in the <constant>struct uart_ops</constant>. Example from drivers/8250.c:<programlisting>
+  In the 2.6.28 kernel, the CONSOLE_POLL API was augmented to include
+  a receive call back which a low level serial driver can call when
+  ever it receives a character.  This had the explicit purpose of
+  allowing a kgdboc to register to receive characters so as to execute
+  an entry point to the debugger upon receiving a specific character.
+  </para>
+  <para>
+  When using kgdboc with a uart, the uart driver must implement two callbacks in the <constant>struct uart_ops</constant>. Example from drivers/serial/8250.c:<programlisting>
 #ifdef CONFIG_CONSOLE_POLL
 	.poll_get_char = serial8250_get_poll_char,
 	.poll_put_char = serial8250_put_poll_char,
@@ -439,6 +459,19 @@
   with any kind of lock you consider, because failing here is most
   going to mean pressing the reset button.
   </para>
+  <para>
+  Each low level serial driver can also call poll_rx_cb().  This is a
+  call back into kgdboc with the purpose allowing kgdboc to intercept
+  characters.  If the function returns a 1, it means that no further
+  processing should be done in the low level driver, as if the
+  character had never been received.  Example from
+  drivers/serial/8250.c:<programlisting>
+#ifdef CONFIG_CONSOLE_POLL
+	if (up->port.poll_rx_cb &amp;&amp; up->port.poll_rx_cb(ch))
+		goto ignore_char;
+#endif
+  </programlisting>
+  </para>
   </sect1>
   </chapter>
   <chapter id="credits">
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 6fb2cbf..fa44e6b 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -311,13 +311,8 @@ struct tty_driver *tty_find_polling_driver(char *name, int *line)
 	list_for_each_entry(p, &tty_drivers, tty_drivers) {
 		if (strncmp(name, p->name, len) != 0)
 			continue;
-		if (*str == ',')
-			str++;
-		if (*str == '\0')
-			str = NULL;
-
 		if (tty_line >= 0 && tty_line <= p->num && p->ops &&
-		    p->ops->poll_init && !p->ops->poll_init(p, tty_line, str)) {
+		    p->ops->poll_init) {
 			res = tty_driver_kref_get(p);
 			*line = tty_line;
 			break;
diff --git a/drivers/serial/kgdboc.c b/drivers/serial/kgdboc.c
index eadc1ab..d5d35c3 100644
--- a/drivers/serial/kgdboc.c
+++ b/drivers/serial/kgdboc.c
@@ -43,6 +43,34 @@ static int kgdboc_option_setup(char *opt)
 	return 0;
 }
 
+static int buffered_char = -1;
+static u8 break_char;
+static int no_polled_breaks;
+static int schedule_breakpoints;
+
+/* Return 1 if a the next layer up should discard the character,
+ * else return 0
+ */
+static int kgdboc_rx_callback(u8 c)
+{
+	if (likely(atomic_read(&kgdb_active) == -1)) {
+		if (no_polled_breaks)
+			return 0;
+		if (c != break_char)
+			buffered_char = c;
+		if (c == break_char ||
+		    (c == '$' && !kgdb_connected && break_char == 0x03)) {
+			if (schedule_breakpoints)
+				kgdb_schedule_breakpoint();
+			else
+				kgdb_breakpoint();
+			return 1;
+		}
+		return 0;
+	}
+	return 1;
+}
+
 __setup("kgdboc=", kgdboc_option_setup);
 
 static int configure_kgdboc(void)
@@ -50,12 +78,18 @@ static int configure_kgdboc(void)
 	struct tty_driver *p;
 	int tty_line = 0;
 	int err;
+	char *str;
 
 	err = kgdboc_option_setup(config);
 	if (err || !strlen(config) || isspace(config[0]))
 		goto noconfig;
 
 	err = -ENODEV;
+	/* If a driver was previously configured remove it now */
+	if (kgdb_tty_driver)
+		kgdb_tty_driver->ops->poll_init(kgdb_tty_driver, kgdb_tty_line,
+						NULL, (void *)-1);
+	kgdb_tty_driver = NULL;
 
 	p = tty_find_polling_driver(config, &tty_line);
 	if (!p)
@@ -63,6 +97,26 @@ static int configure_kgdboc(void)
 
 	kgdb_tty_driver = p;
 	kgdb_tty_line = tty_line;
+	/* Set defaults and parse optional configuration information */
+	no_polled_breaks = 0;
+	schedule_breakpoints = 1;
+	break_char = 0x03;
+	if (strstr(config, ",n"))
+		no_polled_breaks = 1;
+	if (strstr(config, ",B"))
+		schedule_breakpoints = 0;
+	str = strstr(config, ",c");
+	if (str)
+		break_char = simple_strtoul(str+2, &str, 10);
+	str = strrchr(config, ','); /* pointer to baud for init callback */
+	if (str) {
+		str++;
+		if (!(*str >= '0' && *str <= '9'))
+			str = NULL;
+	}
+	/* Initialize the HW level driver for polling */
+	if (p->ops->poll_init(p, tty_line, str, kgdboc_rx_callback))
+		goto noconfig;
 
 	err = kgdb_register_io_module(&kgdboc_io_ops);
 	if (err)
@@ -73,6 +127,10 @@ static int configure_kgdboc(void)
 	return 0;
 
 noconfig:
+	if (kgdb_tty_driver)
+		kgdb_tty_driver->ops->poll_init(kgdb_tty_driver, kgdb_tty_line,
+						NULL, (void *)-1);
+	kgdb_tty_driver = NULL;
 	config[0] = 0;
 	configured = 0;
 
@@ -96,8 +154,11 @@ static void cleanup_kgdboc(void)
 
 static int kgdboc_get_char(void)
 {
+	if (buffered_char >= 0)
+		return xchg(&buffered_char, -1);
+
 	return kgdb_tty_driver->ops->poll_get_char(kgdb_tty_driver,
-						kgdb_tty_line);
+						   kgdb_tty_line);
 }
 
 static void kgdboc_put_char(u8 chr)
@@ -165,6 +226,13 @@ static struct kgdb_io kgdboc_io_ops = {
 module_init(init_kgdboc);
 module_exit(cleanup_kgdboc);
 module_param_call(kgdboc, param_set_kgdboc_var, param_get_string, &kps, 0644);
-MODULE_PARM_DESC(kgdboc, "<serial_device>[,baud]");
+/* The optional paramters to the config string are:
+ * ,n == no monitoring the port for a break char
+ * ,B == monitor the port for a break char and issue a breakpoint in line
+ * ,c### == Use an alternate break character 1-255 instead of ^C (3)
+ * The baud parameter must always be last, if used
+ * ,baud == A baud rate parameter IE: 115200n81
+ */
+MODULE_PARM_DESC(kgdboc, "<serial_device>[,n][,B][,c###][,baud]");
 MODULE_DESCRIPTION("KGDB Console TTY Driver");
 MODULE_LICENSE("GPL");
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 874786a..467b8a4 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2242,7 +2242,22 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
 
 #ifdef CONFIG_CONSOLE_POLL
 
-static int uart_poll_init(struct tty_driver *driver, int line, char *options)
+/**
+ *	uart_poll_init - setup the console polling device
+ *	@driver: pointer to high level tty driver
+ *	@line: tty line number
+ *	@options: baud string for uart initialization
+ *	@rx_callback: call back for character processing
+ *
+ *      uart_poll_init activates the low level initialization of the
+ *      uart device for use with polling access to the uart while the
+ *      interrupts are off, which is primarily used for the debugger.
+ *      If rx_callback is set to -1, the specified tty driver and line
+ *      will have the call back function set to NULL uart_poll_init
+ *      will return immediately.
+ */
+static int uart_poll_init(struct tty_driver *driver, int line,
+		char *options, void *rx_callback)
 {
 	struct uart_driver *drv = driver->driver_state;
 	struct uart_state *state = drv->state + line;
@@ -2256,9 +2271,16 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
 		return -1;
 
 	port = state->port;
+	if (rx_callback + 1 == 0) {
+		port->poll_rx_cb = NULL;
+		return 0;
+	}
+
 	if (!(port->ops->poll_get_char && port->ops->poll_put_char))
 		return -1;
 
+	port->poll_rx_cb = rx_callback;
+
 	if (options) {
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
 		return uart_set_options(port, NULL, baud, parity, bits, flow);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index e27f216..3a57c1d 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -308,6 +308,9 @@ struct uart_port {
 	unsigned char		suspended;
 	unsigned char		unused[2];
 	void			*private_data;		/* generic platform data pointer */
+#ifdef CONFIG_CONSOLE_POLL
+	int		(*poll_rx_cb)(u8);
+#endif
 };
 
 /*
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index 78416b9..4c30f16 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -262,7 +262,8 @@ struct tty_operations {
 				struct winsize *ws);
 	int (*set_termiox)(struct tty_struct *tty, struct termiox *tnew);
 #ifdef CONFIG_CONSOLE_POLL
-	int (*poll_init)(struct tty_driver *driver, int line, char *options);
+	int (*poll_init)(struct tty_driver *driver, int line, char *options,
+			void *rx_callback);
 	int (*poll_get_char)(struct tty_driver *driver, int line);
 	void (*poll_put_char)(struct tty_driver *driver, int line, char ch);
 #endif
diff --git a/kernel/kgdb.c b/kernel/kgdb.c
index 5365d46..d7a30bb 100644
--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -116,6 +116,7 @@ static struct kgdb_bkpt		kgdb_break[KGDB_MAX_BREAKPOINTS] = {
  * The CPU# of the active CPU, or -1 if none:
  */
 atomic_t			kgdb_active = ATOMIC_INIT(-1);
+EXPORT_SYMBOL_GPL(kgdb_active);
 
 /*
  * We use NR_CPUs not PERCPU, in case kgdb is used to debug early
-- 
1.6.0.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 4/7] kgdboc, 8250: Add the rx polling hook to 8250 driver
  2008-10-21 19:14     ` [PATCH 3/7] kgdboc, tty: Add the rx polling call back capability Jason Wessel
@ 2008-10-21 19:14       ` Jason Wessel
  2008-10-21 19:14         ` [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers Jason Wessel
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Wessel @ 2008-10-21 19:14 UTC (permalink / raw)
  To: kgdb-bugreport; +Cc: linux-kernel, Jason Wessel, alan

The RX polling hook allows the debugger to hook character input so as
to allow entry to the kernel debugger with a control-c as an example.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/serial/8250.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 303272a..4f7343e 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1361,6 +1361,10 @@ receive_chars(struct uart_8250_port *up, unsigned int *status)
 			else if (lsr & UART_LSR_FE)
 				flag = TTY_FRAME;
 		}
+#ifdef CONFIG_CONSOLE_POLL
+		if (up->port.poll_rx_cb && up->port.poll_rx_cb(ch))
+			goto ignore_char;
+#endif
 		if (uart_handle_sysrq_char(&up->port, ch))
 			goto ignore_char;
 
-- 
1.6.0.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers
  2008-10-21 19:14       ` [PATCH 4/7] kgdboc, 8250: Add the rx polling hook to 8250 driver Jason Wessel
@ 2008-10-21 19:14         ` Jason Wessel
  2008-10-21 19:14           ` [PATCH 6/7] kgdb: kgdboc console poll hooks for serial_txx9 uart Jason Wessel
                             ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Jason Wessel @ 2008-10-21 19:14 UTC (permalink / raw)
  To: kgdb-bugreport; +Cc: linux-kernel, Jason Wessel, alan

This patch adds two new hooks to the tty layer in tty_io.c to allow
kgdboc to open a tty prior to the start of the user space processes,
as well as to open a tty to serial port which has nothing connected on
it.  This is for the purpose of starting the low level serial drivers
such that a control-c can be caught via the RX poll mechanism.

The two new functions, tty_console_poll_open() and
tty_console_poll_close() take care of setting/cleaning up an empty
"struct file *filp" as well as keeping the reference counts correct
for the tty device.  The new function are only accessible if
CONFIG_CONSOLE_POLL as it is a direct extension to the console poll
API.  All the filp operation for the new functions take place in the
tty_io.c so no further functions have to be exported externally as
well as allowing for the internals of the tty layer to remain private.

A few minor changes were needed in tty_io.c to deal with the fact that
the inode and fpath.dentry were NULL with the generic flip structure.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/char/tty_io.c      |   96 +++++++++++++++++++++++++++++++++++++++++---
 drivers/serial/kgdboc.c    |   26 ++++++++----
 include/linux/tty_driver.h |    5 ++
 3 files changed, 113 insertions(+), 14 deletions(-)

diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index fa44e6b..fe500ec 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -143,6 +143,8 @@ ssize_t redirected_tty_write(struct file *, const char __user *,
 static unsigned int tty_poll(struct file *, poll_table *);
 static int tty_open(struct inode *, struct file *);
 static int tty_release(struct inode *, struct file *);
+static struct tty_struct *tty_driver_lookup_tty(struct tty_driver *driver,
+					 struct inode *inode, int idx);
 long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
 #ifdef CONFIG_COMPAT
 static long tty_compat_ioctl(struct file *file, unsigned int cmd,
@@ -280,6 +282,83 @@ static struct tty_driver *get_tty_driver(dev_t device, int *index)
 }
 
 #ifdef CONFIG_CONSOLE_POLL
+/**
+ *	tty_console_poll_open	-	allocate a tty for a polled device
+ *	@driver: the tty driver
+ *	@filp: file pointer to point to the tty
+ *	@tty_line: the minor device number
+ *
+ *	This routine returns allocates an empty struct file structure
+ *	to allow a polling device to open a tty.  This ultimately
+ *	allows the low level startup code for the uart to only be
+ *	called one time, for either the polling device init or user
+ *	space tty init.
+ */
+int tty_console_poll_open(struct tty_driver *driver, struct file **filp,
+			  int tty_line)
+{
+	int ret = -EIO;
+	struct tty_struct *tty;
+	int inc_tty = 1;
+
+	if (!*filp)
+		*filp = get_empty_filp();
+	if (!*filp)
+		goto out;
+
+	mutex_lock(&tty_mutex);
+	if (!kernel_locked()) {
+		lock_kernel();
+		tty = tty_driver_lookup_tty(driver, NULL, tty_line);
+		if (!tty) {
+			tty = tty_init_dev(driver, tty_line, 0);
+			inc_tty = 0;
+		}
+		(*filp)->private_data = tty;
+		ret = tty->ops->open(tty, *filp);
+		unlock_kernel();
+	} else {
+		tty = tty_driver_lookup_tty(driver, NULL, tty_line);
+		if (!tty) {
+			tty = tty_init_dev(driver, tty_line, 0);
+			inc_tty = 0;
+		}
+		(*filp)->private_data = tty;
+		ret = tty->ops->open(tty, *filp);
+	}
+	if (ret == 0) {
+		file_move(*filp, &tty->tty_files);
+		tty->count += inc_tty;
+	} else {
+		put_filp(*filp);
+		*filp = NULL;
+	}
+	mutex_unlock(&tty_mutex);
+out:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tty_console_poll_open);
+
+/**
+ *	tty_console_poll_close	-	shutdown a tty for a polled device
+ *	@filp: file pointer to point to the tty
+ *
+ *	Deallocate the tty device used by the polling driver, and free
+ *	the associated filp.
+ */
+void tty_console_poll_close(struct file **filp)
+{
+	if (!kernel_locked()) {
+		lock_kernel();
+		tty_release_dev(*filp);
+		unlock_kernel();
+	} else {
+		tty_release_dev(*filp);
+	}
+	put_filp(*filp);
+	*filp = NULL;
+}
+EXPORT_SYMBOL_GPL(tty_console_poll_close);
 
 /**
  *	tty_find_polling_driver	-	find device of a polled tty
@@ -552,6 +631,8 @@ static void do_tty_hangup(struct work_struct *work)
 	file_list_lock();
 	/* This breaks for file handles being sent over AF_UNIX sockets ? */
 	list_for_each_entry(filp, &tty->tty_files, f_u.fu_list) {
+		if (!filp->f_op)
+			continue;
 		if (filp->f_op->write == redirected_tty_write)
 			cons_filp = filp;
 		if (filp->f_op->write != tty_write)
@@ -1524,12 +1605,14 @@ void tty_release_dev(struct file *filp)
 	int	devpts;
 	int	idx;
 	char	buf[64];
-	struct 	inode *inode;
+	struct 	inode *inode = NULL;
 
-	inode = filp->f_path.dentry->d_inode;
 	tty = (struct tty_struct *)filp->private_data;
-	if (tty_paranoia_check(tty, inode, "tty_release_dev"))
-		return;
+	if (filp->f_path.dentry) {
+		inode = filp->f_path.dentry->d_inode;
+		if (tty_paranoia_check(tty, inode, "tty_release_dev"))
+			return;
+	}
 
 	check_tty_count(tty, "tty_release_dev");
 
@@ -1723,7 +1806,7 @@ void tty_release_dev(struct file *filp)
 	release_tty(tty, idx);
 
 	/* Make this pty number available for reallocation */
-	if (devpts)
+	if (devpts && inode)
 		devpts_kill_index(inode, idx);
 }
 
@@ -1950,7 +2033,8 @@ static int tty_fasync(int fd, struct file *filp, int on)
 
 	lock_kernel();
 	tty = (struct tty_struct *)filp->private_data;
-	if (tty_paranoia_check(tty, filp->f_path.dentry->d_inode, "tty_fasync"))
+	if (filp->f_path.dentry &&
+	    tty_paranoia_check(tty, filp->f_path.dentry->d_inode, "tty_fasync"))
 		goto out;
 
 	retval = fasync_helper(fd, filp, on, &tty->fasync);
diff --git a/drivers/serial/kgdboc.c b/drivers/serial/kgdboc.c
index d5d35c3..b274a36 100644
--- a/drivers/serial/kgdboc.c
+++ b/drivers/serial/kgdboc.c
@@ -31,6 +31,7 @@ static struct kparam_string kps = {
 
 static struct tty_driver	*kgdb_tty_driver;
 static int			kgdb_tty_line;
+static struct file		*kgdb_filp;
 
 static int kgdboc_option_setup(char *opt)
 {
@@ -73,6 +74,16 @@ static int kgdboc_rx_callback(u8 c)
 
 __setup("kgdboc=", kgdboc_option_setup);
 
+static void release_kgdboc_tty(void)
+{
+	if (kgdb_tty_driver)
+		kgdb_tty_driver->ops->poll_init(kgdb_tty_driver, kgdb_tty_line,
+						NULL, (void *)-1);
+	if (kgdb_filp)
+		tty_console_poll_close(&kgdb_filp);
+	kgdb_tty_driver = NULL;
+}
+
 static int configure_kgdboc(void)
 {
 	struct tty_driver *p;
@@ -86,10 +97,7 @@ static int configure_kgdboc(void)
 
 	err = -ENODEV;
 	/* If a driver was previously configured remove it now */
-	if (kgdb_tty_driver)
-		kgdb_tty_driver->ops->poll_init(kgdb_tty_driver, kgdb_tty_line,
-						NULL, (void *)-1);
-	kgdb_tty_driver = NULL;
+	release_kgdboc_tty();
 
 	p = tty_find_polling_driver(config, &tty_line);
 	if (!p)
@@ -118,6 +126,11 @@ static int configure_kgdboc(void)
 	if (p->ops->poll_init(p, tty_line, str, kgdboc_rx_callback))
 		goto noconfig;
 
+	/* Open the port and obtain a tty which call low level driver startup */
+	if (tty_console_poll_open(kgdb_tty_driver, &kgdb_filp,
+				  kgdb_tty_line) != 0)
+		goto noconfig;
+
 	err = kgdb_register_io_module(&kgdboc_io_ops);
 	if (err)
 		goto noconfig;
@@ -127,10 +140,7 @@ static int configure_kgdboc(void)
 	return 0;
 
 noconfig:
-	if (kgdb_tty_driver)
-		kgdb_tty_driver->ops->poll_init(kgdb_tty_driver, kgdb_tty_line,
-						NULL, (void *)-1);
-	kgdb_tty_driver = NULL;
+	release_kgdboc_tty();
 	config[0] = 0;
 	configured = 0;
 
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index 4c30f16..1af8393 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -310,7 +310,12 @@ extern struct tty_driver *alloc_tty_driver(int lines);
 extern void put_tty_driver(struct tty_driver *driver);
 extern void tty_set_operations(struct tty_driver *driver,
 			const struct tty_operations *op);
+#ifdef CONFIG_CONSOLE_POLL
 extern struct tty_driver *tty_find_polling_driver(char *name, int *line);
+extern int tty_console_poll_open(struct tty_driver *driver,
+			struct file **filp, int line);
+extern void tty_console_poll_close(struct file **filp);
+#endif
 
 extern void tty_driver_kref_put(struct tty_driver *driver);
 extern inline struct tty_driver *tty_driver_kref_get(struct tty_driver *d)
-- 
1.6.0.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 6/7] kgdb: kgdboc console poll hooks for serial_txx9 uart
  2008-10-21 19:14         ` [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers Jason Wessel
@ 2008-10-21 19:14           ` Jason Wessel
  2008-10-21 19:14             ` [PATCH 7/7] kgdb: Make mem access function weak in kgdb.c andkgdb.h Jason Wessel
  2008-10-21 21:49           ` [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers Jason Wessel
  2008-10-21 22:47           ` Alan Cox
  2 siblings, 1 reply; 16+ messages in thread
From: Jason Wessel @ 2008-10-21 19:14 UTC (permalink / raw)
  To: kgdb-bugreport; +Cc: linux-kernel, Atsushi Nemoto, Jason Wessel

From: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

Implement the serial polling hooks for the serial_txx9 uart for use
with kgdboc.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/serial/serial_txx9.c |  113 ++++++++++++++++++++++++++++++++++--------
 1 files changed, 92 insertions(+), 21 deletions(-)

diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 7313c2e..54dd16d 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -461,6 +461,94 @@ static void serial_txx9_break_ctl(struct uart_port *port, int break_state)
 	spin_unlock_irqrestore(&up->port.lock, flags);
 }
 
+#if defined(CONFIG_SERIAL_TXX9_CONSOLE) || (CONFIG_CONSOLE_POLL)
+/*
+ *	Wait for transmitter & holding register to empty
+ */
+static void wait_for_xmitr(struct uart_txx9_port *up)
+{
+	unsigned int tmout = 10000;
+
+	/* Wait up to 10ms for the character(s) to be sent. */
+	while (--tmout &&
+	       !(sio_in(up, TXX9_SICISR) & TXX9_SICISR_TXALS))
+		udelay(1);
+
+	/* Wait up to 1s for flow control if necessary */
+	if (up->port.flags & UPF_CONS_FLOW) {
+		tmout = 1000000;
+		while (--tmout &&
+		       (sio_in(up, TXX9_SICISR) & TXX9_SICISR_CTSS))
+			udelay(1);
+	}
+}
+#endif
+
+#ifdef CONFIG_CONSOLE_POLL
+/*
+ * Console polling routines for writing and reading from the uart while
+ * in an interrupt or debug context.
+ */
+
+static int serial_txx9_get_poll_char(struct uart_port *port)
+{
+	unsigned int ier;
+	unsigned char c;
+	struct uart_txx9_port *up = (struct uart_txx9_port *)port;
+
+	/*
+	 *	First save the IER then disable the interrupts
+	 */
+	ier = sio_in(up, TXX9_SIDICR);
+	sio_out(up, TXX9_SIDICR, 0);
+
+	while (sio_in(up, TXX9_SIDISR) & TXX9_SIDISR_UVALID)
+		;
+
+	c = sio_in(up, TXX9_SIRFIFO);
+
+	/*
+	 *	Finally, clear RX interrupt status
+	 *	and restore the IER
+	 */
+	sio_mask(up, TXX9_SIDISR, TXX9_SIDISR_RDIS);
+	sio_out(up, TXX9_SIDICR, ier);
+	return c;
+}
+
+
+static void serial_txx9_put_poll_char(struct uart_port *port, unsigned char c)
+{
+	unsigned int ier;
+	struct uart_txx9_port *up = (struct uart_txx9_port *)port;
+
+	/*
+	 *	First save the IER then disable the interrupts
+	 */
+	ier = sio_in(up, TXX9_SIDICR);
+	sio_out(up, TXX9_SIDICR, 0);
+
+	wait_for_xmitr(up);
+	/*
+	 *	Send the character out.
+	 *	If a LF, also do CR...
+	 */
+	sio_out(up, TXX9_SITFIFO, c);
+	if (c == 10) {
+		wait_for_xmitr(up);
+		sio_out(up, TXX9_SITFIFO, 13);
+	}
+
+	/*
+	 *	Finally, wait for transmitter to become empty
+	 *	and restore the IER
+	 */
+	wait_for_xmitr(up);
+	sio_out(up, TXX9_SIDICR, ier);
+}
+
+#endif /* CONFIG_CONSOLE_POLL */
+
 static int serial_txx9_startup(struct uart_port *port)
 {
 	struct uart_txx9_port *up = (struct uart_txx9_port *)port;
@@ -781,6 +869,10 @@ static struct uart_ops serial_txx9_pops = {
 	.release_port	= serial_txx9_release_port,
 	.request_port	= serial_txx9_request_port,
 	.config_port	= serial_txx9_config_port,
+#ifdef CONFIG_CONSOLE_POLL
+	.poll_get_char	= serial_txx9_get_poll_char,
+	.poll_put_char	= serial_txx9_put_poll_char,
+#endif
 };
 
 static struct uart_txx9_port serial_txx9_ports[UART_NR];
@@ -803,27 +895,6 @@ static void __init serial_txx9_register_ports(struct uart_driver *drv,
 
 #ifdef CONFIG_SERIAL_TXX9_CONSOLE
 
-/*
- *	Wait for transmitter & holding register to empty
- */
-static inline void wait_for_xmitr(struct uart_txx9_port *up)
-{
-	unsigned int tmout = 10000;
-
-	/* Wait up to 10ms for the character(s) to be sent. */
-	while (--tmout &&
-	       !(sio_in(up, TXX9_SICISR) & TXX9_SICISR_TXALS))
-		udelay(1);
-
-	/* Wait up to 1s for flow control if necessary */
-	if (up->port.flags & UPF_CONS_FLOW) {
-		tmout = 1000000;
-		while (--tmout &&
-		       (sio_in(up, TXX9_SICISR) & TXX9_SICISR_CTSS))
-			udelay(1);
-	}
-}
-
 static void serial_txx9_console_putchar(struct uart_port *port, int ch)
 {
 	struct uart_txx9_port *up = (struct uart_txx9_port *)port;
-- 
1.6.0.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 7/7] kgdb: Make mem access function weak in kgdb.c andkgdb.h
  2008-10-21 19:14           ` [PATCH 6/7] kgdb: kgdboc console poll hooks for serial_txx9 uart Jason Wessel
@ 2008-10-21 19:14             ` Jason Wessel
  0 siblings, 0 replies; 16+ messages in thread
From: Jason Wessel @ 2008-10-21 19:14 UTC (permalink / raw)
  To: kgdb-bugreport; +Cc: linux-kernel, Sonic Zhang, Jason Wessel

From: Sonic Zhang <sonic.adi@gmail.com>

L1 instruction memory and MMR memory on blackfin can not be accessed by
common functions probe_kernel_read() and probe_kernel_write().
Blackfin asks for 2/4 byte align access to MMR memory and DMA access to
L1 instruction memory. These functions need to be reimplemented in
architecture specific kgdb.c. Update documentation and prototypes as
well.

Signed-off-by: Sonic Zhang <sonic.adi@gmail.com>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 include/linux/kgdb.h |   42 ++++++++++++++++++++++++++++++++++++++++--
 kernel/kgdb.c        |    6 +++---
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 8fdd3cb..3df90af 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -267,8 +267,46 @@ extern int kgdb_register_io_module(struct kgdb_io *local_kgdb_io_ops);
 extern void kgdb_unregister_io_module(struct kgdb_io *local_kgdb_io_ops);
 
 extern int kgdb_hex2long(char **ptr, unsigned long *long_val);
-extern int kgdb_mem2hex(char *mem, char *buf, int count);
-extern int kgdb_hex2mem(char *buf, char *mem, int count);
+
+/**
+ *      kgdb_mem2hex - (optional arch override) translate bin to hex chars
+ *      @mem: source buffer
+ *      @buf: target buffer
+ *      @count: number of bytes in mem
+ *
+ *      Architectures which do not support probe_kernel_(read|write),
+ *      can make an alternate implementation of this function.
+ *      This function safely reads memory into hex
+ *      characters for use with the kgdb protocol.
+ */
+extern int __weak kgdb_mem2hex(char *mem, char *buf, int count);
+
+/**
+ *      kgdb_hex2mem - (optional arch override) translate hex chars to bin
+ *      @buf: source buffer
+ *      @mem: target buffer
+ *      @count: number of bytes in mem
+ *
+ *      Architectures which do not support probe_kernel_(read|write),
+ *      can make an alternate implementation of this function.
+ *      This function safely writes hex characters into memory
+ *      for use with the kgdb protocol.
+ */
+extern int __weak kgdb_hex2mem(char *buf, char *mem, int count);
+
+/**
+ *      kgdb_ebin2mem - (optional arch override) Copy the binary array
+ *     pointed to by buf into mem.
+ *      @buf: source buffer
+ *      @mem: target buffer
+ *      @count: number of bytes in mem
+ *
+ *      Architectures which do not support probe_kernel_(read|write),
+ *      can make an alternate implementation of this function.
+ *      This function safely copies binary array into memory
+ *      for use with the kgdb protocol.
+ */
+extern int __weak kgdb_ebin2mem(char *buf, char *mem, int count);
 
 extern int kgdb_isremovedbreak(unsigned long addr);
 
diff --git a/kernel/kgdb.c b/kernel/kgdb.c
index d7a30bb..e9e0e8a 100644
--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -365,7 +365,7 @@ static void put_packet(char *buffer)
  * Convert the memory pointed to by mem into hex, placing result in buf.
  * Return a pointer to the last char put in buf (null). May return an error.
  */
-int kgdb_mem2hex(char *mem, char *buf, int count)
+int __weak kgdb_mem2hex(char *mem, char *buf, int count)
 {
 	char *tmp;
 	int err;
@@ -395,7 +395,7 @@ int kgdb_mem2hex(char *mem, char *buf, int count)
  * 0x7d escaped with 0x7d.  Return a pointer to the character after
  * the last byte written.
  */
-static int kgdb_ebin2mem(char *buf, char *mem, int count)
+int __weak kgdb_ebin2mem(char *buf, char *mem, int count)
 {
 	int err = 0;
 	char c;
@@ -420,7 +420,7 @@ static int kgdb_ebin2mem(char *buf, char *mem, int count)
  * Return a pointer to the character AFTER the last byte written.
  * May return an error.
  */
-int kgdb_hex2mem(char *buf, char *mem, int count)
+int __weak kgdb_hex2mem(char *buf, char *mem, int count)
 {
 	char *tmp_raw;
 	char *tmp_hex;
-- 
1.6.0.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers
  2008-10-21 19:14         ` [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers Jason Wessel
  2008-10-21 19:14           ` [PATCH 6/7] kgdb: kgdboc console poll hooks for serial_txx9 uart Jason Wessel
@ 2008-10-21 21:49           ` Jason Wessel
  2008-10-21 22:47           ` Alan Cox
  2 siblings, 0 replies; 16+ messages in thread
From: Jason Wessel @ 2008-10-21 21:49 UTC (permalink / raw)
  To: kgdb-bugreport; +Cc: linux-kernel, alan

Jason Wessel wrote:
> A few minor changes were needed in tty_io.c to deal with the fact that
> the inode and fpath.dentry were NULL with the generic flip structure.
>   

After further testing and review one patch hunk has been eliminated.

> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>  drivers/char/tty_io.c      |   96 +++++++++++++++++++++++++++++++++++++++++---
>  drivers/serial/kgdboc.c    |   26 ++++++++----
>  include/linux/tty_driver.h |    5 ++
>  3 files changed, 113 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
> index fa44e6b..fe500ec 100644
> --- a/drivers/char/tty_io.c
> +++ b/drivers/char/tty_io.c
> @@ -1723,7 +1806,7 @@ void tty_release_dev(struct file *filp)
>  	release_tty(tty, idx);
>  
>  	/* Make this pty number available for reallocation */
> -	if (devpts)
> +	if (devpts && inode)
>  		devpts_kill_index(inode, idx);
>  }
>   

Specifically the call to devpts_kill_index() should get executed
and the above patch hunk has been removed.

Thanks,
Jason.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers
  2008-10-21 19:14         ` [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers Jason Wessel
  2008-10-21 19:14           ` [PATCH 6/7] kgdb: kgdboc console poll hooks for serial_txx9 uart Jason Wessel
  2008-10-21 21:49           ` [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers Jason Wessel
@ 2008-10-21 22:47           ` Alan Cox
  2008-10-22  1:54             ` Jason Wessel
  2 siblings, 1 reply; 16+ messages in thread
From: Alan Cox @ 2008-10-21 22:47 UTC (permalink / raw)
  To: Jason Wessel; +Cc: kgdb-bugreport, linux-kernel, Jason Wessel

On Tue, 21 Oct 2008 14:14:51 -0500
Jason Wessel <jason.wessel@windriver.com> wrote:

> This patch adds two new hooks to the tty layer in tty_io.c to allow

Gak

> A few minor changes were needed in tty_io.c to deal with the fact that
> the inode and fpath.dentry were NULL with the generic flip structure.

Gak and more gak

> +		tty = tty_driver_lookup_tty(driver, NULL, tty_line);
> +		if (!tty) {
> +			tty = tty_init_dev(driver, tty_line, 0);
> +			inc_tty = 0;
> +		}
> +		(*filp)->private_data = tty;
> +		ret = tty->ops->open(tty, *filp);

Digging deep into tty innards (some of which btw are going to change)

> +		unlock_kernel();
> +	} else {
> +		tty = tty_driver_lookup_tty(driver, NULL, tty_line);
> +		if (!tty) {
> +			tty = tty_init_dev(driver, tty_line, 0);
> +			inc_tty = 0;
> +		}
> +		(*filp)->private_data = tty;
> +		ret = tty->ops->open(tty, *filp);

and with the error paths wrong


This is to put it bluntly just horrible.

Really kgdb, console and other boot processes need an abstraction for tty
device ports at the device (not ldisc) level and until that work is done
there is no acceptable way to sort this out. Merging gunk like this will
simply make the existing mess worse and provide more crap that will make
it harder to fix properly.

All this stuff needs revisiting when every tty device side object in the
kernel contains a struct tty_port, until then it's a nasty (perhaps
neccessary for the moment) hack that belongs out of tree.

Alan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: kgdb 2.6.28 merge plans
  2008-10-21 19:14 kgdb 2.6.28 merge plans Jason Wessel
  2008-10-21 19:14 ` [PATCH 1/7] kgdb: Add the ability to schedule a breakpoint via a tasklet Jason Wessel
@ 2008-10-21 22:49 ` Alan Cox
  2008-10-22  2:09   ` Jason Wessel
  1 sibling, 1 reply; 16+ messages in thread
From: Alan Cox @ 2008-10-21 22:49 UTC (permalink / raw)
  To: Jason Wessel; +Cc: kgdb-bugreport, linux-kernel

> * kgdboc will request/allocate a tty so as to sync with
>   the user space with respect to having only one
>   invocation of the low level start up code.

The tty stuff for this is not 2.6.28 material

Alan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers
  2008-10-21 22:47           ` Alan Cox
@ 2008-10-22  1:54             ` Jason Wessel
  0 siblings, 0 replies; 16+ messages in thread
From: Jason Wessel @ 2008-10-22  1:54 UTC (permalink / raw)
  To: Alan Cox; +Cc: kgdb-bugreport, linux-kernel

Alan Cox wrote:
> On Tue, 21 Oct 2008 14:14:51 -0500
> Jason Wessel <jason.wessel@windriver.com> wrote:
>
>> +        unlock_kernel();
>> +    } else {
>> +        tty = tty_driver_lookup_tty(driver, NULL, tty_line);
>> +        if (!tty) {
>> +            tty = tty_init_dev(driver, tty_line, 0);
>> +            inc_tty = 0;
>> +        }
>> +        (*filp)->private_data = tty;
>> +        ret = tty->ops->open(tty, *filp);
>
> and with the error paths wrong
>
>
> This is to put it bluntly just horrible.
>

Yeah, I'd say it is pretty horrible too, but this the only API that
exists presently and it is not at all obvious how to use it.

I am fine with keeping it out of the tree, IE this patch is dropped
from merge consideration for now.  It is obvious that it is a moving
target because the patch to 2.6.27 looked different than 2.6.28.  Even
with it out of the tree I am interested in fixing the "error paths"
you mentioned.  Perhaps in this case the return of tty_init_dev()
needed further checking and that was what you were stating is incorrect?

> Really kgdb, console and other boot processes need an abstraction for tty
> device ports at the device (not ldisc) level and until that work is done
> there is no acceptable way to sort this out. Merging gunk like this will
> simply make the existing mess worse and provide more crap that will make
> it harder to fix properly.

If there is another API, please point me to it.  I am not aware of how
to get uart startup code to run without going through the
tty->ops->open().  The kgdboc module has only a generic high level
interface and makes use of only the high level tty calls.  The
debugger's needs are the same as the console and the user space.  The
early debug is still a special case, but in the ideal world there
would be a way to hand off from early debug into the struct tty_port
at the point that console_init() is called.

>
> All this stuff needs revisiting when every tty device side object in the
> kernel contains a struct tty_port, until then it's a nasty (perhaps
> neccessary for the moment) hack that belongs out of tree.

Let me know when we get closer to that stage, as I am interested in
figuring out how to have a uniform way to share the hardware between
the polled and interrupt drive contexts.

Thanks,
Jason.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: kgdb 2.6.28 merge plans
  2008-10-21 22:49 ` kgdb 2.6.28 merge plans Alan Cox
@ 2008-10-22  2:09   ` Jason Wessel
  2008-10-22  8:34     ` Alan Cox
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Wessel @ 2008-10-22  2:09 UTC (permalink / raw)
  To: Alan Cox; +Cc: kgdb-bugreport, linux-kernel

Alan Cox wrote:
>> * kgdboc will request/allocate a tty so as to sync with
>>   the user space with respect to having only one
>>   invocation of the low level start up code.
>
> The tty stuff for this is not 2.6.28 material
>

Anything related to tty allocation has been removed from the merge
request queue, and regression testing is taking place on the revised
patch set at:

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git
for_linus

The rx_poll callback support is still on the docket to request a
merge for 2.6.28.

Cheers,
Jason.

---
Atsushi Nemoto (1):
      kgdb: kgdboc console poll hooks for serial_txx9 uart

Jason Wessel (5):
      kgdb: Add the ability to schedule a breakpoint via a tasklet
      tty: Fix sparse static warning for tty_driver_lookup_tty
      kgdboc, tty: Add the rx polling call back capability
      kgdboc, 8250: rx polling hook for the 8250 driver
      kgdboc, amba-pl011: rx polling hook for the amba-pl011 driver

Sonic Zhang (1):
      kgdb: Make mem access function weak in kgdb.c and kgdb.h

 Documentation/DocBook/kgdb.tmpl |   49 ++++++++++++++---
 drivers/char/tty_io.c           |    9 +---
 drivers/serial/8250.c           |    4 ++
 drivers/serial/amba-pl011.c     |    5 ++-
 drivers/serial/kgdboc.c         |   72 ++++++++++++++++++++++++-
 drivers/serial/serial_core.c    |   24 ++++++++-
 drivers/serial/serial_txx9.c    |  113
+++++++++++++++++++++++++++++++-------
 include/linux/kgdb.h            |   43 ++++++++++++++-
 include/linux/serial_core.h     |    3 +
 include/linux/tty_driver.h      |    3 +-
 kernel/kgdb.c                   |   33 ++++++++++-
 11 files changed, 312 insertions(+), 46 deletions(-)



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: kgdb 2.6.28 merge plans
  2008-10-22  2:09   ` Jason Wessel
@ 2008-10-22  8:34     ` Alan Cox
  2008-10-22 10:07       ` Jason Wessel
  0 siblings, 1 reply; 16+ messages in thread
From: Alan Cox @ 2008-10-22  8:34 UTC (permalink / raw)
  To: Jason Wessel; +Cc: kgdb-bugreport, linux-kernel

>       tty: Fix sparse static warning for tty_driver_lookup_tty
>       kgdboc, tty: Add the rx polling call back capability
>       kgdboc, 8250: rx polling hook for the 8250 driver
>       kgdboc, amba-pl011: rx polling hook for the amba-pl011 driver

This is not 2.6.28 tty layer or serial layer material. The tty driver
tree has been building patches for 2.6.28 for several weeks. Everyone
else managed to send me patches before the big push to Linus to get them
integrated, you didn't.

In addition to which random hooks in random mixtures of uart drivers are
not a practical sensible way to manage general purpose interfaces to
debug tools.

NAK.

You missed the window and the stuff presented simply isn't ready to go
upstream.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: kgdb 2.6.28 merge plans
  2008-10-22  8:34     ` Alan Cox
@ 2008-10-22 10:07       ` Jason Wessel
  2008-10-22 12:25         ` Alan Cox
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Wessel @ 2008-10-22 10:07 UTC (permalink / raw)
  To: Alan Cox; +Cc: kgdb-bugreport, linux-kernel

Alan Cox wrote:
>>       tty: Fix sparse static warning for tty_driver_lookup_tty
>>       kgdboc, tty: Add the rx polling call back capability
>>       kgdboc, 8250: rx polling hook for the 8250 driver
>>       kgdboc, amba-pl011: rx polling hook for the amba-pl011 driver
>
> NAK.

The kgdb merge request queue has had any serial and tty changes
removed.

The only changes that remain are to the kgdb core for the
purpose of moving the blackfin off its existing kgdb stub over to the
common kgdb core API.

>
> In addition to which random hooks in random mixtures of uart drivers are
> not a practical sensible way to manage general purpose interfaces to
> debug tools.
>

This is a discussion that needs to be hashed out.  It is a question of
where would you like it discussed (IE: new thread in lmkl,
linux-serial@...)?

It is a discussion more about general polled vs interrupt driven
serial/tty APIs, not all of which are used for debuggers.  I am open
to help fix/change/extend/refactor the existing kernel's CONSOLE_POLL
API or even deprecate it in favor of something else so long as it
leads to an interface which can allow the sharing of the hardware
among various contexts with early_printk, console, tty, debugger,
etc...

If it was all obvious and easy, it would have already been done.  Rome
was not built in a day after all. :-)

Cheers,
Jason.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: kgdb 2.6.28 merge plans
  2008-10-22 10:07       ` Jason Wessel
@ 2008-10-22 12:25         ` Alan Cox
  0 siblings, 0 replies; 16+ messages in thread
From: Alan Cox @ 2008-10-22 12:25 UTC (permalink / raw)
  To: Jason Wessel; +Cc: kgdb-bugreport, linux-kernel

> This is a discussion that needs to be hashed out.  It is a question of
> where would you like it discussed (IE: new thread in lmkl,
> linux-serial@...)?

Either of those. I'd also love to drive the sysrq hook out of every
driver in the process. It seems the sysrq key, and debugger activation
are all the same thing.

> It is a discussion more about general polled vs interrupt driven
> serial/tty APIs, not all of which are used for debuggers.  I am open
> to help fix/change/extend/refactor the existing kernel's CONSOLE_POLL
> API or even deprecate it in favor of something else so long as it
> leads to an interface which can allow the sharing of the hardware
> among various contexts with early_printk, console, tty, debugger,
> etc...

What I've done so far is begin to create a "tty_port" object, the idea
being that every tty device will eventually contain one which exists for
the lifetime of the hardware object and have enough methods for printk
console (and now for polled input)

> If it was all obvious and easy, it would have already been done.  Rome
> was not built in a day after all. :-)

Agreed entirely - and in this case we have to convert a twisty maze of
ad-hoc building into Rome without breaking it during the rennovations

I certainly was not expecting a quick follow up patch to fix all these
problems, rather that it gets tackled in an orderly manner for 2.6.29/30
somewhere.

Alan

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2008-10-22 12:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-21 19:14 kgdb 2.6.28 merge plans Jason Wessel
2008-10-21 19:14 ` [PATCH 1/7] kgdb: Add the ability to schedule a breakpoint via a tasklet Jason Wessel
2008-10-21 19:14   ` [PATCH 2/7] tty: Fix sparse static warning for tty_driver_lookup_tty Jason Wessel
2008-10-21 19:14     ` [PATCH 3/7] kgdboc, tty: Add the rx polling call back capability Jason Wessel
2008-10-21 19:14       ` [PATCH 4/7] kgdboc, 8250: Add the rx polling hook to 8250 driver Jason Wessel
2008-10-21 19:14         ` [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers Jason Wessel
2008-10-21 19:14           ` [PATCH 6/7] kgdb: kgdboc console poll hooks for serial_txx9 uart Jason Wessel
2008-10-21 19:14             ` [PATCH 7/7] kgdb: Make mem access function weak in kgdb.c andkgdb.h Jason Wessel
2008-10-21 21:49           ` [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers Jason Wessel
2008-10-21 22:47           ` Alan Cox
2008-10-22  1:54             ` Jason Wessel
2008-10-21 22:49 ` kgdb 2.6.28 merge plans Alan Cox
2008-10-22  2:09   ` Jason Wessel
2008-10-22  8:34     ` Alan Cox
2008-10-22 10:07       ` Jason Wessel
2008-10-22 12:25         ` Alan Cox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).