From: Erwan Le Ray <hidden> Date: 2021-02-19 17:48:56
Incorrect characters are observed on console during boot. This issue occurs
when init/main.c is modifying termios settings to open /dev/console on the
rootfs.
This patch adds a waiting loop in set_termios to wait for TX shift register
empty (and TX FIFO if any) before stopping serial port.
Fixes: 48a6092fb41f ("serial: stm32-usart: Add STM32 USART Driver")
Signed-off-by: Erwan Le Ray <redacted>
@@ -748,6 +749,15 @@ static void stm32_usart_set_termios(struct uart_port *port,spin_lock_irqsave(&port->lock,flags);+ret=readl_relaxed_poll_timeout_atomic(port->membase+ofs->isr,+isr,+(isr&USART_SR_TC),+10,100000);++/* Send the TC error message only when ISR_TC is not set. */+if(ret)+dev_err(port->dev,"Transmission is not complete\n");+/* Stop serial port and reset value */writel_relaxed(0,port->membase+ofs->cr1);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:49:24
Deadlock issue is seen when enabling CONFIG_PROVE_LOCKING=Y, and uart
console as wakeup source. Deadlock occurs when resuming from low power
mode if system is waked up via usart console.
The deadlock is triggered 100% when also disabling console suspend prior
to go to suspend.
Simplified call stack, deadlock condition:
- stm32_console_write <-- spin_lock already held
- print_circular_bug
- pm_wakeup_dev_event <-- triggers lockdep as seen above
- stm32_receive_chars
- stm32_interrupt <-- wakeup via uart console, takes the lock
So, revisit spin_lock in stm32-usart driver:
- there is no need to hold the lock to access ICR (atomic clear of status
flags)
- only hold the lock inside stm32_receive_chars() routine (no need to
call pm_wakeup_dev_event with lock held)
- keep stm32_transmit_chars() routine called with lock held
Fixes: 48a6092fb41f ("serial: stm32-usart: Add STM32 USART Driver")
Signed-off-by: Erwan Le Ray <redacted>
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:49:25
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 <redacted>
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:49:25
RX is configured, but usart is not enabled in startup function.
Kernel documentation specifies that startup should enable the port for
reception.
Fix the startup by enabling usart for reception.
Fixes: 84872dc448fe ("serial: stm32: add RX and TX FIFO flush")
Signed-off-by: Erwan Le Ray <redacted>
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:50:10
This patch fixes several issue with wake-up handling:
- the WUF irq is handled several times at wake-up
- the USART is disabled / enabled at suspend to set wake-up flag.
It can cause glitches during RX.
This patch fix those issues:
- clear wake-up flag and disable wake-up irq in WUF irq handling
- enable wake-up from low power on start bit detection at port
configuration
- Unmask the wake-up flag irq at suspend and mask it at resume
In addition, pm_wakeup_event handling is moved from receice_chars to WUF
irq handling.
Fixes: 270e5a74fe4c ("serial: stm32: add wakeup mechanism")
Signed-off-by: Erwan Le Ray <redacted>
@@ -473,9 +471,14 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)writel_relaxed(USART_ICR_RTOCF,port->membase+ofs->icr);-if((sr&USART_SR_WUF)&&ofs->icr!=UNDEF_REG)+if((sr&USART_SR_WUF)&&ofs->icr!=UNDEF_REG){+/* Clear wake up flag and disable wake up interrupt */writel_relaxed(USART_ICR_WUCF,port->membase+ofs->icr);+stm32_usart_clr_bits(port,ofs->cr3,USART_CR3_WUFIE);+if(irqd_is_wakeup_set(irq_get_irq_data(port->irq)))+pm_wakeup_event(tport->tty->dev,0);+}if((sr&USART_SR_RXNE)&&!(stm32_port->rx_ch))stm32_usart_receive_chars(port,false);
@@ -901,6 +904,12 @@ static void stm32_usart_set_termios(struct uart_port *port,cr1&=~(USART_CR1_DEDT_MASK|USART_CR1_DEAT_MASK);}+/* Configure wake up from low power on start bit detection */+if(stm32_port->wakeirq>0){+cr3&=~USART_CR3_WUS_MASK;+cr3|=USART_CR3_WUS_START_BIT;+}+writel_relaxed(cr3,port->membase+ofs->cr3);writel_relaxed(cr2,port->membase+ofs->cr2);writel_relaxed(cr1,port->membase+ofs->cr1);
@@ -1479,23 +1488,20 @@ static void __maybe_unused stm32_usart_serial_en_wakeup(struct uart_port *port,{structstm32_port*stm32_port=to_stm32_port(port);structstm32_usart_offsets*ofs=&stm32_port->info->ofs;-structstm32_usart_config*cfg=&stm32_port->info->cfg;-u32val;if(stm32_port->wakeirq<=0)return;+/*+*Enablelow-powerwake-upandwake-upirqifargumentissetto+*"enable",disablelow-powerwake-upandwake-upirqotherwise+*/if(enable){-stm32_usart_clr_bits(port,ofs->cr1,BIT(cfg->uart_enable_bit));stm32_usart_set_bits(port,ofs->cr1,USART_CR1_UESM);-val=readl_relaxed(port->membase+ofs->cr3);-val&=~USART_CR3_WUS_MASK;-/* Enable Wake up interrupt from low power on start bit */-val|=USART_CR3_WUS_START_BIT|USART_CR3_WUFIE;-writel_relaxed(val,port->membase+ofs->cr3);-stm32_usart_set_bits(port,ofs->cr1,BIT(cfg->uart_enable_bit));+stm32_usart_set_bits(port,ofs->cr3,USART_CR3_WUFIE);}else{stm32_usart_clr_bits(port,ofs->cr1,USART_CR1_UESM);+stm32_usart_clr_bits(port,ofs->cr3,USART_CR3_WUFIE);}}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:50:57
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 <redacted>
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
@@ -291,13 +291,16 @@ static void stm32_usart_tx_dma_complete(void *arg)structuart_port*port=arg;structstm32_port*stm32port=to_stm32_port(port);structstm32_usart_offsets*ofs=&stm32port->info->ofs;+unsignedlongflags;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);}staticvoidstm32_usart_tx_interrupt_enable(structuart_port*port)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:50:57
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 <redacted>
@@ -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)-gotoerr_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)+gotoerr_port;+pm_runtime_put_sync(&pdev->dev);return0;-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)interr;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);--returnerr;+return0;}#ifdef CONFIG_SERIAL_STM32_CONSOLE
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:51:25
CTS/RTS GPIOs support that has been added recently to STM32 UART driver has
introduced scheduled code in a set_termios part protected by a spin lock.
This generates a potential deadlock scenario:
Chain exists of:
&irq_desc_lock_class --> console_owner --> &port_lock_key
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&port_lock_key);
lock(console_owner);
lock(&port_lock_key);
lock(&irq_desc_lock_class);
*** DEADLOCK ***
4 locks held by stty/766:
Move the scheduled code after the spinlock.
Fixes: 6cf61b9bd7cc ("tty: serial: Add modem control gpio support for STM32 UART")
Signed-off-by: Erwan Le Ray <redacted>
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:51:53
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 <redacted>
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:52:18
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 <redacted>
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:52:24
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 <redacted>
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:52:24
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 <redacted>
From: Erwan Le Ray <hidden> Date: 2021-02-19 17:52:50
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 <redacted>
@@ -597,6 +597,19 @@ static void stm32_usart_start_tx(struct uart_port *port)stm32_usart_transmit_chars(port);}+/* Flush the transmit buffer. */+staticvoidstm32_usart_flush_buffer(structuart_port*port)+{+structstm32_port*stm32_port=to_stm32_port(port);+structstm32_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. */staticvoidstm32_usart_throttle(structuart_port*port){
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel