Hi Ulrich-san, > From: Ulrich Hecht, Sent: Saturday, April 10, 2021 1:05 AM > > > On 04/09/2021 2:16 PM Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: <snip> > > So, almost all comments was not needed. > > > > Also, I'm wondering if the following condition is not needed or not. > > This is because this was "20ms". But, perhaps, the driver will not > > set rx_timeout to "20us" or small. > > > > if (s->rx_timeout < 20) > > s->rx_timeout = 20; > > A more helpful version of the comment is in 3089f381fbaf5: > > /* > * Calculate delay for 1.5 DMA buffers: see > * drivers/serial/serial_core.c::uart_update_timeout(). With 10 bits > * (CS8), 250Hz, 115200 baud and 64 bytes FIFO, the above function > * calculates 1 jiffie for the data plus 5 jiffies for the "slop(e)." > * Then below we calculate 3 jiffies (12ms) for 1.5 DMA buffers (3 FIFO > * sizes), but it has been found out experimentally, that this is not > * enough: the driver too often needlessly runs on a DMA timeout. 20ms > * as a minimum seem to work perfectly. > */ > > I think we still want that, but it should of course be 20000, not 20. Hmm, when we use HSCIF with 10 bits, 3000000 baud and 128 bytes FIFO, the rx_timeout value will be set to 1536 (us). So, if we set rx_timeout to 20000 (us) as a minimum value, the sh-sci' behavior will be back to non hrtimer support, IIUC. Perhaps, describing uart_update_timeout() and the jiffies value of uart_port->timeout with 115200 baud here may cause misreading?? I didn't understand the purpose of uart_port->timeout yet thought. But, at least, the current driver uses hrtimer to improve latency for HSCIF, the driver should not set 20000 (us) as a minimum value. Best regards, Yoshihiro Shimoda
