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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4047 bytes --]

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.

[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.

-- 
 i.