Re: [RESEND PATCH v4 1/7] serial: Do not hold the port lock when setting rx-during-tx GPIO

From: Lino Sanfilippo
Date: Wed Nov 22 2023 - 06:56:04 EST




On 19.11.23 12:28, Lino Sanfilippo wrote:
> Both the imx and stm32 driver set the rx-during-tx GPIO in rs485_config().
> Since this function is called with the port lock held, this can be an
> problem in case that setting the GPIO line can sleep (e.g. if a GPIO
> expander is used which is connected via SPI or I2C).
>
> Avoid this issue by moving the GPIO setting outside of the port lock into
> the serial core and thus making it a generic feature.
>
> Since both setting the term and the rx-during-tx GPIO is done at the same
> code place, move it into a common function.

> - const struct serial_rs485 *rs485)
> +/*
> + * Set optional RS485 GPIOs for bus termination and data reception during
> + * transmission. These GPIOs are controlled by the serial core independently
> + * from the UART driver.
> + */
> +static void uart_set_rs485_gpios(struct uart_port *port,
> + const struct serial_rs485 *rs485)
> {
> if (!(rs485->flags & SER_RS485_ENABLED))
> return;
>
> gpiod_set_value_cansleep(port->rs485_term_gpio,
> !!(rs485->flags & SER_RS485_TERMINATE_BUS));
> + gpiod_set_value_cansleep(port->rs485_rx_during_tx_gpio,
> + !!(rs485->flags & SER_RS485_RX_DURING_TX));
> }
>

Rasmus is about to add support for another RS485 related GPIO (see
https://lore.kernel.org/all/20231120151056.148450-3-linux@xxxxxxxxxxxxxxxxxx/ )
which has to be configured both before and after port->rs485_config(). This
does not fit very well with the idea of handling all these GPIOs in
one function.

So I would like to revise this patch and send an updated version in a v5
of this series in which the suggestion from Hugo
(see https://lore.kernel.org/all/20231011183656.5111ba32ec0c9d43171662a1@xxxxxxxxxxx/)
is implemented to use separate functions for the GPIO configurations.

Regards,
Lino