[PATCHv3 0/3] Fix hardware handshake on SAM9x5 platforms

linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCHv3 0/3] Fix hardware handshake on SAM9x5 platforms
@ 2016-09-27 14:13 Richard Genoud
  2016-09-27 14:13 ` [PATCHv3 1/3] serial: mctrl_gpio: implement mctrl_gpio_use_rtscts Richard Genoud
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Richard Genoud @ 2016-09-27 14:13 UTC (permalink / raw)
  To: Uwe Kleine-König, Nicolas Ferre, Alexandre Belloni,
	Greg Kroah-Hartman, Cyrille Pitchen
  Cc: linux-serial, linux-kernel, linux-arm-kernel, Richard Genoud

Since commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled"), hardware handshake is not working
anymore on SAM9x5/SAMA5D3/SAM9 platforms.

The first two patches fix the hardware handshake when CTS/RTS pins are
handled by GPIOs.

The last patch fixes hardware handshake when CTS/RTS pins are not GPIOs.

Changes since v2:
 - remove IS_ERR_OR_NULL() test in patch 1/3 as Uwe suggested.
 - fix typos in patch 2/3
 - rebase on next-20160927
 - simplify the logic in patch 3/3.

Changes since v1:
 - Correct patch 1 with the error found by kbuild.
 - Add Alexandre's Acked-by on patch 2
 - Rewrite patch 3 logic in the light of the on-going discussion
   with Cyrille and Alexandre.

NB: patch 2 NEEDS patch 1 to compile.

Richard Genoud (3):
  serial: mctrl_gpio: implement mctrl_gpio_use_rtscts
  tty/serial: at91: fix hardware handshake with GPIOs
  tty/serial: at91: fix hardware handshake on SAM9x5 (without GPIOs)

 drivers/tty/serial/atmel_serial.c      | 26 +++++++++++++++++---------
 drivers/tty/serial/serial_mctrl_gpio.c |  8 ++++++++
 drivers/tty/serial/serial_mctrl_gpio.h | 10 ++++++++++
 3 files changed, 35 insertions(+), 9 deletions(-)

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

* [PATCHv3 1/3] serial: mctrl_gpio: implement mctrl_gpio_use_rtscts
  2016-09-27 14:13 [PATCHv3 0/3] Fix hardware handshake on SAM9x5 platforms Richard Genoud
@ 2016-09-27 14:13 ` Richard Genoud
  2016-09-27 15:12   ` Uwe Kleine-König
  2016-09-27 14:13 ` [PATCHv3 2/3] tty/serial: at91: fix hardware handshake with GPIOs Richard Genoud
  2016-09-27 14:13 ` [PATCHv3 3/3] tty/serial: at91: fix hardware handshake on SAM9x5 (without GPIOs) Richard Genoud
  2 siblings, 1 reply; 6+ messages in thread
From: Richard Genoud @ 2016-09-27 14:13 UTC (permalink / raw)
  To: Uwe Kleine-König, Nicolas Ferre, Alexandre Belloni,
	Greg Kroah-Hartman, Cyrille Pitchen
  Cc: linux-serial, linux-kernel, linux-arm-kernel, Richard Genoud

This function returns true if CTS and RTS are used as GPIOs.
Some drivers (like atmel_serial) needs to know if the flow control is
handled by the controller or by GPIOs.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/tty/serial/serial_mctrl_gpio.c |  8 ++++++++
 drivers/tty/serial/serial_mctrl_gpio.h | 10 ++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index d2da6aa7f27d..0e5525a64c2a 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -17,6 +17,7 @@
 #include <linux/err.h>
 #include <linux/device.h>
 #include <linux/irq.h>
+#include <linux/err.h>
 #include <linux/gpio/consumer.h>
 #include <linux/termios.h>
 #include <linux/serial_core.h>
@@ -72,6 +73,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
 
+bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios)
+{
+	return mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS) &&
+		mctrl_gpio_to_gpiod(gpios, UART_GPIO_RTS);
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_use_rtscts);
+
 unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
 {
 	enum mctrl_gpio_idx i;
diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
index fa000bcff217..c34269733c62 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.h
+++ b/drivers/tty/serial/serial_mctrl_gpio.h
@@ -101,6 +101,11 @@ void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
  */
 void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
 
+/*
+ * Return true if both CTS and RTS are used with GPIOs
+ */
+bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios);
+
 #else /* GPIOLIB */
 
 static inline
@@ -152,6 +157,11 @@ static inline void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
 {
 }
 
+static inline bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios)
+{
+	return false;
+}
+
 #endif /* GPIOLIB */
 
 #endif

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

* Re: [PATCHv3 1/3] serial: mctrl_gpio: implement mctrl_gpio_use_rtscts
  2016-09-27 14:13 ` [PATCHv3 1/3] serial: mctrl_gpio: implement mctrl_gpio_use_rtscts Richard Genoud
@ 2016-09-27 15:12   ` Uwe Kleine-König
  2016-09-27 15:15     ` Richard Genoud
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2016-09-27 15:12 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Nicolas Ferre, Alexandre Belloni, Greg Kroah-Hartman,
	Cyrille Pitchen, linux-serial, linux-kernel, linux-arm-kernel

Hello,

On Tue, Sep 27, 2016 at 04:13:11PM +0200, Richard Genoud wrote:
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> index d2da6aa7f27d..0e5525a64c2a 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -17,6 +17,7 @@
>  #include <linux/err.h>
>  #include <linux/device.h>
>  #include <linux/irq.h>
> +#include <linux/err.h>

this isn't needed any more, right?

>  #include <linux/gpio/consumer.h>
>  #include <linux/termios.h>
>  #include <linux/serial_core.h>
> @@ -72,6 +73,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>  }
>  EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
>  
> +bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios)
> +{
> +	return mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS) &&
> +		mctrl_gpio_to_gpiod(gpios, UART_GPIO_RTS);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_use_rtscts);
> +
>  unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>  {
>  	enum mctrl_gpio_idx i;

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

* Re: [PATCHv3 1/3] serial: mctrl_gpio: implement mctrl_gpio_use_rtscts
  2016-09-27 15:12   ` Uwe Kleine-König
@ 2016-09-27 15:15     ` Richard Genoud
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Genoud @ 2016-09-27 15:15 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Nicolas Ferre, Alexandre Belloni, Greg Kroah-Hartman,
	Cyrille Pitchen, linux-serial@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org

2016-09-27 17:12 GMT+02:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> Hello,
>
> On Tue, Sep 27, 2016 at 04:13:11PM +0200, Richard Genoud wrote:
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
>> index d2da6aa7f27d..0e5525a64c2a 100644
>> --- a/drivers/tty/serial/serial_mctrl_gpio.c
>> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/err.h>
>>  #include <linux/device.h>
>>  #include <linux/irq.h>
>> +#include <linux/err.h>
>
> this isn't needed any more, right?
indeed !

>
>>  #include <linux/gpio/consumer.h>
>>  #include <linux/termios.h>
>>  #include <linux/serial_core.h>
>> @@ -72,6 +73,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>>  }
>>  EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
>>
>> +bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios)
>> +{
>> +     return mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS) &&
>> +             mctrl_gpio_to_gpiod(gpios, UART_GPIO_RTS);
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_use_rtscts);
>> +
>>  unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>>  {
>>       enum mctrl_gpio_idx i;
>
> Best regards
> Uwe
Thanks !

Richard.

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

* [PATCHv3 2/3] tty/serial: at91: fix hardware handshake with GPIOs
  2016-09-27 14:13 [PATCHv3 0/3] Fix hardware handshake on SAM9x5 platforms Richard Genoud
  2016-09-27 14:13 ` [PATCHv3 1/3] serial: mctrl_gpio: implement mctrl_gpio_use_rtscts Richard Genoud
@ 2016-09-27 14:13 ` Richard Genoud
  2016-09-27 14:13 ` [PATCHv3 3/3] tty/serial: at91: fix hardware handshake on SAM9x5 (without GPIOs) Richard Genoud
  2 siblings, 0 replies; 6+ messages in thread
From: Richard Genoud @ 2016-09-27 14:13 UTC (permalink / raw)
  To: Uwe Kleine-König, Nicolas Ferre, Alexandre Belloni,
	Greg Kroah-Hartman, Cyrille Pitchen
  Cc: linux-serial, linux-kernel, linux-arm-kernel, Richard Genoud

Commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled") broke the hardware handshake when GPIOs
were used.

Hardware handshake with GPIOs used to work before this commit because
the CRTSCTS flag (termios->c_cflag) was set, but not the
ATMEL_US_USMODE_HWHS flag (controller register) ; so hardware handshake
enabled, but not handled by the controller.

This commit restores this behaviour.

NB: -stable is not Cced because it doesn't cleanly apply on 4.1+
and it will also need previous commit:
"serial: mctrl_gpio: implement mctrl_gpio_use_rtscts"

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Fixes: 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when hardware handshake is enabled")
---
 drivers/tty/serial/atmel_serial.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 5f550d9feed9..14467d5e060b 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2130,8 +2130,12 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 		atmel_uart_writel(port, ATMEL_US_TTGR,
 				  port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
-	} else if (termios->c_cflag & CRTSCTS) {
-		/* RS232 with hardware handshake (RTS/CTS) */
+	} else if ((termios->c_cflag & CRTSCTS) &&
+		   !mctrl_gpio_use_rtscts(atmel_port->gpios)) {
+		/*
+		 * RS232 with hardware handshake (RTS/CTS)
+		 * handled by the controller.
+		 */
 		if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
 			dev_info(port->dev, "not enabling hardware flow control because DMA is used");
 			termios->c_cflag &= ~CRTSCTS;
@@ -2139,7 +2143,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 			mode |= ATMEL_US_USMODE_HWHS;
 		}
 	} else {
-		/* RS232 without hadware handshake */
+		/* RS232 without hardware handshake or controlled by GPIOs */
 		mode |= ATMEL_US_USMODE_NORMAL;
 	}
 

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

* [PATCHv3 3/3] tty/serial: at91: fix hardware handshake on SAM9x5 (without GPIOs)
  2016-09-27 14:13 [PATCHv3 0/3] Fix hardware handshake on SAM9x5 platforms Richard Genoud
  2016-09-27 14:13 ` [PATCHv3 1/3] serial: mctrl_gpio: implement mctrl_gpio_use_rtscts Richard Genoud
  2016-09-27 14:13 ` [PATCHv3 2/3] tty/serial: at91: fix hardware handshake with GPIOs Richard Genoud
@ 2016-09-27 14:13 ` Richard Genoud
  2 siblings, 0 replies; 6+ messages in thread
From: Richard Genoud @ 2016-09-27 14:13 UTC (permalink / raw)
  To: Uwe Kleine-König, Nicolas Ferre, Alexandre Belloni,
	Greg Kroah-Hartman, Cyrille Pitchen
  Cc: linux-serial, linux-kernel, linux-arm-kernel, Richard Genoud

Commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled") broke the hardware handshake on SAM9x5
platforms.

On Atmel platforms, the USART can only handle the handware handshake
(ATMEL_US_USMODE_HWHS) if FIFOs or PDC are used.

Thus, ATMEL_US_USMODE_HWHS mode should only be used in this case.

For SAM9x5, there's no FIFOs nor PDC for the USART, so the mode should
be ATMEL_US_USMODE_NORMAL and the RTS pin should be controlled by the
driver.

NB: -stable is not Cced because it doesn't cleanly apply on 4.1+

Tested on SAM9G35-CM with and without DMA

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Fixes: 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when hardware handshake is enabled")
---
 drivers/tty/serial/atmel_serial.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 14467d5e060b..f644d5dcf6d1 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2131,19 +2131,23 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 				  port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
 	} else if ((termios->c_cflag & CRTSCTS) &&
-		   !mctrl_gpio_use_rtscts(atmel_port->gpios)) {
+		   !mctrl_gpio_use_rtscts(atmel_port->gpios) &&
+		   (atmel_use_pdc_rx(port) || atmel_use_fifo(port))) {
 		/*
-		 * RS232 with hardware handshake (RTS/CTS)
-		 * handled by the controller.
+		 * Automatic hardware handshake (RTS/CTS) only work with
+		 * FIFOs or PDC.
+		 * Meaning that on SAM9x5 the controller can't handle
+		 * the hardware handshake (no FIFOs nor PDC on these platforms).
 		 */
-		if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
-			dev_info(port->dev, "not enabling hardware flow control because DMA is used");
-			termios->c_cflag &= ~CRTSCTS;
-		} else {
-			mode |= ATMEL_US_USMODE_HWHS;
-		}
+		mode |= ATMEL_US_USMODE_HWHS;
 	} else {
-		/* RS232 without hardware handshake or controlled by GPIOs */
+		/*
+		 * Other cases are:
+		 * - RS232 without hardware handshake
+		 * - RS232 with hardware handshake and:
+		 *   - controller unable to handle CTS/RTS by itself
+		 *   - or CTS/RTS handled by GPIOs
+		 */
 		mode |= ATMEL_US_USMODE_NORMAL;
 	}
 

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

end of thread, other threads:[~2016-09-27 15:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-27 14:13 [PATCHv3 0/3] Fix hardware handshake on SAM9x5 platforms Richard Genoud
2016-09-27 14:13 ` [PATCHv3 1/3] serial: mctrl_gpio: implement mctrl_gpio_use_rtscts Richard Genoud
2016-09-27 15:12   ` Uwe Kleine-König
2016-09-27 15:15     ` Richard Genoud
2016-09-27 14:13 ` [PATCHv3 2/3] tty/serial: at91: fix hardware handshake with GPIOs Richard Genoud
2016-09-27 14:13 ` [PATCHv3 3/3] tty/serial: at91: fix hardware handshake on SAM9x5 (without GPIOs) 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).