RE: [PATCH] serial: rsci: Remove goto and refactor baud rate clock selection

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 08 April 2026 09:22
> Subject: Re: [PATCH] serial: rsci: Remove goto and refactor baud rate clock selection
> 
> Hi Biju,
> 
> On Tue, 7 Apr 2026 at 17:12, Biju <biju.das.au@xxxxxxxxx> wrote:
> > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > Replace the early-exit goto pattern in rsci_set_termios() with a
> > positive conditional block. When baud rate is zero, the clock
> > selection logic is now simply skipped rather than jumping to a 'done'
> > label, eliminating the goto entirely.
> >
> > No functional change intended.
> >
> > Reported-by: Pavel Machek <pavel@xxxxxxxxxxxx>
> > Closes: https://lore.kernel.org/all/abPpZULsXhRmXTX9@xxxxxxxxxx/
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/drivers/tty/serial/rsci.c
> > +++ b/drivers/tty/serial/rsci.c
> > @@ -265,20 +265,18 @@ static void rsci_set_termios(struct uart_port *port, struct ktermios *termios,
> >         }
> >
> >         baud = uart_get_baud_rate(port, termios, old, 0, max_freq);
> > -       if (!baud)
> > -               goto done;
> 
> As RSCI has only a single possible input clock for bit rate selection, there is indeed no need for the
> "done" label.
> 
> > -
> > -       /* Divided Functional Clock using standard Bit Rate Register */
> > -       err = sci_scbrr_calc(s, baud, &brr1, &srr1, &cks1);
> > -       if (abs(err) < abs(min_err)) {
> > -               best_clk = SCI_FCK;
> > -               ccr0_val = 0;
> > -               min_err = err;
> > -               brr = brr1;
> > -               cks = cks1;
> > +       if (baud) {
> > +               /* Divided Functional Clock using standard Bit Rate Register */
> > +               err = sci_scbrr_calc(s, baud, &brr1, &srr1, &cks1);
> > +               if (abs(err) < abs(min_err)) {
> 
> This check is always true.

Ok will drop this.

> 
> > +                       best_clk = SCI_FCK;
> 
> best_clk can be removed...

OK.

> 
> > +                       ccr0_val = 0;
> > +                       min_err = err;
> 
> ... just like min_err...
OK.
> 
> > +                       brr = brr1;
> > +                       cks = cks1;
> 
> and the brr1, srr1, and cks1 intermediaries.

OK.

> 
> > +               }
> >         }
> >
> > -done:
> >         if (best_clk >= 0)
> >                 dev_dbg(port->dev, "Using clk %pC for %u%+d bps\n",
> >                         s->clks[best_clk], baud, min_err);
> 
> This dev_dbg() can be moved inside the "if (baud)" check.

Agreed.

Will send v2, fixing these.

Cheers,
Biju