On Wed, 9 Nov 2022, Lukas Wunner wrote:
> Commit 801954d1210a ("serial: 8250: 8250_omap: Support native RS485")
> calculates RS485 delays from the baudrate. The baudrate is generated
> with either a 16x or 13x divisor. The divisor is set in the Mode
> Definition Register 1 (MDR1).
>
> The commit erroneously assumes that the register stores the divisor as
> a bitmask and uses a logical AND to differentiate between 16x and 13x
> divisors. However the divisor is really stored as a 3-bit mode
> (see lines 363ff in include/uapi/linux/serial_reg.h).
>
> The logical AND operation is performed with UART_OMAP_MDR1_16X_MODE,
> which is defined as 0x0 and hence yields false. So the commit always
> assumes a 13x divisor. Fix by using an equal comparison. This works
> because we never set any of the other 5 bits in the register. (They
> pertain to IrDA mode, which is not supported by the driver).
>
> Fixes: 801954d1210a ("serial: 8250: 8250_omap: Support native RS485")
> Link: https://lore.kernel.org/linux-serial/202211070440.8hWunFUN-lkp@xxxxxxxxx/
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Reported-by: Dan Carpenter <error27@xxxxxxxxx>
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> ---
> drivers/tty/serial/8250/8250_omap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 1dd8c5b..734f092 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -824,7 +824,7 @@ static int omap8250_rs485_config(struct uart_port *port,
> * of the AM65 TRM: https://www.ti.com/lit/ug/spruid7e/spruid7e.pdf
> */
> if (priv->quot) {
> - if (priv->mdr1 & UART_OMAP_MDR1_16X_MODE)
> + if (priv->mdr1 == UART_OMAP_MDR1_16X_MODE)
> baud = port->uartclk / (16 * priv->quot);
> else
> baud = port->uartclk / (13 * priv->quot);
It was very lucky that 16x mode happened to be zero so that automation
could catch this problem.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
--
i.
