Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Erwan Le Ray <erwan.leray@foss.st.com>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jslaby@suse.com>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>
CC:     <linux-serial@vger.kernel.org>,
        <linux-stm32@st-md-mailman.stormreply.com>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>,
        Erwan Le Ray <erwan.leray@foss.st.com>,
        Fabrice Gasnier <fabrice.gasnier@foss.st.com>,
        Valentin Caron <valentin.caron@foss.st.com>,
        Amelie Delaunay <amelie.delaunay@foss.st.com>
Subject: [PATCH 3/3] serial: stm32: push DMA RX data before suspending
Date:   Mon, 25 Oct 2021 15:42:29 +0200
Message-ID: <20211025134229.8456-4-erwan.leray@foss.st.com>
In-Reply-To: <20211025134229.8456-1-erwan.leray@foss.st.com>
References: <20211025134229.8456-1-erwan.leray@foss.st.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Data may be stored in DMA RX buffer, when suspending. The data needs
to be pushed to the upper layer. We can't rely on the timeout IRQ (RTOR)
that can't be triggered into low power state. So safely clear DMA request
(DMAR), force the DMA reception routines to push RX buffer content, before
disabling RX DMA. This way, handover to pio mode is safe.
Only call tty_flip_buffer_push() when there is RX data to handle.

Move the locking outside of stm32_usart_receive_chars() to prevent a race
condition, when disabling DMA request upon suspend / pm_runtime_suspend.
Data may be received under IRQ and pushed before
stm32_usart_receive_chars() has pushed older data from DMA rx_buf upon
suspend.
The sequence in suspend routine needs proper locking to avoid this.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Signed-off-by: Erwan Le Ray <erwan.leray@foss.st.com>

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 4b5b0748790c..3244e7f6818c 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -210,11 +210,12 @@ static unsigned long stm32_usart_get_char_pio(struct uart_port *port)
 	return c;
 }
 
-static void stm32_usart_receive_chars_pio(struct uart_port *port)
+static unsigned int stm32_usart_receive_chars_pio(struct uart_port *port)
 {
 	struct stm32_port *stm32_port = to_stm32_port(port);
 	const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
 	unsigned long c;
+	unsigned int size = 0;
 	u32 sr;
 	char flag;
 
@@ -239,6 +240,7 @@ static void stm32_usart_receive_chars_pio(struct uart_port *port)
 
 		c = stm32_usart_get_char_pio(port);
 		port->icount.rx++;
+		size++;
 		if (sr & USART_SR_ERR_MASK) {
 			if (sr & USART_SR_ORE) {
 				port->icount.overrun++;
@@ -271,6 +273,8 @@ static void stm32_usart_receive_chars_pio(struct uart_port *port)
 			continue;
 		uart_insert_char(port, sr, USART_SR_ORE, c, flag);
 	}
+
+	return size;
 }
 
 static void stm32_usart_push_buffer_dma(struct uart_port *port, unsigned int dma_size)
@@ -300,50 +304,48 @@ static void stm32_usart_push_buffer_dma(struct uart_port *port, unsigned int dma
 		stm32_port->last_res = RX_BUF_L;
 }
 
-static void stm32_usart_receive_chars_dma(struct uart_port *port)
+static unsigned int stm32_usart_receive_chars_dma(struct uart_port *port)
 {
 	struct stm32_port *stm32_port = to_stm32_port(port);
-	unsigned int dma_size;
+	unsigned int dma_size, size = 0;
 
 	/* DMA buffer is configured in cyclic mode and handles the rollback of the buffer. */
 	if (stm32_port->rx_dma_state.residue > stm32_port->last_res) {
 		/* Conditional first part: from last_res to end of DMA buffer */
 		dma_size = stm32_port->last_res;
 		stm32_usart_push_buffer_dma(port, dma_size);
+		size = dma_size;
 	}
 
 	dma_size = stm32_port->last_res - stm32_port->rx_dma_state.residue;
 	stm32_usart_push_buffer_dma(port, dma_size);
+	size += dma_size;
+
+	return size;
 }
 
-static void stm32_usart_receive_chars(struct uart_port *port, bool irqflag)
+static unsigned int stm32_usart_receive_chars(struct uart_port *port, bool force_dma_flush)
 {
-	struct tty_port *tport = &port->state->port;
 	struct stm32_port *stm32_port = to_stm32_port(port);
 	const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
 	enum dma_status rx_dma_status;
-	unsigned long flags;
 	u32 sr;
+	unsigned int size = 0;
 
-	if (irqflag)
-		spin_lock_irqsave(&port->lock, flags);
-	else
-		spin_lock(&port->lock);
-
-	if (stm32_usart_rx_dma_enabled(port)) {
+	if (stm32_usart_rx_dma_enabled(port) || force_dma_flush) {
 		rx_dma_status = dmaengine_tx_status(stm32_port->rx_ch,
 						    stm32_port->rx_ch->cookie,
 						    &stm32_port->rx_dma_state);
 		if (rx_dma_status == DMA_IN_PROGRESS) {
 			/* Empty DMA buffer */
-			stm32_usart_receive_chars_dma(port);
+			size = stm32_usart_receive_chars_dma(port);
 			sr = readl_relaxed(port->membase + ofs->isr);
 			if (sr & USART_SR_ERR_MASK) {
 				/* Disable DMA request line */
 				stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_DMAR);
 
 				/* Switch to PIO mode to handle the errors */
-				stm32_usart_receive_chars_pio(port);
+				size += stm32_usart_receive_chars_pio(port);
 
 				/* Switch back to DMA mode */
 				stm32_usart_set_bits(port, ofs->cr3, USART_CR3_DMAR);
@@ -354,18 +356,13 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool irqflag)
 			stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_DMAR);
 			/* Fall back to interrupt mode */
 			dev_dbg(port->dev, "DMA error, fallback to irq mode\n");
-			stm32_usart_receive_chars_pio(port);
+			size = stm32_usart_receive_chars_pio(port);
 		}
 	} else {
-		stm32_usart_receive_chars_pio(port);
+		size = stm32_usart_receive_chars_pio(port);
 	}
 
-	if (irqflag)
-		uart_unlock_and_check_sysrq_irqrestore(port, irqflag);
-	else
-		uart_unlock_and_check_sysrq(port);
-
-	tty_flip_buffer_push(tport);
+	return size;
 }
 
 static void stm32_usart_tx_dma_complete(void *arg)
@@ -403,8 +400,15 @@ static void stm32_usart_tx_interrupt_enable(struct uart_port *port)
 static void stm32_usart_rx_dma_complete(void *arg)
 {
 	struct uart_port *port = arg;
+	struct tty_port *tport = &port->state->port;
+	unsigned int size;
+	unsigned long flags;
 
-	stm32_usart_receive_chars(port, true);
+	spin_lock_irqsave(&port->lock, flags);
+	size = stm32_usart_receive_chars(port, false);
+	uart_unlock_and_check_sysrq_irqrestore(port, flags);
+	if (size)
+		tty_flip_buffer_push(tport);
 }
 
 static void stm32_usart_tx_interrupt_disable(struct uart_port *port)
@@ -557,6 +561,7 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
 	struct stm32_port *stm32_port = to_stm32_port(port);
 	const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
 	u32 sr;
+	unsigned int size;
 
 	sr = readl_relaxed(port->membase + ofs->isr);
 
@@ -580,7 +585,11 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
 	if (!stm32_port->throttled) {
 		if (((sr & USART_SR_RXNE) && !stm32_usart_rx_dma_enabled(port)) ||
 		    ((sr & USART_SR_ERR_MASK) && stm32_usart_rx_dma_enabled(port))) {
-			stm32_usart_receive_chars(port, false);
+			spin_lock(&port->lock);
+			size = stm32_usart_receive_chars(port, false);
+			uart_unlock_and_check_sysrq(port);
+			if (size)
+				tty_flip_buffer_push(tport);
 		}
 	}
 
@@ -599,11 +608,19 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
 static irqreturn_t stm32_usart_threaded_interrupt(int irq, void *ptr)
 {
 	struct uart_port *port = ptr;
+	struct tty_port *tport = &port->state->port;
 	struct stm32_port *stm32_port = to_stm32_port(port);
+	unsigned int size;
+	unsigned long flags;
 
 	/* Receiver timeout irq for DMA RX */
-	if (!stm32_port->throttled)
-		stm32_usart_receive_chars(port, false);
+	if (!stm32_port->throttled) {
+		spin_lock_irqsave(&port->lock, flags);
+		size = stm32_usart_receive_chars(port, false);
+		uart_unlock_and_check_sysrq_irqrestore(port, flags);
+		if (size)
+			tty_flip_buffer_push(tport);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -1678,6 +1695,8 @@ static int __maybe_unused stm32_usart_serial_en_wakeup(struct uart_port *port,
 	const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
 	struct tty_port *tport = &port->state->port;
 	int ret;
+	unsigned int size;
+	unsigned long flags;
 
 	if (!stm32_port->wakeup_src || !tty_port_initialized(tport))
 		return 0;
@@ -1696,8 +1715,15 @@ static int __maybe_unused stm32_usart_serial_en_wakeup(struct uart_port *port,
 		 * low-power mode.
 		 */
 		if (stm32_port->rx_ch) {
+			spin_lock_irqsave(&port->lock, flags);
+			/* Avoid race with RX IRQ when DMAR is cleared */
 			stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_DMAR);
-			dmaengine_terminate_sync(stm32_port->rx_ch);
+			/* Poll data from DMA RX buffer if any */
+			size = stm32_usart_receive_chars(port, true);
+			dmaengine_terminate_async(stm32_port->rx_ch);
+			uart_unlock_and_check_sysrq_irqrestore(port, flags);
+			if (size)
+				tty_flip_buffer_push(tport);
 		}
 
 		/* Poll data from RX FIFO if any */
-- 
2.17.1