lkml.org 
[lkml]   [2020]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v2 4/7] serial: 8250: Handle implementations not having TEMT interrupt using em485
    Date
    Hi Lukas,

    Am Samstag, 2. Mai 2020, 15:49:27 CEST schrieb Lukas Wunner:
    > On Thu, Mar 26, 2020 at 12:14:19AM +0100, Heiko Stuebner wrote:
    > > @@ -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.

    Checking for a timeout was strongly suggested in v1 ;-)


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

    I did play around with different baud rates and data amounts today
    and even ran into the timeout with the current 10ms when doing a
    "dmesg > /dev/ttyS3" ... combined with the hardirq issue you mentioned
    I think I found a slightly better variant to do this ... by catching the first
    100us in the interrupt handler and otherwise re-using the existing
    stop-timer infrastructure to move this out of the actual __stop_tx function.

    I've sent a v3 based on your new series just now ... if you find time
    please have a look :-)

    Thanks
    Heiko


    \
     
     \ /
      Last update: 2020-05-18 00:02    [W:4.212 / U:0.408 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site