2023-01-09 07:54:36

by Tobias Schramm

[permalink] [raw]
Subject: [PATCH v2] serial: atmel: fix incorrect baudrate setup

Commit ba47f97a18f2 ("serial: core: remove baud_rates when serial console
setup") changed uart_set_options to select the correct baudrate
configuration based on the absolute error between requested baudrate and
available standard baudrate settings.
Prior to that commit the baudrate was selected based on which predefined
standard baudrate did not exceed the requested baudrate.
This change of selection logic was never reflected in the atmel serial
driver. Thus the comment left in the atmel serial driver is no longer
accurate.
Additionally the manual rounding up described in that comment and applied
via (quot - 1) requests an incorrect baudrate. Since uart_set_options uses
tty_termios_encode_baud_rate to determine the appropriate baudrate flags
this can cause baudrate selection to fail entirely because
tty_termios_encode_baud_rate will only select a baudrate if relative error
between requested and selected baudrate does not exceed +/-2%.
Fix that by requesting actual, exact baudrate used by the serial.

Fixes: ba47f97a18f2 ("serial: core: remove baud_rates when serial console setup")
Signed-off-by: Tobias Schramm <[email protected]>
---
drivers/tty/serial/atmel_serial.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index f1c06e12efa0..9cd7479b03c0 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2657,13 +2657,7 @@ static void __init atmel_console_get_options(struct uart_port *port, int *baud,
else if (mr == ATMEL_US_PAR_ODD)
*parity = 'o';

- /*
- * The serial core only rounds down when matching this to a
- * supported baud rate. Make sure we don't end up slightly
- * lower than one of those, as it would make us fall through
- * to a much lower baud rate than we really want.
- */
- *baud = port->uartclk / (16 * (quot - 1));
+ *baud = port->uartclk / (16 * quot);
}

static int __init atmel_console_setup(struct console *co, char *options)
--
2.30.2


2023-01-12 16:31:57

by Richard Genoud

[permalink] [raw]
Subject: Re: [PATCH v2] serial: atmel: fix incorrect baudrate setup

Le 09/01/2023 à 08:29, Tobias Schramm a écrit :
> Commit ba47f97a18f2 ("serial: core: remove baud_rates when serial console
> setup") changed uart_set_options to select the correct baudrate
> configuration based on the absolute error between requested baudrate and
> available standard baudrate settings.
> Prior to that commit the baudrate was selected based on which predefined
> standard baudrate did not exceed the requested baudrate.
> This change of selection logic was never reflected in the atmel serial
> driver. Thus the comment left in the atmel serial driver is no longer
> accurate.
> Additionally the manual rounding up described in that comment and applied
> via (quot - 1) requests an incorrect baudrate. Since uart_set_options uses
> tty_termios_encode_baud_rate to determine the appropriate baudrate flags
> this can cause baudrate selection to fail entirely because
> tty_termios_encode_baud_rate will only select a baudrate if relative error
> between requested and selected baudrate does not exceed +/-2%.
> Fix that by requesting actual, exact baudrate used by the serial.
>
> Fixes: ba47f97a18f2 ("serial: core: remove baud_rates when serial console setup")
> Signed-off-by: Tobias Schramm <[email protected]>
Acked-by: Richard Genoud <[email protected]>

Tested-on sam9g35
> ---
> drivers/tty/serial/atmel_serial.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index f1c06e12efa0..9cd7479b03c0 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2657,13 +2657,7 @@ static void __init atmel_console_get_options(struct uart_port *port, int *baud,
> else if (mr == ATMEL_US_PAR_ODD)
> *parity = 'o';
>
> - /*
> - * The serial core only rounds down when matching this to a
> - * supported baud rate. Make sure we don't end up slightly
> - * lower than one of those, as it would make us fall through
> - * to a much lower baud rate than we really want.
> - */
> - *baud = port->uartclk / (16 * (quot - 1));
> + *baud = port->uartclk / (16 * quot);
> }
>
> static int __init atmel_console_setup(struct console *co, char *options)
Thanks !


Regards,
Richard