This patchset contains a number of fixes to make it possible to use ttyNMIX as the primary console, providing you also have that the additional architecture specific code to support it.
The first patch is a generic improvement to make the behaviour of poll_put_char consistent for all UARTs. This is especially important for kgdb_nmi, which makes unusually heavy use of poll_put_char, although this fix also benefits other users of polled I/O.
The remaining patches are specific to kgdb_nmi and have no effect on any other part of the kernel.
The series has been runtime tested on ARM and compile tested on x86 and powerpc.
Daniel Thompson (4): serial: core: Consistent LF handling for poll_put_char serial: kgdb_nmi: Use container_of() to locate private data serial: kgdb_nmi: Switch from tasklets to real timers serial: kgdb_nmi: Improve console integration with KDB I/O
drivers/tty/serial/8250/8250_core.c | 5 --- drivers/tty/serial/cpm_uart/cpm_uart_core.c | 8 ++-- drivers/tty/serial/kgdb_nmi.c | 59 +++++++++++++---------------- drivers/tty/serial/pch_uart.c | 5 --- drivers/tty/serial/pxa.c | 5 --- drivers/tty/serial/serial_core.c | 2 + drivers/tty/serial/serial_txx9.c | 5 --- 7 files changed, 33 insertions(+), 56 deletions(-)
The behaviour of the UART poll_put_char infrastructure is inconsistent with respect to linefeed conversions. This in turn leads to difficulty using kdb on serial ports that are not also consoles (e.g. console=ttyAMA0,115200 kgdboc=ttyAMA1,115200).
The following drivers automatically convert '\n' to '\r\n' inside their own poll functions but the remaining seventeen do not:
serial8250, cpm, pch_uart, serial_pxa, serial_txx9,
This can be made fully consistent but performing the conversion in uart_poll_put_char(). A similar conversion is already made inside uart_console_write() but it is optional for drivers to use this function. Fortunately we can be confident the translation is safe because the (very common) 8250 already does this translation.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Kumar Gala galak@kernel.crashing.org Cc: Pantelis Antoniou panto@intracom.gr Cc: Heikki Krogerus heikki.krogerus@linux.intel.com Cc: Joe Schultz jschultz@xes-inc.com Cc: Loic Poulain loic.poulain@intel.com Cc: Kyle McMartin kyle@infradead.org Cc: Stephen Warren swarren@nvidia.com Cc: Ingo Molnar mingo@elte.hu Cc: Paul Gortmaker paul.gortmaker@windriver.com Cc: Grant Likely grant.likely@linaro.org Cc: Rob Herring rob.herring@calxeda.com Cc: Jingoo Han jg1.han@samsung.com Cc: Christophe Leroy christophe.leroy@c-s.fr --- drivers/tty/serial/8250/8250_core.c | 5 ----- drivers/tty/serial/cpm_uart/cpm_uart_core.c | 8 ++++---- drivers/tty/serial/pch_uart.c | 5 ----- drivers/tty/serial/pxa.c | 5 ----- drivers/tty/serial/serial_core.c | 2 ++ drivers/tty/serial/serial_txx9.c | 5 ----- 6 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 2d4bd39..053c200 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -1926,13 +1926,8 @@ static void serial8250_put_poll_char(struct uart_port *port, wait_for_xmitr(up, BOTH_EMPTY); /* * Send the character out. - * If a LF, also do CR... */ serial_port_out(port, UART_TX, c); - if (c == 10) { - wait_for_xmitr(up, BOTH_EMPTY); - serial_port_out(port, UART_TX, 13); - }
/* * Finally, wait for transmitter to become empty diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c index 7d76214..aa60e6d 100644 --- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c +++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c @@ -971,7 +971,7 @@ static void cpm_uart_config_port(struct uart_port *port, int flags) * Note that this is called with interrupts already disabled */ static void cpm_uart_early_write(struct uart_cpm_port *pinfo, - const char *string, u_int count) + const char *string, u_int count, bool handle_linefeed) { unsigned int i; cbd_t __iomem *bdp, *bdbase; @@ -1013,7 +1013,7 @@ static void cpm_uart_early_write(struct uart_cpm_port *pinfo, bdp++;
/* if a LF, also do CR... */ - if (*string == 10) { + if (handle_linefeed && *string == 10) { while ((in_be16(&bdp->cbd_sc) & BD_SC_READY) != 0) ;
@@ -1111,7 +1111,7 @@ static void cpm_put_poll_char(struct uart_port *port, static char ch[2];
ch[0] = (char)c; - cpm_uart_early_write(pinfo, ch, 1); + cpm_uart_early_write(pinfo, ch, 1, false); } #endif /* CONFIG_CONSOLE_POLL */
@@ -1275,7 +1275,7 @@ static void cpm_uart_console_write(struct console *co, const char *s, spin_lock_irqsave(&pinfo->port.lock, flags); }
- cpm_uart_early_write(pinfo, s, count); + cpm_uart_early_write(pinfo, s, count, true);
if (unlikely(nolock)) { local_irq_restore(flags); diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c index 0931b3f..11e631d 100644 --- a/drivers/tty/serial/pch_uart.c +++ b/drivers/tty/serial/pch_uart.c @@ -1588,13 +1588,8 @@ static void pch_uart_put_poll_char(struct uart_port *port, wait_for_xmitr(priv, UART_LSR_THRE); /* * Send the character out. - * If a LF, also do CR... */ iowrite8(c, priv->membase + PCH_UART_THR); - if (c == 10) { - wait_for_xmitr(priv, UART_LSR_THRE); - iowrite8(13, priv->membase + PCH_UART_THR); - }
/* * Finally, wait for transmitter to become empty diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c index f9f20f3..9e7ee39 100644 --- a/drivers/tty/serial/pxa.c +++ b/drivers/tty/serial/pxa.c @@ -711,13 +711,8 @@ static void serial_pxa_put_poll_char(struct uart_port *port, wait_for_xmitr(up); /* * Send the character out. - * If a LF, also do CR... */ serial_out(up, UART_TX, c); - if (c == 10) { - wait_for_xmitr(up); - serial_out(up, UART_TX, 13); - }
/* * Finally, wait for transmitter to become empty diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index b68550d..50167bf 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2239,6 +2239,8 @@ static void uart_poll_put_char(struct tty_driver *driver, int line, char ch) return;
port = state->uart_port; + if (ch == '\n') + port->ops->poll_put_char(port, '\r'); port->ops->poll_put_char(port, ch); } #endif diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c index 90a080b..60f49b9 100644 --- a/drivers/tty/serial/serial_txx9.c +++ b/drivers/tty/serial/serial_txx9.c @@ -535,13 +535,8 @@ static void serial_txx9_put_poll_char(struct uart_port *port, unsigned char c) 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
On 05/14/2014 09:55 AM, Daniel Thompson wrote:
The behaviour of the UART poll_put_char infrastructure is inconsistent with respect to linefeed conversions. This in turn leads to difficulty using kdb on serial ports that are not also consoles (e.g. console=ttyAMA0,115200 kgdboc=ttyAMA1,115200).
The following drivers automatically convert '\n' to '\r\n' inside their own poll functions but the remaining seventeen do not:
serial8250, cpm, pch_uart, serial_pxa, serial_txx9,This can be made fully consistent but performing the conversion in uart_poll_put_char(). A similar conversion is already made inside uart_console_write() but it is optional for drivers to use this function. Fortunately we can be confident the translation is safe because the (very common) 8250 already does this translation.
I'll have to take a look at some of the other drivers. If all the instances of the function calls are going to coded per driver, it might make more sense to add variable to struct uart_port, vs changing the number of arguments to uart_poll_put_char. And then the default can simply be coded in the struct initialization to the most common need.
Cheers, Jason.
On 14/05/14 16:08, Jason Wessel wrote:
On 05/14/2014 09:55 AM, Daniel Thompson wrote:
The behaviour of the UART poll_put_char infrastructure is inconsistent with respect to linefeed conversions. This in turn leads to difficulty using kdb on serial ports that are not also consoles (e.g. console=ttyAMA0,115200 kgdboc=ttyAMA1,115200).
The following drivers automatically convert '\n' to '\r\n' inside their own poll functions but the remaining seventeen do not:
serial8250, cpm, pch_uart, serial_pxa, serial_txx9,This can be made fully consistent but performing the conversion in uart_poll_put_char(). A similar conversion is already made inside uart_console_write() but it is optional for drivers to use this function. Fortunately we can be confident the translation is safe because the (very common) 8250 already does this translation.
I'll have to take a look at some of the other drivers. If all the instances of the function calls are going to coded per driver, it might make more sense to add variable to struct uart_port, vs changing the number of arguments to uart_poll_put_char. And then the default can simply be coded in the struct initialization to the most common need.
I'm proposing a very simply approach: unconditionally make all serial drivers behave like the 8250.
Detailed reasoning is:
1. Making the polled serial drivers behave consistently is good for transferring mainstream testing to less commonly used UARTs,
2. The 8250 gets best test coverage so it is probably the best behaviour to standardize on,
3. kdb normally sends characters to the user using console I/O rather than polled I/O and almost all serial drivers automatically convert linefeeds in the console I/O,
4. kgdb never generates a linefeed character. If it did kgdb would not currently work on 8250 UARTs (its true that I have assumed, whilst reasoning about potential regressions, that is does).
To be absolutely sure of #4 I did a full code review this morning and confirmed that all code that sends arbitrary data to the gdbserver converts the raw data to hex first. Note also that if, in the future, kgdb does ever implement the "modern" gdbserver commands that utilize raw 8-bit data it would still be possible for kgdb to escape linefeeds to ensure arbitrary binary data can be safely transferred.
Daniel.
On Wed, May 14, 2014 at 03:55:32PM +0100, Daniel Thompson wrote:
The behaviour of the UART poll_put_char infrastructure is inconsistent with respect to linefeed conversions. This in turn leads to difficulty using kdb on serial ports that are not also consoles (e.g. console=ttyAMA0,115200 kgdboc=ttyAMA1,115200).
The following drivers automatically convert '\n' to '\r\n' inside their own poll functions but the remaining seventeen do not:
serial8250, cpm, pch_uart, serial_pxa, serial_txx9,This can be made fully consistent but performing the conversion in uart_poll_put_char(). A similar conversion is already made inside uart_console_write() but it is optional for drivers to use this function. Fortunately we can be confident the translation is safe because the (very common) 8250 already does this translation.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Kumar Gala galak@kernel.crashing.org Cc: Pantelis Antoniou panto@intracom.gr Cc: Heikki Krogerus heikki.krogerus@linux.intel.com Cc: Joe Schultz jschultz@xes-inc.com Cc: Loic Poulain loic.poulain@intel.com Cc: Kyle McMartin kyle@infradead.org Cc: Stephen Warren swarren@nvidia.com Cc: Ingo Molnar mingo@elte.hu Cc: Paul Gortmaker paul.gortmaker@windriver.com Cc: Grant Likely grant.likely@linaro.org Cc: Rob Herring rob.herring@calxeda.com Cc: Jingoo Han jg1.han@samsung.com Cc: Christophe Leroy christophe.leroy@c-s.fr
drivers/tty/serial/8250/8250_core.c | 5 -----
This patch doesn't apply to my tty-next branch anymore, as it seems that part of it is already there, right?
Can you respin this whole series and resend?
thanks,
greg k-h
This patchset contains a number of fixes to make it possible to use ttyNMIX as the primary console (providing you also have that the additional architecture specific code which is proposed in a different patch set).
The first patch fixes a bug in the cpm poll_put_char() driver. This is not strictly related to ttyNMIX but appears in this patch series for historical reasons (it was part of a patch that did impact ttyNMIX but most of that patch is already in the kernel thanks to an independent patch from Doug Anderson).
The remaining patches are specific to kgdb_nmi and have no effect on any other part of the kernel.
The series has been runtime tested on ARM and compile tested on x86 and powerpc.
Changes since v1:
- Trimmed down the first patch to the remaining fragments as described above. - Rebased on tty-next as requested by Greg KH.
Daniel Thompson (4): serial: cpm_uart: No LF conversion in put_poll_char() serial: kgdb_nmi: Use container_of() to locate private data serial: kgdb_nmi: Switch from tasklets to real timers serial: kgdb_nmi: Improve console integration with KDB I/O
drivers/tty/serial/cpm_uart/cpm_uart_core.c | 8 ++-- drivers/tty/serial/kgdb_nmi.c | 59 +++++++++++++---------------- 2 files changed, 31 insertions(+), 36 deletions(-)
In (c7d44a02a serial_core: Commonalize crlf when working w/ a non open console port) the core was modified to make the UART poll_put_char() automatically convert LF to CRLF. This driver's poll_put_char() adds a CR itself and this was not disabled by the above patch meaning currently it sends two CR characters.
The code to issue a character is shared by the console write code (where driver must do LF to CRLF conversion, although it can make use of the uart_console_write() helper function) and the poll_put_char (where driver must not do the conversion). For that reason we add a flag rather than simply rip out the conversion code.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Doug Anderson dianders@chromium.org Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Jiri Slaby jslaby@suse.cz Cc: Christophe Leroy christophe.leroy@c-s.fr Cc: linux-serial@vger.kernel.org --- drivers/tty/serial/cpm_uart/cpm_uart_core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c index 7d76214..aa60e6d 100644 --- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c +++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c @@ -971,7 +971,7 @@ static void cpm_uart_config_port(struct uart_port *port, int flags) * Note that this is called with interrupts already disabled */ static void cpm_uart_early_write(struct uart_cpm_port *pinfo, - const char *string, u_int count) + const char *string, u_int count, bool handle_linefeed) { unsigned int i; cbd_t __iomem *bdp, *bdbase; @@ -1013,7 +1013,7 @@ static void cpm_uart_early_write(struct uart_cpm_port *pinfo, bdp++;
/* if a LF, also do CR... */ - if (*string == 10) { + if (handle_linefeed && *string == 10) { while ((in_be16(&bdp->cbd_sc) & BD_SC_READY) != 0) ;
@@ -1111,7 +1111,7 @@ static void cpm_put_poll_char(struct uart_port *port, static char ch[2];
ch[0] = (char)c; - cpm_uart_early_write(pinfo, ch, 1); + cpm_uart_early_write(pinfo, ch, 1, false); } #endif /* CONFIG_CONSOLE_POLL */
@@ -1275,7 +1275,7 @@ static void cpm_uart_console_write(struct console *co, const char *s, spin_lock_irqsave(&pinfo->port.lock, flags); }
- cpm_uart_early_write(pinfo, s, count); + cpm_uart_early_write(pinfo, s, count, true);
if (unlikely(nolock)) { local_irq_restore(flags);
This corrects a crash in kgdb_nmi_tty_shutdown() which occurs when the function is called with port->tty set to NULL.
All conversions between struct tty_port and struct kgdb_nmi_tty_priv have been switched to direct calls to container_of() to improve code clarity and consistancy.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org --- drivers/tty/serial/kgdb_nmi.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c index 5f673b7..d51b2a1 100644 --- a/drivers/tty/serial/kgdb_nmi.c +++ b/drivers/tty/serial/kgdb_nmi.c @@ -84,11 +84,6 @@ struct kgdb_nmi_tty_priv { STRUCT_KFIFO(char, KGDB_NMI_FIFO_SIZE) fifo; };
-static struct kgdb_nmi_tty_priv *kgdb_nmi_port_to_priv(struct tty_port *port) -{ - return container_of(port, struct kgdb_nmi_tty_priv, port); -} - /* * Our debugging console is polled in a tasklet, so we'll check for input * every tick. In HZ-less mode, we should program the next tick. We have @@ -118,7 +113,7 @@ static void kgdb_tty_recv(int ch) * do that, and actually, we can't: we're in NMI context, no locks are * possible. */ - priv = kgdb_nmi_port_to_priv(kgdb_nmi_port); + priv = container_of(kgdb_nmi_port, struct kgdb_nmi_tty_priv, port); kfifo_in(&priv->fifo, &c, 1); kgdb_tty_poke(); } @@ -216,7 +211,8 @@ static void kgdb_nmi_tty_receiver(unsigned long data)
static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty) { - struct kgdb_nmi_tty_priv *priv = tty->driver_data; + struct kgdb_nmi_tty_priv *priv = + container_of(port, struct kgdb_nmi_tty_priv, port);
kgdb_nmi_port = port; tasklet_schedule(&priv->tlet); @@ -225,7 +221,8 @@ static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty)
static void kgdb_nmi_tty_shutdown(struct tty_port *port) { - struct kgdb_nmi_tty_priv *priv = port->tty->driver_data; + struct kgdb_nmi_tty_priv *priv = + container_of(port, struct kgdb_nmi_tty_priv, port);
tasklet_kill(&priv->tlet); kgdb_nmi_port = NULL;
kgdb_nmi uses tasklets on the assumption they will not be scheduled until the next timer tick. This assumption is invalid and can lead to live lock, continually servicing the kgdb_nmi tasklet. This is fixed by using the timer API instead.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org --- drivers/tty/serial/kgdb_nmi.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c index d51b2a1..20d21d0 100644 --- a/drivers/tty/serial/kgdb_nmi.c +++ b/drivers/tty/serial/kgdb_nmi.c @@ -80,24 +80,10 @@ static struct console kgdb_nmi_console = {
struct kgdb_nmi_tty_priv { struct tty_port port; - struct tasklet_struct tlet; + struct timer_list timer; STRUCT_KFIFO(char, KGDB_NMI_FIFO_SIZE) fifo; };
-/* - * Our debugging console is polled in a tasklet, so we'll check for input - * every tick. In HZ-less mode, we should program the next tick. We have - * to use the lowlevel stuff as no locks should be grabbed. - */ -#ifdef CONFIG_HIGH_RES_TIMERS -static void kgdb_tty_poke(void) -{ - tick_program_event(ktime_get(), 0); -} -#else -static inline void kgdb_tty_poke(void) {} -#endif - static struct tty_port *kgdb_nmi_port;
static void kgdb_tty_recv(int ch) @@ -108,14 +94,13 @@ static void kgdb_tty_recv(int ch) if (!kgdb_nmi_port || ch < 0) return; /* - * Can't use port->tty->driver_data as tty might be not there. Tasklet + * Can't use port->tty->driver_data as tty might be not there. Timer * will check for tty and will get the ref, but here we don't have to * do that, and actually, we can't: we're in NMI context, no locks are * possible. */ priv = container_of(kgdb_nmi_port, struct kgdb_nmi_tty_priv, port); kfifo_in(&priv->fifo, &c, 1); - kgdb_tty_poke(); }
static int kgdb_nmi_poll_one_knock(void) @@ -199,7 +184,8 @@ static void kgdb_nmi_tty_receiver(unsigned long data) struct kgdb_nmi_tty_priv *priv = (void *)data; char ch;
- tasklet_schedule(&priv->tlet); + priv->timer.expires = jiffies + (HZ/100); + add_timer(&priv->timer);
if (likely(!kgdb_nmi_tty_enabled || !kfifo_len(&priv->fifo))) return; @@ -215,7 +201,9 @@ static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty) container_of(port, struct kgdb_nmi_tty_priv, port);
kgdb_nmi_port = port; - tasklet_schedule(&priv->tlet); + priv->timer.expires = jiffies + (HZ/100); + add_timer(&priv->timer); + return 0; }
@@ -224,7 +212,7 @@ static void kgdb_nmi_tty_shutdown(struct tty_port *port) struct kgdb_nmi_tty_priv *priv = container_of(port, struct kgdb_nmi_tty_priv, port);
- tasklet_kill(&priv->tlet); + del_timer(&priv->timer); kgdb_nmi_port = NULL; }
@@ -243,7 +231,7 @@ static int kgdb_nmi_tty_install(struct tty_driver *drv, struct tty_struct *tty) return -ENOMEM;
INIT_KFIFO(priv->fifo); - tasklet_init(&priv->tlet, kgdb_nmi_tty_receiver, (unsigned long)priv); + setup_timer(&priv->timer, kgdb_nmi_tty_receiver, (unsigned long)priv); tty_port_init(&priv->port); priv->port.ops = &kgdb_nmi_tty_port_ops; tty->driver_data = priv;
kgdb_nmi_tty_enabled is used for two unrelated purposes, namely to suppress normal TTY input handling and to suppress console output (although it has no effect at all on TTY output). A much better way to handle muting the console is to not have to mute it in the first place! That's what this patch does.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org --- drivers/tty/serial/kgdb_nmi.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c index 20d21d0..cfadf29 100644 --- a/drivers/tty/serial/kgdb_nmi.c +++ b/drivers/tty/serial/kgdb_nmi.c @@ -44,13 +44,22 @@ MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)");
static bool kgdb_nmi_tty_enabled;
+static int kgdb_nmi_console_setup(struct console *co, char *options) +{ + /* The NMI console uses the dbg_io_ops to issue console messages. To + * avoid duplicate messages during kdb sessions we must inform kdb's + * I/O utilities that messages sent to the console will automatically + * be displayed on the dbg_io. + */ + dbg_io_ops->is_console = true; + + return 0; +} + static void kgdb_nmi_console_write(struct console *co, const char *s, uint c) { int i;
- if (!kgdb_nmi_tty_enabled || atomic_read(&kgdb_active) >= 0) - return; - for (i = 0; i < c; i++) dbg_io_ops->write_char(s[i]); } @@ -65,6 +74,7 @@ static struct tty_driver *kgdb_nmi_console_device(struct console *co, int *idx)
static struct console kgdb_nmi_console = { .name = "ttyNMI", + .setup = kgdb_nmi_console_setup, .write = kgdb_nmi_console_write, .device = kgdb_nmi_console_device, .flags = CON_PRINTBUFFER | CON_ANYTIME | CON_ENABLED,
This corrects a crash in kgdb_nmi_tty_shutdown() which occurs when the function is called with port->tty set to NULL.
All conversions between struct tty_port and struct kgdb_nmi_tty_priv have been switched to direct calls to container_of() to improve code clarity and consistancy.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org --- drivers/tty/serial/kgdb_nmi.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c index 5f673b7..d51b2a1 100644 --- a/drivers/tty/serial/kgdb_nmi.c +++ b/drivers/tty/serial/kgdb_nmi.c @@ -84,11 +84,6 @@ struct kgdb_nmi_tty_priv { STRUCT_KFIFO(char, KGDB_NMI_FIFO_SIZE) fifo; };
-static struct kgdb_nmi_tty_priv *kgdb_nmi_port_to_priv(struct tty_port *port) -{ - return container_of(port, struct kgdb_nmi_tty_priv, port); -} - /* * Our debugging console is polled in a tasklet, so we'll check for input * every tick. In HZ-less mode, we should program the next tick. We have @@ -118,7 +113,7 @@ static void kgdb_tty_recv(int ch) * do that, and actually, we can't: we're in NMI context, no locks are * possible. */ - priv = kgdb_nmi_port_to_priv(kgdb_nmi_port); + priv = container_of(kgdb_nmi_port, struct kgdb_nmi_tty_priv, port); kfifo_in(&priv->fifo, &c, 1); kgdb_tty_poke(); } @@ -216,7 +211,8 @@ static void kgdb_nmi_tty_receiver(unsigned long data)
static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty) { - struct kgdb_nmi_tty_priv *priv = tty->driver_data; + struct kgdb_nmi_tty_priv *priv = + container_of(port, struct kgdb_nmi_tty_priv, port);
kgdb_nmi_port = port; tasklet_schedule(&priv->tlet); @@ -225,7 +221,8 @@ static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty)
static void kgdb_nmi_tty_shutdown(struct tty_port *port) { - struct kgdb_nmi_tty_priv *priv = port->tty->driver_data; + struct kgdb_nmi_tty_priv *priv = + container_of(port, struct kgdb_nmi_tty_priv, port);
tasklet_kill(&priv->tlet); kgdb_nmi_port = NULL;
kgdb_nmi uses tasklets on the assumption they will not be scheduled until the next timer tick. This assumption is invalid and can lead to live lock, continually servicing the kgdb_nmi tasklet. This is fixed by using the timer API instead.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org --- drivers/tty/serial/kgdb_nmi.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c index d51b2a1..20d21d0 100644 --- a/drivers/tty/serial/kgdb_nmi.c +++ b/drivers/tty/serial/kgdb_nmi.c @@ -80,24 +80,10 @@ static struct console kgdb_nmi_console = {
struct kgdb_nmi_tty_priv { struct tty_port port; - struct tasklet_struct tlet; + struct timer_list timer; STRUCT_KFIFO(char, KGDB_NMI_FIFO_SIZE) fifo; };
-/* - * Our debugging console is polled in a tasklet, so we'll check for input - * every tick. In HZ-less mode, we should program the next tick. We have - * to use the lowlevel stuff as no locks should be grabbed. - */ -#ifdef CONFIG_HIGH_RES_TIMERS -static void kgdb_tty_poke(void) -{ - tick_program_event(ktime_get(), 0); -} -#else -static inline void kgdb_tty_poke(void) {} -#endif - static struct tty_port *kgdb_nmi_port;
static void kgdb_tty_recv(int ch) @@ -108,14 +94,13 @@ static void kgdb_tty_recv(int ch) if (!kgdb_nmi_port || ch < 0) return; /* - * Can't use port->tty->driver_data as tty might be not there. Tasklet + * Can't use port->tty->driver_data as tty might be not there. Timer * will check for tty and will get the ref, but here we don't have to * do that, and actually, we can't: we're in NMI context, no locks are * possible. */ priv = container_of(kgdb_nmi_port, struct kgdb_nmi_tty_priv, port); kfifo_in(&priv->fifo, &c, 1); - kgdb_tty_poke(); }
static int kgdb_nmi_poll_one_knock(void) @@ -199,7 +184,8 @@ static void kgdb_nmi_tty_receiver(unsigned long data) struct kgdb_nmi_tty_priv *priv = (void *)data; char ch;
- tasklet_schedule(&priv->tlet); + priv->timer.expires = jiffies + (HZ/100); + add_timer(&priv->timer);
if (likely(!kgdb_nmi_tty_enabled || !kfifo_len(&priv->fifo))) return; @@ -215,7 +201,9 @@ static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty) container_of(port, struct kgdb_nmi_tty_priv, port);
kgdb_nmi_port = port; - tasklet_schedule(&priv->tlet); + priv->timer.expires = jiffies + (HZ/100); + add_timer(&priv->timer); + return 0; }
@@ -224,7 +212,7 @@ static void kgdb_nmi_tty_shutdown(struct tty_port *port) struct kgdb_nmi_tty_priv *priv = container_of(port, struct kgdb_nmi_tty_priv, port);
- tasklet_kill(&priv->tlet); + del_timer(&priv->timer); kgdb_nmi_port = NULL; }
@@ -243,7 +231,7 @@ static int kgdb_nmi_tty_install(struct tty_driver *drv, struct tty_struct *tty) return -ENOMEM;
INIT_KFIFO(priv->fifo); - tasklet_init(&priv->tlet, kgdb_nmi_tty_receiver, (unsigned long)priv); + setup_timer(&priv->timer, kgdb_nmi_tty_receiver, (unsigned long)priv); tty_port_init(&priv->port); priv->port.ops = &kgdb_nmi_tty_port_ops; tty->driver_data = priv;
kgdb_nmi_tty_enabled is used for two unrelated purposes, namely to suppress normal TTY input handling and to suppress console output (although it has no effect at all on TTY output). A much better way to handle muting the console is to not have to mute it in the first place! That's what this patch does.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org --- drivers/tty/serial/kgdb_nmi.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c index 20d21d0..cfadf29 100644 --- a/drivers/tty/serial/kgdb_nmi.c +++ b/drivers/tty/serial/kgdb_nmi.c @@ -44,13 +44,22 @@ MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)");
static bool kgdb_nmi_tty_enabled;
+static int kgdb_nmi_console_setup(struct console *co, char *options) +{ + /* The NMI console uses the dbg_io_ops to issue console messages. To + * avoid duplicate messages during kdb sessions we must inform kdb's + * I/O utilities that messages sent to the console will automatically + * be displayed on the dbg_io. + */ + dbg_io_ops->is_console = true; + + return 0; +} + static void kgdb_nmi_console_write(struct console *co, const char *s, uint c) { int i;
- if (!kgdb_nmi_tty_enabled || atomic_read(&kgdb_active) >= 0) - return; - for (i = 0; i < c; i++) dbg_io_ops->write_char(s[i]); } @@ -65,6 +74,7 @@ static struct tty_driver *kgdb_nmi_console_device(struct console *co, int *idx)
static struct console kgdb_nmi_console = { .name = "ttyNMI", + .setup = kgdb_nmi_console_setup, .write = kgdb_nmi_console_write, .device = kgdb_nmi_console_device, .flags = CON_PRINTBUFFER | CON_ANYTIME | CON_ENABLED,
linaro-kernel@lists.linaro.org
-
Daniel Thompson -
Greg Kroah-Hartman -
Jason Wessel