Skip to content

Commit 133f4c0

Browse files
hvilleneuvedoogregkh
authored andcommitted
serial: sc16is7xx: fix TX fifo corruption
Sometimes, when a packet is received on channel A at almost the same time as a packet is about to be transmitted on channel B, we observe with a logic analyzer that the received packet on channel A is transmitted on channel B. In other words, the Tx buffer data on channel B is corrupted with data from channel A. The problem appeared since commit 4409df5 ("serial: sc16is7xx: change EFR lock to operate on each channels"), which changed the EFR locking to operate on each channel instead of chip-wise. This commit has introduced a regression, because the EFR lock is used not only to protect the EFR registers access, but also, in a very obscure and undocumented way, to protect access to the data buffer, which is shared by the Tx and Rx handlers, but also by each channel of the IC. Fix this regression first by switching to kfifo_out_linear_ptr() in sc16is7xx_handle_tx() to eliminate the need for a shared Rx/Tx buffer. Secondly, replace the chip-wise Rx buffer with a separate Rx buffer for each channel. Fixes: 4409df5 ("serial: sc16is7xx: change EFR lock to operate on each channels") Cc: stable@vger.kernel.org Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> Link: https://lore.kernel.org/r/20240723125302.1305372-2-hugo@hugovil.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 6eabce6 commit 133f4c0

File tree

1 file changed

+11
-10
lines changed

1 file changed

+11
-10
lines changed

drivers/tty/serial/sc16is7xx.c

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ struct sc16is7xx_one {
327327
struct kthread_work reg_work;
328328
struct kthread_delayed_work ms_work;
329329
struct sc16is7xx_one_config config;
330+
unsigned char buf[SC16IS7XX_FIFO_SIZE]; /* Rx buffer. */
330331
unsigned int old_mctrl;
331332
u8 old_lcr; /* Value before EFR access. */
332333
bool irda_mode;
@@ -340,7 +341,6 @@ struct sc16is7xx_port {
340341
unsigned long gpio_valid_mask;
341342
#endif
342343
u8 mctrl_mask;
343-
unsigned char buf[SC16IS7XX_FIFO_SIZE];
344344
struct kthread_worker kworker;
345345
struct task_struct *kworker_task;
346346
struct sc16is7xx_one p[];
@@ -612,18 +612,18 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud)
612612
static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen,
613613
unsigned int iir)
614614
{
615-
struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
615+
struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
616616
unsigned int lsr = 0, bytes_read, i;
617617
bool read_lsr = (iir == SC16IS7XX_IIR_RLSE_SRC) ? true : false;
618618
u8 ch, flag;
619619

620-
if (unlikely(rxlen >= sizeof(s->buf))) {
620+
if (unlikely(rxlen >= sizeof(one->buf))) {
621621
dev_warn_ratelimited(port->dev,
622622
"ttySC%i: Possible RX FIFO overrun: %d\n",
623623
port->line, rxlen);
624624
port->icount.buf_overrun++;
625625
/* Ensure sanity of RX level */
626-
rxlen = sizeof(s->buf);
626+
rxlen = sizeof(one->buf);
627627
}
628628

629629
while (rxlen) {
@@ -636,10 +636,10 @@ static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen,
636636
lsr = 0;
637637

638638
if (read_lsr) {
639-
s->buf[0] = sc16is7xx_port_read(port, SC16IS7XX_RHR_REG);
639+
one->buf[0] = sc16is7xx_port_read(port, SC16IS7XX_RHR_REG);
640640
bytes_read = 1;
641641
} else {
642-
sc16is7xx_fifo_read(port, s->buf, rxlen);
642+
sc16is7xx_fifo_read(port, one->buf, rxlen);
643643
bytes_read = rxlen;
644644
}
645645

@@ -672,7 +672,7 @@ static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen,
672672
}
673673

674674
for (i = 0; i < bytes_read; ++i) {
675-
ch = s->buf[i];
675+
ch = one->buf[i];
676676
if (uart_handle_sysrq_char(port, ch))
677677
continue;
678678

@@ -690,10 +690,10 @@ static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen,
690690

691691
static void sc16is7xx_handle_tx(struct uart_port *port)
692692
{
693-
struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
694693
struct tty_port *tport = &port->state->port;
695694
unsigned long flags;
696695
unsigned int txlen;
696+
unsigned char *tail;
697697

698698
if (unlikely(port->x_char)) {
699699
sc16is7xx_port_write(port, SC16IS7XX_THR_REG, port->x_char);
@@ -718,8 +718,9 @@ static void sc16is7xx_handle_tx(struct uart_port *port)
718718
txlen = 0;
719719
}
720720

721-
txlen = uart_fifo_out(port, s->buf, txlen);
722-
sc16is7xx_fifo_write(port, s->buf, txlen);
721+
txlen = kfifo_out_linear_ptr(&tport->xmit_fifo, &tail, txlen);
722+
sc16is7xx_fifo_write(port, tail, txlen);
723+
uart_xmit_advance(port, txlen);
723724

724725
uart_port_lock_irqsave(port, &flags);
725726
if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS)

0 commit comments

Comments
 (0)