Hi, Jiri, On 08.11.2024 14:19, Claudiu Beznea wrote: >>> @@ -885,6 +887,7 @@ static void sci_transmit_chars(struct uart_port *port) >>> } >>> sci_serial_out(port, SCxTDR, c); >>> + s->first_time_tx = true; >>> port->icount.tx++; >>> } while (--count > 0); >>> @@ -1241,6 +1244,8 @@ static void sci_dma_tx_complete(void *arg) >>> if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS) >>> uart_write_wakeup(port); >>> + s->first_time_tx = true; >> This is too late IMO. The first in-flight dma won't be accounted in >> sci_tx_empty(). From DMA submit up to now. > If it's in-flight we can't determine it's status anyway with one variable. > We can set this variable later but it wouldn't tell the truth as the TX > might be in progress anyway or may have been finished? > > The hardware might help with this though the TEND bit. According to the HW > manual, the TEND bit has the following meaning: > > 0: Transmission is in the waiting state or in progress. > 1: Transmission is completed. > > But the problem, from my point of view, is that the 0 has double meaning. > > I noticed the tx_empty() is called in kernel multiple times before > declaring TX is empty or not. E.g., uart_suspend_port() call it 3 times, > uart_wait_until_sent() call it in a while () look with a timeout. There is > the uart_ioctl() which calls it though uart_get_lsr_info() only one time > but I presumed the user space might implement the same multiple trials > approach before declaring it empty. > > Because of this I considered it wouldn't be harmful for the scenario you > described "The first in-flight dma won't be accounted in sci_tx_empty()" > as the user may try again later to check the status. For this reason I also > chose to have no extra locking around this variable. > > Please let me know if you consider otherwise. With the above explanation, can you please let me know if you still consider I should change the approach for this patch? Thank you, Claudiu Beznea
