Date: Wed, 21 May 2025 22:42:27 -0400 From: Ravi Pokala <rpokala@freebsd.org> To: Andriy Gapon <avg@FreeBSD.org>, <mmel@FreeBSD.org>, <src-committers@FreeBSD.org>, <dev-commits-src-all@FreeBSD.org>, <dev-commits-src-main@FreeBSD.org> Subject: Re: 0d2fd5b99c95 - main - ns8250: use LSR_THRE instead of LSR_TEMT for checking tx flush Message-ID: <37BF51C0-C631-493F-B3AF-3AA9FC32551B@panasas.com> In-Reply-To: <bc1afdab-72d6-43c1-9603-1eed07cb25e6@FreeBSD.org> References: <202505201457.54KEvD1r053951@gitrepo.freebsd.org> <1a11f640-be62-4f4e-b537-70806ac54831@FreeBSD.org> <bc1afdab-72d6-43c1-9603-1eed07cb25e6@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> Additionally, ns8250_bus_transmit uses ns8250_drain(UART_DRAIN_TRANSMITTE= R) in broken_txfifo case. FWIW, I just had to enable the 'hw.broken_txfifo=3D1' workaround on physical = hardware; see [Bug 286703]. Thanks, Ravi (rpokala@) =EF=BB=BF-----Original Message----- From: <owner-src-committers@freebsd.org <mailto:owner-src-committers@freebs= d.org>> on behalf of Andriy Gapon <avg@FreeBSD.org <mailto:avg@FreeBSD.org>> Date: Tuesday, May 20, 2025 at 17:12 To: <mmel@FreeBSD.org <mailto:mmel@FreeBSD.org>>, <src-committers@FreeBSD.o= rg <mailto:src-committers@FreeBSD.org>>, <dev-commits-src-all@FreeBSD.org <m= ailto:dev-commits-src-all@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: >=20 >=20 > 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/?>=20 >> id=3D0d2fd5b99c95329085d0700a4dd38507a054a50d >> >> commit 0d2fd5b99c95329085d0700a4dd38507a054a50d >> Author: Andriy Gapon <avg@FreeBSD.org <mailto:avg@FreeBSD.org>> >> AuthorDate: 2024-11-10 11:15:30 +0000 >> Commit: Andriy Gapon <avg@FreeBSD.org <mailto: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.=20 I am not sure to which part of the commit message your "that" refers to, so= I'll=20 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 tha= t. We do not have direct control over the shift register, hardware clears it a= fter=20 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=20 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 sh= ould=20 use LSR_TEMT like you say. At the same time, this UART flush is not like stdout flush, of course, wher= e we=20 ensure that all buffered data is transmitted. For UART, we just clear the F= IFO=20 and the holding register. So, I am not sure if polling for empty transmitte= r is=20 important. Besides, I do not see the code which would flush transmitter when parameter= s are=20 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=20 broken_txfifo case. P.S. Maybe I don't understand the code, but UART_FLUSH_RECEIVER in ns8250_bus_at= tach=20 looks strange to me. It's one thing to flush data while in the loop-back mo= de,=20 but I think that in ns8250_bus_attach the hardware is fully set up to recei= ve=20 data from the outside world. So, how can we hope to drain all of it and to=20 reliably detect whether FIFO flushing works. I mean that something on the o= ther=20 end could be continuously transmitting. --=20 Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?37BF51C0-C631-493F-B3AF-3AA9FC32551B>