[Linux-stm32] [PATCH V3] serial: stm32: Add RS485 RTS GPIO control again

[Fabrice Gasnier]

On 8/31/20 7:10 PM, Marek Vasut wrote:
> While the STM32 does support RS485 drive-enable control within the
> UART IP itself, some systems have the drive-enable line connected
> to a pin which cannot be pinmuxed as RTS. Add support for toggling
> the RTS GPIO line using the modem control GPIOs to provide at least
> some sort of emulation.
> 
> Fixes: 7df5081cbf5e ("serial: stm32: Add RS485 RTS GPIO control")
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Alexandre Torgue <alexandre.torgue at st.com>
> Cc: Andy Shevchenko <andy.shevchenko at gmail.com>
> Cc: Manivannan Sadhasivam <mani at kernel.org>
> Cc: Fabrice Gasnier <fabrice.gasnier at st.com>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> Cc: linux-stm32 at st-md-mailman.stormreply.com
> ---
> V2: Use mctrl_gpio_set() instead of stm32_set_mctrl()
> V3: - Actually toggle the RTS line before and after TX
>     - Undo 7df5081cbf5e ("serial: stm32: Add RS485 RTS GPIO control")
>       which was previous version of this patch ; I messed up.
> ---
>  drivers/tty/serial/stm32-usart.c | 33 ++++++++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 8 deletions(-)


Hi Marek,

This seems sensible. I've few comments on the commit tittle and commit
message:
- tittle: this could be named as a "fix" rather than "add... again" ?
- I may have missed it... Is it a V3 ?

Could explain what is being fixed? Why moving the mctrl_gpio_* calls
away from the stm32_config_rs485()/set_termios() routines to the
start_tx/stop_tx ops improves/fixes the RS485 RTS GPIO control (what was
wrong) ?

Thanks,
Best regards,
Fabrice

> 
> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> index 143300a80090..23f7453441ae 100644
> --- a/drivers/tty/serial/stm32-usart.c
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -129,13 +129,9 @@ static int stm32_config_rs485(struct uart_port *port,
>  		if (rs485conf->flags & SER_RS485_RTS_ON_SEND) {
>  			cr3 &= ~USART_CR3_DEP;
>  			rs485conf->flags &= ~SER_RS485_RTS_AFTER_SEND;
> -			mctrl_gpio_set(stm32_port->gpios,
> -					stm32_port->port.mctrl & ~TIOCM_RTS);
>  		} else {
>  			cr3 |= USART_CR3_DEP;
>  			rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
> -			mctrl_gpio_set(stm32_port->gpios,
> -					stm32_port->port.mctrl | TIOCM_RTS);
>  		}
>  
>  		writel_relaxed(cr3, port->membase + ofs->cr3);
> @@ -541,17 +537,42 @@ static void stm32_disable_ms(struct uart_port *port)
>  /* Transmit stop */
>  static void stm32_stop_tx(struct uart_port *port)
>  {
> +	struct stm32_port *stm32_port = to_stm32_port(port);
> +	struct serial_rs485 *rs485conf = &port->rs485;
> +
>  	stm32_tx_interrupt_disable(port);
> +
> +	if (rs485conf->flags & SER_RS485_ENABLED) {
> +		if (rs485conf->flags & SER_RS485_RTS_ON_SEND) {
> +			mctrl_gpio_set(stm32_port->gpios,
> +					stm32_port->port.mctrl & ~TIOCM_RTS);
> +		} else {
> +			mctrl_gpio_set(stm32_port->gpios,
> +					stm32_port->port.mctrl | TIOCM_RTS);
> +		}
> +	}
>  }
>  
>  /* There are probably characters waiting to be transmitted. */
>  static void stm32_start_tx(struct uart_port *port)
>  {
> +	struct stm32_port *stm32_port = to_stm32_port(port);
> +	struct serial_rs485 *rs485conf = &port->rs485;
>  	struct circ_buf *xmit = &port->state->xmit;
>  
>  	if (uart_circ_empty(xmit))
>  		return;
>  
> +	if (rs485conf->flags & SER_RS485_ENABLED) {
> +		if (rs485conf->flags & SER_RS485_RTS_ON_SEND) {
> +			mctrl_gpio_set(stm32_port->gpios,
> +					stm32_port->port.mctrl | TIOCM_RTS);
> +		} else {
> +			mctrl_gpio_set(stm32_port->gpios,
> +					stm32_port->port.mctrl & ~TIOCM_RTS);
> +		}
> +	}
> +
>  	stm32_transmit_chars(port);
>  }
>  
> @@ -851,13 +872,9 @@ static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>  		if (rs485conf->flags & SER_RS485_RTS_ON_SEND) {
>  			cr3 &= ~USART_CR3_DEP;
>  			rs485conf->flags &= ~SER_RS485_RTS_AFTER_SEND;
> -			mctrl_gpio_set(stm32_port->gpios,
> -					stm32_port->port.mctrl & ~TIOCM_RTS);
>  		} else {
>  			cr3 |= USART_CR3_DEP;
>  			rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
> -			mctrl_gpio_set(stm32_port->gpios,
> -					stm32_port->port.mctrl | TIOCM_RTS);
>  		}
>  
>  	} else {
>