From: Tapio Reijonen <tapio.reijonen@vaisala.com>
To: Hugo Villeneuve <hugo@hugovil.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jirislaby@kernel.org>,
linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
Hugo Villeneuve <hvilleneuve@dimonoff.com>,
Tapio Reijonen <tapio.reijonen@kolumbus.fi>
Subject: Re: [PATCH v5 4/8] serial: max310x: wait for TX to drain before powering down in shutdown
Date: Fri, 2 Oct 2026 10:28:59 +0300 [thread overview]
Message-ID: <e1c4b08c-8e91-4368-8fde-6deb989995e9@vaisala.com> (raw)
In-Reply-To: <20261001160020.5ba190a2b747f0c66f8b30d7@hugovil.com>
Hi Hugo,
On Thu, 1 Oct 2026 16:00:20 -0400, Hugo Villeneuve wrote:
> > + unsigned int one_char_duration_us;
>
> char_time_us?
Renamed in v6.
> > + to_max310x_port(port)->baud = baud;
> > + to_max310x_port(port)->one_char_duration_us =
> > + DIV_ROUND_UP(USEC_PER_SEC * frame_bits, baud);
>
> Would it be a good idea to if you moved these two lines after
> max310x_set_rts_ctl_params(), then you could probably leave the
> original comments and simply add a new comment to indicate "Compute
> time it takes to clock out one character", simplifying the diff
> (review) and readability?
I would prefer not to move them: the helper consumes both values.
The baud is what the millisecond-to-bit-time conversion divides by,
so it must be cached before the call. And as of v6 the helper can
also arm the after-send hold directly - v6 adds a fix for the case
where a reconfigure moves the port off the hardware RTS path while a
transmission is still in flight, and the takeover computes the hold
from char_time_us - so the character time has to be current at that
point as well.
> > + unsigned int loops = port->fifosize + 1;
>
> tries?
Renamed in v6.
> Based on these comments, does it mean that the FIFO has already been
> validated empty at this point by the tty layer, so you don't need the
> loop at all, just the unconditional last fsleep()?
No - that wait is not guaranteed. uart_wait_until_sent() runs only on
the close path and is bounded by closing_wait, which can be configured
to none, and hangup reaches shutdown() with no wait at all. In
testing, a vhangup issued mid-transfer entered shutdown() with the
chip FIFO still holding over a hundred characters; this loop is what
drained them before power-down.
> For certain combinations of large fifo_sizes and high-baud rates,
> that could mean a lot of I2C/SPI transactions?
It is bounded at one FIFO-level read per character time, at most
fifosize + 1 of them, only on the close/hangup path, and it stops as
soon as the FIFO reads empty - in total no longer than the remaining
transmit time of the data itself. At high baud rates the character
time shrinks, so the polls get more frequent but the window they can
occupy shrinks with it.
Tapio
next prev parent reply other threads:[~2026-10-02 7:29 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-29 9:37 [PATCH v5 0/8] serial: max310x: RS485 delay and RTS fixes, software-timed delays Tapio Reijonen
2026-09-29 9:37 ` [PATCH v5 1/8] serial: max310x: don't clobber the TX break bit in set_termios Tapio Reijonen
2026-09-29 13:40 ` Hugo Villeneuve
2026-10-02 7:25 ` Tapio Reijonen
2026-10-02 15:03 ` Hugo Villeneuve
2026-10-04 10:45 ` Tapio Reijonen
2026-09-29 9:37 ` [PATCH v5 2/8] serial: max310x: assert the transceiver during a break Tapio Reijonen
2026-09-29 9:37 ` [PATCH v5 3/8] serial: max310x: convert RS485 delays from milliseconds to bit-times Tapio Reijonen
2026-09-29 13:54 ` Hugo Villeneuve
2026-10-01 19:11 ` Hugo Villeneuve
2026-10-02 7:27 ` Tapio Reijonen
2026-09-29 9:37 ` [PATCH v5 4/8] serial: max310x: wait for TX to drain before powering down in shutdown Tapio Reijonen
2026-10-01 20:00 ` Hugo Villeneuve
2026-10-02 7:28 ` Tapio Reijonen [this message]
2026-10-02 15:01 ` Hugo Villeneuve
2026-10-04 10:56 ` Tapio Reijonen
2026-09-29 9:37 ` [PATCH v5 5/8] serial: max310x: support active-low RTS on the hardware path Tapio Reijonen
2026-09-29 9:38 ` [PATCH v5 6/8] serial: max310x: schedule tx_work directly from the IRQ handler Tapio Reijonen
2026-09-29 9:38 ` [PATCH v5 7/8] serial: max310x: drive RTS in software when hardware delays are too short Tapio Reijonen
2026-09-29 9:38 ` [PATCH v5 8/8] serial: max310x: don't transmit while an RS485 reconfigure is pending Tapio Reijonen
2026-10-01 8:35 ` [PATCH v5 0/8] serial: max310x: RS485 delay and RTS fixes, software-timed delays Greg Kroah-Hartman
2026-10-01 9:10 ` Tapio Reijonen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e1c4b08c-8e91-4368-8fde-6deb989995e9@vaisala.com \
--to=tapio.reijonen@vaisala.com \
--cc=gregkh@linuxfoundation.org \
--cc=hugo@hugovil.com \
--cc=hvilleneuve@dimonoff.com \
--cc=jirislaby@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=tapio.reijonen@kolumbus.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®