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@xxxxxxxxxxxxxx> wrote:

> Hi Hugo,
> 
> > -----Original Message-----
> > From: Hugo Villeneuve <hugo@xxxxxxxxxxx>
> > Sent: 17 April 2026 20:36
> > Subject: [PATCH] serial: sh-sci: optimize max_freq determination
> > 
> > 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>
> > ---
> > Cc: biju.das.jz@xxxxxxxxxxxxxx
> > 
> > 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@xxxxxxxxxxx/

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