[PATCH v1 0/1] Mediatek uart patch

[PATCH v1 0/1] Mediatek uart patch

[Zhiyong Tao]

This series includes 1 patches:
1.fix uart corruption issue when rx power off

when uart is used as a communication port with external device(GPS).
when external device(GPS) power off, the power of rx pin is also from
1.8v to 0v. Even if there is not any data in rx. But uart rx pin can
capture the data "0".
If uart don't receive any data in specified cycle, uart will generates
BI(Break interrupt) interrupt.
If external device(GPS) power off, we found that BI interrupt appeared
continuously and very frequently.
When uart interrupt type is BI, uart IRQ handler(8250 framwork
API:serial8250_handle_irq) will push data to tty buffer.
The code path:
https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/8250/8250_port.c#L1917
mtk8250_dma_rx_complete is a task of mtk_uart_apdma_rx_handler.
mtk8250_dma_rx_complete priority is lower than uart irq
handler(serial8250_handle_irq).
if we are in process of mtk8250_dma_rx_complete, uart appear BI
interrupt:1)serial8250_handle_irq will priority execution.2)it may cause
write tty buffer conflict in mtk8250_dma_rx_complete.
So the spin lock protect the rx receive data process is not break.

Changes in patch v1:
1. remove processing mechanism which count value is 0.
2. change patch title and commit message.
3. explain the detailed reason for the patch in changelog.

Zhiyong Tao (1):
  serial: 8250_mtk: fix uart corruption issue when rx power off

 drivers/tty/serial/8250/8250_mtk.c | 5 +++++
 1 file changed, 5 insertions(+)

--
2.18.0

[PATCH v1] serial: 8250_mtk: fix uart corruption issue when rx power off

[Zhiyong Tao]

Fix uart corruption issue when rx power off.
Add spin lock in mtk8250_dma_rx_complete function in APDMA mode.

Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
---
 drivers/tty/serial/8250/8250_mtk.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index f7d3023f860f..fb65dc601b23 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -93,10 +93,13 @@ static void mtk8250_dma_rx_complete(void *param)
 	struct dma_tx_state state;
 	int copied, total, cnt;
 	unsigned char *ptr;
+	unsigned long flags;
 
 	if (data->rx_status == DMA_RX_SHUTDOWN)
 		return;
 
+	spin_lock_irqsave(&up->port.lock, flags);
+
 	dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
 	total = dma->rx_size - state.residue;
 	cnt = total;
@@ -120,6 +123,8 @@ static void mtk8250_dma_rx_complete(void *param)
 	tty_flip_buffer_push(tty_port);
 
 	mtk8250_rx_dma(up);
+
+	spin_unlock_irqrestore(&up->port.lock, flags);
 }
 
 static void mtk8250_rx_dma(struct uart_8250_port *up)
-- 
2.18.0

Re: [PATCH v1 0/1] Mediatek uart patch

[Greg KH]

On Thu, Jul 29, 2021 at 09:48:16AM +0800, Zhiyong Tao wrote:
> This series includes 1 patches:
> 1.fix uart corruption issue when rx power off
> 
> when uart is used as a communication port with external device(GPS).
> when external device(GPS) power off, the power of rx pin is also from
> 1.8v to 0v. Even if there is not any data in rx. But uart rx pin can
> capture the data "0".
> If uart don't receive any data in specified cycle, uart will generates
> BI(Break interrupt) interrupt.
> If external device(GPS) power off, we found that BI interrupt appeared
> continuously and very frequently.
> When uart interrupt type is BI, uart IRQ handler(8250 framwork
> API:serial8250_handle_irq) will push data to tty buffer.
> The code path:
> https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/8250/8250_port.c#L1917
> mtk8250_dma_rx_complete is a task of mtk_uart_apdma_rx_handler.
> mtk8250_dma_rx_complete priority is lower than uart irq
> handler(serial8250_handle_irq).
> if we are in process of mtk8250_dma_rx_complete, uart appear BI
> interrupt:1)serial8250_handle_irq will priority execution.2)it may cause
> write tty buffer conflict in mtk8250_dma_rx_complete.
> So the spin lock protect the rx receive data process is not break.

All of this information should be in the changelog for the patch itself.
There is no need for a "cover letter" for a single patch like this.

Can you redo your 1/1 patch and add the above information to the
changelog text and resend it as a v2?

thanks,

greg k-h