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

[Geert Uytterhoeven]

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.

> +                       best_clk = SCI_FCK;

best_clk can be removed...

> +                       ccr0_val = 0;
> +                       min_err = err;

... just like min_err...

> +                       brr = brr1;
> +                       cks = cks1;

and the brr1, srr1, and cks1 intermediaries.

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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