From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Tate Whiteberg <whiteberg@arista.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jirislaby@kernel.org>,
Andy Shevchenko <andy@kernel.org>,
linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org
Subject: Re: [PATCH] serial: 8250_mid: wait for LSR tx empty before setting termios
Date: Fri, 28 Aug 2026 18:17:32 +0300 [thread overview]
Message-ID: <apGmjOOI6K5wkhu3@ashevche-desk.local> (raw)
In-Reply-To: <20260828144617.1210599-2-whiteberg@arista.com>
On Fri, Aug 28, 2026 at 02:45:17PM +0000, Tate Whiteberg wrote:
> If mid8250_set_termios is called while data is still in transmission,
mid8250_set_termios()
> the corresponding register updates will corrupt the transmission.
>
> Fix this by locking the port and and waiting for the transmitter to
> empty before performing updates. It is necessary to wait for both
> UART_LSR_THRE and UART_LSR_TEMT to ensure the final character is sent.
Fixes tag?
...
> static void mid8250_set_termios(struct uart_port *p, struct ktermios *termios,
> {
> unsigned int baud = tty_termios_baud_rate(termios);
> struct mid8250 *mid = p->private_data;
> + struct uart_8250_port *up = up_to_u8250p(p);
Move it one line up.
> unsigned short ps = 16;
> unsigned long fuart = baud * ps;
> unsigned long w = BIT(24) - 1;
> /* Gracefully handle the B0 case: fall back to B9600 */
> fuart = fuart ? fuart : 9600 * 16;
>
> + uart_port_lock_irq(p);
+ blank line.
Do we need to protect the preliminary calculations?
> if (mid->board->freq < fuart) {
> /* Find prescaler value that satisfies Fuart < Fref */
> if (mid->board->freq > baud)
> @@ -231,11 +233,14 @@ static void mid8250_set_termios(struct uart_port *p, struct ktermios *termios,
> }
>
> rational_best_approximation(fuart, mid->board->freq, w, w, &mul, &div);
> + serial8250_wait_for_xmitr(up, UART_LSR_BOTH_EMPTY);
Can't it be done one line after? Somewhere...
> p->uartclk = fuart * 16 / ps; /* core uses ps = 16 always */
...here?
> writel(ps, p->membase + INTEL_MID_UART_PS); /* set PS */
> writel(mul, p->membase + INTEL_MID_UART_MUL); /* set MUL */
> writel(div, p->membase + INTEL_MID_UART_DIV);
+ blank line.
> + uart_port_unlock_irq(p);
>
> serial8250_do_set_termios(p, termios, old);
...
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 38fa45e74a37..4eaf417f1fd7 100644
All below better to split to another prerequisite patch.
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2026-08-28 15:17 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-28 14:45 Tate Whiteberg
2026-08-28 15:17 ` Andy Shevchenko [this message]
2026-08-30 18:03 ` kernel test robot
2026-08-31 8:25 ` kernel test robot
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=apGmjOOI6K5wkhu3@ashevche-desk.local \
--to=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=whiteberg@arista.com \
/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®