On Sun, 11 Sep 2022, Lukas Wunner wrote:
> The STM32 USART can control RS-485 Transmit Enable in hardware. Since
> commit 7df5081cbf5e ("serial: stm32: Add RS485 RTS GPIO control"),
> it can alternatively be controlled in software. That was done to allow
> RS-485 even if the RTS pin is unavailable because it's pinmuxed to a
> different function.
>
> However the commit neglected to deassert Transmit Enable upon invocation
> of the ->rs485_config() callback. Fix it.
>
> Avoid forward declarations by moving stm32_usart_tx_empty(),
> stm32_usart_rs485_rts_enable() and stm32_usart_rs485_rts_disable()
> further up in the driver.
>
> Fixes: 7df5081cbf5e ("serial: stm32: Add RS485 RTS GPIO control")
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # v5.9+
> Cc: Marek Vasut <marex@xxxxxxx>
I'd prefer to have those plain function moves to be in a separate patch.
Other than that,
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
--
i.
> ---
> drivers/tty/serial/stm32-usart.c | 100 ++++++++++++++++---------------
> 1 file changed, 53 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> index 0b18615b2ca4..c48f225eba86 100644
> --- a/drivers/tty/serial/stm32-usart.c
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -131,6 +131,53 @@ static void stm32_usart_clr_bits(struct uart_port *port, u32 reg, u32 bits)
> writel_relaxed(val, port->membase + reg);
> }
>
> +static unsigned int stm32_usart_tx_empty(struct uart_port *port)
> +{
> + struct stm32_port *stm32_port = to_stm32_port(port);
> + const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> +
> + if (readl_relaxed(port->membase + ofs->isr) & USART_SR_TC)
> + return TIOCSER_TEMT;
> +
> + return 0;
> +}
> +
> +static void stm32_usart_rs485_rts_enable(struct uart_port *port)
> +{
> + struct stm32_port *stm32_port = to_stm32_port(port);
> + struct serial_rs485 *rs485conf = &port->rs485;
> +
> + if (stm32_port->hw_flow_control ||
> + !(rs485conf->flags & SER_RS485_ENABLED))
> + return;
> +
> + 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);
> + }
> +}
> +
> +static void stm32_usart_rs485_rts_disable(struct uart_port *port)
> +{
> + struct stm32_port *stm32_port = to_stm32_port(port);
> + struct serial_rs485 *rs485conf = &port->rs485;
> +
> + if (stm32_port->hw_flow_control ||
> + !(rs485conf->flags & SER_RS485_ENABLED))
> + return;
> +
> + 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);
> + }
> +}
> +
> static void stm32_usart_config_reg_rs485(u32 *cr1, u32 *cr3, u32 delay_ADE,
> u32 delay_DDE, u32 baud)
> {
> @@ -214,6 +261,12 @@ static int stm32_usart_config_rs485(struct uart_port *port, struct ktermios *ter
>
> stm32_usart_set_bits(port, ofs->cr1, BIT(cfg->uart_enable_bit));
>
> + /* Adjust RTS polarity in case it's driven in software */
> + if (stm32_usart_tx_empty(port))
> + stm32_usart_rs485_rts_disable(port);
> + else
> + stm32_usart_rs485_rts_enable(port);
> +
> return 0;
> }
>
> @@ -529,42 +582,6 @@ static void stm32_usart_tc_interrupt_disable(struct uart_port *port)
> stm32_usart_clr_bits(port, ofs->cr1, USART_CR1_TCIE);
> }
>
> -static void stm32_usart_rs485_rts_enable(struct uart_port *port)
> -{
> - struct stm32_port *stm32_port = to_stm32_port(port);
> - struct serial_rs485 *rs485conf = &port->rs485;
> -
> - if (stm32_port->hw_flow_control ||
> - !(rs485conf->flags & SER_RS485_ENABLED))
> - return;
> -
> - 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);
> - }
> -}
> -
> -static void stm32_usart_rs485_rts_disable(struct uart_port *port)
> -{
> - struct stm32_port *stm32_port = to_stm32_port(port);
> - struct serial_rs485 *rs485conf = &port->rs485;
> -
> - if (stm32_port->hw_flow_control ||
> - !(rs485conf->flags & SER_RS485_ENABLED))
> - return;
> -
> - 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);
> - }
> -}
> -
> static void stm32_usart_transmit_chars_pio(struct uart_port *port)
> {
> struct stm32_port *stm32_port = to_stm32_port(port);
> @@ -807,17 +824,6 @@ static irqreturn_t stm32_usart_threaded_interrupt(int irq, void *ptr)
> return IRQ_HANDLED;
> }
>
> -static unsigned int stm32_usart_tx_empty(struct uart_port *port)
> -{
> - struct stm32_port *stm32_port = to_stm32_port(port);
> - const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> -
> - if (readl_relaxed(port->membase + ofs->isr) & USART_SR_TC)
> - return TIOCSER_TEMT;
> -
> - return 0;
> -}
> -
> static void stm32_usart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> {
> struct stm32_port *stm32_port = to_stm32_port(port);
>
