linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
To: gregkh@linuxfoundation.org
Cc: linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Jiri Slaby (SUSE)" <jirislaby@kernel.org>,
	Richard Genoud <richard.genoud@gmail.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Claudiu Beznea <claudiu.beznea@microchip.com>,
	linux-arm-kernel@lists.infradead.org,
	Michael Walle <michael@walle.cc>
Subject: [PATCH 2/2] serial: atmel: don't stop the transmitter when doing PIO
Date: Wed, 23 Nov 2022 09:27:36 +0100	[thread overview]
Message-ID: <20221123082736.24566-2-jirislaby@kernel.org> (raw)
In-Reply-To: <20221123082736.24566-1-jirislaby@kernel.org>

Writing ATMEL_US_TXDIS to ATMEL_US_CR makes the transmitter NOT to send
the just queued character. This means when the character is last and
uart calls ops->stop_tx(), the character is not sent at all.

The usart datasheet is not much specific on this, it just says the
transmitter is stopped. But apparently, the character is dropped. So
we should stop the transmitter only for DMA and PDC transfers to not
send any more characters. For PIO, this is unexpected and deviates from
other drivers. In particular, the below referenced commit broke TX as it
added a call to ->stop_tx() after the very last character written to the
transmitter.

So fix this by limiting the write of ATMEL_US_TXDIS to DMA transfers
only.

Even there, I don't know if it is correctly implemented. Are all the
queued characters sent once ->start_tx() is called? Anyone tested flow
control -- be it hard (RTSCTS) or the soft (XOFF/XON) one?

Fixes: 2d141e683e9a ("tty: serial: use uart_port_tx() helper")
Cc: Richard Genoud <richard.genoud@gmail.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
Cc: linux-arm-kernel@lists.infradead.org
Reported-by: Michael Walle <michael@walle.cc>
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/serial/atmel_serial.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 65f63dccfd72..f1c06e12efa0 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -553,19 +553,22 @@ static void atmel_stop_tx(struct uart_port *port)
 {
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 	bool is_pdc = atmel_use_pdc_tx(port);
+	bool is_dma = is_pdc || atmel_use_dma_tx(port);
 
 	if (is_pdc) {
 		/* disable PDC transmit */
 		atmel_uart_writel(port, ATMEL_PDC_PTCR, ATMEL_PDC_TXTDIS);
 	}
 
-	/*
-	 * Disable the transmitter.
-	 * This is mandatory when DMA is used, otherwise the DMA buffer
-	 * is fully transmitted.
-	 */
-	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
-	atmel_port->tx_stopped = true;
+	if (is_dma) {
+		/*
+		 * Disable the transmitter.
+		 * This is mandatory when DMA is used, otherwise the DMA buffer
+		 * is fully transmitted.
+		 */
+		atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
+		atmel_port->tx_stopped = true;
+	}
 
 	/* Disable interrupts */
 	atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
@@ -601,9 +604,11 @@ static void atmel_start_tx(struct uart_port *port)
 	/* Enable interrupts */
 	atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask);
 
-	/* re-enable the transmitter */
-	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
-	atmel_port->tx_stopped = false;
+	if (is_dma) {
+		/* re-enable the transmitter */
+		atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
+		atmel_port->tx_stopped = false;
+	}
 }
 
 /*
-- 
2.38.1


  reply	other threads:[~2022-11-23  8:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-23  8:27 [PATCH 1/2] serial: atmel: cleanup atmel_start+stop_tx() Jiri Slaby (SUSE)
2022-11-23  8:27 ` Jiri Slaby (SUSE) [this message]
2022-11-23  8:50   ` [PATCH 2/2] serial: atmel: don't stop the transmitter when doing PIO Michael Walle
2022-11-23 12:46     ` Richard Genoud
2022-11-23  8:50 ` [PATCH 1/2] serial: atmel: cleanup atmel_start+stop_tx() Michael Walle
2022-11-23 12:47   ` Richard Genoud

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=20221123082736.24566-2-jirislaby@kernel.org \
    --to=jirislaby@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=claudiu.beznea@microchip.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=michael@walle.cc \
    --cc=nicolas.ferre@microchip.com \
    --cc=richard.genoud@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).