8250_core.c

This was generated from 3.14 kernel, but since its so small it will likely apply to newer kernels without issue.

A lesser travelled code path specifically crafted for "tx interrupt buggy" UARTs seems to be testing the wrong bit for whether or not to prime the transmit pump. The result is that the tx interrupt never occurs on one chip I am using that falls into the UART_BUG_TXEN category as I understand it.

The 16550 data sheet even names the interrupt "THRE" and not transmit shift register empty. The interrupt is fired when the holding register, or its logical equivalent, the fifo, goes empty; not when the shift register goes empty. This is a code bug in code hoping to fix a bug in hardware.

The reason this is so late coming to the surface is that lately the 8250 interrupt handler polls for all kinds of special status beyond the intention of the hardware's purpose specific interrupt. Therefore so long as you were in the interrupt handler (even if for purposes of servicing a recv interrupt, you could recover from the broken transmit chain. But if you are only transmitting, not receiving, then this bug can manifest itself.

This is the fix. It is harmless because so long as the fifo is empty, it should be legal to re-fill it fully anyways.

=== modified file 'drivers/tty/serial/8250/8250_core.c'

@@ -1302,11 +1302,15 @@ static void serial8250_start_tx(struct u

		up->ier |= UART_IER_THRI;
		serial_port_out(port, UART_IER, up->ier);

+ /*
+ * If setting UART_IER_THRI does not cause an
+ * immediate tx interrupt
+ */

		if (up->bugs & UART_BUG_TXEN) {
			unsigned char lsr;
			lsr = serial_in(up, UART_LSR);
			up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
-			if (lsr & UART_LSR_TEMT)
+			if (lsr & UART_LSR_THRE)
				serial8250_tx_chars(up);
		}
	}

Hi,

Thanks for contributing.

It is the custom that you send your patches in the body of the e-mail instead of as an attachment. You might want to try using git send-email, or at least have a look at other patch submissions here.

You may also want to make sure that you have a proper subject line to your patch, like

8250_core: fix buggy interrupts bit test

A lesser travelled codepath...

I'm sure you can come up with a better one though.

Before you resubmit, you might want to run scripts/checkpatch.pl on your patch. If you resubmit, you should probably also put [email protected] and Greg Kroah-Hartman <[email protected]> in CC. For a full list of suggestions, check scripts/get_maintainer.pl.

Good luck,
Frans

[ +cc linux-serial, Greg, Frans ]

Hi Dick,

Thanks for finding this.

In addition to Frans' comments, I've expanded the patch inline for further comment (which is why attached patches are of limited value).

What UART is this?

Please include a description of why this test is broken. For example,

"So, if the fifo is already empty but the shifter is still transmitting, no further THRE interrupt will occur, and tx will stall."

The remainder of the commit message can be dropped (except Signed-off-by).

		/* Prime tx pump for buggy UARTs */