Re: Serial: UART_BUG_TXEN race conditions

["linux-os \(Dick Johnson\)" <linux-os@analogic.com>]

On Tue, 27 Jun 2006, Ingo van Lil wrote:

> "linux-os (Dick Johnson)" <linux-os@analogic.com> 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.

The sneaky things are other bits being set after you read them.
Look under 'Interrupt Reset Control': Reading IIR register or
writing to the transmitter holding register. You read status,
then do something else because the THRE bit wasn't set yet. It
is now set just before you do something else, but you don't get
another edge interrupt because the IRQ line never moved. It's
always high, no more interrupts ... ever. This problem wouldn't
exist with level interrupts but the 8250/16450 doesn't use level
interrupts.

These problems are why you sometimes see the UART ISR code
being called from a timer-tick interrupt.

Whatever you do, you must completely satisfy the interrupt
condition in the ISR, or your code will never get the CPU back!
All the bits that could cause interrupts must be reset before
you return from your routine.

>
> 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.

Yep.

> 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
>

Cheers,
Dick Johnson
Penguin : Linux version 2.6.16.4 on an i686 machine (5592.71 BogoMips).
New book: http://www.AbominableFirebug.com/
_


****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

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