Skip to content

Commit 99277ee

Browse files
Phil Elwellpopcornmix
Phil Elwell
authored andcommitted
tty: amba-pl011: Make TX optimisation conditional
pl011_tx_chars takes a "from_irq" parameter to reduce the number of register accesses. When from_irq is true the function assumes that the FIFO is half empty and writes up to half a FIFO's worth of bytes without polling the FIFO status register, the reasoning being that the function is being called as a result of the TX interrupt being raised. This logic would work were it not for the fact that pl011_rx_chars, called from pl011_int before pl011_tx_chars, releases the spinlock before calling tty_flip_buffer_push. A user thread writing to the UART claims the spinlock and ultimately calls pl011_tx_chars with from_irq set to false. This reverts to the older logic that polls the FIFO status register before sending every byte. If this happen on an SMP system during the section of the IRQ handler where the spinlock has been released, then by the time the TX interrupt handler is called, the FIFO may already be full, and any further writes are likely to be lost. The fix involves adding a per-port flag that is true iff running from within the interrupt handler and the spinlock has not yet been released. This flag is then used as the value for the from_irq parameter of pl011_tx_chars, causing polling to be used in the unsafe case. Fixes: 1e84d22 ("serial/amba-pl011: Refactor and simplify TX FIFO handling") Signed-off-by: Phil Elwell <[email protected]>
1 parent 47ba524 commit 99277ee

File tree

1 file changed

+6
-1
lines changed

1 file changed

+6
-1
lines changed

drivers/tty/serial/amba-pl011.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ struct uart_amba_port {
270270
unsigned int old_cr; /* state during shutdown */
271271
unsigned int fixed_baud; /* vendor-set fixed baud rate */
272272
char type[12];
273+
bool irq_locked; /* in irq, unreleased lock */
273274
#ifdef CONFIG_DMA_ENGINE
274275
/* DMA stuff */
275276
bool using_tx_dma;
@@ -814,6 +815,7 @@ __acquires(&uap->port.lock)
814815
return;
815816

816817
/* Avoid deadlock with the DMA engine callback */
818+
uap->irq_locked = 0;
817819
spin_unlock(&uap->port.lock);
818820
dmaengine_terminate_all(uap->dmatx.chan);
819821
spin_lock(&uap->port.lock);
@@ -941,6 +943,7 @@ static void pl011_dma_rx_chars(struct uart_amba_port *uap,
941943
fifotaken = pl011_fifo_to_tty(uap);
942944
}
943945

946+
uap->irq_locked = 0;
944947
spin_unlock(&uap->port.lock);
945948
dev_vdbg(uap->port.dev,
946949
"Took %d chars from DMA buffer and %d chars from the FIFO\n",
@@ -1349,6 +1352,7 @@ __acquires(&uap->port.lock)
13491352
{
13501353
pl011_fifo_to_tty(uap);
13511354

1355+
uap->irq_locked = 0;
13521356
spin_unlock(&uap->port.lock);
13531357
tty_flip_buffer_push(&uap->port.state->port);
13541358
/*
@@ -1484,6 +1488,7 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
14841488
int handled = 0;
14851489

14861490
spin_lock_irqsave(&uap->port.lock, flags);
1491+
uap->irq_locked = 1;
14871492
status = pl011_read(uap, REG_RIS) & uap->im;
14881493
if (status) {
14891494
do {
@@ -1503,7 +1508,7 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
15031508
UART011_CTSMIS|UART011_RIMIS))
15041509
pl011_modem_status(uap);
15051510
if (status & UART011_TXIS)
1506-
pl011_tx_chars(uap, true);
1511+
pl011_tx_chars(uap, uap->irq_locked);
15071512

15081513
if (pass_counter-- == 0)
15091514
break;

0 commit comments

Comments
 (0)