From nobody Thu May 22 06:37:49 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4b2zB85T54z5vxf5; Thu, 22 May 2025 06:37:52 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4b2zB850byz3hdq; Thu, 22 May 2025 06:37:52 +0000 (UTC) (envelope-from mmel@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1747895872; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lqC3sHaFUOshDTtJKqSjmYx/3tkGLc/Pbri5hy+n2os=; b=I7nhh/HCW1vDefnaSBMZhAvRTlN0WuPguOSZXl1T/9AcOq2P5f1lM4TDt25GnUeSsVG8em 9UZYN5q0o4vJqS9EeCReVqjhDXWG08tJrXE15he6dhEV036bsnkqb8t5cQXeFHbLrZlUV7 NefLInKQ5zQsbLD9cnkNJOVAmJeZHkf3kkt9As4/LJ+sQBMdOqxqZMnYui6tnCvArO4yrs ba+Upt25HWVpIQN5xpugdoFkbvnzU9IP0wE3ErEubJEfOWY96yZ9UBzAiLxZGW8+g5OqP/ qUeB//hUgzZpgGfQkugnzkz34TiEqVMY7G5GAx1VzNa+AMeoHAPDKQ5aVgDpiQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1747895872; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lqC3sHaFUOshDTtJKqSjmYx/3tkGLc/Pbri5hy+n2os=; b=iqqp/jrFZLGdYa9cpYxg9NUMi1ELrzhrqxj3Iznj0ZHHew4Yh3hoaWQw2yh0XHf1yntwUe lC66QZIRPqn40rYG5F/MBlAYYgnDVlrCmMS8LsCL7fJdq9eZA+E8sv34zc6TNjpzUwegoD cHrkmmAzB7ldYW1vgY6U35SZja4yLj6cmNzJ27USvq3oXNmhvlbF6/M7q5KDofeOp5Rgix U+hQuW2G14+VmoSlrUMmRM5rEkkIR463xKJzNRPbUeteBXjTcHodgQF2V48PWVDosi8JDO 7h0uQIlCOcmEYOiW+/lH4zp4MPHBNk3en0qUt0AqstMlkFMrjsepJlUhSysRMA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1747895872; a=rsa-sha256; cv=none; b=PYer8OlCsUcMQMuj33/YAzicCzg6gyxtQYcdfRbbfpPrCeq3tHyHqX1nN2QkGs1wsw+rjC CiId1GKoImwbuk5OshJRHiVXiHMQ/XmGfo3cM08zdyFHBM38kCgqowmnibch6Cg+2/cJeD JNT7Un9cxzA6S6C7PlmtCtUMAe6QYEcXzHSMI7BosMJmVQDIJ4wYZnSkHTuPPdJ1jdHasa KgjhizG7zQcFsow2WpR6BlTnoWcfOSyYSS4v+73I21w08MYztIOyGaf7uCFiU4iNid5x+o ULdLydAWJf5E1M+z+rF+SMyDvKkImiej4Gqs6DoK19OkkF8bNXgafeTJF8fTzg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from [IPV6:2001:67c:14a0:5fe0:74eb:656:ad38:73a0] (unknown [IPv6:2001:67c:14a0:5fe0:74eb:656:ad38:73a0]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: mmel/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4b2zB76JCpzZKC; Thu, 22 May 2025 06:37:51 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Message-ID: <13b1bbfe-4a60-4c4a-aa4e-ed168cb1286a@FreeBSD.org> Date: Thu, 22 May 2025 08:37:49 +0200 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: mmel@FreeBSD.org Reply-To: mmel@FreeBSD.org Subject: Re: 0d2fd5b99c95 - main - ns8250: use LSR_THRE instead of LSR_TEMT for checking tx flush To: Ravi Pokala , Andriy Gapon , src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org References: <202505201457.54KEvD1r053951@gitrepo.freebsd.org> <1a11f640-be62-4f4e-b537-70806ac54831@FreeBSD.org> <37BF51C0-C631-493F-B3AF-3AA9FC32551B@panasas.com> Content-Language: cs, en-US In-Reply-To: <37BF51C0-C631-493F-B3AF-3AA9FC32551B@panasas.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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: > on behalf of Andriy Gapon > > Date: Tuesday, May 20, 2025 at 17:12 > To: >, >, >, > > 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/? >>> id=0d2fd5b99c95329085d0700a4dd38507a054a50d >>> >>> commit 0d2fd5b99c95329085d0700a4dd38507a054a50d >>> Author: Andriy Gapon > >>> AuthorDate: 2024-11-10 11:15:30 +0000 >>> Commit: Andriy Gapon > >>> 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