Linux-Serial Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH V3] serial: stm32: Add RS485 RTS GPIO control again
@ 2020-08-31 17:10 Marek Vasut
  2020-09-02  8:08 ` Fabrice Gasnier
  0 siblings, 1 reply; 3+ messages in thread
From: Marek Vasut @ 2020-08-31 17:10 UTC (permalink / raw)
  To: linux-serial
  Cc: Marek Vasut, Alexandre Torgue, Andy Shevchenko,
	Manivannan Sadhasivam, Fabrice Gasnier, Greg Kroah-Hartman,
	linux-stm32

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@denx.de>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Fabrice Gasnier <fabrice.gasnier@st.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-stm32@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(-)

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH V3] serial: stm32: Add RS485 RTS GPIO control again
  2020-08-31 17:10 [PATCH V3] serial: stm32: Add RS485 RTS GPIO control again Marek Vasut
@ 2020-09-02  8:08 ` Fabrice Gasnier
  2020-09-02 12:50   ` Marek Vasut
  0 siblings, 1 reply; 3+ messages in thread
From: Fabrice Gasnier @ 2020-09-02  8:08 UTC (permalink / raw)
  To: Marek Vasut, linux-serial
  Cc: Alexandre Torgue, Andy Shevchenko, Manivannan Sadhasivam,
	Greg Kroah-Hartman, linux-stm32, Erwan LE RAY

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@denx.de>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Manivannan Sadhasivam <mani@kernel.org>
> Cc: Fabrice Gasnier <fabrice.gasnier@st.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-stm32@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 {
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH V3] serial: stm32: Add RS485 RTS GPIO control again
  2020-09-02  8:08 ` Fabrice Gasnier
@ 2020-09-02 12:50   ` Marek Vasut
  0 siblings, 0 replies; 3+ messages in thread
From: Marek Vasut @ 2020-09-02 12:50 UTC (permalink / raw)
  To: Fabrice Gasnier, linux-serial
  Cc: Alexandre Torgue, Andy Shevchenko, Manivannan Sadhasivam,
	Greg Kroah-Hartman, linux-stm32, Erwan LE RAY

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@denx.de>
>> Cc: Alexandre Torgue <alexandre.torgue@st.com>
>> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Cc: Manivannan Sadhasivam <mani@kernel.org>
>> Cc: Fabrice Gasnier <fabrice.gasnier@st.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: linux-stm32@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 {
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-31 17:10 [PATCH V3] serial: stm32: Add RS485 RTS GPIO control again Marek Vasut
2020-09-02  8:08 ` Fabrice Gasnier
2020-09-02 12:50   ` Marek Vasut

Linux-Serial Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-serial/0 linux-serial/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-serial linux-serial/ https://lore.kernel.org/linux-serial \
		linux-serial@vger.kernel.org
	public-inbox-index linux-serial

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-serial


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git