On Wed, 22 Jun 2022, Lino Sanfilippo wrote:
> From: Lino Sanfilippo <l.sanfilippo@xxxxxxxxxx>
>
> In serial8250_em485_config() the termination GPIO is set with the uart_port
> spinlock held. This is an issue if setting the GPIO line can sleep (e.g.
> since the concerning GPIO expander is connected via SPI or I2C).
>
> Fix this by setting the termination line outside of the uart_port spinlock
> in the serial core.
>
> This also makes setting the termination GPIO generic for all uart drivers.
>
> Signed-off-by: Lino Sanfilippo <l.sanfilippo@xxxxxxxxxx>
> ---
> drivers/tty/serial/8250/8250_port.c | 3 ---
> drivers/tty/serial/serial_core.c | 12 ++++++++++++
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 3e3d784aa628..5245c179cc51 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -675,9 +675,6 @@ int serial8250_em485_config(struct uart_port *port, struct serial_rs485 *rs485)
> rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
> }
>
> - gpiod_set_value(port->rs485_term_gpio,
> - rs485->flags & SER_RS485_TERMINATE_BUS);
> -
> /*
> * Both serial8250_em485_init() and serial8250_em485_destroy()
> * are idempotent.
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 015f4e1da647..b387376e6fa2 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1352,12 +1352,23 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
> memset(rs485->padding, 0, sizeof(rs485->padding));
> }
>
> +static void uart_set_rs485_termination(struct uart_port *port,
> + const struct serial_rs485 *rs485)
> +{
> + if (!port->rs485_term_gpio || !(rs485->flags & SER_RS485_ENABLED))
> + return;
> +
> + gpiod_set_value_cansleep(port->rs485_term_gpio,
> + !!(rs485->flags & SER_RS485_TERMINATE_BUS));
> +}
> +
> int uart_rs485_config(struct uart_port *port)
> {
> struct serial_rs485 *rs485 = &port->rs485;
> int ret;
>
> uart_sanitize_serial_rs485(port, rs485);
> + uart_set_rs485_termination(port, rs485);
>
> ret = port->rs485_config(port, rs485);
> if (ret)
> @@ -1400,6 +1411,7 @@ static int uart_set_rs485_config(struct uart_port *port,
> if (ret)
> return ret;
> uart_sanitize_serial_rs485(port, &rs485);
> + uart_set_rs485_termination(port, &rs485);
>
> spin_lock_irqsave(&port->lock, flags);
> ret = port->rs485_config(port, &rs485);
When port->rs485_config(port, &rs485) returns non-zero, the input got
partially applied?
--
i.
