Re: [PATCH v7 1/2] serial: core: fix baud rate fallback in uart_get_baud_rate()

[Hugo Villeneuve <hugo@hugovil.com>]

Hi Ilpo,

On Wed, 7 Oct 2026 13:38:55 +0300 (EEST)
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:

> On Tue, 6 Oct 2026, Hugo Villeneuve wrote:
> 
> > On Thu, 1 Oct 2026 12:46:05 +0300 (EEST)
> > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > 
> > > On Thu, 1 Oct 2026, Greg Kroah-Hartman wrote:
> > > 
> > > > On Wed, Sep 30, 2026 at 12:59:00PM +0000, Hui Peng wrote:
> > > > > When uart_get_baud_rate() is called with a baud rate exceeding the port's
> > > > > maximum supported speed (port->uartclk / 16), it clips baud to [min,
> > > > > max - 1] and encodes it into termios via tty_termios_encode_baud_rate().
> > > > > 
> > > > > However, because the loop bound is for (try = 0; try < 2; try++), the
> > > > > loop terminates immediately after try == 1 without re-evaluating
> > > > 
> > > > But try == 1 should keep the loop going as it is < 2, right?  What am I
> > > > missing here?  Do I need more coffee?
> > > >
> > > > > baud = tty_termios_baud_rate(termios) for the clipped rate, hitting
> > > > > WARN_ON(1) and returning 0, which then triggers a fatal divide-by-zero
> > > > > (Oops: divide error) in uart_get_divisor():
> > > > > 
> > > > >   WARNING: drivers/tty/serial/serial_core.c:548 at uart_get_baud_rate+0x136/0x260
> > > > >   [ ... ]
> > > > >   divide error: 0000 [#1] PREEMPT SMP KASAN
> > > > > 
> > > > > Increase the retry count in uart_get_baud_rate() from 2 to 3 iterations so
> > > > > that clipped baud rates are re-evaluated in the third iteration.
> > > > 
> > > > What is the magic 2 here, and why turning it into a magic 3 somehow fix
> > > > things?
> > > 
> > > Hi Greg & Hui,
> > > 
> > > First of all, I'm withdrawing my Reviewed-by from this!!!
> > > 
> > > Lets hope the submitter can finally get his/her act together and not make 
> > > unlisted changes between versions or send non-sense.
> > > 
> > > While the code change is still fine, it seems the submitter (or more 
> > > likely AI) has changed the changelog from what I read when I reviewed 
> > > this. And the new one is way worse than it used to be so not being able 
> > > to follow what's going on is very understandable given the lackluster 
> > > explanation that remains.
> > > 
> > > 
> > > What you're missing is that the baud returns happens within the loop, so:
> > > 
> > > try == 0: use new, if baud is out of bound and old is available, switch to 
> > >           old
> > > try == 1: if old is also out of bounds, there's the last resort rule 
> > >           towards the end of the loop which is applied forcing baud to the 
> > >           accetable range.
> > 
> > Hi all,
> > is it at all possible that old is out of bounds in the first place?
> 
> Apparently it is, given the WARNING above (unless that too is fabricate 
> by LLM which seems well within possiblities given the track record of this 
> particular submitter).
> 
> > If yes, is it something that should be fixed?
> 
> I agree.
> 
> I think the scenario here is (but take it with grain of salt, as the 
> information seems to be constantly changing thanks to LLM/submitter 
> failing to keep one's act to together) [1]:
> 
> Tested in QEMU against Linux 7.3.0-rc3 by setting /dev/ttyS1 to B115200,
> lowering baud_base to 9600 (max = 9600) via TIOCSSERIAL, and calling
> tcsetattr() with B57600 (old = B115200), reproducing the WARNING and
> Oops: divide error in uart_get_divisor() on the unfixed kernel and
> verifying clean execution with 0 warnings/faults with the fix applied.
> 
> So when rate gets lowered, it should alter the termios to prevent what is 
> here seem as "old" having an invalid value.

That makes sense...

> 
> [1] https://lore.kernel.org/linux-serial/20260930041216.155911-3-benquike@gmail.com/
> 
> > > try == 2: loop exits => WARN_ON(1) triggers.
> > > 
> > > What we'd want to happen with try == 2, is for it to use the return baud 
> > > which is within the loop body.
> > 
> > Then should the "return 0" statement be modified to "return baud", and
> > possibly the WARN_ON() removed? Then you wouldn't need to increase
> > max try?
> 
> IMO, the entire loop constructs feels somewhat artificial in this case so 
> I'd prefer to kill the loop entirely. But such refactoring is not going to 
> be a minimal fix to the issue.

Yes, when I updated uart_get_baud_rate() a few months ago, the loop
also felt a little bit weird, but I did not remove it because I assumed
it was working ok if we assumed, like I did, that the old baud rate was
always valid.

I tried in the past to simplify it, make it more logical and
understandable by normal humans :) But I always end up with
something not so simple and obvious because of all the special cases
(B0, spd_* flags, etc).

I am still trying though, maybe I will have better luck this time :)


-- 
Hugo Villeneuve