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

[Marek Vasut]

On 9/2/20 10:08 AM, Fabrice Gasnier wrote:
> 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 ?

There was a V2 which got applied before I had the chance to send 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) ?

Because set_termios is not called every time there is a transfer, but
the DE GPIOs must be toggled every time there is a transfer (to enable
the DE on TX and disable it right after TX).

> 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 {
>>