2021-02-19 17:53:08

by Erwan Le Ray

[permalink] [raw]
Subject: [PATCH 00/13] stm32 usart various fixes

This series brings various fixes to stm32-usart driver.

Erwan Le Ray (13):
serial: stm32: fix probe and remove order for dma
serial: stm32: fix startup by enabling usart for reception
serial: stm32: fix incorrect characters on console
serial: stm32: fix TX and RX FIFO thresholds
serial: stm32: fix a deadlock condition with wakeup event
serial: stm32: fix wake-up flag handling
serial: stm32: fix a deadlock in set_termios
serial: stm32: fix tx dma completion, release channel
serial: stm32: call stm32_transmit_chars locked
serial: stm32: fix FIFO flush in startup and set_termios
serial: stm32: add FIFO flush when port is closed
serial: stm32: fix tx_empty condition
serial: stm32: add support for "flush_buffer" ops

drivers/tty/serial/stm32-usart.c | 198 +++++++++++++++++++++----------
drivers/tty/serial/stm32-usart.h | 3 -
2 files changed, 135 insertions(+), 66 deletions(-)

--
2.17.1


2021-02-19 17:53:11

by Erwan Le Ray

[permalink] [raw]
Subject: [PATCH 04/13] serial: stm32: fix TX and RX FIFO thresholds

TX and RX FIFO thresholds may be cleared after suspend/resume, depending
on the low power mode.

Those configurations (done in startup) are not effective for UART console,
as:
- the reference manual indicates that FIFOEN bit can only be written when
the USART is disabled (UE=0)
- a set_termios (where UE is set) is requested firstly for console
enabling, before the startup.

Fixes: 84872dc448fe ("serial: stm32: add RX and TX FIFO flush")
Signed-off-by: Erwan Le Ray <[email protected]>

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 7710de947aa3..d409a23806b1 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -649,19 +649,8 @@ static int stm32_usart_startup(struct uart_port *port)
if (ofs->rqr != UNDEF_REG)
stm32_usart_set_bits(port, ofs->rqr, USART_RQR_RXFRQ);

- /* Tx and RX FIFO configuration */
- if (stm32_port->fifoen) {
- val = readl_relaxed(port->membase + ofs->cr3);
- val &= ~(USART_CR3_TXFTCFG_MASK | USART_CR3_RXFTCFG_MASK);
- val |= USART_CR3_TXFTCFG_HALF << USART_CR3_TXFTCFG_SHIFT;
- val |= USART_CR3_RXFTCFG_HALF << USART_CR3_RXFTCFG_SHIFT;
- writel_relaxed(val, port->membase + ofs->cr3);
- }
-
- /* RX FIFO enabling */
+ /* RX enabling */
val = stm32_port->cr1_irq | USART_CR1_RE | BIT(cfg->uart_enable_bit);
- if (stm32_port->fifoen)
- val |= USART_CR1_FIFOEN;
stm32_usart_set_bits(port, ofs->cr1, val);

return 0;
@@ -770,9 +759,15 @@ static void stm32_usart_set_termios(struct uart_port *port,
if (stm32_port->fifoen)
cr1 |= USART_CR1_FIFOEN;
cr2 = 0;
+
+ /* Tx and RX FIFO configuration */
cr3 = readl_relaxed(port->membase + ofs->cr3);
- cr3 &= USART_CR3_TXFTIE | USART_CR3_RXFTCFG_MASK | USART_CR3_RXFTIE
- | USART_CR3_TXFTCFG_MASK;
+ cr3 &= USART_CR3_TXFTIE | USART_CR3_RXFTIE;
+ if (stm32_port->fifoen) {
+ cr3 &= ~(USART_CR3_TXFTCFG_MASK | USART_CR3_RXFTCFG_MASK);
+ cr3 |= USART_CR3_TXFTCFG_HALF << USART_CR3_TXFTCFG_SHIFT;
+ cr3 |= USART_CR3_RXFTCFG_HALF << USART_CR3_RXFTCFG_SHIFT;
+ }

if (cflag & CSTOPB)
cr2 |= USART_CR2_STOP_2B;
--
2.17.1

2021-02-19 17:53:52

by Erwan Le Ray

[permalink] [raw]
Subject: [PATCH 11/13] serial: stm32: add FIFO flush when port is closed

Transmission complete error is sent when ISR_TC is not set. If port closure
is requested despite data in TDR / TX FIFO has not been sent (because of
flow control), ISR_TC is not set and error message is sent on port closure
but also when a new port is opened.

Flush the data when port is closed, so the error isn't printed twice upon
next port opening.

Fixes: 64c32eab6603 ("serial: stm32: Add support of TC bit status check")
Signed-off-by: Erwan Le Ray <[email protected]>

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 45203648172b..dcc309fd5d2f 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -691,6 +691,11 @@ static void stm32_usart_shutdown(struct uart_port *port)
if (ret)
dev_err(port->dev, "Transmission is not complete\n");

+ /* flush RX & TX FIFO */
+ if (ofs->rqr != UNDEF_REG)
+ writel_relaxed(USART_RQR_TXFRQ | USART_RQR_RXFRQ,
+ port->membase + ofs->rqr);
+
stm32_usart_clr_bits(port, ofs->cr1, val);

free_irq(port->irq, port);
--
2.17.1

2021-02-19 17:54:18

by Erwan Le Ray

[permalink] [raw]
Subject: [PATCH 01/13] serial: stm32: fix probe and remove order for dma

The probe and remove orders are wrong as the uart_port is registered
before saving device data in the probe, and unregistered after DMA
resource deallocation in the remove. uart_port registering should be
done at the end of probe and unregistering should be done at the begin of
remove to avoid resource allocation issues.

Fix probe and remove orders. This enforce resource allocation occur at
proper time.
Terminate both DMA rx and tx transfers before removing device.

Move pm_runtime after uart_remove_one_port() call in remove() to keep the
probe error path.

Fixes: 3489187204eb ("serial: stm32: adding dma support")
Signed-off-by: Erwan Le Ray <[email protected]>

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index dde6d526362d..c67029ebcac8 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -1255,10 +1255,6 @@ static int stm32_usart_serial_probe(struct platform_device *pdev)
device_set_wakeup_enable(&pdev->dev, false);
}

- ret = uart_add_one_port(&stm32_usart_driver, &stm32port->port);
- if (ret)
- goto err_wirq;
-
ret = stm32_usart_of_dma_rx_probe(stm32port, pdev);
if (ret)
dev_info(&pdev->dev, "interrupt mode used for rx (no dma)\n");
@@ -1272,11 +1268,40 @@ static int stm32_usart_serial_probe(struct platform_device *pdev)
pm_runtime_get_noresume(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
+
+ ret = uart_add_one_port(&stm32_usart_driver, &stm32port->port);
+ if (ret)
+ goto err_port;
+
pm_runtime_put_sync(&pdev->dev);

return 0;

-err_wirq:
+err_port:
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
+
+ if (stm32port->rx_ch) {
+ dmaengine_terminate_async(stm32port->rx_ch);
+ dma_release_channel(stm32port->rx_ch);
+ }
+
+ if (stm32port->rx_dma_buf)
+ dma_free_coherent(&pdev->dev,
+ RX_BUF_L, stm32port->rx_buf,
+ stm32port->rx_dma_buf);
+
+ if (stm32port->tx_ch) {
+ dmaengine_terminate_async(stm32port->tx_ch);
+ dma_release_channel(stm32port->tx_ch);
+ }
+
+ if (stm32port->tx_dma_buf)
+ dma_free_coherent(&pdev->dev,
+ TX_BUF_L, stm32port->tx_buf,
+ stm32port->tx_dma_buf);
+
if (stm32port->wakeirq > 0)
dev_pm_clear_wake_irq(&pdev->dev);

@@ -1298,11 +1323,20 @@ static int stm32_usart_serial_remove(struct platform_device *pdev)
int err;

pm_runtime_get_sync(&pdev->dev);
+ err = uart_remove_one_port(&stm32_usart_driver, port);
+ if (err)
+ return(err);
+
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);

stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_DMAR);

- if (stm32_port->rx_ch)
+ if (stm32_port->rx_ch) {
+ dmaengine_terminate_async(stm32_port->rx_ch);
dma_release_channel(stm32_port->rx_ch);
+ }

if (stm32_port->rx_dma_buf)
dma_free_coherent(&pdev->dev,
@@ -1311,8 +1345,10 @@ static int stm32_usart_serial_remove(struct platform_device *pdev)

stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_DMAT);

- if (stm32_port->tx_ch)
+ if (stm32_port->tx_ch) {
+ dmaengine_terminate_async(stm32_port->tx_ch);
dma_release_channel(stm32_port->tx_ch);
+ }

if (stm32_port->tx_dma_buf)
dma_free_coherent(&pdev->dev,
@@ -1326,12 +1362,7 @@ static int stm32_usart_serial_remove(struct platform_device *pdev)

stm32_usart_deinit_port(stm32_port);

- err = uart_remove_one_port(&stm32_usart_driver, port);
-
- pm_runtime_disable(&pdev->dev);
- pm_runtime_put_noidle(&pdev->dev);
-
- return err;
+ return 0;
}

#ifdef CONFIG_SERIAL_STM32_CONSOLE
--
2.17.1

2021-02-19 17:54:31

by Erwan Le Ray

[permalink] [raw]
Subject: [PATCH 09/13] serial: stm32: call stm32_transmit_chars locked

stm32_transmit_chars should be called under lock also in tx DMA callback.

Fixes: 3489187204eb ("serial: stm32: adding dma support")
Signed-off-by: Erwan Le Ray <[email protected]>
Signed-off-by: Fabrice Gasnier <[email protected]>

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 7da16d468b84..bdd7ca490021 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -291,13 +291,16 @@ static void stm32_usart_tx_dma_complete(void *arg)
struct uart_port *port = arg;
struct stm32_port *stm32port = to_stm32_port(port);
struct stm32_usart_offsets *ofs = &stm32port->info->ofs;
+ unsigned long flags;

dmaengine_terminate_async(stm32port->tx_ch);
stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_DMAT);
stm32port->tx_dma_busy = false;

/* Let's see if we have pending data to send */
+ spin_lock_irqsave(&port->lock, flags);
stm32_usart_transmit_chars(port);
+ spin_unlock_irqrestore(&port->lock, flags);
}

static void stm32_usart_tx_interrupt_enable(struct uart_port *port)
--
2.17.1

2021-02-19 17:54:50

by Erwan Le Ray

[permalink] [raw]
Subject: [PATCH 13/13] serial: stm32: add support for "flush_buffer" ops

Add the support for "flush_buffer" ops in order to flush any write buffers,
reset any DMA state and stop any ongoing DMA transfers when the
port->state->xmit circular buffer is cleared.

Signed-off-by: Erwan Le Ray <[email protected]>

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 114408f3892a..92836068e5ec 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -597,6 +597,19 @@ static void stm32_usart_start_tx(struct uart_port *port)
stm32_usart_transmit_chars(port);
}

+/* Flush the transmit buffer. */
+static void stm32_usart_flush_buffer(struct uart_port *port)
+{
+ struct stm32_port *stm32_port = to_stm32_port(port);
+ struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
+
+ if (stm32_port->tx_ch) {
+ dmaengine_terminate_async(stm32_port->tx_ch);
+ stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_DMAT);
+ stm32_port->tx_dma_busy = false;
+ }
+}
+
/* Throttle the remote when input buffer is about to overflow. */
static void stm32_usart_throttle(struct uart_port *port)
{
@@ -992,6 +1005,7 @@ static const struct uart_ops stm32_uart_ops = {
.break_ctl = stm32_usart_break_ctl,
.startup = stm32_usart_startup,
.shutdown = stm32_usart_shutdown,
+ .flush_buffer = stm32_usart_flush_buffer,
.set_termios = stm32_usart_set_termios,
.pm = stm32_usart_pm,
.type = stm32_usart_type,
--
2.17.1

2021-02-19 17:54:52

by Erwan Le Ray

[permalink] [raw]
Subject: [PATCH 10/13] serial: stm32: fix FIFO flush in startup and set_termios

Fifo flush set USART_RQR register by calling stm32_usart_set_bits
routine (Read/Modify/Write). USART_RQR register is a write only
register. So, read before write isn't correct / relevant to flush
the FIFOs.
Replace stm32_usart_set_bits call by writel_relaxed.

Fixes: 84872dc448fe ("serial: stm32: add RX and TX FIFO flush")
Signed-off-by: Erwan Le Ray <[email protected]>

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index bdd7ca490021..45203648172b 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -657,7 +657,7 @@ static int stm32_usart_startup(struct uart_port *port)

/* RX FIFO Flush */
if (ofs->rqr != UNDEF_REG)
- stm32_usart_set_bits(port, ofs->rqr, USART_RQR_RXFRQ);
+ writel_relaxed(USART_RQR_RXFRQ, port->membase + ofs->rqr);

/* RX enabling */
val = stm32_port->cr1_irq | USART_CR1_RE | BIT(cfg->uart_enable_bit);
@@ -762,8 +762,8 @@ static void stm32_usart_set_termios(struct uart_port *port,

/* flush RX & TX FIFO */
if (ofs->rqr != UNDEF_REG)
- stm32_usart_set_bits(port, ofs->rqr,
- USART_RQR_TXFRQ | USART_RQR_RXFRQ);
+ writel_relaxed(USART_RQR_TXFRQ | USART_RQR_RXFRQ,
+ port->membase + ofs->rqr);

cr1 = USART_CR1_TE | USART_CR1_RE;
if (stm32_port->fifoen)
--
2.17.1

2021-02-19 17:55:24

by Erwan Le Ray

[permalink] [raw]
Subject: [PATCH 08/13] serial: stm32: fix tx dma completion, release channel

This patch add a proper release of dma channels when completing dma tx.

Fixes: 3489187204eb ("serial: stm32: adding dma support")
Signed-off-by: Erwan Le Ray <[email protected]>

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 4ba164820904..7da16d468b84 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -292,6 +292,7 @@ static void stm32_usart_tx_dma_complete(void *arg)
struct stm32_port *stm32port = to_stm32_port(port);
struct stm32_usart_offsets *ofs = &stm32port->info->ofs;

+ dmaengine_terminate_async(stm32port->tx_ch);
stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_DMAT);
stm32port->tx_dma_busy = false;

--
2.17.1

2021-02-19 17:56:20

by Erwan Le Ray

[permalink] [raw]
Subject: [PATCH 12/13] serial: stm32: fix tx_empty condition

In "tx_empty", we should poll TC bit in both DMA and PIO modes (instead of
TXE) to check transmission data register has been transmitted independently
of the FIFO mode. TC indicates that both transmit register and shift
register are empty. When shift register is empty, tx_empty should return
TIOCSER_TEMT instead of TC value.

Cleans the USART_CR_TC TCCF register define (transmission complete clear
flag) as it is duplicate of USART_ICR_TCCF.

Fixes: 48a6092fb41f ("serial: stm32-usart: Add STM32 USART Driver")
Signed-off-by: Erwan Le Ray <[email protected]>

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index dcc309fd5d2f..114408f3892a 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -515,7 +515,10 @@ static unsigned int stm32_usart_tx_empty(struct uart_port *port)
struct stm32_port *stm32_port = to_stm32_port(port);
struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;

- return readl_relaxed(port->membase + ofs->isr) & USART_SR_TXE;
+ if (readl_relaxed(port->membase + ofs->isr) & USART_SR_TC)
+ return TIOCSER_TEMT;
+
+ return 0;
}

static void stm32_usart_set_mctrl(struct uart_port *port, unsigned int mctrl)
diff --git a/drivers/tty/serial/stm32-usart.h b/drivers/tty/serial/stm32-usart.h
index d4c916e78d40..e305462d0de6 100644
--- a/drivers/tty/serial/stm32-usart.h
+++ b/drivers/tty/serial/stm32-usart.h
@@ -127,9 +127,6 @@ struct stm32_usart_info stm32h7_info = {
/* Dummy bits */
#define USART_SR_DUMMY_RX BIT(16)

-/* USART_ICR (F7) */
-#define USART_CR_TC BIT(6)
-
/* USART_DR */
#define USART_DR_MASK GENMASK(8, 0)

--
2.17.1

2021-03-04 12:43:28

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 00/13] stm32 usart various fixes

On Fri, Feb 19, 2021 at 06:47:23PM +0100, Erwan Le Ray wrote:
> This series brings various fixes to stm32-usart driver.
>
> Erwan Le Ray (13):
> serial: stm32: fix probe and remove order for dma
> serial: stm32: fix startup by enabling usart for reception
> serial: stm32: fix incorrect characters on console
> serial: stm32: fix TX and RX FIFO thresholds
> serial: stm32: fix a deadlock condition with wakeup event
> serial: stm32: fix wake-up flag handling
> serial: stm32: fix a deadlock in set_termios
> serial: stm32: fix tx dma completion, release channel
> serial: stm32: call stm32_transmit_chars locked
> serial: stm32: fix FIFO flush in startup and set_termios
> serial: stm32: add FIFO flush when port is closed
> serial: stm32: fix tx_empty condition
> serial: stm32: add support for "flush_buffer" ops
>
> drivers/tty/serial/stm32-usart.c | 198 +++++++++++++++++++++----------
> drivers/tty/serial/stm32-usart.h | 3 -
> 2 files changed, 135 insertions(+), 66 deletions(-)

This series does not apply cleanly to my tree. Can you rebase it
against 5.12-rc1 and resend?

thanks,

greg k-h