Re: git: 0d2fd5b99c95 - main - ns8250: use LSR_THRE instead of LSR_TEMT for checking tx flush
Date: Tue, 20 May 2025 21:12:11 UTC
On 20/05/2025 21:28, Michal Meloun wrote: > > > On 20.05.2025 16:57, Andriy Gapon wrote: >> The branch main has been updated by avg: >> >> URL: https://cgit.FreeBSD.org/src/commit/? >> id=0d2fd5b99c95329085d0700a4dd38507a054a50d >> >> commit 0d2fd5b99c95329085d0700a4dd38507a054a50d >> Author: Andriy Gapon <avg@FreeBSD.org> >> AuthorDate: 2024-11-10 11:15:30 +0000 >> Commit: Andriy Gapon <avg@FreeBSD.org> >> CommitDate: 2025-05-20 14:55:18 +0000 >> >> ns8250: use LSR_THRE instead of LSR_TEMT for checking tx flush >> LSR_TEMT bit is set if both transmit hold and shift registers are >> empty, but the flush command flushes only the hold register. > I don't think that's true. I am not sure to which part of the commit message your "that" refers to, so I'll try to justify everything. T_H_R_E - transmitter holding register empty T_EMPT - transmitter empty All hardware documentation that I have around describes those bits like that. We do not have direct control over the shift register, hardware clears it after sending. > Imho, ns8250_flush() is used also before changing > baud rate, so we need to ensure that all bits are flushed, including the > transmit register. That's an interesting point. My intention was actually to avoid bogus "FCR is broken" message which can happen because of a race between the UART transmission and code execution. I think that LSR_THRE is proper for checking that FCR works. But to actually detect and ensure that all transmission has completed we should use LSR_TEMT like you say. At the same time, this UART flush is not like stdout flush, of course, where we ensure that all buffered data is transmitted. For UART, we just clear the FIFO and the holding register. So, I am not sure if polling for empty transmitter is important. Besides, I do not see the code which would flush transmitter when parameters are changing. I can find only two places where UART_FLUSH_TRANSMITTER is passed: - ns8250_bus_attach - ns8250_bus_probe Additionally, ns8250_bus_transmit uses ns8250_drain(UART_DRAIN_TRANSMITTER) in broken_txfifo case. P.S. Maybe I don't understand the code, but UART_FLUSH_RECEIVER in ns8250_bus_attach looks strange to me. It's one thing to flush data while in the loop-back mode, but I think that in ns8250_bus_attach the hardware is fully set up to receive data from the outside world. So, how can we hope to drain all of it and to reliably detect whether FIFO flushing works. I mean that something on the other end could be continuously transmitting. -- Andriy Gapon