All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Organov <sorganov@gmail.com>
To: linux-serial@vger.kernel.org
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Sergey Organov" <sorganov@gmail.com>,
	"NXP Linux Team" <linux-imx@nxp.com>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 4/5] serial: imx: fix data breakage on termios change
Date: Wed, 28 Aug 2019 21:37:54 +0300	[thread overview]
Message-ID: <1567017475-11919-5-git-send-email-sorganov@gmail.com> (raw)
In-Reply-To: <1567017475-11919-1-git-send-email-sorganov@gmail.com>

imx_set_termios(): avoid writing baud rate divider registers when the
values to be written are the same as current. Any writing seems to
restart transmission/receiving logic in the hardware, that leads to
data breakage even when rate doesn't in fact change. E.g., user
switches RTS/CTS handshake and suddenly gets broken bytes.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index cc3783c..e89045a 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1545,7 +1545,7 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	unsigned int baud, quot;
 	unsigned int old_csize = old ? old->c_cflag & CSIZE : CS8;
 	unsigned long div;
-	unsigned long num, denom;
+	unsigned long num, denom, old_ubir, old_ubmr;
 	uint64_t tdiv64;
 
 	/*
@@ -1670,8 +1670,21 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	ufcr = (ufcr & (~UFCR_RFDIV)) | UFCR_RFDIV_REG(div);
 	imx_uart_writel(sport, ufcr, UFCR);
 
-	imx_uart_writel(sport, num, UBIR);
-	imx_uart_writel(sport, denom, UBMR);
+	/*
+	 *  Two registers below should always be written both and in this
+	 *  particular order. One consequence is that we need to check if any of
+	 *  them changes and then update both. We do need the check for change
+	 *  as even writing the same values seem to "restart"
+	 *  transmission/receiving logic in the hardware, that leads to data
+	 *  breakage even when rate doesn't in fact change. E.g., user switches
+	 *  RTS/CTS handshake and suddenly gets broken bytes.
+	 */
+	old_ubir = imx_uart_readl(sport, UBIR);
+	old_ubmr = imx_uart_readl(sport, UBMR);
+	if (old_ubir != num || old_ubmr != denom) {
+		imx_uart_writel(sport, num, UBIR);
+		imx_uart_writel(sport, denom, UBMR);
+	}
 
 	if (!imx_uart_is_imx1(sport))
 		imx_uart_writel(sport, sport->port.uartclk / div / 1000,
-- 
2.10.0.1.g57b01a3

WARNING: multiple messages have this Message-ID (diff)
From: Sergey Organov <sorganov@gmail.com>
To: linux-serial@vger.kernel.org
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Sergey Organov" <sorganov@gmail.com>,
	"NXP Linux Team" <linux-imx@nxp.com>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 4/5] serial: imx: fix data breakage on termios change
Date: Wed, 28 Aug 2019 21:37:54 +0300	[thread overview]
Message-ID: <1567017475-11919-5-git-send-email-sorganov@gmail.com> (raw)
In-Reply-To: <1567017475-11919-1-git-send-email-sorganov@gmail.com>

imx_set_termios(): avoid writing baud rate divider registers when the
values to be written are the same as current. Any writing seems to
restart transmission/receiving logic in the hardware, that leads to
data breakage even when rate doesn't in fact change. E.g., user
switches RTS/CTS handshake and suddenly gets broken bytes.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index cc3783c..e89045a 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1545,7 +1545,7 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	unsigned int baud, quot;
 	unsigned int old_csize = old ? old->c_cflag & CSIZE : CS8;
 	unsigned long div;
-	unsigned long num, denom;
+	unsigned long num, denom, old_ubir, old_ubmr;
 	uint64_t tdiv64;
 
 	/*
@@ -1670,8 +1670,21 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	ufcr = (ufcr & (~UFCR_RFDIV)) | UFCR_RFDIV_REG(div);
 	imx_uart_writel(sport, ufcr, UFCR);
 
-	imx_uart_writel(sport, num, UBIR);
-	imx_uart_writel(sport, denom, UBMR);
+	/*
+	 *  Two registers below should always be written both and in this
+	 *  particular order. One consequence is that we need to check if any of
+	 *  them changes and then update both. We do need the check for change
+	 *  as even writing the same values seem to "restart"
+	 *  transmission/receiving logic in the hardware, that leads to data
+	 *  breakage even when rate doesn't in fact change. E.g., user switches
+	 *  RTS/CTS handshake and suddenly gets broken bytes.
+	 */
+	old_ubir = imx_uart_readl(sport, UBIR);
+	old_ubmr = imx_uart_readl(sport, UBMR);
+	if (old_ubir != num || old_ubmr != denom) {
+		imx_uart_writel(sport, num, UBIR);
+		imx_uart_writel(sport, denom, UBMR);
+	}
 
 	if (!imx_uart_is_imx1(sport))
 		imx_uart_writel(sport, sport->port.uartclk / div / 1000,
-- 
2.10.0.1.g57b01a3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2019-08-28 18:37 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190530152950.25377-1-sorganov@gmail.com>
2019-08-28 18:37 ` [PATCH v2 0/5] Serial: imx: various fixes Sergey Organov
2019-08-28 18:37   ` Sergey Organov
2019-08-28 18:37   ` [PATCH v2 1/5] serial: imx: get rid of unbounded busy-waiting loop Sergey Organov
2019-08-28 18:37     ` Sergey Organov
2019-08-28 18:37   ` [PATCH v2 2/5] serial: imx: do not stop Rx/Tx on termios change Sergey Organov
2019-08-28 18:37     ` Sergey Organov
2019-08-28 18:37   ` [PATCH v2 3/5] serial: imx: do not disable individual irqs during " Sergey Organov
2019-08-28 18:37     ` Sergey Organov
2019-08-28 18:37   ` Sergey Organov [this message]
2019-08-28 18:37     ` [PATCH v2 4/5] serial: imx: fix data breakage on " Sergey Organov
2019-08-28 18:37   ` [PATCH v2 5/5] serial: imx: use Tx ready rather than Tx empty irq Sergey Organov
2019-08-28 18:37     ` Sergey Organov

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=1567017475-11919-5-git-send-email-sorganov@gmail.com \
    --to=sorganov@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-serial@vger.kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=u.kleine-koenig@pengutronix.de \
    /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.