* [PATCH v2 0/2] tty/serial: atmel: Fix RS485 half duplex operation @ 2019-03-19 13:20 Razvan Stefanescu 2019-03-19 13:20 ` [PATCH v2 1/2] tty/serial: atmel: Add is_half_duplex helper Razvan Stefanescu 2019-03-19 13:20 ` [PATCH v2 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped Razvan Stefanescu 0 siblings, 2 replies; 5+ messages in thread From: Razvan Stefanescu @ 2019-03-19 13:20 UTC (permalink / raw) To: Richard Genoud, Greg Kroah-Hartman, Jiri Slaby Cc: Gil Weber, Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-serial, linux-arm-kernel, linux-kernel Using a loopback serial cable with RS485 protocol shows that data is received: $ stty -F /dev/ttyS3 raw -echo speed 4800 $ cat /dev/ttyS3 & $ echo "Hello, world" > /dev/ttyS3 Hello, world Last line should not be displayed, as it indicates that RX was started before TX finished. This happens because driver activates RX when the DMA transfer completes, but that does not necessarily mean the TX FIFO was emptied. First patch will add a helper that checks if the transmission is half-duplex and uses it throughout the driver, replacing multiple lines of code. Second patch implements the fix by adding a variable to the port struct. This is used to indicate that RX needs to be started. When the DMA transfer completes, the variable is set and the ATMEL_US_TXEMPTY is reactivated. In the interrupt handler, if the variable is set, RX is started. Changelog: v1 -> v2: - remove wrongly added check; - start rx and display warning message in case of error - add fix info Razvan Stefanescu (2): tty/serial: atmel: Add is_half_duplex helper tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped drivers/tty/serial/atmel_serial.c | 48 +++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 15 deletions(-) -- 2.19.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] tty/serial: atmel: Add is_half_duplex helper 2019-03-19 13:20 [PATCH v2 0/2] tty/serial: atmel: Fix RS485 half duplex operation Razvan Stefanescu @ 2019-03-19 13:20 ` Razvan Stefanescu 2019-03-26 15:10 ` Richard Genoud 2019-03-19 13:20 ` [PATCH v2 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped Razvan Stefanescu 1 sibling, 1 reply; 5+ messages in thread From: Razvan Stefanescu @ 2019-03-19 13:20 UTC (permalink / raw) To: Richard Genoud, Greg Kroah-Hartman, Jiri Slaby Cc: Gil Weber, Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-serial, linux-arm-kernel, linux-kernel Use a helper function to check that a port needs to use half duplex communication, replacing several occurrences of multi-line bit checking. Fixes: b389f173aaa1 ("tty/serial: atmel: RS485 half duplex w/DMA: enable RX after TX is done") Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com> --- Changelog: v2: - remove extra check - add fix info drivers/tty/serial/atmel_serial.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c index 05147fe24343..b4b89a16a41b 100644 --- a/drivers/tty/serial/atmel_serial.c +++ b/drivers/tty/serial/atmel_serial.c @@ -231,6 +231,13 @@ static inline void atmel_uart_write_char(struct uart_port *port, u8 value) __raw_writeb(value, port->membase + ATMEL_US_THR); } +static inline int atmel_uart_is_half_duplex(struct uart_port *port) +{ + return ((port->rs485.flags & SER_RS485_ENABLED) && + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) || + (port->iso7816.flags & SER_ISO7816_ENABLED); +} + #ifdef CONFIG_SERIAL_ATMEL_PDC static bool atmel_use_pdc_rx(struct uart_port *port) { @@ -608,10 +615,9 @@ static void atmel_stop_tx(struct uart_port *port) /* Disable interrupts */ atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask); - if (((port->rs485.flags & SER_RS485_ENABLED) && - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) || - port->iso7816.flags & SER_ISO7816_ENABLED) + if (atmel_uart_is_half_duplex(port)) atmel_start_rx(port); + } /* @@ -628,9 +634,7 @@ static void atmel_start_tx(struct uart_port *port) return; if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port)) - if (((port->rs485.flags & SER_RS485_ENABLED) && - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) || - port->iso7816.flags & SER_ISO7816_ENABLED) + if (atmel_uart_is_half_duplex(port)) atmel_stop_rx(port); if (atmel_use_pdc_tx(port)) @@ -928,9 +932,7 @@ static void atmel_complete_tx_dma(void *arg) */ if (!uart_circ_empty(xmit)) atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx); - else if (((port->rs485.flags & SER_RS485_ENABLED) && - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) || - port->iso7816.flags & SER_ISO7816_ENABLED) { + else if (atmel_uart_is_half_duplex(port)) { /* DMA done, stop TX, start RX for RS485 */ atmel_start_rx(port); } @@ -1508,9 +1510,7 @@ static void atmel_tx_pdc(struct uart_port *port) atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask); } else { - if (((port->rs485.flags & SER_RS485_ENABLED) && - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) || - port->iso7816.flags & SER_ISO7816_ENABLED) { + if (atmel_uart_is_half_duplex(port)) { /* DMA done, stop TX, start RX for RS485 */ atmel_start_rx(port); } -- 2.19.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] tty/serial: atmel: Add is_half_duplex helper 2019-03-19 13:20 ` [PATCH v2 1/2] tty/serial: atmel: Add is_half_duplex helper Razvan Stefanescu @ 2019-03-26 15:10 ` Richard Genoud 0 siblings, 0 replies; 5+ messages in thread From: Richard Genoud @ 2019-03-26 15:10 UTC (permalink / raw) To: Razvan Stefanescu, Greg Kroah-Hartman, Jiri Slaby Cc: Gil Weber, Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-serial, linux-arm-kernel, linux-kernel Le 19/03/2019 à 14:20, Razvan Stefanescu a écrit : > Use a helper function to check that a port needs to use half duplex > communication, replacing several occurrences of multi-line bit checking. > > Fixes: b389f173aaa1 ("tty/serial: atmel: RS485 half duplex w/DMA: enable > RX after TX is done") > Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com> Acked-by: Richard Genoud <richard.genoud@gmail.com> NB: backport on kernel older than 4.20 will fail because of the SER_ISO7816_ENABLED flag. > --- > Changelog: > v2: > - remove extra check > - add fix info > > drivers/tty/serial/atmel_serial.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c > index 05147fe24343..b4b89a16a41b 100644 > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -231,6 +231,13 @@ static inline void atmel_uart_write_char(struct uart_port *port, u8 value) > __raw_writeb(value, port->membase + ATMEL_US_THR); > } > > +static inline int atmel_uart_is_half_duplex(struct uart_port *port) > +{ > + return ((port->rs485.flags & SER_RS485_ENABLED) && > + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) || > + (port->iso7816.flags & SER_ISO7816_ENABLED); > +} > + > #ifdef CONFIG_SERIAL_ATMEL_PDC > static bool atmel_use_pdc_rx(struct uart_port *port) > { > @@ -608,10 +615,9 @@ static void atmel_stop_tx(struct uart_port *port) > /* Disable interrupts */ > atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask); > > - if (((port->rs485.flags & SER_RS485_ENABLED) && > - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) || > - port->iso7816.flags & SER_ISO7816_ENABLED) > + if (atmel_uart_is_half_duplex(port)) > atmel_start_rx(port); > + > } > > /* > @@ -628,9 +634,7 @@ static void atmel_start_tx(struct uart_port *port) > return; > > if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port)) > - if (((port->rs485.flags & SER_RS485_ENABLED) && > - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) || > - port->iso7816.flags & SER_ISO7816_ENABLED) > + if (atmel_uart_is_half_duplex(port)) > atmel_stop_rx(port); > > if (atmel_use_pdc_tx(port)) > @@ -928,9 +932,7 @@ static void atmel_complete_tx_dma(void *arg) > */ > if (!uart_circ_empty(xmit)) > atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx); > - else if (((port->rs485.flags & SER_RS485_ENABLED) && > - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) || > - port->iso7816.flags & SER_ISO7816_ENABLED) { > + else if (atmel_uart_is_half_duplex(port)) { > /* DMA done, stop TX, start RX for RS485 */ > atmel_start_rx(port); > } > @@ -1508,9 +1510,7 @@ static void atmel_tx_pdc(struct uart_port *port) > atmel_uart_writel(port, ATMEL_US_IER, > atmel_port->tx_done_mask); > } else { > - if (((port->rs485.flags & SER_RS485_ENABLED) && > - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) || > - port->iso7816.flags & SER_ISO7816_ENABLED) { > + if (atmel_uart_is_half_duplex(port)) { > /* DMA done, stop TX, start RX for RS485 */ > atmel_start_rx(port); > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped 2019-03-19 13:20 [PATCH v2 0/2] tty/serial: atmel: Fix RS485 half duplex operation Razvan Stefanescu 2019-03-19 13:20 ` [PATCH v2 1/2] tty/serial: atmel: Add is_half_duplex helper Razvan Stefanescu @ 2019-03-19 13:20 ` Razvan Stefanescu 2019-03-26 15:12 ` Richard Genoud 1 sibling, 1 reply; 5+ messages in thread From: Razvan Stefanescu @ 2019-03-19 13:20 UTC (permalink / raw) To: Richard Genoud, Greg Kroah-Hartman, Jiri Slaby Cc: Gil Weber, Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-serial, linux-arm-kernel, linux-kernel In half-duplex operation, RX should be started after TX completes. If DMA is used, there is a case when the DMA transfer completes but the TX FIFO is not emptied, so the RX cannot be restarted just yet. Use a boolean variable to store this state and rearm TX interrupt mask to be signaled again that the transfer finished. In interrupt transmit handler this variable is used to start RX. A warning message is generated if RX is activated before TX fifo is cleared. Fixes: b389f173aaa1 ("tty/serial: atmel: RS485 half duplex w/DMA: enable RX after TX is done") Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com> --- Changelog: v2: - start RX and display warning in case of error - add fix info drivers/tty/serial/atmel_serial.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c index b4b89a16a41b..5b2f859c327c 100644 --- a/drivers/tty/serial/atmel_serial.c +++ b/drivers/tty/serial/atmel_serial.c @@ -166,6 +166,8 @@ struct atmel_uart_port { unsigned int pending_status; spinlock_t lock_suspended; + bool hd_start_rx; /* can start RX during half-duplex operation */ + /* ISO7816 */ unsigned int fidi_min; unsigned int fidi_max; @@ -933,8 +935,13 @@ static void atmel_complete_tx_dma(void *arg) if (!uart_circ_empty(xmit)) atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx); else if (atmel_uart_is_half_duplex(port)) { - /* DMA done, stop TX, start RX for RS485 */ - atmel_start_rx(port); + /* + * DMA done, re-enable TXEMPTY and signal that we can stop + * TX and start RX for RS485 + */ + atmel_port->hd_start_rx = true; + atmel_uart_writel(port, ATMEL_US_IER, + atmel_port->tx_done_mask); } spin_unlock_irqrestore(&port->lock, flags); @@ -1378,9 +1385,20 @@ atmel_handle_transmit(struct uart_port *port, unsigned int pending) struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); if (pending & atmel_port->tx_done_mask) { - /* Either PDC or interrupt transmission */ atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask); + + /* Start RX if flag was set and FIFO is empty */ + if (atmel_port->hd_start_rx) { + if (!(atmel_uart_readl(port, ATMEL_US_CSR) + & ATMEL_US_TXEMPTY)) + dev_warn(port->dev, "Should start RX, but TX fifo is not empty\n"); + + atmel_port->hd_start_rx = false; + atmel_start_rx(port); + return; + } + atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx); } } -- 2.19.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped 2019-03-19 13:20 ` [PATCH v2 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped Razvan Stefanescu @ 2019-03-26 15:12 ` Richard Genoud 0 siblings, 0 replies; 5+ messages in thread From: Richard Genoud @ 2019-03-26 15:12 UTC (permalink / raw) To: Razvan Stefanescu, Greg Kroah-Hartman, Jiri Slaby Cc: Gil Weber, Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-serial, linux-arm-kernel, linux-kernel Le 19/03/2019 à 14:20, Razvan Stefanescu a écrit : > In half-duplex operation, RX should be started after TX completes. > > If DMA is used, there is a case when the DMA transfer completes but the > TX FIFO is not emptied, so the RX cannot be restarted just yet. > > Use a boolean variable to store this state and rearm TX interrupt mask > to be signaled again that the transfer finished. In interrupt transmit > handler this variable is used to start RX. A warning message is generated > if RX is activated before TX fifo is cleared. > > Fixes: b389f173aaa1 ("tty/serial: atmel: RS485 half duplex w/DMA: enable > RX after TX is done") > Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com> Acked-by: Richard Genoud <richard.genoud@gmail.com> NB: backport on kernel older than 4.20 will fail because of the iso7816 variables fidi_min/fidi_max. > --- > Changelog: > v2: > - start RX and display warning in case of error > - add fix info > > drivers/tty/serial/atmel_serial.c | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c > index b4b89a16a41b..5b2f859c327c 100644 > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -166,6 +166,8 @@ struct atmel_uart_port { > unsigned int pending_status; > spinlock_t lock_suspended; > > + bool hd_start_rx; /* can start RX during half-duplex operation */ > + > /* ISO7816 */ > unsigned int fidi_min; > unsigned int fidi_max; > @@ -933,8 +935,13 @@ static void atmel_complete_tx_dma(void *arg) > if (!uart_circ_empty(xmit)) > atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx); > else if (atmel_uart_is_half_duplex(port)) { > - /* DMA done, stop TX, start RX for RS485 */ > - atmel_start_rx(port); > + /* > + * DMA done, re-enable TXEMPTY and signal that we can stop > + * TX and start RX for RS485 > + */ > + atmel_port->hd_start_rx = true; > + atmel_uart_writel(port, ATMEL_US_IER, > + atmel_port->tx_done_mask); > } > > spin_unlock_irqrestore(&port->lock, flags); > @@ -1378,9 +1385,20 @@ atmel_handle_transmit(struct uart_port *port, unsigned int pending) > struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); > > if (pending & atmel_port->tx_done_mask) { > - /* Either PDC or interrupt transmission */ > atmel_uart_writel(port, ATMEL_US_IDR, > atmel_port->tx_done_mask); > + > + /* Start RX if flag was set and FIFO is empty */ > + if (atmel_port->hd_start_rx) { > + if (!(atmel_uart_readl(port, ATMEL_US_CSR) > + & ATMEL_US_TXEMPTY)) > + dev_warn(port->dev, "Should start RX, but TX fifo is not empty\n"); > + > + atmel_port->hd_start_rx = false; > + atmel_start_rx(port); > + return; > + } > + > atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx); > } > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-26 15:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-03-19 13:20 [PATCH v2 0/2] tty/serial: atmel: Fix RS485 half duplex operation Razvan Stefanescu 2019-03-19 13:20 ` [PATCH v2 1/2] tty/serial: atmel: Add is_half_duplex helper Razvan Stefanescu 2019-03-26 15:10 ` Richard Genoud 2019-03-19 13:20 ` [PATCH v2 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped Razvan Stefanescu 2019-03-26 15:12 ` Richard Genoud
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).