git: 353e4c5a068d - main - uart(4): Honor hardware state of NS8250-class for tsw_busy
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sun, 14 Jan 2024 07:04:32 UTC
The branch main has been updated by marius:
URL: https://cgit.FreeBSD.org/src/commit/?id=353e4c5a068d06b0d6dcfa9eb736ecb16e9eae45
commit 353e4c5a068d06b0d6dcfa9eb736ecb16e9eae45
Author: Marius Strobl <marius@FreeBSD.org>
AuthorDate: 2024-01-12 22:27:07 +0000
Commit: Marius Strobl <marius@FreeBSD.org>
CommitDate: 2024-01-14 07:03:59 +0000
uart(4): Honor hardware state of NS8250-class for tsw_busy
In 9750d9e5, I brought the equivalent of the TS_BUSY flag back in a
mostly hardware-agnostic way in order to fix tty_drain() and, thus,
TIOCDRAIN for UARTs with TX FIFOs. This proved to be sufficient for
fixing the regression reported. So in light of the release cycle of
FreeBSD 10.3, I decided that this change was be good enough for the
time being and opted to go with the smallest possible yet generic
(for all UARTs driven by uart(4)) solution addressing the problem at
hand.
However, at least for the NS8250-class the above isn't a complete
fix as these UARTs only trigger an interrupt when the TX FIFO became
empty. At this point, there still can be an outstanding character
left in the transmit shift register as indicated via the LSR. Thus,
this change adds the 3rd (besides the tty(4) and generic uart(4) bits)
part I had in my tree ever since, adding a uart_txbusy method to be
queried in addition for tsw_busy and hooking it up as appropriate
for the NS8250-class.
As it turns out, the exact equivalent of this 3rd part later on was
implemented for uftdi(4) in 9ad221a5.
While at it, explain the rational behind the deliberately missing
locking in uart_tty_busy() (also applying to the generic sc_txbusy
testing already present).
---
sys/arm/nvidia/tegra_uart.c | 3 ++-
sys/dev/uart/uart_dev_ns8250.c | 14 +++++++++++++-
sys/dev/uart/uart_dev_ns8250.h | 1 +
sys/dev/uart/uart_dev_snps.c | 1 +
sys/dev/uart/uart_dev_ti8250.c | 1 +
sys/dev/uart/uart_if.m | 21 +++++++++++++++++++++
sys/dev/uart/uart_tty.c | 14 ++++++++++++--
7 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/sys/arm/nvidia/tegra_uart.c b/sys/arm/nvidia/tegra_uart.c
index e18b77ecc321..9c518997e85c 100644
--- a/sys/arm/nvidia/tegra_uart.c
+++ b/sys/arm/nvidia/tegra_uart.c
@@ -136,6 +136,7 @@ static kobj_method_t tegra_methods[] = {
KOBJMETHOD(uart_receive, ns8250_bus_receive),
KOBJMETHOD(uart_setsig, ns8250_bus_setsig),
KOBJMETHOD(uart_transmit, ns8250_bus_transmit),
+ KOBJMETHOD(uart_txbusy, ns8250_bus_txbusy),
KOBJMETHOD(uart_grab, tegra_uart_grab),
KOBJMETHOD(uart_ungrab, tegra_uart_ungrab),
KOBJMETHOD_END
@@ -237,7 +238,7 @@ static device_method_t tegra_uart_bus_methods[] = {
DEVMETHOD(device_probe, tegra_uart_probe),
DEVMETHOD(device_attach, uart_bus_attach),
DEVMETHOD(device_detach, tegra_uart_detach),
- { 0, 0 }
+ DEVMETHOD_END
};
static driver_t tegra_uart_driver = {
diff --git a/sys/dev/uart/uart_dev_ns8250.c b/sys/dev/uart/uart_dev_ns8250.c
index 4f2f8f7753b9..f211084cb013 100644
--- a/sys/dev/uart/uart_dev_ns8250.c
+++ b/sys/dev/uart/uart_dev_ns8250.c
@@ -433,9 +433,10 @@ static kobj_method_t ns8250_methods[] = {
KOBJMETHOD(uart_receive, ns8250_bus_receive),
KOBJMETHOD(uart_setsig, ns8250_bus_setsig),
KOBJMETHOD(uart_transmit, ns8250_bus_transmit),
+ KOBJMETHOD(uart_txbusy, ns8250_bus_txbusy),
KOBJMETHOD(uart_grab, ns8250_bus_grab),
KOBJMETHOD(uart_ungrab, ns8250_bus_ungrab),
- { 0, 0 }
+ KOBJMETHOD_END
};
struct uart_class uart_ns8250_class = {
@@ -1070,6 +1071,17 @@ ns8250_bus_transmit(struct uart_softc *sc)
return (0);
}
+bool
+ns8250_bus_txbusy(struct uart_softc *sc)
+{
+ struct uart_bas *bas = &sc->sc_bas;
+
+ if ((uart_getreg(bas, REG_LSR) & (LSR_TEMT | LSR_THRE)) !=
+ (LSR_TEMT | LSR_THRE))
+ return (true);
+ return (false);
+}
+
void
ns8250_bus_grab(struct uart_softc *sc)
{
diff --git a/sys/dev/uart/uart_dev_ns8250.h b/sys/dev/uart/uart_dev_ns8250.h
index 357f4e7f80df..324ff72f6e5d 100644
--- a/sys/dev/uart/uart_dev_ns8250.h
+++ b/sys/dev/uart/uart_dev_ns8250.h
@@ -57,6 +57,7 @@ int ns8250_bus_receive(struct uart_softc *);
int ns8250_bus_setsig(struct uart_softc *, int);
int ns8250_bus_transmit(struct uart_softc *);
void ns8250_bus_grab(struct uart_softc *);
+bool ns8250_bus_txbusy(struct uart_softc *);
void ns8250_bus_ungrab(struct uart_softc *);
#endif /* _DEV_UART_DEV_NS8250_H_ */
diff --git a/sys/dev/uart/uart_dev_snps.c b/sys/dev/uart/uart_dev_snps.c
index c3ada581c7cd..b8b1f1f78142 100644
--- a/sys/dev/uart/uart_dev_snps.c
+++ b/sys/dev/uart/uart_dev_snps.c
@@ -101,6 +101,7 @@ static kobj_method_t snps_methods[] = {
KOBJMETHOD(uart_receive, ns8250_bus_receive),
KOBJMETHOD(uart_setsig, ns8250_bus_setsig),
KOBJMETHOD(uart_transmit, ns8250_bus_transmit),
+ KOBJMETHOD(uart_txbusy, ns8250_bus_txbusy),
KOBJMETHOD(uart_grab, ns8250_bus_grab),
KOBJMETHOD(uart_ungrab, ns8250_bus_ungrab),
KOBJMETHOD_END
diff --git a/sys/dev/uart/uart_dev_ti8250.c b/sys/dev/uart/uart_dev_ti8250.c
index 636f94872ce6..1c575defa5f4 100644
--- a/sys/dev/uart/uart_dev_ti8250.c
+++ b/sys/dev/uart/uart_dev_ti8250.c
@@ -105,6 +105,7 @@ static kobj_method_t ti8250_methods[] = {
KOBJMETHOD(uart_receive, ns8250_bus_receive),
KOBJMETHOD(uart_setsig, ns8250_bus_setsig),
KOBJMETHOD(uart_transmit, ns8250_bus_transmit),
+ KOBJMETHOD(uart_txbusy, ns8250_bus_txbusy),
KOBJMETHOD_END
};
diff --git a/sys/dev/uart/uart_if.m b/sys/dev/uart/uart_if.m
index 516e8b0811df..7efe63a10248 100644
--- a/sys/dev/uart/uart_if.m
+++ b/sys/dev/uart/uart_if.m
@@ -38,6 +38,17 @@
INTERFACE uart;
+CODE {
+ static uart_txbusy_t uart_default_txbusy;
+
+ static bool
+ uart_default_txbusy(struct uart_softc *this __unused)
+ {
+
+ return (false);
+ }
+};
+
# attach() - attach hardware.
# This method is called when the device is being attached. All resources
# have been allocated. The transmit and receive buffers exist, but no
@@ -141,6 +152,16 @@ METHOD int transmit {
struct uart_softc *this;
};
+# txbusy() - report if Tx is still busy.
+# This method is called by the tty glue for reporting upward that output is
+# still being drained despite sc_txbusy unset. Non-DEFAULT implementations
+# allow for extra checks, i. e. beyond what can be determined in ipend(),
+# that the Tx path actually is idle. For example, whether the last character
+# has left the transmit shift register in addition to the FIFO being empty.
+METHOD bool txbusy {
+ struct uart_softc *this;
+} DEFAULT uart_default_txbusy;
+
# grab() - Up call from the console to the upper layers of the driver when
# the kernel asks to grab the console. This is valid only for console
# drivers. This method is responsible for transitioning the hardware
diff --git a/sys/dev/uart/uart_tty.c b/sys/dev/uart/uart_tty.c
index 672203cc5935..faae077916f3 100644
--- a/sys/dev/uart/uart_tty.c
+++ b/sys/dev/uart/uart_tty.c
@@ -388,9 +388,19 @@ uart_tty_busy(struct tty *tp)
sc = tty_softc(tp);
if (sc == NULL || sc->sc_leaving)
- return (FALSE);
+ return (false);
- return (sc->sc_txbusy);
+ /*
+ * The tty locking is sufficient here; we may lose the race against
+ * uart_bus_ihand()/uart_intr() clearing sc_txbusy underneath us, in
+ * which case we will incorrectly but non-fatally report a busy Tx
+ * path upward. However, tty locking ensures that no additional output
+ * is enqueued before UART_TXBUSY() returns, which means that there
+ * are no Tx interrupts to be lost.
+ */
+ if (sc->sc_txbusy)
+ return (true);
+ return (UART_TXBUSY(sc));
}
static struct ttydevsw uart_tty_class = {