Re: [PATCH] serial: mvebu-uart: correctly report configured baudrate value

From: Ilpo Järvinen
Date: Tue Jun 28 2022 - 06:23:44 EST


On Tue, 28 Jun 2022, Pali Rohár wrote:

> Functions tty_termios_encode_baud_rate() and uart_update_timeout() should
> be called with the baudrate value which was set to hardware. Linux then
> report exact values via ioctl(TCGETS2) to userspace.
>
> Change mvebu_uart_baud_rate_set() function to return baudrate value which
> was set to hardware and propagate this value to above mentioned functions.
>
> With this change userspace would see precise value in termios c_ospeed
> field.
>
> Signed-off-by: Pali Rohár <pali@xxxxxxxxxx>
> Fixes: 68a0db1d7da2 ("serial: mvebu-uart: add function to change baudrate")

Look better than my patch covering cases I didn't even realize
existed. Thanks.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>

--
i.


> ---
> drivers/tty/serial/mvebu-uart.c | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
> index 0429c2a54290..93489fe334d0 100644
> --- a/drivers/tty/serial/mvebu-uart.c
> +++ b/drivers/tty/serial/mvebu-uart.c
> @@ -470,14 +470,14 @@ static void mvebu_uart_shutdown(struct uart_port *port)
> }
> }
>
> -static int mvebu_uart_baud_rate_set(struct uart_port *port, unsigned int baud)
> +static unsigned int mvebu_uart_baud_rate_set(struct uart_port *port, unsigned int baud)
> {
> unsigned int d_divisor, m_divisor;
> unsigned long flags;
> u32 brdv, osamp;
>
> if (!port->uartclk)
> - return -EOPNOTSUPP;
> + return 0;
>
> /*
> * The baudrate is derived from the UART clock thanks to divisors:
> @@ -548,7 +548,7 @@ static int mvebu_uart_baud_rate_set(struct uart_port *port, unsigned int baud)
> (m_divisor << 16) | (m_divisor << 24);
> writel(osamp, port->membase + UART_OSAMP);
>
> - return 0;
> + return DIV_ROUND_CLOSEST(port->uartclk, d_divisor * m_divisor);
> }
>
> static void mvebu_uart_set_termios(struct uart_port *port,
> @@ -587,15 +587,11 @@ static void mvebu_uart_set_termios(struct uart_port *port,
> max_baud = port->uartclk / 80;
>
> baud = uart_get_baud_rate(port, termios, old, min_baud, max_baud);
> - if (mvebu_uart_baud_rate_set(port, baud)) {
> - /* No clock available, baudrate cannot be changed */
> - if (old)
> - baud = uart_get_baud_rate(port, old, NULL,
> - min_baud, max_baud);
> - } else {
> - tty_termios_encode_baud_rate(termios, baud, baud);
> - uart_update_timeout(port, termios->c_cflag, baud);
> - }
> + baud = mvebu_uart_baud_rate_set(port, baud);
> +
> + /* In case baudrate cannot be changed, report previous old value */
> + if (baud == 0 && old)
> + baud = tty_termios_baud_rate(old);
>
> /* Only the following flag changes are supported */
> if (old) {
> @@ -606,6 +602,11 @@ static void mvebu_uart_set_termios(struct uart_port *port,
> termios->c_cflag |= CS8;
> }
>
> + if (baud != 0) {
> + tty_termios_encode_baud_rate(termios, baud, baud);
> + uart_update_timeout(port, termios->c_cflag, baud);
> + }
> +
> spin_unlock_irqrestore(&port->lock, flags);
> }
>
>