All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frank Rowand <frowand.list@gmail.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>,
	David Brown <davidb@codeaurora.org>,
	Daniel Walker <dwalker@fifo99.com>,
	Bryan Huntsman <bryanh@codeaurora.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.cz>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	linux-serial@vger.kernel.org,
	Linux Kernel list <linux-kernel@vger.kernel.org>,
	Jason Wessel <jason.wessel@windriver.com>,
	kgdb-bugreport@lists.sourceforge.net
Subject: [test patch 3/4] use poll_post_exception in msm_serial
Date: Mon, 04 Aug 2014 18:06:32 -0700	[thread overview]
Message-ID: <53E02E18.60602@gmail.com> (raw)
In-Reply-To: <53E02C7C.4090206@gmail.com>

From: Frank Rowand <frank.rowand@sonymobile.com>


**********************************************************
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 <frank.rowand@sonymobile.com>

---
 drivers/tty/serial/msm_serial.c |  196 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 196 insertions(+)

Index: b/drivers/tty/serial/msm_serial.c
===================================================================
--- a/drivers/tty/serial/msm_serial.c
+++ 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
 };
 

WARNING: multiple messages have this Message-ID (diff)
From: Frank Rowand <frowand.list@gmail.com>
To: unlisted-recipients:; (no To-header on input)
Cc: Stephen Boyd <sboyd@codeaurora.org>,
	David Brown <davidb@codeaurora.org>,
	Daniel Walker <dwalker@fifo99.com>,
	Bryan Huntsman <bryanh@codeaurora.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.cz>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	linux-serial@vger.kernel.org,
	Linux Kernel list <linux-kernel@vger.kernel.org>,
	Jason Wessel <jason.wessel@windriver.com>,
	kgdb-bugreport@lists.sourceforge.net
Subject: [test patch 3/4] use poll_post_exception in msm_serial
Date: Mon, 04 Aug 2014 18:06:32 -0700	[thread overview]
Message-ID: <53E02E18.60602@gmail.com> (raw)
In-Reply-To: <53E02C7C.4090206@gmail.com>

From: Frank Rowand <frank.rowand@sonymobile.com>


**********************************************************
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 <frank.rowand@sonymobile.com>

---
 drivers/tty/serial/msm_serial.c |  196 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 196 insertions(+)

Index: b/drivers/tty/serial/msm_serial.c
===================================================================
--- a/drivers/tty/serial/msm_serial.c
+++ 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
 };
 

  parent reply	other threads:[~2014-08-05  1:06 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-05  0:59 [request for help] tty: serial: kgdb: fix msm_serial for kgdb Frank Rowand
2014-08-05  1:03 ` [test patch 1/4] " Frank Rowand
2014-08-05  1:04 ` [test patch 2/4] add poll_post_exception framework Frank Rowand
2014-08-05  1:06 ` Frank Rowand [this message]
2014-08-05  1:06   ` [test patch 3/4] use poll_post_exception in msm_serial Frank Rowand
2014-08-05  1:08 ` [test patch 4/4] debug hackery to trace msm_serial receive interrupts Frank Rowand
2014-08-05  1:08   ` Frank Rowand
2014-08-05  1:33 ` [request for help] tty: serial: kgdb: fix msm_serial for kgdb Stephen Boyd
2014-08-05  2:09   ` Frank Rowand
2014-08-05 19:22     ` Stephen Boyd
2014-08-05 23:53       ` Stephen Boyd
2014-08-06  0:55         ` Frank Rowand
2014-08-06  1:19           ` Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53E02E18.60602@gmail.com \
    --to=frowand.list@gmail.com \
    --cc=bryanh@codeaurora.org \
    --cc=davidb@codeaurora.org \
    --cc=dwalker@fifo99.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jason.wessel@windriver.com \
    --cc=jslaby@suse.cz \
    --cc=kgdb-bugreport@lists.sourceforge.net \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=sboyd@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.