Re: [PATCH] serial: sh-sci: optimize max_freq determination

[Hugo Villeneuve]

Hi Geert,

On Mon, 20 Apr 2026 09:23:55 +0200
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:

> Hi Hugo,
> 
> On Sat, 18 Apr 2026 at 07:24, Hugo Villeneuve <hugo@xxxxxxxxxxx> wrote:
> > From: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
> >
> > Follow example of rsci driver to avoid code duplication and useless
> > max_freq search when port->uartclk is set to zero.
> >
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -2711,14 +2711,15 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
> >          * setup the baud rate generator hardware for us already.
> >          */
> >         if (!port->uartclk) {
> > -               baud = uart_get_baud_rate(port, termios, old, 0, 115200);
> > -               goto done;
> 
> There was no "useless max_freq search", due to this goto?

You are right, this part of the comment is wrong.

> 
> > +               max_freq = 115200;
> > +       } else {
> > +               for (i = 0; i < SCI_NUM_CLKS; i++)
> > +                       max_freq = max(max_freq, s->clk_rates[i]);
> > +
> > +               max_freq /= min_sr(s);
> >         }
> >
> > -       for (i = 0; i < SCI_NUM_CLKS; i++)
> > -               max_freq = max(max_freq, s->clk_rates[i]);
> > -
> > -       baud = uart_get_baud_rate(port, termios, old, 0, max_freq / min_sr(s));
> > +       baud = uart_get_baud_rate(port, termios, old, 0, max_freq);
> >         if (!baud)
> >                 goto done;
> >
> 
> Due to removing the goto above (for the casual reader: this is the
> earlyprintk case, when port->uartclk is zero), the code will now
> continue here, calculating transmission parameters and setting best_clk,
> and overwriting the register configuration done by the bootloader.

Yes, I see it now for sh-sci where we have "if (best_clk >= 0) {"... to
not configure the registers.

However, for the rsci driver, I think that, even after Biju's cleanup
serie V3, we still overwrite the register configuration done by the
bootloader?

Maybe configure only "if (!port->uartclk) {" ?

Hugo.



> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 


-- 
Hugo Villeneuve