[PATCH] serial: uart: add hw flow control support configuration

[PATCH] serial: uart: add hw flow control support configuration

[Murali Karicheri]

8250 uart driver currently supports only software assisted hw flow
control. The software assisted hw flow control maintains a hw_stopped
flag in the tty structure to stop and start transmission and use modem
status interrupt for the event to drive the handshake signals. This is
not needed if hw has flow control capabilities. This patch adds a
DT attribute for enabling hw flow control for a uart port. Also skip
stop and start if this flag is present in flag field of the port
structure.

CC: Rob Herring <robh+dt@kernel.org>
CC: Pawel Moll <pawel.moll@arm.com> 
CC: Mark Rutland <mark.rutland@arm.com>
CC: Ian Campbell <ijc+devicetree@hellion.org.uk>
CC: Kumar Gala <galak@codeaurora.org>
CC: Randy Dunlap <rdunlap@infradead.org>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Jiri Slaby <jslaby@suse.cz>

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 .../devicetree/bindings/serial/of-serial.txt       |    2 ++
 drivers/tty/serial/8250/8250_core.c                |    6 ++++--
 drivers/tty/serial/of_serial.c                     |    4 ++++
 drivers/tty/serial/serial_core.c                   |   12 +++++++++---
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt b/Documentation/devicetree/bindings/serial/of-serial.txt
index 1928a3e..123de01 100644
--- a/Documentation/devicetree/bindings/serial/of-serial.txt
+++ b/Documentation/devicetree/bindings/serial/of-serial.txt
@@ -37,6 +37,8 @@ Optional properties:
 - auto-flow-control: one way to enable automatic flow control support. The
   driver is allowed to detect support for the capability even without this
   property.
+- hw-flow-control: this enables pure hw flow control and no software
+  intervention needed.
 
 Example:
 
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 81f909c..6b1b868 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2338,9 +2338,11 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 	 * the trigger, or the MCR RTS bit is cleared.  In the case where
 	 * the remote UART is not using CTS auto flow control, we must
 	 * have sufficient FIFO entries for the latency of the remote
-	 * UART to respond.  IOW, at least 32 bytes of FIFO.
+	 * UART to respond.  IOW, at least 32 bytes of FIFO. Also enable
+	 * AFE if hw flow control is supported
 	 */
-	if (up->capabilities & UART_CAP_AFE && port->fifosize >= 32) {
+	if ((up->capabilities & UART_CAP_AFE && (port->fifosize >= 32)) ||
+	    port->flags & UPF_HARD_FLOW) {
 		up->mcr &= ~UART_MCR_AFE;
 		if (termios->c_cflag & CRTSCTS)
 			up->mcr |= UART_MCR_AFE;
diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index 9924660..1a5f528 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -182,6 +182,10 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
 					  "auto-flow-control"))
 			port8250.capabilities |= UART_CAP_AFE;
 
+		if (of_property_read_bool(ofdev->dev.of_node,
+					  "hw-flow-control"))
+			port8250.port.flags |= UPF_HARD_FLOW;
+
 		ret = serial8250_register_8250_port(&port8250);
 		break;
 	}
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 2cf5649..16e3615 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -170,8 +170,12 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
 			if (tty->termios.c_cflag & CBAUD)
 				uart_set_mctrl(uport, TIOCM_RTS | TIOCM_DTR);
 		}
-
-		if (tty_port_cts_enabled(port)) {
+		/*
+		 * if hw support flow control without software intervention,
+		 * then skip the below check
+		 */
+		if (tty_port_cts_enabled(port) &&
+		    !(uport->flags & UPF_HARD_FLOW)) {
 			spin_lock_irq(&uport->lock);
 			if (!(uport->ops->get_mctrl(uport) & TIOCM_CTS))
 				tty->hw_stopped = 1;
@@ -2768,7 +2772,9 @@ void uart_handle_cts_change(struct uart_port *uport, unsigned int status)
 
 	uport->icount.cts++;
 
-	if (tty_port_cts_enabled(port)) {
+	/* skip below code if the hw flow control is supported */
+	if (tty_port_cts_enabled(port) &&
+	    !(uport->flags & UPF_HARD_FLOW)) {
 		if (tty->hw_stopped) {
 			if (status) {
 				tty->hw_stopped = 0;
-- 
1.7.9.5

Re: [PATCH] serial: uart: add hw flow control support configuration

[One Thousand Gnomes]

> not needed if hw has flow control capabilities. This patch adds a
> DT attribute for enabling hw flow control for a uart port. Also skip
> stop and start if this flag is present in flag field of the port
> structure.

That seems a sensible thing to add.

> -	if (up->capabilities & UART_CAP_AFE && port->fifosize >= 32) {
> +	if ((up->capabilities & UART_CAP_AFE && (port->fifosize >= 32)) ||
> +	    port->flags & UPF_HARD_FLOW) {

Surely you want brackets on the port->flags & UPF_HARD_FLOW ??

Alan

RE: [PATCH] serial: uart: add hw flow control support configuration

[Karicheri, Muralidharan]

Alan,

Sorry, I had to cut-n-paste your comment as I lost your email from my inbox.

>-----Original Message-----
>From: Karicheri, Muralidharan
>Sent: Friday, April 11, 2014 3:18 PM
>To: devicetree@vger.kernel.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
>linux-serial@vger.kernel.org
>Cc: Balbi, Felipe; Karicheri, Muralidharan; Rob Herring; Mark Rutland; Ian Campbell;
>Kumar Gala; Randy Dunlap; Greg Kroah-Hartman; Jiri Slaby
>Subject: [PATCH] serial: uart: add hw flow control support configuration
>
>8250 uart driver currently supports only software assisted hw flow control. The software
>assisted hw flow control maintains a hw_stopped flag in the tty structure to stop and start
>transmission and use modem status interrupt for the event to drive the handshake signals.
>This is not needed if hw has flow control capabilities. This patch adds a DT attribute for
>enabling hw flow control for a uart port. Also skip stop and start if this flag is present in flag
>field of the port structure.

[Alan] That seems a sensible thing to add.

>
>CC: Rob Herring <robh+dt@kernel.org>
>CC: Pawel Moll <pawel.moll@arm.com>
>CC: Mark Rutland <mark.rutland@arm.com>
>CC: Ian Campbell <ijc+devicetree@hellion.org.uk>
>CC: Kumar Gala <galak@codeaurora.org>
>CC: Randy Dunlap <rdunlap@infradead.org>
>CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>CC: Jiri Slaby <jslaby@suse.cz>
>
>Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>---
> .../devicetree/bindings/serial/of-serial.txt       |    2 ++
> drivers/tty/serial/8250/8250_core.c                |    6 ++++--
> drivers/tty/serial/of_serial.c                     |    4 ++++
> drivers/tty/serial/serial_core.c                   |   12 +++++++++---
> 4 files changed, 19 insertions(+), 5 deletions(-)
>
>diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt
>b/Documentation/devicetree/bindings/serial/of-serial.txt
>index 1928a3e..123de01 100644
>--- a/Documentation/devicetree/bindings/serial/of-serial.txt
>+++ b/Documentation/devicetree/bindings/serial/of-serial.txt
>@@ -37,6 +37,8 @@ Optional properties:
> - auto-flow-control: one way to enable automatic flow control support. The
>   driver is allowed to detect support for the capability even without this
>   property.
>+- hw-flow-control: this enables pure hw flow control and no software
>+  intervention needed.
>
> Example:
>
>diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>index 81f909c..6b1b868 100644
>--- a/drivers/tty/serial/8250/8250_core.c
>+++ b/drivers/tty/serial/8250/8250_core.c
>@@ -2338,9 +2338,11 @@ serial8250_do_set_termios(struct uart_port *port, struct
>ktermios *termios,
> 	 * the trigger, or the MCR RTS bit is cleared.  In the case where
> 	 * the remote UART is not using CTS auto flow control, we must
> 	 * have sufficient FIFO entries for the latency of the remote
>-	 * UART to respond.  IOW, at least 32 bytes of FIFO.
>+	 * UART to respond.  IOW, at least 32 bytes of FIFO. Also enable
>+	 * AFE if hw flow control is supported
> 	 */
>-	if (up->capabilities & UART_CAP_AFE && port->fifosize >= 32) {
>+	if ((up->capabilities & UART_CAP_AFE && (port->fifosize >= 32)) ||
>+	    port->flags & UPF_HARD_FLOW) {

[Alan] Surely you want brackets on the port->flags & UPF_HARD_FLOW ??

The C precedence table shows following order:=
-> & ||

So flags will be accessed first and then AND-ed with  UPF_HARD_FLOW and
result will be OR-ed with left side expression value. So no need for bracket IMO.

Murali

Re: [PATCH] serial: uart: add hw flow control support configuration

[One Thousand Gnomes]

> >-	if (up->capabilities & UART_CAP_AFE && port->fifosize >= 32) {
> >+	if ((up->capabilities & UART_CAP_AFE && (port->fifosize >= 32)) ||
> >+	    port->flags & UPF_HARD_FLOW) {
> 
> [Alan] Surely you want brackets on the port->flags & UPF_HARD_FLOW ??
> 
> The C precedence table shows following order:=
> -> & ||
> 
> So flags will be accessed first and then AND-ed with  UPF_HARD_FLOW and
> result will be OR-ed with left side expression value. So no need for bracket IMO.

Yep... agreed.

Alan

RE: [PATCH] serial: uart: add hw flow control support configuration

[Karicheri, Muralidharan]

>-----Original Message-----
>From: Karicheri, Muralidharan
>Sent: Friday, April 11, 2014 3:18 PM
>To: devicetree@vger.kernel.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
>linux-serial@vger.kernel.org
>Cc: Balbi, Felipe; Karicheri, Muralidharan; Rob Herring; Mark Rutland; Ian Campbell;
>Kumar Gala; Randy Dunlap; Greg Kroah-Hartman; Jiri Slaby
>Subject: [PATCH] serial: uart: add hw flow control support configuration
>
>8250 uart driver currently supports only software assisted hw flow control. The software
>assisted hw flow control maintains a hw_stopped flag in the tty structure to stop and start
>transmission and use modem status interrupt for the event to drive the handshake signals.
>This is not needed if hw has flow control capabilities. This patch adds a DT attribute for
>enabling hw flow control for a uart port. Also skip stop and start if this flag is present in flag
>field of the port structure.
>
>CC: Rob Herring <robh+dt@kernel.org>
>CC: Pawel Moll <pawel.moll@arm.com>
>CC: Mark Rutland <mark.rutland@arm.com>
>CC: Ian Campbell <ijc+devicetree@hellion.org.uk>
>CC: Kumar Gala <galak@codeaurora.org>
>CC: Randy Dunlap <rdunlap@infradead.org>
>CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>CC: Jiri Slaby <jslaby@suse.cz>
>
>Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>---
> .../devicetree/bindings/serial/of-serial.txt       |    2 ++
> drivers/tty/serial/8250/8250_core.c                |    6 ++++--
> drivers/tty/serial/of_serial.c                     |    4 ++++
> drivers/tty/serial/serial_core.c                   |   12 +++++++++---
> 4 files changed, 19 insertions(+), 5 deletions(-)
>
>diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt
>b/Documentation/devicetree/bindings/serial/of-serial.txt
>index 1928a3e..123de01 100644
>--- a/Documentation/devicetree/bindings/serial/of-serial.txt
>+++ b/Documentation/devicetree/bindings/serial/of-serial.txt
>@@ -37,6 +37,8 @@ Optional properties:
> - auto-flow-control: one way to enable automatic flow control support. The
>   driver is allowed to detect support for the capability even without this
>   property.
>+- hw-flow-control: this enables pure hw flow control and no software
>+  intervention needed.
>
> Example:
>
>diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>index 81f909c..6b1b868 100644
>--- a/drivers/tty/serial/8250/8250_core.c
>+++ b/drivers/tty/serial/8250/8250_core.c
>@@ -2338,9 +2338,11 @@ serial8250_do_set_termios(struct uart_port *port, struct
>ktermios *termios,
> 	 * the trigger, or the MCR RTS bit is cleared.  In the case where
> 	 * the remote UART is not using CTS auto flow control, we must
> 	 * have sufficient FIFO entries for the latency of the remote
>-	 * UART to respond.  IOW, at least 32 bytes of FIFO.
>+	 * UART to respond.  IOW, at least 32 bytes of FIFO. Also enable
>+	 * AFE if hw flow control is supported
> 	 */
>-	if (up->capabilities & UART_CAP_AFE && port->fifosize >= 32) {
>+	if ((up->capabilities & UART_CAP_AFE && (port->fifosize >= 32)) ||
>+	    port->flags & UPF_HARD_FLOW) {
> 		up->mcr &= ~UART_MCR_AFE;
> 		if (termios->c_cflag & CRTSCTS)
> 			up->mcr |= UART_MCR_AFE;
>diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c index
>9924660..1a5f528 100644
>--- a/drivers/tty/serial/of_serial.c
>+++ b/drivers/tty/serial/of_serial.c
>@@ -182,6 +182,10 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
> 					  "auto-flow-control"))
> 			port8250.capabilities |= UART_CAP_AFE;
>
>+		if (of_property_read_bool(ofdev->dev.of_node,
>+					  "hw-flow-control"))
>+			port8250.port.flags |= UPF_HARD_FLOW;
>+
> 		ret = serial8250_register_8250_port(&port8250);
> 		break;
> 	}
>diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>index 2cf5649..16e3615 100644
>--- a/drivers/tty/serial/serial_core.c
>+++ b/drivers/tty/serial/serial_core.c
>@@ -170,8 +170,12 @@ static int uart_port_startup(struct tty_struct *tty, struct
>uart_state *state,
> 			if (tty->termios.c_cflag & CBAUD)
> 				uart_set_mctrl(uport, TIOCM_RTS | TIOCM_DTR);
> 		}
>-
>-		if (tty_port_cts_enabled(port)) {
>+		/*
>+		 * if hw support flow control without software intervention,
>+		 * then skip the below check
>+		 */
>+		if (tty_port_cts_enabled(port) &&
>+		    !(uport->flags & UPF_HARD_FLOW)) {
> 			spin_lock_irq(&uport->lock);
> 			if (!(uport->ops->get_mctrl(uport) & TIOCM_CTS))
> 				tty->hw_stopped = 1;
>@@ -2768,7 +2772,9 @@ void uart_handle_cts_change(struct uart_port *uport, unsigned
>int status)
>
> 	uport->icount.cts++;
>
>-	if (tty_port_cts_enabled(port)) {
>+	/* skip below code if the hw flow control is supported */
>+	if (tty_port_cts_enabled(port) &&
>+	    !(uport->flags & UPF_HARD_FLOW)) {
> 		if (tty->hw_stopped) {
> 			if (status) {
> 				tty->hw_stopped = 0;
>--
>1.7.9.5

Can this be queued up for merge if looks good?

Murali Karicheri
Linux Kernel, Software Development

Re: [PATCH] serial: uart: add hw flow control support configuration

[Greg Kroah-Hartman]

On Wed, Apr 23, 2014 at 03:27:17PM +0100, One Thousand Gnomes wrote:
> > >-	if (up->capabilities & UART_CAP_AFE && port->fifosize >= 32) {
> > >+	if ((up->capabilities & UART_CAP_AFE && (port->fifosize >= 32)) ||
> > >+	    port->flags & UPF_HARD_FLOW) {
> > 
> > [Alan] Surely you want brackets on the port->flags & UPF_HARD_FLOW ??
> > 
> > The C precedence table shows following order:=
> > -> & ||
> > 
> > So flags will be accessed first and then AND-ed with  UPF_HARD_FLOW and
> > result will be OR-ed with left side expression value. So no need for bracket IMO.
> 
> Yep... agreed.

Perhaps, but please put them there so we all _know_ what it is doing is
correct.

greg k-h

Re: [PATCH] serial: uart: add hw flow control support configuration

[Greg Kroah-Hartman]

On Wed, Apr 23, 2014 at 02:31:51PM +0000, Karicheri, Muralidharan wrote:
> 
> Can this be queued up for merge if looks good?

Can you fix up the line Alan had questions about?

thanks,

greg k-h

RE: [PATCH] serial: uart: add hw flow control support configuration

[Karicheri, Muralidharan]

>-----Original Message-----
>From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
>Sent: Thursday, April 24, 2014 7:31 PM
>To: Karicheri, Muralidharan
>Cc: devicetree@vger.kernel.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
>linux-serial@vger.kernel.org; Balbi, Felipe; Rob Herring; Mark Rutland; Ian Campbell;
>Kumar Gala; Randy Dunlap; Jiri Slaby
>Subject: Re: [PATCH] serial: uart: add hw flow control support configuration
>
>On Wed, Apr 23, 2014 at 02:31:51PM +0000, Karicheri, Muralidharan wrote:
>>
>> Can this be queued up for merge if looks good?
>
>Can you fix up the line Alan had questions about?
>
>thanks,
>
>greg k-h

Will fix and re-send the patch

Thanks

Murali

RE: [PATCH] serial: uart: add hw flow control support configuration

[Karicheri, Muralidharan]

>-----Original Message-----
>From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
>Sent: Thursday, April 24, 2014 7:31 PM
>To: Karicheri, Muralidharan
>Cc: devicetree@vger.kernel.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
>linux-serial@vger.kernel.org; Balbi, Felipe; Rob Herring; Mark Rutland; Ian Campbell;
>Kumar Gala; Randy Dunlap; Jiri Slaby
>Subject: Re: [PATCH] serial: uart: add hw flow control support configuration
>
>On Wed, Apr 23, 2014 at 02:31:51PM +0000, Karicheri, Muralidharan wrote:
>>
>> Can this be queued up for merge if looks good?
>
>Can you fix up the line Alan had questions about?
>
>thanks,
>
>greg k-h

Greg,

I have posted v1 of the patch to list with the above change. Please pick it for
merge if it looks good.

Murali