Re: Serial: UART_BUG_TXEN race conditions

[Ingo van Lil]

"linux-os (Dick Johnson)" <linux-os@xxxxxxxxxxxx> schrieb:

> A common problem with 8250 UART code I've seen is that
> writers often do this:
>
> 	clear interrupts (spinlock)
> 	read/check status
> 	do something
> 	enable interrupts (spin unlock)
>
> This 'seems' correct, but allows sneaky things to happen between
> when you read/check status and you actually handle it.

What "sneaky thinks" could possibly happen between there? Either the 
IIR read tells you that the TX FIFO is empty (in that case you can 
safely write data into it) or it will tell you something different. In 
the latter case another interrupt will be generated as soon as the FIFO 
becomes empty.

As far as I can see the main reason for all the UART confusion is the 
requirement that the interrupt condition is to be automatically cleared 
 by an IIR read, if (and only if) the indicated state ist THRE. This 
sounds like a particularly bad idea to me, but I'm not an expert.
Anyway, as the IIR read might have been performed by some other piece 
of code (e.g. serial8250_startup) the 8250 driver depends on the UART 
to trigger another interrupt as soon the THRE bit in the IER is set (by 
start_tx). The opencores.org UART behaves that way, and I guess that's 
what most controllers do, but some more exotic chips appear not to.

> To implement the above algorithm, the following has been successful.
[...]

This seems to be pretty much what I suggested yesterday: After adding 
new characters to the ring buffer, check whether the transmitter is 
idle and, if it is, write a first load of characters into the FIFO. 
This should be a safe for solution for all kinds of UARTs, and it would 
allow us to entirely drop the UART_BUG_TXEN detection.

I'm gonna have a look at the kernel patch submission guidelines and, 
unless somebody is quicker than me, propose a patch.

Cheers,
Ingo


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/