Skip to contentSkip to conversation

KGDB-8250: refactor configuration

PATCH In review Jan Kiszka 2 participants 6 messages Jan 29, 2008 – Feb 6, 2008

Subsystems (2)
Library code KGDB / KDB /debug_core
lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 4/5 KGDB-8250: refactor configuration

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 <[email protected]>
---
 drivers/serial/8250.c        |   47 ++++++++++++----------------
 drivers/serial/8250_kgdb.c   |   72 +++++++++++++------------------------------
 drivers/serial/serial_core.c |    7 ----
 include/linux/kgdb.h         |    1 
 lib/Kconfig.kgdb             |   16 ++++-----
 5 files changed, 52 insertions(+), 91 deletions(-)
drivers/serial/8250_kgdb.c +23 -49
Index: 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];

@@ -255,12 +245,18 @@ static int kgdb8250_uart_init(void)
  * had values specifically configured in.  We need to make sure this only
  * happens once.
  */
-static void __init kgdb8250_copy_rs_table(void)
+static void kgdb8250_copy_rs_table(void)
 {
 #ifdef CONFIG_KGDB_SIMPLE_SERIAL
+	static struct serial_state old_rs_table[] = {
+#ifdef SERIAL_PORT_DFNS
+		SERIAL_PORT_DFNS
+#endif
+	};
+	static int rs_table_copied;
 	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;
 }
drivers/serial/8250.c +21 -26

65 lines not on this page · raw message

drivers/serial/serial_core.c +0 -7
Index: 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);
include/linux/kgdb.h +0 -1
Index: 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);
lib/Kconfig.kgdb +8 -8
Index: 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
author
#1 lore raw

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 <[email protected]>
---
 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(-)
drivers/serial/8250_kgdb.c +22 -48
Index: 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;
 }
drivers/serial/8250.c +21 -26

65 lines not on this page · raw message

drivers/serial/serial_core.c +0 -7
Index: 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);
include/linux/kgdb.h +0 -1
Index: 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);
lib/Kconfig.kgdb +8 -8
Index: 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
#2 lore raw

Hello.

Quoted 8 lines · Jan Kiszka

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:

Quoted 9 lines

- 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)

You left powerpc-lite.patch broken with this change as it has multiple
calls to kgdb8250_add_port()...

Quoted 6 lines
Signed-off-by Jan Kiszka <[email protected]>

Index: b/drivers/serial/serial_core.c
===================================================================

drivers/serial/serial_core.c +0 -0

[...]

drivers/serial/serial_core.c int uart_add_one_port(struct uart_driver · line 2378

Quoted 10 lines
drivers/serial/serial_core.c +0 -6
@@ -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
-

I'm afraid this wasn't correct from the very start since this can add
ports with .iotype that 8250_kgdb.c does not support. So, nothing to regret here...

WBR, Sergei

author
#3 lore raw
Quoted 22 lines · Sergei Shtylyov

Hello.

Quoted 8 lines · Jan Kiszka

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:

Quoted 9 lines

- 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)

Quoted 2 lines

You left powerpc-lite.patch broken with this change as it has
multiple calls to kgdb8250_add_port()...

I see. But I wonder if there ever was a real need for these hooks (in 2.4 times?): If I look at bamboo_early_serial_map() e.g., I find it calling into early_serial_setup() which fills serial8250_ports[] - and that content is now retrieved via serial8250_get_port_def() when we parse the runtime or build-time provided parameters (port number & baudrate).

Quoted 12 lines
Quoted 7 lines
Signed-off-by Jan Kiszka <[email protected]>

Index: b/drivers/serial/serial_core.c
===================================================================

drivers/serial/serial_core.c +0 -0

[...]

Quoted 4 lines
drivers/serial/serial_core.c +0 -0
@@ -2370,12 +2369,6 @@ int uart_add_one_port(struct uart_driver
      */
     port->flags &= ~UPF_DEAD;
Quoted 10 lines

drivers/serial/serial_core.c int uart_add_one_port(struct uart_driver · line 2378

Quoted 6 lines
drivers/serial/serial_core.c +0 -6
-#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
-

I'm afraid this wasn't correct from the very start since this can add
ports with .iotype that 8250_kgdb.c does not support. So, nothing to regret here...

I think a lot of cruft piled up in the kgdb patches over their long life. :)

Thanks for your feedback!

Jan

#4 lore raw

Hello.

Quoted 21 lines · Jan Kiszka
Quoted 18 lines
Quoted 8 lines

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:

Quoted 9 lines

- 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)

Quoted 2 lines

You left powerpc-lite.patch broken with this change as it has
multiple calls to kgdb8250_add_port()...

Quoted 6 lines

I see. But I wonder if there ever was a real need for these hooks (in 2.4 times?): If I look at bamboo_early_serial_map() e.g., I find it calling into early_serial_setup() which fills serial8250_ports[] - and that content is now retrieved via serial8250_get_port_def() when we parse the runtime or build-time provided parameters (port number & baudrate).

Of course. But now the kgdb8250_add_port() calls need to be removed.

Quoted 1 line
Quoted 1 line
Quoted 1 line
Signed-off-by Jan Kiszka <[email protected]>

WBR, Sergei

author
#5 lore raw
Quoted 11 lines · Sergei Shtylyov
Quoted 9 lines
Quoted 2 lines

You left powerpc-lite.patch broken with this change as it has
multiple calls to kgdb8250_add_port()...

I see. But I wonder if there ever was a real need for these hooks (in 2.4 times?): If I look at bamboo_early_serial_map() e.g., I find it calling into early_serial_setup() which fills serial8250_ports[] - and that content is now retrieved via serial8250_get_port_def() when we parse the runtime or build-time provided parameters (port number & baudrate).

Of course. But now the kgdb8250_add_port() calls need to be removed.

For sure. Most arch patches need to go through some refactoring anyway when preparing them for upstream. Cleaning up no longer required hooks should be no problem at this chance.

If you want to accelerate this process, please check out Jason's linux-2.6-kgdb.git for 2.6.25 and start rebasing the powerpc patch. He just recently said that support around kgdb for non-x86 would be highly welcome. And if you stumble over ppc-related issues that cannot be solved with latest kgdb design, please let us know. The sooner, the better.

Jan