Re: [PATCH 01/26] Revert "serial: uart: add hw flow control support configuration"
On 09/02/2014 05:58 PM, Peter Hurley wrote: On 09/02/2014 05:50 PM, Murali Karicheri wrote: Peter, On 09/02/2014 05:39 PM, Peter Hurley wrote: Finally, this patch supposes to fix existing bugs in the serial core for auto CTS-enabled hardware, but does not include the class of hardware for which these bugs exist. What is this? What class of hardware are you referring to? UART_CAP_AFE and UART_CAP_EFR hardware. As discussed, I will send an updated patch for this as discussed.. Great. And since that patch will touch every single line this reverts (and more) with a different solution, I see no reason not to back this out. No Issues and I am fine with the revert. Plus, as you can see, you're holding up progress. I need to make progress on the hardware that I deal with for which this patch was submitted :) Unfortunately this patch didn't get proper review before merge causing the issue. Just want to give right context. Regards, Murali Regards, Peter Hurley -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/26] Revert "serial: uart: add hw flow control support configuration"
On 09/02/2014 05:50 PM, Murali Karicheri wrote: > Peter, > > On 09/02/2014 05:39 PM, Peter Hurley wrote: >> Finally, this patch >> supposes to fix existing bugs in the serial core for auto CTS-enabled >> hardware, but does not include the class of hardware for which these >> bugs exist. > > What is this? What class of hardware are you referring to? UART_CAP_AFE and UART_CAP_EFR hardware. > As discussed, I will send an updated patch for this as discussed.. Great. And since that patch will touch every single line this reverts (and more) with a different solution, I see no reason not to back this out. Plus, as you can see, you're holding up progress. Regards, Peter Hurley -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/26] Revert "serial: uart: add hw flow control support configuration"
Peter, On 09/02/2014 05:39 PM, Peter Hurley wrote: Finally, this patch supposes to fix existing bugs in the serial core for auto CTS-enabled hardware, but does not include the class of hardware for which these bugs exist. What is this? What class of hardware are you referring to? As discussed, I will send an updated patch for this as discussed.. Regards, Murali CC: Murali Karicheri CC: Rob Herring Signed-off-by: Peter Hurley --- Documentation/devicetree/bindings/serial/of-serial.txt | 1 - 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, 5 insertions(+), 18 deletions(-) diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt b/Documentation/devicetree/bindings/serial/of-serial.txt index 7705477..1928a3e 100644 --- a/Documentation/devicetree/bindings/serial/of-serial.txt +++ b/Documentation/devicetree/bindings/serial/of-serial.txt @@ -37,7 +37,6 @@ 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. -- has-hw-flow-control: the hardware has flow control capability. Example: diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 7a91c6d..109da60 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -2333,11 +2333,9 @@ 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. Also enable -* AFE if hw flow control is supported +* UART to respond. IOW, at least 32 bytes of FIFO. */ - if ((up->capabilities& UART_CAP_AFE&& (port->fifosize>= 32)) || - (port->flags& UPF_HARD_FLOW)) { + if (up->capabilities& UART_CAP_AFE&& port->fifosize>= 32) { 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 68d4455..27981e2 100644 --- a/drivers/tty/serial/of_serial.c +++ b/drivers/tty/serial/of_serial.c @@ -183,10 +183,6 @@ 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, - "has-hw-flow-control")) - port8250.port.flags |= UPF_HARD_FLOW; - ret = serial8250_register_8250_port(); break; } diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index c2f90ec..261e49e 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -174,12 +174,8 @@ 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 hw support flow control without software intervention, -* then skip the below check -*/ - if (tty_port_cts_enabled(port)&& - !(uport->flags& UPF_HARD_FLOW)) { + + if (tty_port_cts_enabled(port)) { spin_lock_irq(>lock); if (!(uport->ops->get_mctrl(uport)& TIOCM_CTS)) tty->hw_stopped = 1; @@ -2777,9 +2773,7 @@ void uart_handle_cts_change(struct uart_port *uport, unsigned int status) uport->icount.cts++; - /* skip below code if the hw flow control is supported */ - if (tty_port_cts_enabled(port)&& - !(uport->flags& UPF_HARD_FLOW)) { + if (tty_port_cts_enabled(port)) { if (tty->hw_stopped) { if (status) { tty->hw_stopped = 0; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 01/26] Revert "serial: uart: add hw flow control support configuration"
This reverts commit 06aa82e498c144c7784a6f3d3b55458b272d6146. This commit purports to enable auto CTS flow control for the 8250 UART driver. However, the 8250 UART driver already supports auto CTS flow control via UART_CAP_AFE and UART_CAP_EFR. Indeed, this patch introduces another DT attribute for which an existing firmware flag already exists ("auto-flow-control"). Furthermore, the use of UPF_HARD_FLOW requires the UART driver to define .throttle and .unthrottle methods, neither of which are defined for the 8250 UART driver (which will result in a NULL ptr dereference). Finally, this patch supposes to fix existing bugs in the serial core for auto CTS-enabled hardware, but does not include the class of hardware for which these bugs exist. CC: Murali Karicheri CC: Rob Herring Signed-off-by: Peter Hurley --- Documentation/devicetree/bindings/serial/of-serial.txt | 1 - 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, 5 insertions(+), 18 deletions(-) diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt b/Documentation/devicetree/bindings/serial/of-serial.txt index 7705477..1928a3e 100644 --- a/Documentation/devicetree/bindings/serial/of-serial.txt +++ b/Documentation/devicetree/bindings/serial/of-serial.txt @@ -37,7 +37,6 @@ 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. -- has-hw-flow-control: the hardware has flow control capability. Example: diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 7a91c6d..109da60 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -2333,11 +2333,9 @@ 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. Also enable -* AFE if hw flow control is supported +* UART to respond. IOW, at least 32 bytes of FIFO. */ - if ((up->capabilities & UART_CAP_AFE && (port->fifosize >= 32)) || - (port->flags & UPF_HARD_FLOW)) { + if (up->capabilities & UART_CAP_AFE && port->fifosize >= 32) { 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 68d4455..27981e2 100644 --- a/drivers/tty/serial/of_serial.c +++ b/drivers/tty/serial/of_serial.c @@ -183,10 +183,6 @@ 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, - "has-hw-flow-control")) - port8250.port.flags |= UPF_HARD_FLOW; - ret = serial8250_register_8250_port(); break; } diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index c2f90ec..261e49e 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -174,12 +174,8 @@ 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 hw support flow control without software intervention, -* then skip the below check -*/ - if (tty_port_cts_enabled(port) && - !(uport->flags & UPF_HARD_FLOW)) { + + if (tty_port_cts_enabled(port)) { spin_lock_irq(>lock); if (!(uport->ops->get_mctrl(uport) & TIOCM_CTS)) tty->hw_stopped = 1; @@ -2777,9 +2773,7 @@ void uart_handle_cts_change(struct uart_port *uport, unsigned int status) uport->icount.cts++; - /* skip below code if the hw flow control is supported */ - if (tty_port_cts_enabled(port) && - !(uport->flags & UPF_HARD_FLOW)) { + if (tty_port_cts_enabled(port)) { if (tty->hw_stopped) { if (status) { tty->hw_stopped = 0; -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at
[PATCH 01/26] Revert serial: uart: add hw flow control support configuration
This reverts commit 06aa82e498c144c7784a6f3d3b55458b272d6146. This commit purports to enable auto CTS flow control for the 8250 UART driver. However, the 8250 UART driver already supports auto CTS flow control via UART_CAP_AFE and UART_CAP_EFR. Indeed, this patch introduces another DT attribute for which an existing firmware flag already exists (auto-flow-control). Furthermore, the use of UPF_HARD_FLOW requires the UART driver to define .throttle and .unthrottle methods, neither of which are defined for the 8250 UART driver (which will result in a NULL ptr dereference). Finally, this patch supposes to fix existing bugs in the serial core for auto CTS-enabled hardware, but does not include the class of hardware for which these bugs exist. CC: Murali Karicheri m-kariche...@ti.com CC: Rob Herring robh...@kernel.org Signed-off-by: Peter Hurley pe...@hurleysoftware.com --- Documentation/devicetree/bindings/serial/of-serial.txt | 1 - 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, 5 insertions(+), 18 deletions(-) diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt b/Documentation/devicetree/bindings/serial/of-serial.txt index 7705477..1928a3e 100644 --- a/Documentation/devicetree/bindings/serial/of-serial.txt +++ b/Documentation/devicetree/bindings/serial/of-serial.txt @@ -37,7 +37,6 @@ 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. -- has-hw-flow-control: the hardware has flow control capability. Example: diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 7a91c6d..109da60 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -2333,11 +2333,9 @@ 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. Also enable -* AFE if hw flow control is supported +* UART to respond. IOW, at least 32 bytes of FIFO. */ - if ((up-capabilities UART_CAP_AFE (port-fifosize = 32)) || - (port-flags UPF_HARD_FLOW)) { + if (up-capabilities UART_CAP_AFE port-fifosize = 32) { 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 68d4455..27981e2 100644 --- a/drivers/tty/serial/of_serial.c +++ b/drivers/tty/serial/of_serial.c @@ -183,10 +183,6 @@ 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, - has-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 c2f90ec..261e49e 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -174,12 +174,8 @@ 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 hw support flow control without software intervention, -* then skip the below check -*/ - if (tty_port_cts_enabled(port) - !(uport-flags UPF_HARD_FLOW)) { + + if (tty_port_cts_enabled(port)) { spin_lock_irq(uport-lock); if (!(uport-ops-get_mctrl(uport) TIOCM_CTS)) tty-hw_stopped = 1; @@ -2777,9 +2773,7 @@ void uart_handle_cts_change(struct uart_port *uport, unsigned int status) uport-icount.cts++; - /* skip below code if the hw flow control is supported */ - if (tty_port_cts_enabled(port) - !(uport-flags UPF_HARD_FLOW)) { + if (tty_port_cts_enabled(port)) { if (tty-hw_stopped) { if (status) { tty-hw_stopped = 0; -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More
Re: [PATCH 01/26] Revert serial: uart: add hw flow control support configuration
Peter, On 09/02/2014 05:39 PM, Peter Hurley wrote: Finally, this patch supposes to fix existing bugs in the serial core for auto CTS-enabled hardware, but does not include the class of hardware for which these bugs exist. What is this? What class of hardware are you referring to? As discussed, I will send an updated patch for this as discussed.. Regards, Murali CC: Murali Karicherim-kariche...@ti.com CC: Rob Herringrobh...@kernel.org Signed-off-by: Peter Hurleype...@hurleysoftware.com --- Documentation/devicetree/bindings/serial/of-serial.txt | 1 - 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, 5 insertions(+), 18 deletions(-) diff --git a/Documentation/devicetree/bindings/serial/of-serial.txt b/Documentation/devicetree/bindings/serial/of-serial.txt index 7705477..1928a3e 100644 --- a/Documentation/devicetree/bindings/serial/of-serial.txt +++ b/Documentation/devicetree/bindings/serial/of-serial.txt @@ -37,7 +37,6 @@ 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. -- has-hw-flow-control: the hardware has flow control capability. Example: diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 7a91c6d..109da60 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -2333,11 +2333,9 @@ 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. Also enable -* AFE if hw flow control is supported +* UART to respond. IOW, at least 32 bytes of FIFO. */ - if ((up-capabilities UART_CAP_AFE (port-fifosize= 32)) || - (port-flags UPF_HARD_FLOW)) { + if (up-capabilities UART_CAP_AFE port-fifosize= 32) { 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 68d4455..27981e2 100644 --- a/drivers/tty/serial/of_serial.c +++ b/drivers/tty/serial/of_serial.c @@ -183,10 +183,6 @@ 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, - has-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 c2f90ec..261e49e 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -174,12 +174,8 @@ 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 hw support flow control without software intervention, -* then skip the below check -*/ - if (tty_port_cts_enabled(port) - !(uport-flags UPF_HARD_FLOW)) { + + if (tty_port_cts_enabled(port)) { spin_lock_irq(uport-lock); if (!(uport-ops-get_mctrl(uport) TIOCM_CTS)) tty-hw_stopped = 1; @@ -2777,9 +2773,7 @@ void uart_handle_cts_change(struct uart_port *uport, unsigned int status) uport-icount.cts++; - /* skip below code if the hw flow control is supported */ - if (tty_port_cts_enabled(port) - !(uport-flags UPF_HARD_FLOW)) { + if (tty_port_cts_enabled(port)) { if (tty-hw_stopped) { if (status) { tty-hw_stopped = 0; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/26] Revert serial: uart: add hw flow control support configuration
On 09/02/2014 05:50 PM, Murali Karicheri wrote: Peter, On 09/02/2014 05:39 PM, Peter Hurley wrote: Finally, this patch supposes to fix existing bugs in the serial core for auto CTS-enabled hardware, but does not include the class of hardware for which these bugs exist. What is this? What class of hardware are you referring to? UART_CAP_AFE and UART_CAP_EFR hardware. As discussed, I will send an updated patch for this as discussed.. Great. And since that patch will touch every single line this reverts (and more) with a different solution, I see no reason not to back this out. Plus, as you can see, you're holding up progress. Regards, Peter Hurley -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/26] Revert serial: uart: add hw flow control support configuration
On 09/02/2014 05:58 PM, Peter Hurley wrote: On 09/02/2014 05:50 PM, Murali Karicheri wrote: Peter, On 09/02/2014 05:39 PM, Peter Hurley wrote: Finally, this patch supposes to fix existing bugs in the serial core for auto CTS-enabled hardware, but does not include the class of hardware for which these bugs exist. What is this? What class of hardware are you referring to? UART_CAP_AFE and UART_CAP_EFR hardware. As discussed, I will send an updated patch for this as discussed.. Great. And since that patch will touch every single line this reverts (and more) with a different solution, I see no reason not to back this out. No Issues and I am fine with the revert. Plus, as you can see, you're holding up progress. I need to make progress on the hardware that I deal with for which this patch was submitted :) Unfortunately this patch didn't get proper review before merge causing the issue. Just want to give right context. Regards, Murali Regards, Peter Hurley -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/