Re: [PATCH 4/5] KGDB-8250: refactor configuration
From: Jan Kiszka
Date: Tue Jan 29 2008 - 19:32:19 EST
Sorry, previous version was missing some __init[data] attributes which
were dropped in an intermediate stage. Here comes an updated patch:
<---snip--->
This major refactoring of the quite complex kgdb8250 configuration does
the following:
- ensures that static configurations according to SERIAL_PORT_DFNS are
always loaded first
- tries to pull more accurate configuration via serial8250_get_port_def
if simple-config is used
- detects empty/invalid simple-configs
- enforces KGDB_PORT_NUM <= SERIAL_8250_NR_UARTS at kconfig level
- removes kgdb8250_add_port and its hook in serial_core (calling
serial8250_get_port_def in demand should provide us the same
information)
Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxx>
---
drivers/serial/8250.c | 47 ++++++++++++----------------
drivers/serial/8250_kgdb.c | 70 +++++++++++++------------------------------
drivers/serial/serial_core.c | 7 ----
include/linux/kgdb.h | 1
lib/Kconfig.kgdb | 16 ++++-----
5 files changed, 51 insertions(+), 90 deletions(-)
Index: b/drivers/serial/8250_kgdb.c
===================================================================
--- a/drivers/serial/8250_kgdb.c
+++ b/drivers/serial/8250_kgdb.c
@@ -53,16 +53,6 @@ static char kgdb8250_buf[GDB_BUF_SIZE];
static atomic_t kgdb8250_buf_in_cnt;
static int kgdb8250_buf_out_inx;
-/* Old-style serial definitions, if existant, and a counter. */
-#ifdef CONFIG_KGDB_SIMPLE_SERIAL
-static int should_copy_rs_table = 1;
-static struct serial_state old_rs_table[] __initdata = {
-#ifdef SERIAL_PORT_DFNS
- SERIAL_PORT_DFNS
-#endif
-};
-#endif
-
/* Our internal table of UARTS. */
#define UART_NR CONFIG_SERIAL_8250_NR_UARTS
static struct uart_port kgdb8250_ports[UART_NR];
@@ -258,9 +248,15 @@ static int kgdb8250_uart_init(void)
static void __init kgdb8250_copy_rs_table(void)
{
#ifdef CONFIG_KGDB_SIMPLE_SERIAL
+ static struct serial_state old_rs_table[] __initdata = {
+#ifdef SERIAL_PORT_DFNS
+ SERIAL_PORT_DFNS
+#endif
+ };
+ static int rs_table_copied __initdata;
int i;
- if (!should_copy_rs_table)
+ if (rs_table_copied)
return;
for (i = 0; i < ARRAY_SIZE(old_rs_table); i++) {
@@ -273,8 +269,8 @@ static void __init kgdb8250_copy_rs_tabl
kgdb8250_ports[i].line = i;
}
- should_copy_rs_table = 0;
-#endif
+ rs_table_copied = 1;
+#endif /* CONFIG_KGDB_SIMPLE_SERIAL */
}
/*
@@ -284,9 +280,6 @@ static void __init kgdb8250_copy_rs_tabl
*/
static void __init kgdb8250_late_init(void)
{
- /* Try and copy the old_rs_table. */
- kgdb8250_copy_rs_table();
-
#if defined(CONFIG_SERIAL_8250) || defined(CONFIG_SERIAL_8250_MODULE)
/* Take the port away from the main driver. */
serial8250_unregister_by_port(current_port);
@@ -306,9 +299,6 @@ static void __init kgdb8250_late_init(vo
static __init int kgdb_init_io(void)
{
- /* Give us the basic table of uarts. */
- kgdb8250_copy_rs_table();
-
/* We're either a module and parse a config string, or we have a
* semi-static config. */
#ifdef CONFIG_KGDB_8250_MODULE
@@ -325,8 +315,15 @@ static __init int kgdb_init_io(void)
#else /* !CONFIG_KGDB_8250_MODULE */
if (!params_evaluated) {
#ifdef CONFIG_KGDB_SIMPLE_SERIAL
+ kgdb8250_copy_rs_table();
+
kgdb8250_baud = CONFIG_KGDB_BAUDRATE;
current_port = &kgdb8250_ports[CONFIG_KGDB_PORT_NUM];
+
+ if (serial8250_get_port_def(current_port,
+ CONFIG_KGDB_PORT_NUM) < 0 &&
+ !current_port->iobase && !current_port->membase)
+ return -EINVAL;
#else /* !CONFIG_KGDB_SIMPLE_SERIAL */
if (kgdb8250_opt(CONFIG_KGDB_8250_CONF_STRING))
return -EINVAL;
@@ -395,29 +392,6 @@ struct kgdb_io kgdb_io_ops = {
};
/**
- * kgdb8250_add_port - Define a serial port for use with KGDB
- * @i: The index of the port being added
- * @serial_req: The &struct uart_port describing the port
- *
- * On platforms where we must register the serial device
- * dynamically, this is the best option if a platform also normally
- * calls early_serial_setup().
- */
-void kgdb8250_add_port(int i, struct uart_port *serial_req)
-{
-#ifdef CONFIG_KGDB_SIMPLE_SERIAL
- if (should_copy_rs_table)
- printk(KERN_ERR "8250_kgdb: warning will over write serial"
- " port definitions at kgdb init time\n");
-#endif
-
- /* Copy the whole thing over. */
- if (current_port != &kgdb8250_ports[i])
- memcpy(&kgdb8250_ports[i], serial_req,
- sizeof(struct uart_port));
-}
-
-/**
* kgdb8250_add_platform_port - Define a serial port for use with KGDB
* @i: The index of the port being added
* @p: The &struct plat_serial8250_port describing the port
@@ -447,7 +421,10 @@ void __init kgdb8250_add_platform_port(i
*/
static int __init kgdb8250_opt(char *str)
{
- /* We'll fill out and use the first slot. */
+ /* Give us the basic table of uarts. */
+ kgdb8250_copy_rs_table();
+
+ /* If we overwrite, we'll fill out and use the first slot. */
current_port = &kgdb8250_ports[0];
if (!strncmp(str, "io", 2)) {
@@ -467,7 +444,8 @@ static int __init kgdb8250_opt(char *str
if (line >= UART_NR)
goto errout;
current_port = &kgdb8250_ports[line];
- if (serial8250_get_port_def(current_port, line))
+ if (serial8250_get_port_def(current_port, line) < 0 &&
+ !current_port->iobase && !current_port->membase)
goto errout;
str++;
if (*str != ',')
@@ -512,16 +490,12 @@ static int __init kgdb8250_opt(char *str
current_port->irq = simple_strtoul(str, &str, 10);
finish:
-#ifdef CONFIG_KGDB_SIMPLE_SERIAL
- should_copy_rs_table = 0;
-#endif
#ifdef CONFIG_KGDB_8250
params_evaluated = 1;
#endif
return 0;
errout:
- printk(KERN_ERR "Invalid syntax for option kgdb8250=\n");
return 1;
}
Index: b/drivers/serial/8250.c
===================================================================
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -2860,37 +2860,32 @@ void serial8250_unregister_port(int line
EXPORT_SYMBOL(serial8250_unregister_port);
/**
- * serial8250_get_port_def - Get port definition for a specific line
- * @port: generic uart_port output for a specific serial line
- * @line: specific serial line index
+ * serial8250_get_port_def - Get port definition for a specific line
+ * @port: generic uart_port output for a specific serial line
+ * @line: specific serial line index
*
- * Return 0 if the port existed
- * Return 1 on failure
+ * Returns 0 on success, -ENODEV if the port does not exist.
*/
-
int serial8250_get_port_def(struct uart_port *port, int line)
{
- struct uart_8250_port *uart;
+ struct uart_port *port8250 = &serial8250_ports[line].port;
- if (line < 0 || line >= UART_NR)
- return 1;
- uart = &serial8250_ports[line];
- if (uart) {
- port->iobase = uart->port.iobase;
- port->membase = uart->port.membase;
- port->irq = uart->port.irq;
- port->uartclk = uart->port.uartclk;
- port->fifosize = uart->port.fifosize;
- port->regshift = uart->port.regshift;
- port->iotype = uart->port.iotype;
- port->flags = uart->port.flags;
- port->mapbase = uart->port.mapbase;
- port->dev = uart->port.dev;
- return 0;
- }
- return 1;
+ if (!port8250->iobase && !port8250->membase)
+ return -ENODEV;
+
+ port->iobase = port8250->iobase;
+ port->membase = port8250->membase;
+ port->irq = port8250->irq;
+ port->uartclk = port8250->uartclk;
+ port->fifosize = port8250->fifosize;
+ port->regshift = port8250->regshift;
+ port->iotype = port8250->iotype;
+ port->flags = port8250->flags;
+ port->mapbase = port8250->mapbase;
+ port->dev = port8250->dev;
+ return 0;
}
-EXPORT_SYMBOL(serial8250_get_port_def);
+EXPORT_SYMBOL_GPL(serial8250_get_port_def);
/**
* serial8250_unregister_by_port - remove a 16x50 serial port
@@ -2909,7 +2904,7 @@ void serial8250_unregister_by_port(struc
if (uart)
serial8250_unregister_port(uart->port.line);
}
-EXPORT_SYMBOL(serial8250_unregister_by_port);
+EXPORT_SYMBOL_GPL(serial8250_unregister_by_port);
static int __init serial8250_init(void)
{
Index: b/drivers/serial/serial_core.c
===================================================================
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -33,7 +33,6 @@
#include <linux/serial.h> /* for serial_state and serial_icounter_struct */
#include <linux/delay.h>
#include <linux/mutex.h>
-#include <linux/kgdb.h>
#include <asm/irq.h>
#include <asm/uaccess.h>
@@ -2370,12 +2369,6 @@ int uart_add_one_port(struct uart_driver
*/
port->flags &= ~UPF_DEAD;
-#if defined(CONFIG_KGDB_8250)
- /* Add any 8250-like ports we find later. */
- if (port->type <= PORT_MAX_8250)
- kgdb8250_add_port(port->line, port);
-#endif
-
out:
mutex_unlock(&state->mutex);
mutex_unlock(&port_mutex);
Index: b/include/linux/kgdb.h
===================================================================
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -271,7 +271,6 @@ extern struct kgdb_arch arch_kgdb_ops;
int kgdb_register_io_module(struct kgdb_io *local_kgdb_io_ops);
void kgdb_unregister_io_module(struct kgdb_io *local_kgdb_io_ops);
-void kgdb8250_add_port(int i, struct uart_port *serial_req);
void __init kgdb8250_add_platform_port(int i,
struct plat_serial8250_port *serial_req);
Index: b/lib/Kconfig.kgdb
===================================================================
--- a/lib/Kconfig.kgdb
+++ b/lib/Kconfig.kgdb
@@ -36,6 +36,14 @@ config KGDB_SIMPLE_SERIAL
or on the command line, the type (I/O or MMIO), IRQ and
address to use. If in doubt, say Y.
+config KGDB_PORT_NUM
+ int "Serial port number for KGDB"
+ range 0 SERIAL_8250_NR_UARTS
+ depends on KGDB_SIMPLE_SERIAL
+ default "1"
+ help
+ Pick the port number (0 based) for KGDB to use.
+
config KGDB_BAUDRATE
int "Debug serial port baud rate"
depends on KGDB_SIMPLE_SERIAL
@@ -45,14 +53,6 @@ config KGDB_BAUDRATE
used. Standard rates from 9600 to 115200 are allowed, and this
may be overridden via the commandline.
-config KGDB_PORT_NUM
- int "Serial port number for KGDB"
- range 0 3
- depends on KGDB_SIMPLE_SERIAL
- default "1"
- help
- Pick the port number (0 based) for KGDB to use.
-
config KGDB_8250_CONF_STRING
string "Configuration string for KGDB"
depends on (KGDB_8250 = y) && !KGDB_SIMPLE_SERIAL
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/