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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231122151006.GB18949@wunner.de>
Date:   Wed, 22 Nov 2023 16:10:06 +0100
From:   Lukas Wunner <lukas@...ner.de>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>, devicetree@...r.kernel.org,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
        Lino Sanfilippo <LinoSanfilippo@....de>,
        Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH 2/2] serial: core: implement support for rs485-mux-gpios

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