Re: [PATCH 2/2] serial: stm32: Use TC interrupt to deassert GPIO RTS in RS485 mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marek,

On 4/30/22 18:28, Marek Vasut wrote:
In case the RS485 mode is emulated using GPIO RTS, use the TC interrupt
to deassert the GPIO RTS, otherwise the GPIO RTS stays asserted after a
transmission ended and the RS485 cannot work.

Could you please add a cover letter to explain the rational of the first patch ? I understood the goal of the first by reading the commit message of this second patch.

Signed-off-by: Marek Vasut <marex@xxxxxxx>
Cc: Alexandre Torgue <alexandre.torgue@xxxxxxxxxxx>
Cc: Erwan Le Ray <erwan.leray@xxxxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: Jean Philippe Romain <jean-philippe.romain@xxxxxxxxxxx>
Cc: Valentin Caron <valentin.caron@xxxxxxxxxxx>
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Cc: linux-stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
To: linux-serial@xxxxxxxxxxxxxxx
---
  drivers/tty/serial/stm32-usart.c | 42 ++++++++++++++++++++++++++++++--
  drivers/tty/serial/stm32-usart.h |  1 +
  2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 224f359c6051e..764415b8e8f03 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -417,6 +417,14 @@ static void stm32_usart_tx_interrupt_enable(struct uart_port *port)
  		stm32_usart_set_bits(port, ofs->cr1, USART_CR1_TXEIE);
  }
+static void stm32_usart_tc_interrupt_enable(struct uart_port *port)
+{
+	struct stm32_port *stm32_port = to_stm32_port(port);
+	const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
+
+	stm32_usart_set_bits(port, ofs->cr1, USART_CR1_TCIE);
+}
+
I don't see the added value of this helper (only 1 instruction used 1 time), and other Interrupt Enabled bits are already set/unset in others functions of this driver. To keep an homogeneous code in the driver, could you please remove this helper and set TCIE directly when you need it ?

  static void stm32_usart_rx_dma_complete(void *arg)
  {
  	struct uart_port *port = arg;
@@ -442,6 +450,14 @@ static void stm32_usart_tx_interrupt_disable(struct uart_port *port)
  		stm32_usart_clr_bits(port, ofs->cr1, USART_CR1_TXEIE);
  }
+static void stm32_usart_tc_interrupt_disable(struct uart_port *port)
+{
+	struct stm32_port *stm32_port = to_stm32_port(port);
+	const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
+
+	stm32_usart_clr_bits(port, ofs->cr1, USART_CR1_TCIE);
+}
+
Same comment here.

  static void stm32_usart_rs485_rts_enable(struct uart_port *port)
  {
  	struct stm32_port *stm32_port = to_stm32_port(port);
@@ -585,6 +601,13 @@ static void stm32_usart_transmit_chars(struct uart_port *port)
  	u32 isr;
  	int ret;
+ if (!stm32_port->hw_flow_control &&
+	    port->rs485.flags & SER_RS485_ENABLED) {
+		stm32_port->txdone = false;
+		stm32_usart_tc_interrupt_disable(port);
+		stm32_usart_rs485_rts_enable(port);
+	}
+
  	if (port->x_char) {
  		if (stm32_usart_tx_dma_started(stm32_port) &&
  		    stm32_usart_tx_dma_enabled(stm32_port))
@@ -625,8 +648,14 @@ static void stm32_usart_transmit_chars(struct uart_port *port)
  	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
  		uart_write_wakeup(port);
- if (uart_circ_empty(xmit))
+	if (uart_circ_empty(xmit)) {
  		stm32_usart_tx_interrupt_disable(port);
+		if (!stm32_port->hw_flow_control &&
+		    port->rs485.flags & SER_RS485_ENABLED) {
+			stm32_port->txdone = true;
+			stm32_usart_tc_interrupt_enable(port);
+		}
+	}
  }
static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
@@ -640,6 +669,13 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
sr = readl_relaxed(port->membase + ofs->isr); + if (!stm32_port->hw_flow_control &&
+	    port->rs485.flags & SER_RS485_ENABLED &&
+	    (sr & USART_SR_TC)) {
+		stm32_usart_tc_interrupt_disable(port);
+		stm32_usart_rs485_rts_disable(port);
+	}
+
  	if ((sr & USART_SR_RTOF) && ofs->icr != UNDEF_REG)
  		writel_relaxed(USART_ICR_RTOCF,
  			       port->membase + ofs->icr);
@@ -763,8 +799,10 @@ static void stm32_usart_start_tx(struct uart_port *port)
  {
  	struct circ_buf *xmit = &port->state->xmit;
- if (uart_circ_empty(xmit) && !port->x_char)
+	if (uart_circ_empty(xmit) && !port->x_char) {
+		stm32_usart_rs485_rts_disable(port);
  		return;
+	}
stm32_usart_rs485_rts_enable(port); diff --git a/drivers/tty/serial/stm32-usart.h b/drivers/tty/serial/stm32-usart.h
index d734c4a5fd24c..ee69c203b926d 100644
--- a/drivers/tty/serial/stm32-usart.h
+++ b/drivers/tty/serial/stm32-usart.h
@@ -271,6 +271,7 @@ struct stm32_port {
  	bool hw_flow_control;
  	bool swap;		 /* swap RX & TX pins */
  	bool fifoen;
+	bool txdone;
  	int rxftcfg;		/* RX FIFO threshold CFG      */
  	int txftcfg;		/* TX FIFO threshold CFG      */
  	bool wakeup_src;

Regards, Erwan.



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux