Re: [PATCH 2/2] serial: core: implement support for rs485-mux-gpios

[Lukas Wunner <lukas@wunner.de>]

On Mon, Nov 20, 2023 at 04:10:55PM +0100, Rasmus Villemoes wrote:
> Add code for handling a rs485-mux-gpio specified in device tree.

Hm, that's a bit terse as a commit message.


> @@ -1457,6 +1466,14 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
>  		return ret;
>  	uart_sanitize_serial_rs485(port, &rs485);
>  	uart_set_rs485_termination(port, &rs485);
> +	/*
> +	 * To avoid glitches on the transmit enable pin, the mux must
> +	 * be set before calling the driver's ->rs485_config when
> +	 * disabling rs485 mode, but after when enabling rs485
> +	 * mode.
> +	 */
> +	if (!(rs485.flags & SER_RS485_ENABLED))
> +		uart_set_rs485_mux(port, &rs485);

Can it happen that the UART's FIFO contains characters such that
suddenly switching the mux causes some of them to appear on the
RS-485 transceiver and some on the RS-232 driver?

Shouldn't we wait for the FIFO to drain before making the switch?
I think that would be closer to the behavior expected by user space.


> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -584,6 +584,7 @@ struct uart_port {
>  	struct serial_rs485	rs485_supported;	/* Supported mask for serial_rs485 */
>  	struct gpio_desc	*rs485_term_gpio;	/* enable RS485 bus termination */
>  	struct gpio_desc	*rs485_rx_during_tx_gpio; /* Output GPIO that sets the state of RS485 RX during TX */
> +	struct gpio_desc	*rs485_mux_gpio;	/* gpio for selecting RS485 mode */

Again, the code comment isn't really helpful as it doesn't add a whole
lot of information to the variable name "rs485_mux_gpio".  How about:
"select between RS-232 and RS-485 transceiver" ?

(I realize I made a typo in my previous e-mail about the DT-binding,
sorry about that: s/connect/connected/)

Thanks,

Lukas