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@xxxxxxx>
> Cc: Alexandre Torgue <alexandre.torgue@xxxxxx>
> Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> Cc: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> Cc: Fabrice Gasnier <fabrice.gasnier@xxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: linux-stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
> ---
> 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 {
>
