On 22.05.2025 4:42, Ravi Pokala wrote:
Additionally, ns8250_bus_transmit uses ns8250_drain(UART_DRAIN_TRANSMITTER) in
broken_txfifo case.
FWIW, I just had to enable the 'hw.broken_txfifo=1' workaround on physical
hardware; see [Bug 286703].
Thanks,
Ravi (rpokala@)
-----Original Message-----
From: <owner-src-committ...@freebsd.org <mailto:owner-src-committ...@freebsd.org>> on
behalf of Andriy Gapon <a...@freebsd.org <mailto:a...@freebsd.org>>
Date: Tuesday, May 20, 2025 at 17:12
To: <m...@freebsd.org <mailto:m...@freebsd.org>>, <src-committ...@freebsd.org
<mailto:src-committ...@freebsd.org>>, <dev-commits-src-...@freebsd.org
<mailto:dev-commits-src-...@freebsd.org>>, <dev-commits-src-main@FreeBSD.org
<mailto:dev-commits-src-main@FreeBSD.org>>
Subject: Re: git: 0d2fd5b99c95 - main - ns8250: use LSR_THRE instead of
LSR_TEMT for checking tx flush
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/?
<https://cgit.FreeBSD.org/src/commit/?>
id=0d2fd5b99c95329085d0700a4dd38507a054a50d
commit 0d2fd5b99c95329085d0700a4dd38507a054a50d
Author: Andriy Gapon <a...@freebsd.org <mailto:a...@freebsd.org>>
AuthorDate: 2024-11-10 11:15:30 +0000
Commit: Andriy Gapon <a...@freebsd.org <mailto:a...@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.
Oups, I take it back and apologize for the noise.
The short story is that I was confused by my local uart driver changes
and mistook ns8250_flush() for ns8250_drain().
The long story is that I have several control and measurement devices
connected via serial port. These need a dynamic baud rate change because
they communicate at a low baud rate (for robustness), but perform bulk
transfers of measurement data at a higher baud rate. Because of this, I
modified the uart ioctl to handle TIOCSETA/W/F/ ioctls which are, of
course, implemented by ns8250_drain() and ns8250_flush(). This allows
the control application to send a baud rate change message, wait for all
data (including the transmit register) to be sent, and then safely
change the baud rate.
Again,sorry for the noise.
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.
I'm not sure if I understand you exactly, but in loop-back mode the RX
input is internally connected to the TX output (i.e. disconnected from
the outside world). So from that perspective, the FIFO detection and
flush seems fine to me.
Michal
