diff options
author | Marius Strobl <marius@FreeBSD.org> | 2024-01-12 22:27:07 +0000 |
---|---|---|
committer | Marius Strobl <marius@FreeBSD.org> | 2024-01-18 20:14:56 +0000 |
commit | e7fe996990a8bdb13632a5137731dc57549f05be (patch) | |
tree | 71a2c2ca1a26c025f541b02db2da6759208b5060 | |
parent | bf81fc51ccf04a6ae58f7c4d281266d5ab59d4e0 (diff) | |
download | src-e7fe996990a8bdb13632a5137731dc57549f05be.tar.gz src-e7fe996990a8bdb13632a5137731dc57549f05be.zip |
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).
(cherry picked from commit 353e4c5a068d06b0d6dcfa9eb736ecb16e9eae45)
-rw-r--r-- | sys/arm/nvidia/tegra_uart.c | 3 | ||||
-rw-r--r-- | sys/dev/uart/uart_dev_ns8250.c | 14 | ||||
-rw-r--r-- | sys/dev/uart/uart_dev_ns8250.h | 1 | ||||
-rw-r--r-- | sys/dev/uart/uart_dev_snps.c | 1 | ||||
-rw-r--r-- | sys/dev/uart/uart_dev_ti8250.c | 1 | ||||
-rw-r--r-- | sys/dev/uart/uart_if.m | 21 | ||||
-rw-r--r-- | sys/dev/uart/uart_tty.c | 14 | ||||
-rw-r--r-- | sys/mips/cavium/uart_dev_oct16550.c | 17 | ||||
-rw-r--r-- | sys/mips/ingenic/jz4780_uart.c | 3 |
9 files changed, 67 insertions, 8 deletions
diff --git a/sys/arm/nvidia/tegra_uart.c b/sys/arm/nvidia/tegra_uart.c index 4332976ea187..fe2701c02cb4 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 ec9aea18379a..879fe6e04a70 100644 --- a/sys/dev/uart/uart_dev_ns8250.c +++ b/sys/dev/uart/uart_dev_ns8250.c @@ -392,9 +392,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 = { @@ -1048,6 +1049,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 1e3192fe3ab2..7149a13b7004 100644 --- a/sys/dev/uart/uart_dev_snps.c +++ b/sys/dev/uart/uart_dev_snps.c @@ -102,6 +102,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 5a5fc97fd04e..96639f365a9d 100644 --- a/sys/dev/uart/uart_dev_ti8250.c +++ b/sys/dev/uart/uart_dev_ti8250.c @@ -106,6 +106,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 0c69fa25f6cf..736c756322a4 100644 --- a/sys/dev/uart/uart_tty.c +++ b/sys/dev/uart/uart_tty.c @@ -389,9 +389,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 = { diff --git a/sys/mips/cavium/uart_dev_oct16550.c b/sys/mips/cavium/uart_dev_oct16550.c index 2a932da73dcb..bb9776352819 100644 --- a/sys/mips/cavium/uart_dev_oct16550.c +++ b/sys/mips/cavium/uart_dev_oct16550.c @@ -51,8 +51,6 @@ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * */ #include <sys/cdefs.h> @@ -397,6 +395,7 @@ static int oct16550_bus_probe(struct uart_softc *); static int oct16550_bus_receive(struct uart_softc *); static int oct16550_bus_setsig(struct uart_softc *, int); static int oct16550_bus_transmit(struct uart_softc *); +static bool oct16550_bus_txbusy(struct uart_softc *); static void oct16550_bus_grab(struct uart_softc *); static void oct16550_bus_ungrab(struct uart_softc *); @@ -412,9 +411,10 @@ static kobj_method_t oct16550_methods[] = { KOBJMETHOD(uart_receive, oct16550_bus_receive), KOBJMETHOD(uart_setsig, oct16550_bus_setsig), KOBJMETHOD(uart_transmit, oct16550_bus_transmit), + KOBJMETHOD(uart_txbusy, oct16550_bus_txbusy), KOBJMETHOD(uart_grab, oct16550_bus_grab), KOBJMETHOD(uart_ungrab, oct16550_bus_ungrab), - { 0, 0 } + KOBJMETHOD_END }; struct uart_class uart_oct16550_class = { @@ -812,6 +812,17 @@ oct16550_bus_transmit (struct uart_softc *sc) return (0); } +static bool +oct16550_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); +} + static void oct16550_bus_grab(struct uart_softc *sc) { diff --git a/sys/mips/ingenic/jz4780_uart.c b/sys/mips/ingenic/jz4780_uart.c index 897ae73ec210..d46d2566c178 100644 --- a/sys/mips/ingenic/jz4780_uart.c +++ b/sys/mips/ingenic/jz4780_uart.c @@ -93,6 +93,7 @@ static kobj_method_t jz4780_uart_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 @@ -205,7 +206,7 @@ static device_method_t jz4780_uart_bus_methods[] = { DEVMETHOD(device_probe, jz4780_uart_probe), DEVMETHOD(device_attach, uart_bus_attach), DEVMETHOD(device_detach, jz4780_uart_detach), - { 0, 0 } + DEVMETHOD_END }; static driver_t jz4780_uart_driver = { |