Date: Thu, 18 Jan 2024 20:15:25 GMT From: Marius Strobl <marius@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: e7fe996990a8 - stable/13 - uart(4): Honor hardware state of NS8250-class for tsw_busy Message-ID: <202401182015.40IKFPEJ068399@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by marius: URL: https://cgit.FreeBSD.org/src/commit/?id=e7fe996990a8bdb13632a5137731dc57549f05be commit e7fe996990a8bdb13632a5137731dc57549f05be Author: Marius Strobl <marius@FreeBSD.org> AuthorDate: 2024-01-12 22:27:07 +0000 Commit: Marius Strobl <marius@FreeBSD.org> CommitDate: 2024-01-18 20:14:56 +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). (cherry picked from commit 353e4c5a068d06b0d6dcfa9eb736ecb16e9eae45) --- 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 ++++++++++++-- sys/mips/cavium/uart_dev_oct16550.c | 17 ++++++++++++++--- 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 = {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202401182015.40IKFPEJ068399>