diff mbox

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

Message ID 1398792122-18592-1-git-send-email-m-karicheri2@ti.com
State Accepted
Commit 06aa82e498c144c7784a6f3d3b55458b272d6146
Headers show

Commit Message

Murali Karicheri April 29, 2014, 5:22 p.m. UTC
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.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>

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>
CC: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 - Fixed brackets around port->flags & UPF_HARD_FLOW as per comments 
 .../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(-)

Comments

Mark Rutland April 29, 2014, 10:27 p.m. UTC | #1
Hi,

On Tue, Apr 29, 2014 at 06:22:02PM +0100, Murali Karicheri wrote:
> 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.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> 
> 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>
> CC: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  - Fixed brackets around port->flags & UPF_HARD_FLOW as per comments 
>  .../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.

From the wording of the commit message, I believe this would be better worded
something like:

- hw-flow-control: the hardware has flow control capability.

While the OS needs to know that flow control is present if it wishes to make
use of it, whether or not the OS does so is not a matter for the binding.

It might also be better worded as "has-hw-flow-control", which would make that
distinction clearer.

Otherwise this looks fine to me.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Murali Karicheri May 1, 2014, 6:57 p.m. UTC | #2
>-----Original Message-----
>From: Mark Rutland [mailto:mark.rutland@arm.com]
>Sent: Tuesday, April 29, 2014 6:28 PM
>To: Karicheri, Muralidharan
>Cc: linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; linux-serial@vger.kernel.org;
>devicetree@vger.kernel.org; Rob Herring; Ian Campbell; Kumar Gala; Randy Dunlap; Greg
>Kroah-Hartman; Jiri Slaby; Shilimkar, Santosh
>Subject: Re: [PATCH v1] serial: uart: add hw flow control support configuration
>
>Hi,
>
>On Tue, Apr 29, 2014 at 06:22:02PM +0100, Murali Karicheri wrote:
>> 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.
>>
>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>
>> 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>
>> CC: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> ---
>>  - Fixed brackets around port->flags & UPF_HARD_FLOW as per comments
>>  .../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.
>
>From the wording of the commit message, I believe this would be better worded
>something like:
>
>- hw-flow-control: the hardware has flow control capability.
>
>While the OS needs to know that flow control is present if it wishes to make use of it,
>whether or not the OS does so is not a matter for the binding.
>
>It might also be better worded as "has-hw-flow-control", which would make that
>distinction clearer.
>
>Otherwise this looks fine to me.

Fine. Will re-send with rewording as

has-hw-flow-control: the hardware has flow control capability.

Murali
>
>Thanks,
>Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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..f145103 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;