Skip to contentSkip to conversation

tty: serial: kgdb: fix msm_serial for kgdb

request for help New Frank Rowand 2 participants 11 messages Aug 5, 2014 – Aug 6, 2014

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

Stephen,

I made some changes to drivers/tty/serial/msm_serial.c to allow kgdb to work with the dragon board (which has a qcom,msm-uartdm-v1.4 serial port).

I will reply to this email with the patches.

With these fixes, kgdb properly communicates with the dragon board, but following the continue command, the serial driver does not get any stale (UART_IMR_RXSTALE) interrupts until 48 characters have been read, which triggers a high water interrupt. After the high water interrupt has been processed, the driver resumes properly getting stale interrupts.

I am hoping you can help me figure out how to get the stale interrupt to occur for the first character after gdb continues, instead of having to first get a high water interrupt.

Patch 2 adds a framework to fix up the state of the driver and/or hardware after gdb continues. Patch 3 is a series of experiments trying to fix things up for msm_serial. I don't have complete documentation and understanding of the hardware, so some of the experiments might be less than intelligent.

Can you give me some suggestions of What should I be doing in the fix up function?

Thanks,

Frank

This is the console output from a dragonboard. Comments are between dashed lines:

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Connect to gdb (kgdb):
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

/ # echo g >/proc/sysrq-trigger
SysRq : DEBUG
Entering KGDB

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - gdb connects successfully.
The gdb 'c' command continues from the connect.
The command "echo abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQR" does not appear on the console until the final character ('R') is typed. Then the entire command is written out to the console.
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

/ # echo abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQR abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQR

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Now the console is behaving normally, and each character is echoed immediately when it is typed.
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

/ # echo 123456789
123456789

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - The 'sync' command prints out the trace of calls to handle_rx_dm(), which handles the msm_serial stale and high water interrupts. Events 0 - 27 are stale interrupts, as expected.

The connect to gdb occurs as a result of the sysrq triggered after event 27. The next msm_serial interrupt after gdb tells the kernel to continue is event 28, which is a high water interrupt, processing 48 characters.

Following event 28, the driver resumes getting stale interrupts for each character, as expected.
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

/ # sync
B S E
r t x old_
e a h snap_state
a l s ---------- tot
k t t 1 2 snap count c_idx --- data
---- - - - ---- ---- ---- ----- ----- --- --------

   0   S       0    0    1     1     1 --- 00000065 ...e 
   1   S       0    0    1     1     1 --- 00000063 ...c 
   2   S       0    0    1     1     1 --- 00000068 ...h 
   3   S       0    0    1     1     1 --- 0000006f ...o 
   4   S       0    0    1     1     1 --- 00000020 ...  
   5   S       0    0    1     1     1 --- 00000067 ...g 
   6   S       0    0    1     1     1 --- 00000020 ...  
   7   S       0    0    1     1     1 --- 0000003e ...> 
   8   S       0    0    1     1     1 --- 0000002f .../ 
   9   S       0    0    1     1     1 --- 00000070 ...p 
  10   S       0    0    1     1     1 --- 00000072 ...r 
  11   S       0    0    1     1     1 --- 0000006f ...o 
  12   S       0    0    1     1     1 --- 00000063 ...c 
  13   S       0    0    1     1     1 --- 0000002f .../ 
  14   S       0    0    1     1     1 --- 00000073 ...s 
  15   S       0    0    1     1     1 --- 00000079 ...y 
  16   S       0    0    1     1     1 --- 00000073 ...s 
  17   S       0    0    1     1     1 --- 00000072 ...r 
  18   S       0    0    1     1     1 --- 00000071 ...q 
  19   S       0    0    1     1     1 --- 0000002d ...- 
  20   S       0    0    1     1     1 --- 00000074 ...t 
  21   S       0    0    1     1     1 --- 00000072 ...r 
  22   S       0    0    1     1     1 --- 00000069 ...i 
  23   S       0    0    1     1     1 --- 00000067 ...g 
  24   S       0    0    1     1     1 --- 00000067 ...g 
  25   S       0    0    1     1     1 --- 00000065 ...e 
  26   S       0    0    1     1     1 --- 00000072 ...r 
  27   S       0    0    1     1     1 --- 0000000d .... 
  28   H       0   48    0    48    48 --- 00000065 00000063 00000068 0000006f 00000020 00000061 000

00062 00000063 00000064 00000065 00000066 00000067 00000068 00000069 0000006a 0000006b 0000006c 0000006d 0000006e 0 000006f 00000070 00000071 00000072 00000073 00000074 00000075 00000076 00000077 00000078 00000079 0000007a 00000041
00000042 00000043 00000044 00000045 00000046 00000047 00000048 00000049 0000004a 0000004b 0000004c 0000004d 000000
4e 0000004f 00000050 00000051 ...e ...c ...h ...o ... ...a ...b ...c ...d ...e ...f ...g ...h ...i ...j ...k ...l ...m ...n ...o ...p ...q ...r ...s ...t ...u ...v ...w ...x ...y ...z ...A ...B ...C ...D ...E ...F ...G ...H ...I ...J ...K ...L ...M ...N ...O ...P ...Q

  29   S      48    0   49     1     1 --- 00000052 ...R 
  30   S       0    0    1     1     1 --- 0000000d .... 
  31   S       0    0    1     1     1 --- 00000065 ...e 
  32   S       0    0    1     1     1 --- 00000063 ...c 
  33   S       0    0    1     1     1 --- 00000068 ...h 
  34   S       0    0    1     1     1 --- 0000006f ...o 
  35   S       0    0    1     1     1 --- 00000020 ...  
  36   S       0    0    1     1     1 --- 00000031 ...1 
  37   S       0    0    1     1     1 --- 00000032 ...2 
  38   S       0    0    1     1     1 --- 00000033 ...3 
  39   S       0    0    1     1     1 --- 00000034 ...4 
  40   S       0    0    1     1     1 --- 00000035 ...5 
  41   S       0    0    1     1     1 --- 00000036 ...6 
  42   S       0    0    1     1     1 --- 00000037 ...7 
  43   S       0    0    1     1     1 --- 00000038 ...8 
  44   S       0    0    1     1     1 --- 00000039 ...9 
  45   S       0    0    1     1     1 --- 0000000d .... 
  46   S       0    0    1     1     1 --- 00000073 ...s 
  47   S       0    0    1     1     1 --- 00000079 ...y 
  48   S       0    0    1     1     1 --- 0000006e ...n 
  49   S       0    0    1     1     1 --- 00000063 ...c 
  50   S       0    0    1     1     1 --- 0000000d .... 

/ #

#1 lore raw

PATCH 1/4 fix msm_serial for kgdb

From: Frank Rowand <[email protected]>

f7e54d7ad743 added support for poll_{get,put}_char() Additional fixes to cope with single character mode on RX FIFO for qcom,msm-uartdm-v1.4.

With these fixes, kgdb properly communicates with the dragon board, but following the continue command, the serial driver does not get any stale (UART_IMR_RXSTALE) interrupts until 48 characters have been read, which triggers a high water interrupt. After the high water interrupt has been processed, the driver resumes properly getting stale interrupts.

Not-signed-off-by-yet: Frank Rowand <[email protected]>

---
 drivers/tty/serial/msm_serial.c |   74 ++++++++++++++++++++++++++++++++--------
 1 file changed, 61 insertions(+), 13 deletions(-)
drivers/tty/serial/msm_serial.c +61 -13
Index: b/drivers/tty/serial/msm_serial.c
===================================================================
@@ -54,6 +54,7 @@ struct msm_port {
 	unsigned int		imr;
 	void __iomem		*gsbi_base;
 	int			is_uartdm;
+	int			rx_sc_enabled;
 	unsigned int		old_snap_state;
 };
 
@@ -104,7 +105,10 @@ static void handle_rx_dm(struct uart_por
 	struct tty_port *tport = &port->state->port;
 	unsigned int sr;
 	int count = 0;
+	int imr_rx_stale = misr & UART_IMR_RXSTALE;
 	struct msm_port *msm_port = UART_TO_MSM(port);
+	int res;
+	char *cp;
 
 	if ((msm_read(port, UART_SR) & UART_SR_OVERRUN)) {
 		port->icount.overrun++;
@@ -112,12 +116,14 @@ static void handle_rx_dm(struct uart_por
 		msm_write(port, UART_CR_CMD_RESET_ERR, UART_CR);
 	}
 
-	if (misr & UART_IMR_RXSTALE) {
+	if (imr_rx_stale) {
 		count = msm_read(port, UARTDM_RX_TOTAL_SNAP) -
 			msm_port->old_snap_state;
 		msm_port->old_snap_state = 0;
 	} else {
-		count = 4 * (msm_read(port, UART_RFWR));
+		count = msm_read(port, UART_RFWR);
+		if (!msm_port->rx_sc_enabled)
+			count = 4 * count;
 		msm_port->old_snap_state += count;
 	}
 
@@ -130,28 +136,60 @@ static void handle_rx_dm(struct uart_por
 
 		sr = msm_read(port, UART_SR);
 		if ((sr & UART_SR_RX_READY) == 0) {
-			msm_port->old_snap_state -= count;
+			if (!imr_rx_stale)
+				msm_port->old_snap_state -= count;
 			break;
 		}
+
 		c = msm_read(port, UARTDM_RF);
+
 		if (sr & UART_SR_RX_BREAK) {
 			port->icount.brk++;
-			if (uart_handle_break(port))
-				continue;
+			uart_handle_break(port);
+			if (msm_port->rx_sc_enabled)
+				count -= 1;
+			else
+				count -= 4;
+			continue;
 		} else if (sr & UART_SR_PAR_FRAME_ERR)
 			port->icount.frame++;
 
-		/* TODO: handle sysrq */
-		tty_insert_flip_string(tport, (char *)&c,
-				       (count > 4) ? 4 : count);
-		count -= 4;
+		if (msm_port->rx_sc_enabled) {
+			cp = (char *)&c;
+
+			spin_unlock(&port->lock);
+			res = uart_handle_sysrq_char(port, *cp);
+			spin_lock(&port->lock);
+
+			if (!res)
+				tty_insert_flip_string(tport, cp, 1);
+			count -= 1;
+		} else {
+			cp = (char *)&c;
+
+			spin_unlock(&port->lock);
+			res = uart_handle_sysrq_char(port, *cp);
+			spin_lock(&port->lock);
+
+			if (res) {
+				count -= 1;
+				cp++;
+				tty_insert_flip_string(tport, cp,
+					       (count > 3) ? 3 : count);
+				count -= 3;
+			} else {
+				tty_insert_flip_string(tport, cp,
+						       (count > 4) ? 4 : count);
+				count -= 4;
+			}
+		}
 	}
 
 	spin_unlock(&port->lock);
 	tty_flip_buffer_push(tport);
 	spin_lock(&port->lock);
 
-	if (misr & (UART_IMR_RXSTALE))
+	if (imr_rx_stale)
 		msm_write(port, UART_CR_CMD_RESET_STALE_INT, UART_CR);
 	msm_write(port, 0xFFFFFF, UARTDM_DMRX);
 	msm_write(port, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR);
@@ -413,8 +451,11 @@ static int msm_set_baud_rate(struct uart
 	watermark |= UART_IPR_STALE_TIMEOUT_MSB & (rxstale << 2);
 	msm_write(port, watermark, UART_IPR);
 
-	/* set RX watermark */
-	watermark = (port->fifosize * 3) / 4;
+	/* set RX watermark (number of words) */
+	if (msm_port->rx_sc_enabled)
+		watermark = (port->fifosize * 3) / 4;  /* 1 byte per word */
+	else
+		watermark = (port->fifosize * 3) / 16;  /* 4 bytes per word */
 	msm_write(port, watermark, UART_RFWR);
 
 	/* set TX watermark */
@@ -728,10 +769,17 @@ static void msm_power(struct uart_port *
 static int msm_poll_init(struct uart_port *port)
 {
 	struct msm_port *msm_port = UART_TO_MSM(port);
+	unsigned int watermark;
 
 	/* Enable single character mode on RX FIFO */
-	if (msm_port->is_uartdm >= UARTDM_1P4)
+	if (msm_port->is_uartdm >= UARTDM_1P4) {
 		msm_write(port, UARTDM_DMEN_RX_SC_ENABLE, UARTDM_DMEN);
+		msm_port->rx_sc_enabled = 1;
+	}
+
+	/* set RX watermark (number of words) */
+	watermark = (port->fifosize * 3) / 4;  /* 1 byte per word */
+	msm_write(port, watermark, UART_RFWR);
 
 	return 0;
 }
#2 lore raw

PATCH 2/4 add poll_post_exception framework

In reply to Frank Rowand

From: Frank Rowand <[email protected]>

Add framework to allow serial driver to fixup state after operating in polled mode, before returning to interrupt mode.

Not-signed-off-by-yet: Frank Rowand <[email protected]>

---
 drivers/tty/serial/kgdboc.c      |    3 +++
 drivers/tty/serial/serial_core.c |   15 +++++++++++++++
 include/linux/serial_core.h      |    1 +
 include/linux/tty_driver.h       |    1 +
 4 files changed, 20 insertions(+)
drivers/tty/serial/kgdboc.c +3 -0
Index: b/drivers/tty/serial/kgdboc.c
===================================================================
@@ -302,6 +302,9 @@ static void kgdboc_post_exp_handler(void
 		con_debug_leave();
 	}
 	kgdboc_restore_input();
+
+	if (kgdb_tty_driver->ops->poll_post_exception)
+		kgdb_tty_driver->ops->poll_post_exception(kgdb_tty_driver, kgdb_tty_line);
 }
 
 static struct kgdb_io kgdboc_io_ops = {
drivers/tty/serial/serial_core.c +15 -0
Index: b/drivers/tty/serial/serial_core.c
===================================================================
@@ -2237,6 +2237,20 @@ static void uart_poll_put_char(struct tt
 	port = state->uart_port;
 	port->ops->poll_put_char(port, ch);
 }
+
+static void uart_poll_post_exception(struct tty_driver *driver, int line)
+{
+	struct uart_driver *drv = driver->driver_state;
+	struct uart_state *state = drv->state + line;
+	struct uart_port *port;
+
+	if (!state || !state->uart_port)
+		return;
+
+	port = state->uart_port;
+	if (port->ops->poll_post_exception)
+		port->ops->poll_post_exception(port);
+}
 #endif
 
 static const struct tty_operations uart_ops = {
@@ -2269,6 +2283,7 @@ static const struct tty_operations uart_
 	.poll_init	= uart_poll_init,
 	.poll_get_char	= uart_poll_get_char,
 	.poll_put_char	= uart_poll_put_char,
+	.poll_post_exception = uart_poll_post_exception,
 #endif
 };
 
include/linux/tty_driver.h +1 -0
Index: b/include/linux/tty_driver.h
===================================================================
@@ -283,6 +283,7 @@ struct tty_operations {
 	int (*poll_init)(struct tty_driver *driver, int line, char *options);
 	int (*poll_get_char)(struct tty_driver *driver, int line);
 	void (*poll_put_char)(struct tty_driver *driver, int line, char ch);
+	void (*poll_post_exception)(struct tty_driver *driver, int line);
 #endif
 	const struct file_operations *proc_fops;
 };
include/linux/serial_core.h +1 -0
Index: b/include/linux/serial_core.h
===================================================================
@@ -90,6 +90,7 @@ struct uart_ops {
 	int		(*poll_init)(struct uart_port *);
 	void		(*poll_put_char)(struct uart_port *, unsigned char);
 	int		(*poll_get_char)(struct uart_port *);
+	void		(*poll_post_exception)(struct uart_port *);
 #endif
 };
 
#3 lore raw

PATCH 3/4 use poll_post_exception in msm_serial

In reply to Frank Rowand

From: Frank Rowand <[email protected]>

**********************************************************
I need help with this patch - it does not fix the issue.
**********************************************************

Use framework to allow msm_serial driver to fixup state after operating in polled mode, before returning to interrupt mode.

I do not have complete documentation on the hardware, so this patch is a set of shots in the dark. Each experiment can be enabled by enabling a #define.

The issue I am trying to resolve with this patch is:

kgdb properly communicates with the dragon board, but following the continue command, the serial driver does not get any stale (UART_IMR_RXSTALE) interrupts until 48 characters have been read, which triggers a high water interrupt. After the high water interrupt has been processed, the driver resumes properly getting stale interrupts.

msm_poll_post_exception() will be called near the tail end of kgdb_cpu_enter(), via the path invoked by:

   /* Call the I/O driver's post_exception routine */
   if (dbg_io_ops->post_exception)
      dbg_io_ops->post_exception();

Not-signed-off-by-yet: Frank Rowand <[email protected]>

---
 drivers/tty/serial/msm_serial.c |  196 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 196 insertions(+)
drivers/tty/serial/msm_serial.c +196 -0
Index: b/drivers/tty/serial/msm_serial.c
===================================================================
@@ -881,6 +881,201 @@ static void msm_poll_put_char(struct uar
 
 	return;
 }
+
+int zzz_poll_put_char_reset;
+static void msm_poll_post_exception(struct uart_port *port)
+{
+	int misr;
+	struct msm_port *msm_port = UART_TO_MSM(port);
+
+	zzz_poll_put_char_reset++;
+
+	msm_port->old_snap_state = 0;
+
+// #define ZZZ_ALT_1
+#ifdef ZZZ_ALT_1
+	/*
+	 * zzz alternate 1 -- trigger high water after continue <---- bad
+	 *        high water count == 48, set old_snap_state = 48
+	 *        then stale total snap == 49, so count == 1    <---- good
+	 *
+	 *     better than alternate 2
+	 */
+	msm_read(port, UARTDM_RX_TOTAL_SNAP);
+
+#if 0
+	misr = msm_read(port, UART_MISR);
+	if (misr & (UART_IMR_RXSTALE))
+#endif
+		msm_write(port, UART_CR_CMD_RESET_STALE_INT, UART_CR);
+	msm_write(port, 0xFFFFFF, UARTDM_DMRX);
+	msm_write(port, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR);
+#endif /* ZZZ_ALT_1 */
+
+
+// #define ZZZ_ALT_2
+#ifdef ZZZ_ALT_2
+	/*
+	 * zzz alternate 2 -- trigger high water after continue <---- bad
+	 *        high water count == 48, set old_snap_state = 48
+	 *        next total snap == 310
+	 *        then stale total snap == 310, so count == 262 <---- bad
+	 */
+	msm_read(port, UARTDM_RX_TOTAL_SNAP);
+
+	msm_write(port, UART_CR_CMD_FORCE_STALE, UART_CR);
+#endif
+
+
+// #define ZZZ_ALT_3_4
+#ifdef ZZZ_ALT_3_4
+	/*
+	 * zzz alternate 3 -- never trigger after continue <---- bad
+	 */
+	msm_read(port, UARTDM_RX_TOTAL_SNAP);
+
+	msm_write(port, 0xFFFFFF, UARTDM_DMRX);
+	msm_write(port, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR);
+
+	msm_write(port, UART_CR_CMD_RESET_STALE_INT, UART_CR);
+	msm_write(port, UART_CR_CMD_FORCE_STALE, UART_CR);
+
+#if 0
+	/*
+	 * zzz alternate 3, #if 1 this block
+	 * zzz alternate 4, #if 0 this block
+	 * zzz alternate 4 -- trigger high water after continue <---- bad
+	 *        high water count == 48, set old_snap_state = 48
+	 *        then stale total snap == 49, so count == 1    <---- good
+	 */
+	misr = msm_read(port, UART_MISR);
+	while (!(misr & (UART_IMR_RXSTALE))) {
+		cpu_relax();
+		misr = msm_read(port, UART_MISR);
+	}
+#endif
+
+#endif /* ZZZ_ALT_3_4 */
+
+
+// #define ZZZ_ALT_5
+#ifdef ZZZ_ALT_5
+	/*
+	 * zzz alternate 5 -- trigger high water after continue <---- bad
+	 *        high water count == 48, set old_snap_state = 48
+	 *        then stale total snap == 49, so count == 1    <---- good
+	 */
+	msm_read(port, UARTDM_RX_TOTAL_SNAP);
+
+#if 0
+	misr = msm_read(port, UART_MISR);
+	if (misr & (UART_IMR_RXSTALE))
+#endif
+		msm_write(port, UART_CR_CMD_RESET_STALE_INT, UART_CR);
+	msm_write(port, 0xFFFFFF, UARTDM_DMRX);
+	msm_write(port, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR);
+
+	/* zzz should not have to do this, trying anyway */
+	/* restore interrupt */
+	msm_write(port, msm_port->imr, UART_IMR);
+#endif /* ZZZ_ALT_5 */
+
+
+// #define ZZZ_ALT_6
+#ifdef ZZZ_ALT_6
+	/*
+	 * zzz alternate 6 -- trigger high water after continue <---- bad
+	 *        high water count == 48, set old_snap_state = 48
+	 *        then stale total snap == 49, so count == 1    <---- good
+	 */
+	msm_read(port, UARTDM_RX_TOTAL_SNAP);
+
+	msm_write(port, 0, UART_IMR); /* disable interrupt */
+
+	misr = msm_read(port, UART_MISR);
+	if (misr & (UART_IMR_RXSTALE))
+		msm_write(port, UART_CR_CMD_RESET_STALE_INT, UART_CR);
+	msm_write(port, 0xFFFFFF, UARTDM_DMRX);
+	msm_write(port, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR);
+
+	msm_write(port, UART_CR_CMD_FORCE_STALE, UART_CR);
+
+	/* restore interrupt */
+	msm_write(port, msm_port->imr, UART_IMR);
+
+#endif /* ZZZ_ALT_6 */
+
+
+#define ZZZ_ALT_7
+#ifdef ZZZ_ALT_7
+	/*
+	 * Try to emulate entire interrupt driver read path....
+	 *
+	 * zzzzzz  what is missing that is fixed by the high water irq???
+	 *
+	 * same result for versions A, B, C
+	 * zzz alternate 7 -- trigger high water after continue <---- bad
+	 *        high water count == 48, set old_snap_state = 48
+	 *        then stale total snap == 49, so count == 1    <---- good
+	 */
+
+{
+	int imr_rx_stale;
+	unsigned int sr;
+
+	/*
+	 * zzz Grabbing a lock here will result in a deadlock if a breakpoint
+	 * zzz is hit while the lock is held elsewhere.  It would be best to
+	 * zzz avoid this lock if possible.
+	 *
+	 * zzz It would be better to do a trylock and warn on failure to
+	 * zzz acquire.
+	 */
+	spin_lock(&port->lock);
+
+	misr = msm_read(port, UART_MISR);
+	imr_rx_stale = misr & UART_IMR_RXSTALE;
+
+	msm_write(port, 0, UART_IMR); /* disable interrupt */
+
+	if ((msm_read(port, UART_SR) & UART_SR_OVERRUN))
+		msm_write(port, UART_CR_CMD_RESET_ERR, UART_CR);
+
+
+	/* zzz version A, read _either_ UARTDM_RX_TOTAL_SNAP or UART_RFWR */
+	/* zzz version B, read UARTDM_RX_TOTAL_SNAP and UART_RFWR */
+	/* zzz version C, read UART_RFWR */
+	if (imr_rx_stale)
+		msm_read(port, UARTDM_RX_TOTAL_SNAP);
+	else
+		msm_read(port, UART_RFWR);
+
+	sr = msm_read(port, UART_SR);
+	/* zzz this could be: while (sr & ...) {UARTDM_RF; UART_SR;} */
+	while ((sr & UART_SR_RX_READY) != 0) {
+		msm_read(port, UARTDM_RF);
+		sr = msm_read(port, UART_SR);
+	}
+
+	if (imr_rx_stale)
+		msm_write(port, UART_CR_CMD_RESET_STALE_INT, UART_CR);
+	msm_write(port, 0xFFFFFF, UARTDM_DMRX);
+	msm_write(port, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR);
+
+	msm_write(port, msm_port->imr, UART_IMR); /* restore interrupt */
+
+	spin_unlock(&port->lock);
+}
+
+#endif /* ZZZ_ALT_7 */
+
+
+
+	return;
+}
+
+
+
 #endif
 
 static struct uart_ops msm_uart_pops = {
@@ -905,6 +1100,7 @@ static struct uart_ops msm_uart_pops = {
 	.poll_init = msm_poll_init,
 	.poll_get_char	= msm_poll_get_char,
 	.poll_put_char	= msm_poll_put_char,
+	.poll_post_exception = msm_poll_post_exception,
 #endif
 };
 
#4 lore raw

PATCH 4/4 debug hackery to trace msm_serial receive interrupts

In reply to Frank Rowand

From: Frank Rowand <[email protected]>

Not intended to be applied to mainline.

This is debug code that captures a trace of msm_serial reads by handle_rx_dm().

The trace is printed to the console when the sys_sync() system call is invoked (that is, when the sync user space command is executed).

Not-signed-off-by-yet: Frank Rowand <[email protected]>

---
 arch/arm/boot/dts/qcom-msm8974.dtsi |    1 
 drivers/tty/serial/msm_serial.c     |  110 ++++++++++++++++++++++++++++++++++++
 fs/sync.c                           |    5 +
 3 files changed, 116 insertions(+)
drivers/tty/serial/msm_serial.c +110 -0
Index: b/drivers/tty/serial/msm_serial.c
===================================================================
@@ -100,6 +100,80 @@ static void msm_enable_ms(struct uart_po
 	msm_write(port, msm_port->imr, UART_IMR);
 }
 
+#define dbg_rx_c_MAX 95
+struct dbg_rx {
+	unsigned int	rx_break;
+	unsigned int	stale;
+	unsigned int	count_exhausted;
+	unsigned int	rx_total_snap;
+	unsigned int	count;
+	unsigned int	old_snap_state_1;
+	unsigned int	old_snap_state_2;
+	unsigned int	c_idx;
+	unsigned int	c[dbg_rx_c_MAX + 1];
+};
+
+#define dbg_rx_MAX 4000
+static struct dbg_rx dbg_rx[dbg_rx_MAX + 1];
+static int dbg_rx_idx = -1;
+
+#define BUF_MAX 4095
+#define BUF_SIZE (BUF_MAX + 1)
+static unsigned char buf[BUF_SIZE];
+void print_dbg_rx(void)
+{
+	unsigned char c;
+	int k;
+	int j;
+	int i;
+	struct dbg_rx *d = &dbg_rx[0];
+	unsigned char *bufp;
+
+	pr_err(
+	       "     B S E\n"
+	       "     r t x old_\n"
+	       "     e a h snap_state\n"
+	       "     a l s ---------- tot\n"
+	       "     k t t   1    2   snap count c_idx --- data\n"
+	       "---- - - -  ---- ---- ---- ----- ----- --- --------\n");
+
+	for (k = 0; k <= dbg_rx_idx; k++, d++) {
+		bufp = &buf[0];
+		bufp += scnprintf(bufp, BUF_SIZE, "%4d %c %c %c  %4d %4d %4d %5d %5d --- ",
+			k,
+			d->rx_break ? 'B' : ' ',
+			d->stale ? 'S' : 'H',
+			d->count_exhausted ? 'E' : ' ',
+			d->old_snap_state_1,
+			d->old_snap_state_2,
+			d->rx_total_snap,
+			d->count,
+			d->c_idx
+			);
+		for (j = 0; j < d->c_idx; j++) {
+			bufp += scnprintf(bufp,
+				BUF_SIZE - (bufp - &buf[0]),
+				"%08x ", d->c[j]);
+		}
+		for (j = 0; j < d->c_idx; j++) {
+			for (i = 3; i >= 0; i--) {
+				c = (d->c[j] >> (8 * i)) & 0xff;
+				bufp += scnprintf(bufp,
+					BUF_SIZE - (bufp - &buf[0]),
+					"%c",
+				       ((c >= 0x20) && (c <= 0x7e)) ? c : '.');
+			}
+			bufp += scnprintf(bufp,
+				BUF_SIZE - (bufp - &buf[0]),
+				" ");
+		}
+		bufp += scnprintf(bufp,
+			BUF_SIZE - (bufp - &buf[0]),
+			"\n");
+		pr_err("%s", buf);
+	}
+}
+
 static void handle_rx_dm(struct uart_port *port, unsigned int misr)
 {
 	struct tty_port *tport = &port->state->port;
@@ -110,6 +184,13 @@ static void handle_rx_dm(struct uart_por
 	int res;
 	char *cp;
 
+	dbg_rx_idx++;
+	if (dbg_rx_idx > dbg_rx_MAX)
+		dbg_rx_idx = 0;
+	memset(&dbg_rx[dbg_rx_idx], 0, sizeof(dbg_rx[0]));
+	dbg_rx[dbg_rx_idx].old_snap_state_1 = msm_port->old_snap_state;
+	dbg_rx[dbg_rx_idx].stale = imr_rx_stale;
+
 	if ((msm_read(port, UART_SR) & UART_SR_OVERRUN)) {
 		port->icount.overrun++;
 		tty_insert_flip_char(tport, 0, TTY_OVERRUN);
@@ -117,8 +198,14 @@ static void handle_rx_dm(struct uart_por
 	}
 
 	if (imr_rx_stale) {
+#if 1
+		int rx_total_snap = msm_read(port, UARTDM_RX_TOTAL_SNAP);
+		dbg_rx[dbg_rx_idx].rx_total_snap = rx_total_snap;
+		count = rx_total_snap - msm_port->old_snap_state;
+#else
 		count = msm_read(port, UARTDM_RX_TOTAL_SNAP) -
 			msm_port->old_snap_state;
+#endif
 		msm_port->old_snap_state = 0;
 	} else {
 		count = msm_read(port, UART_RFWR);
@@ -126,6 +213,7 @@ static void handle_rx_dm(struct uart_por
 			count = 4 * count;
 		msm_port->old_snap_state += count;
 	}
+	dbg_rx[dbg_rx_idx].count = count;
 
 	/* TODO: Precise error reporting */
 
@@ -138,12 +226,15 @@ static void handle_rx_dm(struct uart_por
 		if ((sr & UART_SR_RX_READY) == 0) {
 			if (!imr_rx_stale)
 				msm_port->old_snap_state -= count;
+			dbg_rx[dbg_rx_idx].count_exhausted = 1;
 			break;
 		}
 
 		c = msm_read(port, UARTDM_RF);
+		dbg_rx[dbg_rx_idx].c[dbg_rx[dbg_rx_idx].c_idx++] = c;
 
 		if (sr & UART_SR_RX_BREAK) {
+			dbg_rx[dbg_rx_idx].rx_break = 1;
 			port->icount.brk++;
 			uart_handle_break(port);
 			if (msm_port->rx_sc_enabled)
@@ -193,6 +284,7 @@ static void handle_rx_dm(struct uart_por
 		msm_write(port, UART_CR_CMD_RESET_STALE_INT, UART_CR);
 	msm_write(port, 0xFFFFFF, UARTDM_DMRX);
 	msm_write(port, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR);
+	dbg_rx[dbg_rx_idx].old_snap_state_2 = msm_port->old_snap_state;
 }
 
 static void handle_rx(struct uart_port *port)
@@ -771,6 +863,9 @@ static int msm_poll_init(struct uart_por
 	struct msm_port *msm_port = UART_TO_MSM(port);
 	unsigned int watermark;
 
+	pr_err("enter msm_poll_init()\n");
+	pr_err("msm_port->is_uartdm %d\n", msm_port->is_uartdm);
+
 	/* Enable single character mode on RX FIFO */
 	if (msm_port->is_uartdm >= UARTDM_1P4) {
 		msm_write(port, UARTDM_DMEN_RX_SC_ENABLE, UARTDM_DMEN);
@@ -1273,6 +1368,21 @@ static int __init msm_console_setup(stru
 
 	printk(KERN_INFO "msm_serial: console setup on port #%d\n", port->line);
 
+#if 1
+{
+	/* zzz debug info */
+
+	unsigned int version;
+
+	version = msm_read(port, 0xd8);
+	pr_err("msm_serial: HW_VERSION %d.%d.%d\n",
+	       (version >> 28) & 0xf,
+	       (version >> 16) & 0xfff,
+	       (version      ) & 0xffff);
+
+}
+#endif
+
 	return uart_set_options(port, co, baud, parity, bits, flow);
 }
 
fs/sync.c +5 -0
Index: b/fs/sync.c
===================================================================
@@ -99,10 +99,15 @@ static void fdatawait_one_bdev(struct bl
  * just write metadata (such as inodes or bitmaps) to block device page cache
  * and do not sync it on their own in ->sync_fs().
  */
+void print_dbg_rx(void);
+int zzz_print_dbg_rx = 1;
 SYSCALL_DEFINE0(sync)
 {
 	int nowait = 0, wait = 1;
 
+	if (zzz_print_dbg_rx)
+		print_dbg_rx();
+
 	wakeup_flusher_threads(0, WB_REASON_SYNC);
 	iterate_supers(sync_inodes_one_sb, NULL);
 	iterate_supers(sync_fs_one_sb, &nowait);
arch/arm/boot/dts/qcom-msm8974.dtsi +1 -0
Index: b/arch/arm/boot/dts/qcom-msm8974.dtsi
===================================================================
@@ -227,6 +227,7 @@
 
 		serial@f991e000 {
 			compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
+			/* compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm"; */
 			reg = <0xf991e000 0x1000>;
 			interrupts = <0 108 0x0>;
 			clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
#5 lore raw
In reply to Frank Rowand
Quoted 18 lines · Frank Rowand

Stephen,

I made some changes to drivers/tty/serial/msm_serial.c to allow kgdb to work with the dragon board (which has a qcom,msm-uartdm-v1.4 serial port).

I will reply to this email with the patches.

With these fixes, kgdb properly communicates with the dragon board, but following the continue command, the serial driver does not get any stale (UART_IMR_RXSTALE) interrupts until 48 characters have been read, which triggers a high water interrupt. After the high water interrupt has been processed, the driver resumes properly getting stale interrupts.

I am hoping you can help me figure out how to get the stale interrupt to occur for the first character after gdb continues, instead of having to first get a high water interrupt.

Quoted 8 lines

Patch 2 adds a framework to fix up the state of the driver and/or hardware after gdb continues. Patch 3 is a series of experiments trying to fix things up for msm_serial. I don't have complete documentation and understanding of the hardware, so some of the experiments might be less than intelligent.

Can you give me some suggestions of What should I be doing in the fix up function?

Hmm... I've never gotten the serial console to come back after continuing because I only have one serial port and I use it for the console so switching it into kgdb makes me lose the console. How do you work around this? Use adb shell?

I think the big problem is

msm_write(port, UARTDM_DMEN_RX_SC_ENABLE, UARTDM_DMEN)

in msm_poll_init() which forces the uart into single character mode. That causes all sorts of problems but is required to make the get_char stuff work cleanly. I really wanted the msm_poll_get_char_dm_1p3() to work for the 1.4 version of the hardware but I guess they made the hardware different again for some unknown reason so it doesn't work. I never got around to figuring it out or talking to the hardware people. It took a while to develop the 1p3 version of the get_char code because of the "fifo" centric design.

Do you know if that 1p3 version of the code works for you on 1.3 hardware? Just curious if that actually has the same bugs or not. I probably won't have much time to look at this anytime soon, but I'll try and send an email off to the hardware designers to figure out a way to avoid switching the mode.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#6 lore raw
Quoted 25 lines · Stephen Boyd
Quoted 24 lines · Frank Rowand

Stephen,

I made some changes to drivers/tty/serial/msm_serial.c to allow kgdb to work with the dragon board (which has a qcom,msm-uartdm-v1.4 serial port).

I will reply to this email with the patches.

With these fixes, kgdb properly communicates with the dragon board, but following the continue command, the serial driver does not get any stale (UART_IMR_RXSTALE) interrupts until 48 characters have been read, which triggers a high water interrupt. After the high water interrupt has been processed, the driver resumes properly getting stale interrupts.

I am hoping you can help me figure out how to get the stale interrupt to occur for the first character after gdb continues, instead of having to first get a high water interrupt.

Patch 2 adds a framework to fix up the state of the driver and/or hardware after gdb continues. Patch 3 is a series of experiments trying to fix things up for msm_serial. I don't have complete documentation and understanding of the hardware, so some of the experiments might be less than intelligent.

Quoted 7 lines
Quoted 2 lines

Can you give me some suggestions of What should I be doing in the fix up function?

Hmm... I've never gotten the serial console to come back after continuing because I only have one serial port and I use it for the console so switching it into kgdb makes me lose the console. How do you work around this? Use adb shell?

I use kdmx2 to share the host serial port between gdb for kgdb and minicom for the console input and output. I had to update kdmx2 to work in my environment. You can download it from:

http://elinux.org/images/a/a7/Kdmx-2.00_v140730a.tar.gz

Whenever that link is updated, the new link can be found at:

http://elinux.org/Kgdb

Quoted 6 lines

I think the big problem is

msm_write(port, UARTDM_DMEN_RX_SC_ENABLE, UARTDM_DMEN)

in msm_poll_init() which forces the uart into single character mode.

Yep, most of my fixes deal with the fact that in single character mode echo fifo word only contains a single character of data. All of that works now. So my only problem is not getting a stale interrupt until after I've gotten a high water interrupt.

Quoted 8 lines

That causes all sorts of problems but is required to make the get_char stuff work cleanly. I really wanted the msm_poll_get_char_dm_1p3() to work for the 1.4 version of the hardware but I guess they made the hardware different again for some unknown reason so it doesn't work. I never got around to figuring it out or talking to the hardware people. It took a while to develop the 1p3 version of the get_char code because of the "fifo" centric design.

Quoted 6 lines

Do you know if that 1p3 version of the code works for you on 1.3 hardware? Just curious if that actually has the same bugs or not. I probably won't have much time to look at this anytime soon, but I'll try and send an email off to the hardware designers to figure out a way to avoid switching the mode.

I don't have any 1.3 hardware on my desk. I'll have to see if I can find some when I get a chance. I did try telling the driver that I was using 1.3 hardware even though it is 1.4; I had to make some changes to msm_poll_get_char_dm_1p3() to make it kind of sort of work, but not well. Instead of spending more time on that I just went back to making the single character mode work.

-Frank

#7 lore raw
Quoted 36 lines · Frank Rowand
Quoted 25 lines · Stephen Boyd
Quoted 24 lines · Frank Rowand

Stephen,

I made some changes to drivers/tty/serial/msm_serial.c to allow kgdb to work with the dragon board (which has a qcom,msm-uartdm-v1.4 serial port).

I will reply to this email with the patches.

With these fixes, kgdb properly communicates with the dragon board, but following the continue command, the serial driver does not get any stale (UART_IMR_RXSTALE) interrupts until 48 characters have been read, which triggers a high water interrupt. After the high water interrupt has been processed, the driver resumes properly getting stale interrupts.

I am hoping you can help me figure out how to get the stale interrupt to occur for the first character after gdb continues, instead of having to first get a high water interrupt.

Patch 2 adds a framework to fix up the state of the driver and/or hardware after gdb continues. Patch 3 is a series of experiments trying to fix things up for msm_serial. I don't have complete documentation and understanding of the hardware, so some of the experiments might be less than intelligent.

Quoted 6 lines
Quoted 2 lines

Can you give me some suggestions of What should I be doing in the fix up function?

Hmm... I've never gotten the serial console to come back after continuing because I only have one serial port and I use it for the console so switching it into kgdb makes me lose the console. How do you work around this? Use adb shell?

I use kdmx2 to share the host serial port between gdb for kgdb and minicom for the console input and output. I had to update kdmx2 to work in my environment. You can download it from:

Quoted 5 lines
   http://elinux.org/images/a/a7/Kdmx-2.00_v140730a.tar.gz

Whenever that link is updated, the new link can be found at:

http://elinux.org/Kgdb

Cool. Thanks for the pointer.

Quoted 25 lines
Quoted 6 lines

I think the big problem is

msm_write(port, UARTDM_DMEN_RX_SC_ENABLE, UARTDM_DMEN)

in msm_poll_init() which forces the uart into single character mode.

Yep, most of my fixes deal with the fact that in single character mode echo fifo word only contains a single character of data. All of that works now. So my only problem is not getting a stale interrupt until after I've gotten a high water interrupt.

Quoted 8 lines

That causes all sorts of problems but is required to make the get_char stuff work cleanly. I really wanted the msm_poll_get_char_dm_1p3() to work for the 1.4 version of the hardware but I guess they made the hardware different again for some unknown reason so it doesn't work. I never got around to figuring it out or talking to the hardware people. It took a while to develop the 1p3 version of the get_char code because of the "fifo" centric design.

Quoted 6 lines

Do you know if that 1p3 version of the code works for you on 1.3 hardware? Just curious if that actually has the same bugs or not. I probably won't have much time to look at this anytime soon, but I'll try and send an email off to the hardware designers to figure out a way to avoid switching the mode.

Quoted 8 lines

I don't have any 1.3 hardware on my desk. I'll have to see if I can find some when I get a chance. I did try telling the driver that I was using 1.3 hardware even though it is 1.4; I had to make some changes to msm_poll_get_char_dm_1p3() to make it kind of sort of work, but not well. Instead of spending more time on that I just went back to making the single character mode work.

I tried it and it doesn't work either. Typing lots of characters finally unjams it like you see on 1.4 hardware.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#8 lore raw
Quoted 4 lines · Stephen Boyd

I tried it and it doesn't work either. Typing lots of characters finally unjams it like you see on 1.4 hardware.

Can you try this? It seems to work for me on both 1.3 and 1.4 hardware

drivers/tty/serial/msm_serial.c +6 -15
index 72000a6d5af0..fbcb0c657d5f 100644
@@ -680,17 +680,6 @@ static void msm_power(struct uart_port *port, unsigned int state,
 }
 
 #ifdef CONFIG_CONSOLE_POLL
-static int msm_poll_init(struct uart_port *port)
-{
-	struct msm_port *msm_port = UART_TO_MSM(port);
-
-	/* Enable single character mode on RX FIFO */
-	if (msm_port->is_uartdm >= UARTDM_1P4)
-		msm_write(port, UARTDM_DMEN_RX_SC_ENABLE, UARTDM_DMEN);
-
-	return 0;
-}
-
 static int msm_poll_get_char_single(struct uart_port *port)
 {
 	struct msm_port *msm_port = UART_TO_MSM(port);
@@ -702,7 +691,7 @@ static int msm_poll_get_char_single(struct uart_port *port)
 		return msm_read(port, rf_reg) & 0xff;
 }
 
-static int msm_poll_get_char_dm_1p3(struct uart_port *port)
+static int msm_poll_get_char_dm(struct uart_port *port)
 {
 	int c;
 	static u32 slop;
@@ -726,6 +715,9 @@ static int msm_poll_get_char_dm_1p3(struct uart_port *port)
 			slop = msm_read(port, UARTDM_RF);
 			c = sp[0];
 			count--;
+			msm_write(port, UART_CR_CMD_RESET_STALE_INT, UART_CR);
+			msm_write(port, 0xFFFFFF, UARTDM_DMRX);
+			msm_write(port, UART_CR_CMD_STALE_EVENT_ENABLE, UART_CR);
 		} else {
 			c = NO_POLL_CHAR;
 		}
@@ -749,8 +741,8 @@ static int msm_poll_get_char(struct uart_port *port)
 	imr = msm_read(port, UART_IMR);
 	msm_write(port, 0, UART_IMR);
 
-	if (msm_port->is_uartdm == UARTDM_1P3)
-		c = msm_poll_get_char_dm_1p3(port);
+	if (msm_port->is_uartdm)
+		c = msm_poll_get_char_dm(port);
 	else
 		c = msm_poll_get_char_single(port);
 
@@ -809,7 +801,6 @@ static struct uart_ops msm_uart_pops = {
 	.verify_port = msm_verify_port,
 	.pm = msm_power,
 #ifdef CONFIG_CONSOLE_POLL
-	.poll_init = msm_poll_init,
 	.poll_get_char	= msm_poll_get_char,
 	.poll_put_char	= msm_poll_put_char,
 #endif
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
author Reviewed-by Tested-by
#9 lore raw
Quoted 7 lines · Stephen Boyd
Quoted 4 lines · Stephen Boyd

I tried it and it doesn't work either. Typing lots of characters finally unjams it like you see on 1.4 hardware.

Can you try this? It seems to work for me on both 1.3 and 1.4 hardware

Yes, much better getting rid of the single character mode! Works for me on the dragonboard (1.4 hardware).

Thanks for figuring this out.

I'll send a patch to handle sysrq after I see this patch upstream.

-Frank

#10 lore raw
Quoted 5 lines · Frank Rowand
Quoted 4 lines · Stephen Boyd
Quoted 3 lines · Stephen Boyd

I tried it and it doesn't work either. Typing lots of characters finally unjams it like you see on 1.4 hardware.

Quoted 8 lines
Quoted 1 line

Can you try this? It seems to work for me on both 1.3 and 1.4 hardware

Yes, much better getting rid of the single character mode! Works for me on the dragonboard (1.4 hardware).

Great, I'll send an official patch with some commit text that hopefully makes sense.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation