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

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

[Hugo Villeneuve]

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

[Biju Das]

Hi Hugo,

> -----Original Message-----
> From: Hugo Villeneuve <hugo@hugovil.com>
> Sent: 17 April 2026 20:36
> Subject: [PATCH] serial: sh-sci: optimize max_freq determination
> 
> 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;

I have thought about this change. but the below comment made me not to do this change.

<snippet from Geert>
IIRC, baud == 0 can (only?) happen when using earlyprintk on a non-DT
system, where the serial console should just keep on using the settings
programmed by the firmware.  So any config register writes should
be skipped.
</snippet>

Cheers,
Biju

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

[Hugo Villeneuve]

Hi Biju,

On Sat, 18 Apr 2026 07:12:57 +0000
Biju Das <biju.das.jz@bp.renesas.com> wrote:

> Hi Hugo,
> 
> > -----Original Message-----
> > From: Hugo Villeneuve <hugo@hugovil.com>
> > Sent: 17 April 2026 20:36
> > Subject: [PATCH] serial: sh-sci: optimize max_freq determination
> > 
> > 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;
> 
> I have thought about this change. but the below comment made me not to do this change.
> 
> <snippet from Geert>
> IIRC, baud == 0 can (only?) happen when using earlyprintk on a non-DT
> system, where the serial console should just keep on using the settings
> programmed by the firmware.  So any config register writes should
> be skipped.
> </snippet>

I think Geert comments referred to the clock (port->uartclk) being zero,
not the baud (the baud rate cannot be zero)...

But I am not sure how it is relevant here because the modification is
not changing the behavior of the existing code, it is an optimization
and a simplification. In fact, it is exactly the change I proposed [1]
when you submitted the rsci driver (and yoyu accepted), unless I am
missing something?

[1]
https://lore.kernel.org/all/20251028112236.c832fb48ad9fafcd2cf34b57@hugovil.com/

The goal of this patch is to have both rsci and sh-sci code to be the
same for this specific section at least. If you modify it to take into
account Geert's comments, then It think you need to do it both for rsci
and sh-sci drivers, no?


> 
> Cheers,
> Biju
> 
> > +	} 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
> 
> 


-- 
Hugo Villeneuve

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

[Geert Uytterhoeven]

Hi Hugo,

On Sat, 18 Apr 2026 at 16:39, Hugo Villeneuve <hugo@hugovil.com> wrote:
> On Sat, 18 Apr 2026 07:12:57 +0000
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > From: Hugo Villeneuve <hugo@hugovil.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;
> >
> > I have thought about this change. but the below comment made me not to do this change.
> >
> > <snippet from Geert>
> > IIRC, baud == 0 can (only?) happen when using earlyprintk on a non-DT
> > system, where the serial console should just keep on using the settings
> > programmed by the firmware.  So any config register writes should
> > be skipped.
> > </snippet>
>
> I think Geert comments referred to the clock (port->uartclk) being zero,
> not the baud (the baud rate cannot be zero)...

I did mean the baud variable being zero.

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

[Hugo Villeneuve]

Hi Geert,

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

> Hi Hugo,
> 
> On Sat, 18 Apr 2026 at 16:39, Hugo Villeneuve <hugo@hugovil.com> wrote:
> > On Sat, 18 Apr 2026 07:12:57 +0000
> > Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > From: Hugo Villeneuve <hugo@hugovil.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;
> > >
> > > I have thought about this change. but the below comment made me not to do this change.
> > >
> > > <snippet from Geert>
> > > IIRC, baud == 0 can (only?) happen when using earlyprintk on a non-DT
> > > system, where the serial console should just keep on using the settings
> > > programmed by the firmware.  So any config register writes should
> > > be skipped.
> > > </snippet>
> >
> > I think Geert comments referred to the clock (port->uartclk) being zero,
> > not the baud (the baud rate cannot be zero)...
> 
> I did mean the baud variable being zero.

uart_get_baud_rate() will not return zero. Even if the baud rate
specified in termios (or the old) is set to 0 (hang up), it will return
9600...

-- 
Hugo Villeneuve

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

[Geert Uytterhoeven]

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

[Hugo Villeneuve]

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