1. Patchew
  2. linux
  3. View series

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

Hugo Villeneuve posted 1 patch 1 month, 4 weeks ago 
drivers/tty/serial/sh-sci.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
[PATCH] serial: sh-sci: optimize max_freq determination
Posted by Hugo Villeneuve 1 month, 4 weeks ago 
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

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@dimonoff.com>
---
Cc: biju.das.jz@bp.renesas.com

Biju: if you want, feel free to pickup this patch when you resubmit your
serie for "sh-sci/rsci: Fix divide by zero and clean up baud rate logic".
---
 drivers/tty/serial/sh-sci.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 6c819b6b24258..dcee8b69adab2 100644
--- 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;
+		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;
 

base-commit: a1a81aef99e853dec84241d701fbf587d713eb5b
-- 
2.47.3
Re: [PATCH] serial: sh-sci: optimize max_freq determination
Posted by Geert Uytterhoeven 1 month, 3 weeks ago 
Hi Hugo,

On Sat, 18 Apr 2026 at 07:24, Hugo Villeneuve <hugo@hugovil.com> wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> 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@dimonoff.com>

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?

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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
Re: [PATCH] serial: sh-sci: optimize max_freq determination
Posted by Hugo Villeneuve 1 month, 3 weeks ago 
Hi Geert,

On Mon, 20 Apr 2026 09:23:55 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Hugo,
> 
> On Sat, 18 Apr 2026 at 07:24, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >
> > 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@dimonoff.com>
> 
> 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@linux-m68k.org
> 
> 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
Re: [PATCH] serial: sh-sci: optimize max_freq determination
Posted by Hugo Villeneuve 1 month, 3 weeks ago 
On Mon, 20 Apr 2026 12:12:05 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> Hi Geert,
> 
> On Mon, 20 Apr 2026 09:23:55 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> > Hi Hugo,
> > 
> > On Sat, 18 Apr 2026 at 07:24, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > >
> > > 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@dimonoff.com>
> > 
> > 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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.