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

[Hugo Villeneuve]

On Mon, 20 Apr 2026 12:12:05 -0400
Hugo Villeneuve <hugo@xxxxxxxxxxx> wrote:

> 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) {" ?

Argh :) I meant configure only "if (port->uartclk) {" ?

Hugo.



-- 
Hugo Villeneuve