On Thu, Mar 26, 2020 at 12:14:19AM +0100, Heiko Stuebner wrote:
> Some 8250 ports have a TEMT interrupt but it's not a part of the 8250
> standard, instead only available on some implementations.
> 
> The current em485 implementation does not work on ports without it.
> The only chance to make it work is to loop-read on LSR register.
> 
> So add UART_CAP_TEMT to mark 8250 uarts having this interrupt,
> update all current em485 users with that capability and make
> the stop_tx function loop-read on uarts not having it.

Just to get a better understanding:  According to the Dw_apb_uart_db.pdf
databook I've found, the UART does have a "THR empty" interrupt.  So you
get an interrupt once the Transmit Holding Register (and by consequence
the FIFO) has been drained.  Then what do you need a TEMT interrupt for?
Why is the THR interrupt not sufficient?


> @@ -1529,11 +1535,22 @@ static inline void __stop_tx(struct uart_8250_port *p)
>               /*
>                * To provide required timeing and allow FIFO transfer,
>                * __stop_tx_rs485() must be called only when both FIFO and
> -              * shift register are empty. It is for device driver to enable
> -              * interrupt on TEMT.
> +              * shift register are empty. If 8250 port supports it,
> +              * it is for device driver to enable interrupt on TEMT.
> +              * Otherwise must loop-read until TEMT and THRE flags are set.
>                */
> -             if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
> -                     return;
> +             if (p->capabilities & UART_CAP_TEMT) {
> +                     if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
> +                             return;
> +             } else {
> +                     int lsr;
> +
> +                     if (readx_poll_timeout(__get_lsr, p, lsr,
> +                                     (lsr & BOTH_EMPTY) == BOTH_EMPTY,
> +                                     0, 10000) < 0)
> +                             pr_warn("%s: timeout waiting for fifos to 
> empty\n",
> +                                     p->port.name);
> +             }

Do you actually need to check for the timeout?  How could this happen?
Only if some other part of the driver would disable the transmitter
I guess, which would be a bug.

Also, note that __stop_tx() may be called from hardirq context via
serial8250_tx_chars().  If the baudrate is low, you may spin for a
fairly long time in IRQ context.  E.g. with 9600 8N1, it takes about
1 msec for one char to transmit.

Thanks,

Lukas

Reply via email to