OMAP serial driver flow control fixes, and preparation for DMA engine conversion
Subsystems (3)
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
Hi,
This series of patches fixes multiple flow control issues with the OMAP serial driver, and prepares the driver for DMA engine conversion. We require hardware assisted flow control to work properly for DMA support otherwise we have no way to properly pause the transmitter.
This is generated against v3.6, and has been developed mainly by testing on the OMAP4430 SDP platform.
Flow control seems to be really broken in the OMAP serial driver as things stand today. It just about works with software flow control because the generic serial core layer is inserting those characters, but only when the legacy DMA support is not being used. Otherwise, flow control is completely non-functional.
Issues identified in the OMAP serial driver are:
- set_mctrl() can only assert modem control lines, once asserted it is not possible to deassert them.
- IXOFF controls sending of XON/XOFF characters, not the reception of these sequences.
- IXON controls the recognition of XON/XOFF characters, not the transmission of the same.
- Wrong bitmasks for hardware assisted software flow control. Bit 2 in EFR enables sending of XON2/XOFF2 which are never set.
- No point comparing received characters against XOFF2 ('special character detect') as XOFF2 is not set.
- Fix multiple places where bits 6 and 5 of MCR are attempted to be altered, but because EFR ECB is unset, these bits remain unaffected. This effectively prevents us accessing the right XON/XOFF/TCR/TLR registers.
- Remove unnecessary read-backs of EFR/MCR/LCR registers - these registers don't change beneath us, they are configuration registers which hold their values. Not only does this simplify the code, but it makes it more readable, and more importantly ensures that we work from a consistent state where ->efr never has ECB set, and ->mcr never has the TCRTLR bit set.
- Fix disablement of hardware flow control and IXANY modes; once enabled these could never be disabled because nothing in the code ever clears these configuration bits.
Once that lot is fixed, these patches expand serial_core to permit hardware assisted flow control by:
- adding throttle/unthrottle callbacks into low level serial drivers, which allow them to take whatever action is necessary with hardware assisted flow control to throttle the remote end. In the case of OMAP serial, this means disabling the RX interrupts so that the FIFO fills to the watermark.
We then have a number of cleanups to the OMAP serial code to make the set_termios() function clearer and less prone to the kinds of mistakes identified above. This results in a great simplification of the flow control configuration code.
The OMAP serial driver hacks around with the transmit buffer allocation; lets clean that up so that drivers can cleanly allocate their transmitter buffer using coherent memory if that's what they desire.
Finally, the last few patches clean up the plat/omap-serial.h header file, moving most of its contents into the OMAP serial driver itself. Most of this is private to the OMAP serial driver and should never have been shared with anything else.
I have omitted to include the conversion of the transmit paths to DMA engine. Even with all the above fixed, it has issues when DMA transmit is in progress, and a program issues a TCSETS call (as less' does after it has written its prompt.) At the moment, this causes lots of junk to be emitted from the serial port when issuing dmesg | less' which sometimes brings the port to a complete halt.
As the OMAP DMA hardware does not have a clean pause when performing a MEM->DEV transfer (it discards its FIFO) I do not see a solution to this, which probably means that we can _not_ ever support transmit DMA on OMAP platforms.
This means the xmit buffer allocation patches are not that useful unless a solution to that can be found.
Now, the remaining question is, how much of this patch set do we think about merging, and when. Given that flow control in this driver has been broken for a very long time, and no one has apparantly noticed, I don't think there's any urgency to this, so given its size, my preference would be to queue it up for the next merge window. The thing that would worry me about applying some of the initial patches is that they may change the behaviour today and make any problems here more visible.
RFC 1/24 SERIAL: omap: fix set_mctrl() breakage
c538d20c7f (and maybe previous commits) broke set_mctrl() by making it only capable of setting bits in the MCR register. This prevents software controlled flow control and modem control line manipulation via TIOCMSET/TIOCMBIC from working correctly.
--- drivers/tty/serial/omap-serial.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
drivers/tty/serial/omap-serial.c +5 -3
@@ -488,7 +488,7 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)static void serial_omap_set_mctrl (struct uart_port *port,unsigned int mctrl) {struct uart_omap_port *up = (struct uart_omap_port *)port; -unsigned char mcr =0 ; +unsigned char mcr =0 , old_mcr;dev_dbg (up->port.dev,"serial_omap_set_mctrl+%d\n" , up->port.line);if (mctrl &TIOCM_RTS )
@@ -503,8 +503,10 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl) mcr |=UART_MCR_LOOP ;pm_runtime_get_sync (&up->pdev->dev); - up->mcr =serial_in (up,UART_MCR ); - up->mcr |= mcr; + old_mcr =serial_in (up,UART_MCR ); + old_mcr &= ~(UART_MCR_LOOP |UART_MCR_OUT2 |UART_MCR_OUT1 | +UART_MCR_DTR |UART_MCR_RTS ); + up->mcr = old_mcr | mcr;serial_out (up,UART_MCR , up->mcr);pm_runtime_put (&up->pdev->dev); }
Signature · 1 line
1.7.4.4
RFC 2/24 SERIAL: omap: fix bit masks for software flow control
In reply to Russell King - ARM LinuxThis makes ixoff control whether the XON/XOFF characters are transmitted, and ixon control whether XON/XOFF are actioned when received. This is as required by the POSIX specification.
--- arch/arm/plat-omap/include/plat/omap-serial.h | 4 ++-- drivers/tty/serial/omap-serial.c | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-)
arch/arm/plat-omap/include/plat/omap-serial.h +2 -2
@@ -42,10 +42,10 @@#define OMAP_UART_WER_MOD_WKUP 0X7F/* Enable XON/XOFF flow control on output */ -#define OMAP_UART_SW_TX 0x04 +#define OMAP_UART_SW_TX 0x08/* Enable XON/XOFF flow control on input */ -#define OMAP_UART_SW_RX 0x04 +#define OMAP_UART_SW_RX 0x02#define OMAP_UART_SYSC_RESET 0X07#define OMAP_UART_TCR_TRIG 0X0F
drivers/tty/serial/omap-serial.c +6 -6
@@ -669,19 +669,19 @@ serial_omap_configure_xonxoff/* * IXON Flag: -* Enable XON/XOFF flow control on output. -* Transmit XON1, XOFF1 +* Enable XON/XOFF flow control on input. +* Receiver compares XON1, XOFF1. */ if (termios->c_iflag &IXON ) - up->efr |=OMAP_UART_SW_TX ; + up->efr |=OMAP_UART_SW_RX ;/* * IXOFF Flag: -* Enable XON/XOFF flow control on input. -* Receiver compares XON1, XOFF1. +* Enable XON/XOFF flow control on output. +* Transmit XON1, XOFF1 */ if (termios->c_iflag &IXOFF ) - up->efr |=OMAP_UART_SW_RX ; + up->efr |=OMAP_UART_SW_TX ;serial_out (up,UART_EFR , up->efr |UART_EFR_ECB );serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A );
Signature · 1 line
1.7.4.4
RFC 3/24 SERIAL: omap: remove setting of EFR SCD bit
In reply to Russell King - ARM LinuxThe SCD (special character detect) bit enables comparisons with XOFF2, which we do not program. As the XOFF2 character remains unprogrammed, there's little point enabling this feature along with its associated interrupt. Remove this.
--- drivers/tty/serial/omap-serial.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)
drivers/tty/serial/omap-serial.c +2 -6
@@ -700,13 +700,8 @@ serial_omap_configure_xonxoffserial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR );serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_TI752_TCR ,OMAP_UART_TCR_TRIG ); -/* Enable special char function UARTi.EFR_REG[5] and -* load the new software flow control mode IXON or IXOFF -* and restore the UARTi.EFR_REG[4] ENHANCED_EN value. -*/ -serial_out (up,UART_EFR , up->efr |UART_EFR_SCD ); +serial_out (up,UART_EFR , up->efr);serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); -serial_out (up,UART_MCR , up->mcr & ~UART_MCR_TCRTLR );serial_out (up,UART_LCR , up->lcr); }
@@ -838,6 +833,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); up->efr =serial_in (up,UART_EFR ); + up->efr &= ~UART_EFR_SCD ;serial_out (up,UART_EFR , up->efr |UART_EFR_ECB );serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A );
Signature · 1 line
1.7.4.4
RFC 4/24 SERIAL: omap: fix MCR TCRTLR bit handling
In reply to Russell King - ARM LinuxThe MCR TCRTLR bit can only be changed when ECB is set in the EFR. Unfortunately, several places were trying to alter this bit while ECB was clear:
- serial_omap_configure_xonxoff() was attempting to clear the bit after explicitly clearing the ECB bit.
- serial_omap_set_termios() was trying the same trick after setting the SCR, and when trying to change the TCR register when hardware flow control was enabled.
Fix this by ensuring that we always have ECB set whenever the TCRTLR bit is changed.
Moreover, we start out by reading the EFR and MCR registers, which may have indeterminent bit settings for the ECB and TCRTLR bits. Ensure that these bits always start off in a known state.
--- drivers/tty/serial/omap-serial.c | 32 ++++++++++++++++++-------------- 1 files changed, 18 insertions(+), 14 deletions(-)
drivers/tty/serial/omap-serial.c +18 -14
@@ -700,9 +700,10 @@ serial_omap_configure_xonxoffserial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR );serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_TI752_TCR ,OMAP_UART_TCR_TRIG ); -serial_out (up,UART_EFR , up->efr);serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A );serial_out (up,UART_MCR , up->mcr & ~UART_MCR_TCRTLR ); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); +serial_out (up,UART_EFR , up->efr);serial_out (up,UART_LCR , up->lcr); }
@@ -720,7 +721,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios, {struct uart_omap_port *up = (struct uart_omap_port *)port;unsigned char cval =0 ; -unsigned char efr =0 ;unsigned long flags =0 ;unsigned int baud, quot;
@@ -832,12 +832,12 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); - up->efr =serial_in (up,UART_EFR ); + up->efr =serial_in (up,UART_EFR ) & ~UART_EFR_ECB ; up->efr &= ~UART_EFR_SCD ;serial_out (up,UART_EFR , up->efr |UART_EFR_ECB );serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); - up->mcr =serial_in (up,UART_MCR ); + up->mcr =serial_in (up,UART_MCR ) & ~UART_MCR_TCRTLR ;serial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR );/* FIFO ENABLE, DMA MODE */
@@ -857,9 +857,12 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,serial_out (up,UART_OMAP_SCR , up->scr); -serial_out (up,UART_EFR , up->efr); +/* Reset UART_MCR_TCRTLR: this must be done with the EFR_ECB bit set */ serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A );serial_out (up,UART_MCR , up->mcr); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); +serial_out (up,UART_EFR , up->efr); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A );/* Protocol, Baud Rate, and Interrupt Settings */
@@ -900,20 +903,21 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,/* Hardware Flow Control Configuration */ if (termios->c_cflag &CRTSCTS ) { - efr |= (UART_EFR_CTS |UART_EFR_RTS ); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); - - up->mcr =serial_in (up,UART_MCR ); -serial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR ); - +/* Enable access to TCR/TLR */ serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); - up->efr =serial_in (up,UART_EFR );serial_out (up,UART_EFR , up->efr |UART_EFR_ECB ); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); +serial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR );serial_out (up,UART_TI752_TCR ,OMAP_UART_TCR_TRIG ); -serial_out (up,UART_EFR , efr);/* Enable AUTORTS and AUTOCTS */ -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); + +/* Enable AUTORTS and AUTOCTS */ + up->efr |=UART_EFR_CTS |UART_EFR_RTS ; + +/* Disable access to TCR/TLR */ serial_out (up,UART_MCR , up->mcr |UART_MCR_RTS ); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); +serial_out (up,UART_EFR , up->efr);serial_out (up,UART_LCR , cval); }
Signature · 1 line
1.7.4.4
RFC 5/24 SERIAL: omap: no need to re-read EFR
In reply to Russell King - ARM LinuxThere's no need to re-read EFR after we've recently written it; the register is a configuration register which doesn't change its value without us writing to it.
Removing this re-reading avoids the possibility that we end up with up->efr having the ECB bit set, which should only be temporarily set when accessing the enhanced features.
--- drivers/tty/serial/omap-serial.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/tty/serial/omap-serial.c +0 -2
@@ -872,8 +872,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,serial_out (up,UART_OMAP_MDR1 , up->mdr1);serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); - - up->efr =serial_in (up,UART_EFR );serial_out (up,UART_EFR , up->efr |UART_EFR_ECB );serial_out (up,UART_LCR ,0 );
Signature · 1 line
1.7.4.4
RFC 6/24 SERIAL: omap: allow hardware assisted rts/cts modes to be disabled
In reply to Russell King - ARM LinuxThere is nothing which clears the auto RTS/CTS bits, so once hardware flow control gets enabled, there's no possibility to disable it. So, clear these bits when CRTSCTS is cleared.
--- drivers/tty/serial/omap-serial.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
drivers/tty/serial/omap-serial.c +7 -0
@@ -917,6 +917,13 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_EFR , up->efr);serial_out (up,UART_LCR , cval); + }else { +/* Disable AUTORTS and AUTOCTS */ + up->efr &= ~(UART_EFR_CTS |UART_EFR_RTS ); + +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); +serial_out (up,UART_EFR , up->efr); +serial_out (up,UART_LCR , cval); }serial_omap_set_mctrl (&up->port, up->port.mctrl);
Signature · 1 line
1.7.4.4
RFC 7/24 SERIAL: omap: allow hardware assisted IXANY mode to be disabled
In reply to Russell King - ARM LinuxNothing was clearing the UART_MCR_XONANY bit, so once the ixany mode gets set, there's no possibility to disable it. Clear this bit when IXANY mode is cleared.
--- drivers/tty/serial/omap-serial.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/tty/serial/omap-serial.c +2 -0
@@ -696,6 +696,8 @@ serial_omap_configure_xonxoff*/ if (termios->c_iflag &IXANY ) up->mcr |=UART_MCR_XONANY ; +else + up->mcr &= ~UART_MCR_XONANY ;serial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR );serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );
Signature · 1 line
1.7.4.4
RFC 8/24 SERIAL: core: use local variable uport in uart_set_termios()
In reply to Russell King - ARM LinuxThis is to make the following change more clear.
--- drivers/tty/serial/serial_core.c | 17 +++++++++-------- 1 files changed, 9 insertions(+), 8 deletions(-)
drivers/tty/serial/serial_core.c +9 -8
@@ -1194,6 +1194,7 @@ static void uart_set_termios(struct tty_struct *tty, struct ktermios *old_termios) {struct uart_state *state = tty->driver_data; +struct uart_port *uport = state->uart_port;unsigned long flags;unsigned int cflag = tty->termios->c_cflag;
@@ -1216,31 +1217,31 @@ static void uart_set_termios(struct tty_struct *tty,/* Handle transition to B0 status */ if ((old_termios->c_cflag &CBAUD ) && !(cflag &CBAUD )) -uart_clear_mctrl (state->uart_port,TIOCM_RTS |TIOCM_DTR ); +uart_clear_mctrl (uport,TIOCM_RTS |TIOCM_DTR );/* Handle transition away from B0 status */ else if (!(old_termios->c_cflag &CBAUD ) && (cflag &CBAUD )) {unsigned int mask =TIOCM_DTR ;if (!(cflag &CRTSCTS ) || !test_bit (TTY_THROTTLED , &tty->flags)) mask |=TIOCM_RTS ; -uart_set_mctrl (state->uart_port, mask); +uart_set_mctrl (uport, mask); }/* Handle turning off CRTSCTS */ if ((old_termios->c_cflag &CRTSCTS ) && !(cflag &CRTSCTS )) { -spin_lock_irqsave (&state->uart_port->lock, flags); +spin_lock_irqsave (&uport->lock, flags); tty->hw_stopped =0 ;__uart_start (tty); -spin_unlock_irqrestore (&state->uart_port->lock, flags); +spin_unlock_irqrestore (&uport->lock, flags); }/* Handle turning on CRTSCTS */ else if (!(old_termios->c_cflag &CRTSCTS ) && (cflag &CRTSCTS )) { -spin_lock_irqsave (&state->uart_port->lock, flags); -if (!(state->uart_port->ops->get_mctrl (state->uart_port) &TIOCM_CTS )) { +spin_lock_irqsave (&uport->lock, flags); +if (!(uport->ops->get_mctrl (uport) &TIOCM_CTS )) { tty->hw_stopped =1 ; - state->uart_port->ops->stop_tx (state->uart_port); + uport->ops->stop_tx (uport); } -spin_unlock_irqrestore (&state->uart_port->lock, flags); +spin_unlock_irqrestore (&uport->lock, flags); } }
Signature · 1 line
1.7.4.4
RFC 9/24 SERIAL: core: add hardware assisted s/w flow control support
In reply to Russell King - ARM LinuxPorts which are capable of handling s/w flow control in hardware to know when the s/w flow control termios settings are changed. Add a flag to allow the low level serial drivers to indicate that they support this, and these changes should be propagated to them.
--- drivers/tty/serial/serial_core.c | 16 ++++++++++++++-- include/linux/serial_core.h | 2 ++ 2 files changed, 16 insertions(+), 2 deletions(-)
drivers/tty/serial/serial_core.c +14 -2
@@ -1197,7 +1197,19 @@ static void uart_set_termios(struct tty_struct *tty,struct uart_port *uport = state->uart_port;unsigned long flags;unsigned int cflag = tty->termios->c_cflag; +unsigned int iflag_mask =IGNBRK |BRKINT |IGNPAR |PARMRK |INPCK ; +bool sw_changed = false; +/* +* Drivers doing software flow control also need to know +* about changes to these input settings. +*/ +if (uport->flags &UPF_SOFT_FLOW ) { + iflag_mask |=IXANY |IXON |IXOFF ; + sw_changed = + tty->termios->c_cc[VSTART ] != old_termios->c_cc[VSTART ] || + tty->termios->c_cc[VSTOP ] != old_termios->c_cc[VSTOP ]; + }/* * These are the bits that are used to setup various
@@ -1205,11 +1217,11 @@ static void uart_set_termios(struct tty_struct *tty,* bits in c_cflag; c_[io]speed will always be set * appropriately by set_termios() in tty_ioctl.c */ -#define RELEVANT_IFLAG (iflag) ((iflag) & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK))if ((cflag ^ old_termios->c_cflag) ==0 && tty->termios->c_ospeed == old_termios->c_ospeed && tty->termios->c_ispeed == old_termios->c_ispeed && -RELEVANT_IFLAG (tty->termios->c_iflag ^ old_termios->c_iflag) ==0 ) { + ((tty->termios->c_iflag ^ old_termios->c_iflag) & iflag_mask) ==0 && + !sw_changed) {return ; }
include/linux/serial_core.h +2 -0
@@ -356,6 +356,8 @@ struct uart_port {#define UPF_BUGGY_UART ((__force upf_t) (1 << 14))#define UPF_NO_TXEN_TEST ((__force upf_t) (1 << 15))#define UPF_MAGIC_MULTIPLIER ((__force upf_t) (1 << 16)) +/* Port has hardware-assisted s/w flow control */ +#define UPF_SOFT_FLOW ((__force upf_t) (1 << 22))#define UPF_CONS_FLOW ((__force upf_t) (1 << 23))#define UPF_SHARE_IRQ ((__force upf_t) (1 << 24))#define UPF_EXAR_EFR ((__force upf_t) (1 << 25))
Signature · 1 line
1.7.4.4
RFC 10/24 SERIAL: core: add hardware assisted h/w flow control support
In reply to Russell King - ARM LinuxPorts which are handling h/w flow control in hardware must not have their RTS state altered depending on the tty's hardware-stopped state. Avoid this additional logic when setting the termios state.
--- drivers/tty/serial/serial_core.c | 7 +++++++ include/linux/serial_core.h | 2 ++ 2 files changed, 9 insertions(+), 0 deletions(-)
drivers/tty/serial/serial_core.c +7 -0
@@ -1239,6 +1239,13 @@ static void uart_set_termios(struct tty_struct *tty,uart_set_mctrl (uport, mask); } +/* +* If the port is doing h/w assisted flow control, do nothing. +* We assume that tty->hw_stopped has never been set. +*/ +if (uport->flags &UPF_HARD_FLOW ) +return ; +/* Handle turning off CRTSCTS */ if ((old_termios->c_cflag &CRTSCTS ) && !(cflag &CRTSCTS )) {spin_lock_irqsave (&uport->lock, flags);
include/linux/serial_core.h +2 -0
@@ -356,6 +356,8 @@ struct uart_port {#define UPF_BUGGY_UART ((__force upf_t) (1 << 14))#define UPF_NO_TXEN_TEST ((__force upf_t) (1 << 15))#define UPF_MAGIC_MULTIPLIER ((__force upf_t) (1 << 16)) +/* Port has hardware-assisted h/w flow control (iow, auto-RTS *not* auto-CTS) */ +#define UPF_HARD_FLOW ((__force upf_t) (1 << 21))/* Port has hardware-assisted s/w flow control */ #define UPF_SOFT_FLOW ((__force upf_t) (1 << 22))#define UPF_CONS_FLOW ((__force upf_t) (1 << 23))
Signature · 1 line
1.7.4.4
RFC 11/24 SERIAL: core: add throttle/unthrottle callbacks for hardware assisted flow control
In reply to Russell King - ARM LinuxAdd two callbacks for hardware assisted flow control; we need to know when the tty layers want us to stop and restart due to their buffer levels.
Call a driver specific throttle/unthrottle function if and only if the driver indicates that it is using an enabled hardware assisted flow control method, otherwise fall back to the non-hardware assisted methods.
--- drivers/tty/serial/serial_core.c | 31 +++++++++++++++++++++++++++---- include/linux/serial_core.h | 2 ++ 2 files changed, 29 insertions(+), 4 deletions(-)
drivers/tty/serial/serial_core.c +27 -4
@@ -610,27 +610,50 @@ static void uart_send_xchar(struct tty_struct *tty, char ch)static void uart_throttle (struct tty_struct *tty) {struct uart_state *state = tty->driver_data; +struct uart_port *port = state->uart_port; +uint32_t mask =0 ;if (I_IXOFF (tty)) + mask |=UPF_SOFT_FLOW ; +if (tty->termios->c_cflag &CRTSCTS ) + mask |=UPF_HARD_FLOW ; + +if (port->flags & mask) { + port->ops->throttle (port); + mask &= ~port->flags; + } + +if (mask &UPF_SOFT_FLOW )uart_send_xchar (tty,STOP_CHAR (tty)); -if (tty->termios->c_cflag &CRTSCTS ) -uart_clear_mctrl (state->uart_port,TIOCM_RTS ); +if (mask &UPF_HARD_FLOW ) +uart_clear_mctrl (port,TIOCM_RTS ); }static void uart_unthrottle (struct tty_struct *tty) {struct uart_state *state = tty->driver_data;struct uart_port *port = state->uart_port; +uint32_t mask =0 ; -if (I_IXOFF (tty)) { +if (I_IXOFF (tty)) + mask |=UPF_SOFT_FLOW ; +if (tty->termios->c_cflag &CRTSCTS ) + mask |=UPF_HARD_FLOW ; + +if (port->flags & mask) { + port->ops->unthrottle (port); + mask &= ~port->flags; + } + +if (mask &UPF_SOFT_FLOW ) {if (port->x_char) port->x_char =0 ;else uart_send_xchar (tty,START_CHAR (tty)); } -if (tty->termios->c_cflag &CRTSCTS ) +if (mask &UPF_HARD_FLOW )uart_set_mctrl (port,TIOCM_RTS ); }
include/linux/serial_core.h +2 -0
@@ -240,6 +240,8 @@ struct uart_ops {unsigned int (*get_mctrl)(struct uart_port *);void (*stop_tx)(struct uart_port *);void (*start_tx)(struct uart_port *); +void (*throttle)(struct uart_port *); +void (*unthrottle)(struct uart_port *);void (*send_xchar)(struct uart_port *,char ch);void (*stop_rx)(struct uart_port *);void (*enable_ms)(struct uart_port *);
Signature · 1 line
1.7.4.4
RFC 12/24 SERIAL: omap: fix hardware assisted flow control
In reply to Russell King - ARM LinuxWhen the UART device has hardware flow control enabled, it ignores the MCR RTS bit in the MCR register, and keeps RTS asserted as long as we continue to read characters from the UART receiver FIFO. This means that when the TTY buffers become full, the UART doesn't tell the remote end to stop sending, which causes the TTY layer to start dropping characters.
A similar problem exists with software flow control. We need the FIFO register to fill when software flow control is enabled to provoke the UART to send the XOFF character.
Fix this by implementing the throttle/unthrottle callbacks, and use these to disable receiver interrupts. This in turn means that the UART FIFO will fill, which will then cause the UART's hardware to deassert the RTS signal and/or send the XOFF character, stopping the remote end.
--- drivers/tty/serial/omap-serial.c | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-)
drivers/tty/serial/omap-serial.c +31 -0
@@ -373,6 +373,34 @@ static void serial_omap_start_tx(struct uart_port *port)omap_start_dma (up->uart_dma.tx_dma_channel); } +static void serial_omap_throttle (struct uart_port *port) +{ +struct uart_omap_port *up = (struct uart_omap_port *)port; +unsigned long flags; + +pm_runtime_get_sync (&up->pdev->dev); +spin_lock_irqsave (&up->port.lock, flags); + up->ier &= ~(UART_IER_RLSI |UART_IER_RDI ); +serial_out (up,UART_IER , up->ier); +spin_unlock_irqrestore (&up->port.lock, flags); +pm_runtime_mark_last_busy (&up->pdev->dev); +pm_runtime_put_autosuspend (&up->pdev->dev); +} + +static void serial_omap_unthrottle (struct uart_port *port) +{ +struct uart_omap_port *up = (struct uart_omap_port *)port; +unsigned long flags; + +pm_runtime_get_sync (&up->pdev->dev); +spin_lock_irqsave (&up->port.lock, flags); + up->ier |=UART_IER_RLSI |UART_IER_RDI ; +serial_out (up,UART_IER , up->ier); +spin_unlock_irqrestore (&up->port.lock, flags); +pm_runtime_mark_last_busy (&up->pdev->dev); +pm_runtime_put_autosuspend (&up->pdev->dev); +} +static unsigned int check_modem_status (struct uart_omap_port *up) {unsigned int status;
@@ -985,6 +1013,7 @@ static void serial_omap_config_port(struct uart_port *port, int flags)dev_dbg (up->port.dev,"serial_omap_config_port+%d\n" , up->port.line); up->port.type =PORT_OMAP ; + up->port.flags |=UPF_SOFT_FLOW |UPF_HARD_FLOW ; } static int
@@ -1181,6 +1210,8 @@ static struct uart_ops serial_omap_pops = { .get_mctrl = serial_omap_get_mctrl, .stop_tx = serial_omap_stop_tx, .start_tx = serial_omap_start_tx, + .throttle = serial_omap_throttle, + .unthrottle = serial_omap_unthrottle, .stop_rx = serial_omap_stop_rx, .enable_ms = serial_omap_enable_ms, .break_ctl = serial_omap_break_ctl,
Signature · 1 line
1.7.4.4
RFC 13/24 SERIAL: omap: configure xon/xoff before setting modem control lines
In reply to Russell King - ARM Linux--- drivers/tty/serial/omap-serial.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/tty/serial/omap-serial.c +2 -1
@@ -956,10 +956,11 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,serial_out (up,UART_LCR , cval); } -serial_omap_set_mctrl (&up->port, up->port.mctrl);/* Software Flow Control Configuration */ serial_omap_configure_xonxoff (up, termios); +serial_omap_set_mctrl (&up->port, up->port.mctrl); +spin_unlock_irqrestore (&up->port.lock, flags);pm_runtime_put (&up->pdev->dev);dev_dbg (up->port.dev,"serial_omap_set_termios+%d\n" , up->port.line);
Signature · 1 line
1.7.4.4
RFC 14/24 SERIAL: omap: serial_omap_configure_xonxoff() contents into set_termios
In reply to Russell King - ARM Linux--- drivers/tty/serial/omap-serial.c | 109 ++++++++++++++++++-------------------- 1 files changed, 51 insertions(+), 58 deletions(-)
drivers/tty/serial/omap-serial.c +51 -58
@@ -680,63 +680,6 @@ static void serial_omap_shutdown(struct uart_port *port)free_irq (up->port.irq, up); } -static inline void -serial_omap_configure_xonxoff - (struct uart_omap_port *up,struct ktermios *termios) -{ - up->lcr =serial_in (up,UART_LCR ); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); - up->efr =serial_in (up,UART_EFR ); -serial_out (up,UART_EFR , up->efr & ~UART_EFR_ECB ); - -serial_out (up,UART_XON1 , termios->c_cc[VSTART ]); -serial_out (up,UART_XOFF1 , termios->c_cc[VSTOP ]); - -/* clear SW control mode bits */ - up->efr &=OMAP_UART_SW_CLR ; - -/* -* IXON Flag: -* Enable XON/XOFF flow control on input. -* Receiver compares XON1, XOFF1. -*/ -if (termios->c_iflag &IXON ) - up->efr |=OMAP_UART_SW_RX ; - -/* -* IXOFF Flag: -* Enable XON/XOFF flow control on output. -* Transmit XON1, XOFF1 -*/ -if (termios->c_iflag &IXOFF ) - up->efr |=OMAP_UART_SW_TX ; - -serial_out (up,UART_EFR , up->efr |UART_EFR_ECB ); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); - - up->mcr =serial_in (up,UART_MCR ); - -/* -* IXANY Flag: -* Enable any character to restart output. -* Operation resumes after receiving any -* character after recognition of the XOFF character -*/ -if (termios->c_iflag &IXANY ) - up->mcr |=UART_MCR_XONANY ; -else - up->mcr &= ~UART_MCR_XONANY ; - -serial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR ); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); -serial_out (up,UART_TI752_TCR ,OMAP_UART_TCR_TRIG ); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); -serial_out (up,UART_MCR , up->mcr & ~UART_MCR_TCRTLR ); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); -serial_out (up,UART_EFR , up->efr); -serial_out (up,UART_LCR , up->lcr); -} -static void serial_omap_uart_qos_work (struct work_struct *work) { struct uart_omap_port *up = container_of(work, struct uart_omap_port,
@@ -957,7 +900,57 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios, }/* Software Flow Control Configuration */ -serial_omap_configure_xonxoff (up, termios); + up->lcr =serial_in (up,UART_LCR ); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); + up->efr =serial_in (up,UART_EFR ); +serial_out (up,UART_EFR , up->efr & ~UART_EFR_ECB ); + +serial_out (up,UART_XON1 , termios->c_cc[VSTART ]); +serial_out (up,UART_XOFF1 , termios->c_cc[VSTOP ]); + +/* clear SW control mode bits */ + up->efr &=OMAP_UART_SW_CLR ; + +/* +* IXON Flag: +* Enable XON/XOFF flow control on input. +* Receiver compares XON1, XOFF1. +*/ +if (termios->c_iflag &IXON ) + up->efr |=OMAP_UART_SW_RX ; + +/* +* IXOFF Flag: +* Enable XON/XOFF flow control on output. +* Transmit XON1, XOFF1 +*/ +if (termios->c_iflag &IXOFF ) + up->efr |=OMAP_UART_SW_TX ; + +serial_out (up,UART_EFR , up->efr |UART_EFR_ECB ); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); + + up->mcr =serial_in (up,UART_MCR ); + +/* +* IXANY Flag: +* Enable any character to restart output. +* Operation resumes after receiving any +* character after recognition of the XOFF character +*/ +if (termios->c_iflag &IXANY ) + up->mcr |=UART_MCR_XONANY ; +else + up->mcr &= ~UART_MCR_XONANY ; + +serial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR ); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); +serial_out (up,UART_TI752_TCR ,OMAP_UART_TCR_TRIG ); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); +serial_out (up,UART_MCR , up->mcr & ~UART_MCR_TCRTLR ); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); +serial_out (up,UART_EFR , up->efr); +serial_out (up,UART_LCR , up->lcr);serial_omap_set_mctrl (&up->port, up->port.mctrl);
Signature · 1 line
1.7.4.4
RFC 15/24 SERIAL: omap: don't read back LCR/MCR/EFR
In reply to Russell King - ARM LinuxThere's really no reason to read back these registers while setting the termios modes, provided we keep our cached copies up to date. Remove these readbacks.
This has the benefit that we know that the EFR_ECB and MCR_TCRTLR bits will always be clear, so we don't need to keep masking these bits throughout the code.
--- drivers/tty/serial/omap-serial.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-)
drivers/tty/serial/omap-serial.c +6 -7
@@ -885,8 +885,11 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,/* Enable AUTORTS and AUTOCTS */ up->efr |=UART_EFR_CTS |UART_EFR_RTS ; +/* Ensure MCR RTS is asserted */ + up->mcr |=UART_MCR_RTS ; +/* Disable access to TCR/TLR */ -serial_out (up,UART_MCR , up->mcr |UART_MCR_RTS ); +serial_out (up,UART_MCR , up->mcr);serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_EFR , up->efr);serial_out (up,UART_LCR , cval);
@@ -900,10 +903,8 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios, }/* Software Flow Control Configuration */ - up->lcr =serial_in (up,UART_LCR );serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); - up->efr =serial_in (up,UART_EFR ); -serial_out (up,UART_EFR , up->efr & ~UART_EFR_ECB ); +serial_out (up,UART_EFR , up->efr);serial_out (up,UART_XON1 , termios->c_cc[VSTART ]);serial_out (up,UART_XOFF1 , termios->c_cc[VSTOP ]);
@@ -930,8 +931,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,serial_out (up,UART_EFR , up->efr |UART_EFR_ECB );serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); - up->mcr =serial_in (up,UART_MCR ); -/* * IXANY Flag: * Enable any character to restart output.
@@ -947,7 +946,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_TI752_TCR ,OMAP_UART_TCR_TRIG );serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); -serial_out (up,UART_MCR , up->mcr & ~UART_MCR_TCRTLR ); +serial_out (up,UART_MCR , up->mcr);serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_EFR , up->efr);serial_out (up,UART_LCR , up->lcr);
Signature · 1 line
1.7.4.4
RFC 16/24 SERIAL: omap: simplify
In reply to Russell King - ARM LinuxWe have the sequence:
- LCR mode B
- write EFR with ECB clear
- LCR mode normal
- LCR mode B
- write EFR with ECB clear
This can be simplified to just two writes:
- LCR mode B
- write EFR with ECB clear
--- drivers/tty/serial/omap-serial.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-)
drivers/tty/serial/omap-serial.c +0 -8
@@ -890,19 +890,11 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,/* Disable access to TCR/TLR */ serial_out (up,UART_MCR , up->mcr); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); -serial_out (up,UART_EFR , up->efr); -serial_out (up,UART_LCR , cval); }else {/* Disable AUTORTS and AUTOCTS */ up->efr &= ~(UART_EFR_CTS |UART_EFR_RTS ); - -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); -serial_out (up,UART_EFR , up->efr); -serial_out (up,UART_LCR , cval); } -/* Software Flow Control Configuration */ serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_EFR , up->efr);
Signature · 1 line
1.7.4.4
RFC 17/24 SERIAL: omap: always set TCR
In reply to Russell King - ARM LinuxWe always setup the TCR register in the software flow control path, and when hardware flow control is enabled. Remove this redundant setup, and place it before we setup any hardware flow control.
--- drivers/tty/serial/omap-serial.c | 25 ++++++++++--------------- 1 files changed, 10 insertions(+), 15 deletions(-)
drivers/tty/serial/omap-serial.c +10 -15
@@ -871,30 +871,29 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,else serial_out (up,UART_OMAP_MDR1 , up->mdr1); -/* Hardware Flow Control Configuration */ +/* Enable access to TCR/TLR */ +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); +serial_out (up,UART_EFR , up->efr |UART_EFR_ECB ); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); +serial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR ); -if (termios->c_cflag &CRTSCTS ) { -/* Enable access to TCR/TLR */ -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); -serial_out (up,UART_EFR , up->efr |UART_EFR_ECB ); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); -serial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR ); +serial_out (up,UART_TI752_TCR ,OMAP_UART_TCR_TRIG ); -serial_out (up,UART_TI752_TCR ,OMAP_UART_TCR_TRIG ); +/* Hardware Flow Control Configuration */ +if (termios->c_cflag &CRTSCTS ) {/* Enable AUTORTS and AUTOCTS */ up->efr |=UART_EFR_CTS |UART_EFR_RTS ;/* Ensure MCR RTS is asserted */ up->mcr |=UART_MCR_RTS ; - -/* Disable access to TCR/TLR */ -serial_out (up,UART_MCR , up->mcr); }else {/* Disable AUTORTS and AUTOCTS */ up->efr &= ~(UART_EFR_CTS |UART_EFR_RTS ); } +/* Disable access to TCR/TLR */ +serial_out (up,UART_MCR , up->mcr);serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_EFR , up->efr);
@@ -934,10 +933,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,else up->mcr &= ~UART_MCR_XONANY ; -serial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR ); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); -serial_out (up,UART_TI752_TCR ,OMAP_UART_TCR_TRIG ); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A );serial_out (up,UART_MCR , up->mcr);serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_EFR , up->efr);
Signature · 1 line
1.7.4.4
RFC 18/24 SERIAL: omap: move xon/xoff setting earlier
In reply to Russell King - ARM LinuxTake advantage of the switch to mode B for accessing the TCR register, and move the xon/xoff configuration there. This allows further simplication of this sequence.
--- drivers/tty/serial/omap-serial.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-)
drivers/tty/serial/omap-serial.c +7 -6
@@ -871,16 +871,20 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,else serial_out (up,UART_OMAP_MDR1 , up->mdr1); -/* Enable access to TCR/TLR */ +/* Configure flow control */ serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); + +/* XON1/XOFF1 accessible mode B, TCRTLR=0, ECB=0 */ +serial_out (up,UART_XON1 , termios->c_cc[VSTART ]); +serial_out (up,UART_XOFF1 , termios->c_cc[VSTOP ]); + +/* Enable access to TCR/TLR */ serial_out (up,UART_EFR , up->efr |UART_EFR_ECB );serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A );serial_out (up,UART_MCR , up->mcr |UART_MCR_TCRTLR );serial_out (up,UART_TI752_TCR ,OMAP_UART_TCR_TRIG ); -/* Hardware Flow Control Configuration */ -if (termios->c_cflag &CRTSCTS ) {/* Enable AUTORTS and AUTOCTS */ up->efr |=UART_EFR_CTS |UART_EFR_RTS ;
@@ -897,9 +901,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_EFR , up->efr); -serial_out (up,UART_XON1 , termios->c_cc[VSTART ]); -serial_out (up,UART_XOFF1 , termios->c_cc[VSTOP ]); -/* clear SW control mode bits */ up->efr &=OMAP_UART_SW_CLR ;
Signature · 1 line
1.7.4.4
RFC 19/24 SERIAL: omap: simplify (2)
In reply to Russell King - ARM LinuxSimplify:
- set ECB
...
- LCR mode A
- clear TCRTLR
- LCR mode B
- clear ECB
- set ECB and update other bits
- LCR mode A
- update XONANY
to:
- set ECB
...
- LCR mode B
- set ECB and update other bits
- LCR mode A
- update XONANY and clear TCRTLR
--- drivers/tty/serial/omap-serial.c | 11 +++-------- 1 files changed, 3 insertions(+), 8 deletions(-)
drivers/tty/serial/omap-serial.c +3 -8
@@ -896,11 +896,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios, up->efr &= ~(UART_EFR_CTS |UART_EFR_RTS ); } -/* Disable access to TCR/TLR */ -serial_out (up,UART_MCR , up->mcr); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); -serial_out (up,UART_EFR , up->efr); -/* clear SW control mode bits */ up->efr &=OMAP_UART_SW_CLR ;
@@ -920,9 +915,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,if (termios->c_iflag &IXOFF ) up->efr |=OMAP_UART_SW_TX ; -serial_out (up,UART_EFR , up->efr |UART_EFR_ECB ); -serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A ); -/* * IXANY Flag: * Enable any character to restart output.
@@ -934,6 +926,9 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,else up->mcr &= ~UART_MCR_XONANY ; +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B ); +serial_out (up,UART_EFR , up->efr |UART_EFR_ECB ); +serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_A );serial_out (up,UART_MCR , up->mcr);serial_out (up,UART_LCR ,UART_LCR_CONF_MODE_B );serial_out (up,UART_EFR , up->efr);
Signature · 1 line
1.7.4.4
RFC 20/24 SERIAL: core: add xmit buffer allocation callbacks
In reply to Russell King - ARM LinuxThis allows drivers (such as OMAP serial) to allocate and free their transmit buffers in a sane manner, rather than working around the buffer allocation provided by serial_core.
--- drivers/tty/serial/serial_core.c | 23 ++++++++++++++++------- include/linux/serial_core.h | 2 ++ 2 files changed, 18 insertions(+), 7 deletions(-)
drivers/tty/serial/serial_core.c +15 -6
@@ -147,12 +147,18 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,* buffer. */ if (!state->xmit.buf) { -/* This is protected by the per port mutex */ - page =get_zeroed_page (GFP_KERNEL ); -if (!page) -return -ENOMEM ; +if (uport->ops->alloc_xmit) { + retval = uport->ops->alloc_xmit (uport, &state->xmit); +if (retval) +return retval; + }else { +/* This is protected by the per port mutex */ + page =get_zeroed_page (GFP_KERNEL ); +if (!page) +return -ENOMEM ; - state->xmit.buf = (unsigned char *) page; + state->xmit.buf = (unsigned char *) page; + }uart_circ_clear (&state->xmit); }
@@ -257,7 +263,10 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)* Free the transmit buffer page. */ if (state->xmit.buf) { -free_page ((unsigned long )state->xmit.buf); +if (uport->ops->free_xmit) + uport->ops->free_xmit (uport, &state->xmit); +else +free_page ((unsigned long )state->xmit.buf); state->xmit.buf =NULL ; } }
include/linux/serial_core.h +2 -0
@@ -248,6 +248,8 @@ struct uart_ops {void (*break_ctl)(struct uart_port *,int ctl);int (*startup)(struct uart_port *);void (*shutdown)(struct uart_port *); +int (*alloc_xmit)(struct uart_port *,struct circ_buf *); +void (*free_xmit)(struct uart_port *,struct circ_buf *);void (*flush_buffer)(struct uart_port *);void (*set_termios)(struct uart_port *,struct ktermios *new,struct ktermios *old);
Signature · 1 line
1.7.4.4
RFC 20/24 SERIAL: core: add xmit buffer allocation callbacks
Quoted 3 lines · On Sat, 06 Oct 2012 13:45:47 +0100 Russell King <[email protected]>…
This allows drivers (such as OMAP serial) to allocate and free their transmit buffers in a sane manner, rather than working around the buffer allocation provided by
serial_core.
I think this just illustrates how broken the serial_core layering is in that drivers can't treat it as a library but get constrained by it.
Fine for now and actually probably a useful hook to begin removing the circ buffers for the kernel generic kfifo and other work that needs doing eventually.
Alan
RFC 20/24 SERIAL: core: add xmit buffer allocation callbacks
Quoted 7 lines · Alan Cox
Quoted 3 lines · On Sat, 06 Oct 2012 13:45:47 +0100 Russell King <[email protected]>…
This allows drivers (such as OMAP serial) to allocate and free their transmit buffers in a sane manner, rather than working around the buffer allocation provided by
serial_core.
Quoted 6 lines
I think this just illustrates how broken the
serial_corelayering is in that drivers can't treat it as a library but get constrained by it.Fine for now and actually probably a useful hook to begin removing the circ buffers for the kernel generic kfifo and other work that needs doing eventually.
Yes, I was thinking about turning the 'default' buffer allocation into a library call which drivers would set, but I felt that's just asking for a big patch and problems with new drivers appearing.
We could transition it by providing a default buffer allocation call, converting all the drivers, and then making that call mandatory. That probably would then give us a better path to convert to kfifo.
The view that serial_core should be a library is one that Ted mentioned when I created serial_core originally - I disagreed at the time, and I still believe it would've ended up being much more complex - and hairy - to go that route at that time. Maybe as things have moved forward in the tty layer that's changed, but I've been out of tty stuff for too long to properly comment.
RFC 21/24 SERIAL: omap: use tx buffer allocation API
In reply to Russell King - ARM LinuxRather than working around serial_core, use the newly provided buffer allocation API to allow a saner method to manage this buffer.
--- drivers/tty/serial/omap-serial.c | 38 +++++++++++++++++++++++++++++--------- 1 files changed, 29 insertions(+), 9 deletions(-)
drivers/tty/serial/omap-serial.c +29 -9
@@ -604,11 +604,6 @@ static int serial_omap_startup(struct uart_port *port) up->msr_saved_flags =0 ;if (up->use_dma) { -free_page ((unsigned long )up->port.state->xmit.buf); - up->port.state->xmit.buf =dma_alloc_coherent (NULL , -UART_XMIT_SIZE , - (dma_addr_t *)&(up->uart_dma.tx_buf_dma_phys), -0 );init_timer (&(up->uart_dma.rx_timer)); up->uart_dma.rx_timer.function = serial_omap_rxdma_poll; up->uart_dma.rx_timer.data = up->port.line;
@@ -665,10 +660,6 @@ static void serial_omap_shutdown(struct uart_port *port)if (serial_in (up,UART_LSR ) &UART_LSR_DR ) (void )serial_in (up,UART_RX );if (up->use_dma) { -dma_free_coherent (up->port.dev, -UART_XMIT_SIZE , up->port.state->xmit.buf, - up->uart_dma.tx_buf_dma_phys); - up->port.state->xmit.buf =NULL ;serial_omap_stop_rx (port); dma_free_coherent(up->port.dev, up->uart_dma.rx_buf_size, up->uart_dma.rx_buf,
@@ -680,6 +671,33 @@ static void serial_omap_shutdown(struct uart_port *port)free_irq (up->port.irq, up); } +static int serial_omap_alloc_xmit (struct uart_port *port,struct circ_buf *xmit) +{ +struct uart_omap_port *up = (struct uart_omap_port *)port; + +if (up->use_dma) { + xmit->buf =dma_alloc_coherent (NULL ,UART_XMIT_SIZE , + &up->uart_dma.tx_buf_dma_phys,0 ); + }else { +unsigned long page =get_zeroed_page (GFP_KERNEL ); + xmit->buf = (unsigned char *)page; + } + +return xmit->buf ?0 : -ENOMEM ; +} + +static void serial_omap_free_xmit (struct uart_port *port,struct circ_buf *xmit) +{ +struct uart_omap_port *up = (struct uart_omap_port *)port; + +if (up->use_dma) { +dma_free_coherent (NULL ,UART_XMIT_SIZE , + xmit->buf, up->uart_dma.tx_buf_dma_phys); + }else { +free_page ((unsigned long )xmit->buf); + } +} +static void serial_omap_uart_qos_work (struct work_struct *work) { struct uart_omap_port *up = container_of(work, struct uart_omap_port,
@@ -1193,6 +1211,8 @@ static struct uart_ops serial_omap_pops = { .break_ctl = serial_omap_break_ctl, .startup = serial_omap_startup, .shutdown = serial_omap_shutdown, + .alloc_xmit = serial_omap_alloc_xmit, + .free_xmit = serial_omap_free_xmit, .set_termios = serial_omap_set_termios, .pm = serial_omap_pm, .type = serial_omap_type,
Signature · 1 line
1.7.4.4
RFC 22/24 SERIAL: omap: typesafe conversion from uart_port to uart_omap_port
In reply to Russell King - ARM Linux--- drivers/tty/serial/omap-serial.c | 49 +++++++++++++++++++++----------------- 1 files changed, 27 insertions(+), 22 deletions(-)
drivers/tty/serial/omap-serial.c +27 -22
@@ -71,6 +71,11 @@#define OMAP_UART_MVR_MAJ_SHIFT 8#define OMAP_UART_MVR_MIN_MASK 0x3f +static struct uart_omap_port *serial_omap_port (struct uart_port *port) +{ + return container_of(port, struct uart_omap_port, port); +} +static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS ];/* Forward declaration of functions */
@@ -141,7 +146,7 @@ static void serial_omap_stop_rxdma(struct uart_omap_port *up)static void serial_omap_enable_ms (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);dev_dbg (up->port.dev,"serial_omap_enable_ms+%d\n" , up->port.line);
@@ -153,7 +158,7 @@ static void serial_omap_enable_ms(struct uart_port *port)static void serial_omap_stop_tx (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);struct omap_uart_port_info *pdata = up->pdev->dev.platform_data; if (up->use_dma &&
@@ -186,7 +191,7 @@ static void serial_omap_stop_tx(struct uart_port *port)static void serial_omap_stop_rx (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);pm_runtime_get_sync (&up->pdev->dev);if (up->use_dma)
@@ -307,7 +312,7 @@ static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)static void serial_omap_start_tx (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;struct circ_buf *xmit;unsigned int start;
@@ -375,7 +380,7 @@ static void serial_omap_start_tx(struct uart_port *port)static void serial_omap_throttle (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);unsigned long flags;pm_runtime_get_sync (&up->pdev->dev);
@@ -389,7 +394,7 @@ static void serial_omap_throttle(struct uart_port *port)static void serial_omap_unthrottle (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);unsigned long flags;pm_runtime_get_sync (&up->pdev->dev);
@@ -477,7 +482,7 @@ static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)static unsigned int serial_omap_tx_empty (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);unsigned long flags =0 ;unsigned int ret =0 ;
@@ -492,7 +497,7 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)static unsigned int serial_omap_get_mctrl (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);unsigned int status;unsigned int ret =0 ;
@@ -515,7 +520,7 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)static void serial_omap_set_mctrl (struct uart_port *port,unsigned int mctrl) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);unsigned char mcr =0 , old_mcr;dev_dbg (up->port.dev,"serial_omap_set_mctrl+%d\n" , up->port.line);
@@ -541,7 +546,7 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)static void serial_omap_break_ctl (struct uart_port *port,int break_state) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);unsigned long flags =0 ;dev_dbg (up->port.dev,"serial_omap_break_ctl+%d\n" , up->port.line);
@@ -558,7 +563,7 @@ static void serial_omap_break_ctl(struct uart_port *port, int break_state)static int serial_omap_startup (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);unsigned long flags =0 ;int retval;
@@ -631,7 +636,7 @@ static int serial_omap_startup(struct uart_port *port)static void serial_omap_shutdown (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);unsigned long flags =0 ;dev_dbg (up->port.dev,"serial_omap_shutdown+%d\n" , up->port.line);
@@ -673,7 +678,7 @@ static void serial_omap_shutdown(struct uart_port *port)static int serial_omap_alloc_xmit (struct uart_port *port,struct circ_buf *xmit) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);if (up->use_dma) { xmit->buf = dma_alloc_coherent(NULL, UART_XMIT_SIZE,
@@ -688,7 +693,7 @@ static int serial_omap_alloc_xmit(struct uart_port *port, struct circ_buf *xmit)static void serial_omap_free_xmit (struct uart_port *port,struct circ_buf *xmit) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);if (up->use_dma) { dma_free_coherent(NULL, UART_XMIT_SIZE,
@@ -710,7 +715,7 @@ static void serial_omap_set_termios(struct uart_port *port, struct ktermios *termios, struct ktermios *old) { - struct uart_omap_port *up = (struct uart_omap_port *)port; + struct uart_omap_port *up = serial_omap_port(port);unsigned char cval =0 ;unsigned long flags =0 ;unsigned int baud, quot;
@@ -963,7 +968,7 @@ static void serial_omap_pm(struct uart_port *port, unsigned int state, unsigned int oldstate) { - struct uart_omap_port *up = (struct uart_omap_port *)port; + struct uart_omap_port *up = serial_omap_port(port);unsigned char efr;dev_dbg (up->port.dev,"serial_omap_pm+%d\n" , up->port.line);
@@ -1002,7 +1007,7 @@ static int serial_omap_request_port(struct uart_port *port)static void serial_omap_config_port (struct uart_port *port,int flags) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);dev_dbg (up->port.dev,"serial_omap_config_port+%d\n" , up->port.line);
@@ -1021,7 +1026,7 @@ serial_omap_verify_port(struct uart_port *port, struct serial_struct *ser)static const char *serial_omap_type (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);dev_dbg (up->port.dev,"serial_omap_type+%d\n" , up->port.line);return up->name;
@@ -1064,7 +1069,7 @@ static inline void wait_for_xmitr(struct uart_omap_port *up)static void serial_omap_poll_put_char (struct uart_port *port,unsigned char ch) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);pm_runtime_get_sync (&up->pdev->dev);wait_for_xmitr (up);
@@ -1074,7 +1079,7 @@ static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)static int serial_omap_poll_get_char (struct uart_port *port) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);unsigned int status;pm_runtime_get_sync (&up->pdev->dev);
@@ -1097,7 +1102,7 @@ static struct uart_driver serial_omap_reg;static void serial_omap_console_putchar (struct uart_port *port,int ch) { -struct uart_omap_port *up = (struct uart_omap_port *)port; +struct uart_omap_port *up =serial_omap_port (port);wait_for_xmitr (up);serial_out (up,UART_TX , ch);
@@ -1377,7 +1382,7 @@ static void serial_omap_continue_tx(struct uart_omap_port *up)static void uart_tx_dma_callback (int lch,u16 ch_status,void *data) { -struct uart_omap_port *up = (struct uart_omap_port *)data; +struct uart_omap_port *up =serial_omap_port (data);struct circ_buf *xmit = &up->port.state->xmit; xmit->tail = (xmit->tail + up->uart_dma.tx_buf_size) & \
Signature · 1 line
1.7.4.4
RFC 23/24 SERIAL: omap: move driver private definitions and structures to driver
In reply to Russell King - ARM Linuxstruct uart_omap_port and struct uart_omap_dma, and associated definitions are private to the driver, so there's no point them sitting in an include file under arch/arm. Move them into the driver itself.
--- arch/arm/plat-omap/include/plat/omap-serial.h | 87 ------------------------ drivers/tty/serial/omap-serial.c | 88 +++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 87 deletions(-)
arch/arm/plat-omap/include/plat/omap-serial.h +0 -87
@@ -32,35 +32,9 @@*/ #define OMAP_SERIAL_NAME "ttyO" -#define OMAP_MODE13X_SPEED 230400 - -#define OMAP_UART_SCR_TX_EMPTY 0x08 - -/* WER = 0x7F -* Enable module level wakeup in WER reg -*/ -#define OMAP_UART_WER_MOD_WKUP 0X7F - -/* Enable XON/XOFF flow control on output */ -#define OMAP_UART_SW_TX 0x08 - -/* Enable XON/XOFF flow control on input */ -#define OMAP_UART_SW_RX 0x02 -#define OMAP_UART_SYSC_RESET 0X07 -#define OMAP_UART_TCR_TRIG 0X0F -#define OMAP_UART_SW_CLR 0XF0#define OMAP_UART_FIFO_CLR 0X06 -#define OMAP_UART_DMA_CH_FREE -1 - -#define OMAP_MAX_HSUART_PORTS 4 - -#define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA - -#define UART_ERRATA_i202_MDR1_ACCESS BIT(0) -#define UART_ERRATA_i291_DMA_FORCEIDLE BIT(1) -struct omap_uart_port_info {bool dma_enabled;/* To specify DMA Mode */ unsigned int uartclk;/* UART clock rate */
@@ -76,65 +50,4 @@ struct omap_uart_port_info { void (*enable_wakeup)(struct platform_device *, bool); }; -struct uart_omap_dma { -u8 uart_dma_tx; -u8 uart_dma_rx; -int rx_dma_channel; -int tx_dma_channel; -dma_addr_t rx_buf_dma_phys; -dma_addr_t tx_buf_dma_phys; -unsigned int uart_base; -/* -* Buffer for rx dma.It is not required for tx because the buffer -* comes from port structure. -*/ -unsigned char *rx_buf; -unsigned int prev_rx_dma_pos; -int tx_buf_size; -int tx_dma_used; -int rx_dma_used; -spinlock_t tx_lock; -spinlock_t rx_lock; -/* timer to poll activity on rx dma */ -struct timer_list rx_timer; -unsigned int rx_buf_size; -unsigned int rx_poll_rate; -unsigned int rx_timeout; -}; - -struct uart_omap_port { -struct uart_port port; -struct uart_omap_dma uart_dma; -struct platform_device *pdev; - -unsigned char ier; -unsigned char lcr; -unsigned char mcr; -unsigned char fcr; -unsigned char efr; -unsigned char dll; -unsigned char dlh; -unsigned char mdr1; -unsigned char scr; - -int use_dma; -/* -* Some bits in registers are cleared on a read, so they must -* be saved whenever the register is read but the bits will not -* be immediately processed. -*/ -unsigned int lsr_break_flag; -unsigned char msr_saved_flags; -char name[20 ]; -unsigned long port_activity; -u32 context_loss_cnt; -u32 errata; -u8 wakeups_enabled; - -struct pm_qos_request pm_qos_request; -u32 latency; -u32 calc_latency; -struct work_struct qos_work; -}; -#endif /* __OMAP_SERIAL_H__ */
drivers/tty/serial/omap-serial.c +88 -0
106 lines not on this page · raw message
Signature · 1 line
1.7.4.4
RFC 24/24 SERIAL: omap: remove OMAP_UART_SYSC_RESET and OMAP_UART_FIFO_CLR
In reply to Russell King - ARM LinuxOMAP_UART_SYSC_RESET and OMAP_UART_FIFO_CLR are unused, remove them.
--- arch/arm/plat-omap/include/plat/omap-serial.h | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)
arch/arm/plat-omap/include/plat/omap-serial.h +0 -3
@@ -32,9 +32,6 @@*/ #define OMAP_SERIAL_NAME "ttyO" -#define OMAP_UART_SYSC_RESET 0X07 -#define OMAP_UART_FIFO_CLR 0X06 -struct omap_uart_port_info {bool dma_enabled;/* To specify DMA Mode */ unsigned int uartclk;/* UART clock rate */
Signature · 1 line
1.7.4.4
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
In reply to Russell King - ARM LinuxQuoted 78 lines · Russell King - ARM Linux
Hi,
This series of patches fixes multiple flow control issues with the OMAP serial driver, and prepares the driver for DMA engine conversion. We require hardware assisted flow control to work properly for DMA support otherwise we have no way to properly pause the transmitter.
This is generated against v3.6, and has been developed mainly by testing on the OMAP4430 SDP platform.
Flow control seems to be really broken in the OMAP serial driver as things stand today. It just about works with software flow control because the generic serial core layer is inserting those characters, but only when the legacy DMA support is not being used. Otherwise, flow control is completely non-functional.
Issues identified in the OMAP serial driver are:
-set_mctrl()can only assert modem control lines, once asserted it is not possible to deassert them.
- IXOFF controls sending of XON/XOFF characters, not the reception of these sequences.
- IXON controls the recognition of XON/XOFF characters, not the transmission of the same.
- Wrong bitmasks for hardware assisted software flow control. Bit 2 in EFR enables sending of XON2/XOFF2 which are never set.
- No point comparing received characters against XOFF2 ('special character detect') as XOFF2 is not set.
- Fix multiple places where bits 6 and 5 of MCR are attempted to be altered, but because EFR ECB is unset, these bits remain unaffected. This effectively prevents us accessing the right XON/XOFF/TCR/TLR registers.
- Remove unnecessary read-backs of EFR/MCR/LCR registers - these registers don't change beneath us, they are configuration registers which hold their values. Not only does this simplify the code, but it makes it more readable, and more importantly ensures that we work from a consistent state where ->efr never has ECB set, and ->mcr never has the TCRTLR bit set.
- Fix disablement of hardware flow control and IXANY modes; once enabled these could never be disabled because nothing in the code ever clears these configuration bits.Once that lot is fixed, these patches expand
serial_coreto permit hardware assisted flow control by:
- adding throttle/unthrottle callbacks into low level serial drivers, which allow them to take whatever action is necessary with hardware assisted flow control to throttle the remote end. In the case of OMAP serial, this means disabling the RX interrupts so that the FIFO fills to the watermark.We then have a number of cleanups to the OMAP serial code to make the
set_termios()function clearer and less prone to the kinds of mistakes identified above. This results in a great simplification of the flow control configuration code.The OMAP serial driver hacks around with the transmit buffer allocation; lets clean that up so that drivers can cleanly allocate their transmitter buffer using coherent memory if that's what they desire.
Finally, the last few patches clean up the
plat/omap-serial.hheader file, moving most of its contents into the OMAP serial driver itself. Most of this is private to the OMAP serial driver and should never have been shared with anything else.I have omitted to include the conversion of the transmit paths to DMA engine. Even with all the above fixed, it has issues when DMA transmit is in progress, and a program issues a TCSETS call (as
less' does after it has written its prompt.) At the moment, this causes lots of junk to be emitted from the serial port when issuingdmesg | less' which sometimes brings the port to a complete halt.As the OMAP DMA hardware does not have a clean pause when performing a
MEM->DEVtransfer (it discards its FIFO) I do not see a solution to this, which probably means that we can _not_ ever support transmit DMA on OMAP platforms.This means the xmit buffer allocation patches are not that useful unless a solution to that can be found.
Quoted 7 lines
Now, the remaining question is, how much of this patch set do we think about merging, and when. Given that flow control in this driver has been broken for a very long time, and no one has apparantly noticed, I don't think there's any urgency to this, so given its size, my preference would be to queue it up for the next merge window. The thing that would worry me about applying some of the initial patches is that they may change the behaviour today and make any problems here more visible.
I'll add another point to this: serial_omap_restore_context() makes no attempt to restore neither the protected bits in the MCR register nor the contents of the TCR and TLR registers. So, hardware assisted flow control probably won't work at all well after a context loss event. When I work out how to test this, I'll see about cooking up yet another fix to this driver.
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
In reply to Russell King - ARM LinuxAnother potential bug - in serial_omap_set_termios() there is this:
if (up->use_dma) {
serial_out(up, UART_TI752_TLR, 0);
up->scr |= UART_FCR_TRIGGER_4;
} else {
/* Set receive FIFO threshold to 1 byte */
up->fcr &= ~OMAP_UART_FCR_RX_FIFO_TRIG_MASK;
up->fcr |= (0x1 << OMAP_UART_FCR_RX_FIFO_TRIG_SHIFT);
}Is that:
up->scr |= UART_FCR_TRIGGER_4;
line really correct? It looks wrong to be using a FCR register mask with something that ends up in the OMAP SCR register...
I'm beginning to wonder how many of the "workarounds" which have been applied to this driver, particularly those to do with FIFO triggers and idle stuff are actually down to the horribly broken and buggy set_termios code. I'm coming to the conclusion that all those workarounds should be removed, and we start again from a clean slate with a properly written driver which observes the register access rules - thereby ensuring that the correct values get written to the registers that we expect them to.
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
In reply to Russell King - ARM LinuxAnother issue:
serial_omap_set_termios()
{
...
/* FIFOs and DMA Settings */
/* FCR can be changed only when the
* baud clock is not running
* DLL_REG and DLH_REG set to 0.
*/
serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
serial_out(up, UART_DLL, 0);
serial_out(up, UART_DLM, 0);
serial_out(up, UART_LCR, 0);
...
serial_out(up, UART_FCR, up->fcr);...
}
serial_omap_restore_context()
{
serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
serial_out(up, UART_DLL, up->dll);
serial_out(up, UART_DLM, up->dlh);
serial_out(up, UART_LCR, 0x0); /* Operational mode */
serial_out(up, UART_IER, up->ier);
serial_out(up, UART_FCR, up->fcr);
}Either the comment is wrong, or the code in serial_omap_restore_context() is wrong; they can't both be right. Please can someone let me know which is the right version so we can fix that inconsistency.
Thanks.
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
In reply to Russell King - ARM LinuxQuoted 6 lines · On Sat, 6 Oct 2012 13:38:03 +0100 Russell King - ARM Linux…
Hi,
This series of patches fixes multiple flow control issues with the OMAP serial driver, and prepares the driver for DMA engine conversion. We require hardware assisted flow control to work properly for DMA support otherwise we have no way to properly pause the transmitter.
All the generic parts
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
In reply to Russell King - ARM LinuxHi,
Quoted 86 lines · Russell King - ARM Linux
Hi,
This series of patches fixes multiple flow control issues with the OMAP serial driver, and prepares the driver for DMA engine conversion. We require hardware assisted flow control to work properly for DMA support otherwise we have no way to properly pause the transmitter.
This is generated against v3.6, and has been developed mainly by testing on the OMAP4430 SDP platform.
Flow control seems to be really broken in the OMAP serial driver as things stand today. It just about works with software flow control because the generic serial core layer is inserting those characters, but only when the legacy DMA support is not being used. Otherwise, flow control is completely non-functional.
Issues identified in the OMAP serial driver are:
-set_mctrl()can only assert modem control lines, once asserted it is not possible to deassert them.
- IXOFF controls sending of XON/XOFF characters, not the reception of these sequences.
- IXON controls the recognition of XON/XOFF characters, not the transmission of the same.
- Wrong bitmasks for hardware assisted software flow control. Bit 2 in EFR enables sending of XON2/XOFF2 which are never set.
- No point comparing received characters against XOFF2 ('special character detect') as XOFF2 is not set.
- Fix multiple places where bits 6 and 5 of MCR are attempted to be altered, but because EFR ECB is unset, these bits remain unaffected. This effectively prevents us accessing the right XON/XOFF/TCR/TLR registers.
- Remove unnecessary read-backs of EFR/MCR/LCR registers - these registers don't change beneath us, they are configuration registers which hold their values. Not only does this simplify the code, but it makes it more readable, and more importantly ensures that we work from a consistent state where ->efr never has ECB set, and ->mcr never has the TCRTLR bit set.
- Fix disablement of hardware flow control and IXANY modes; once enabled these could never be disabled because nothing in the code ever clears these configuration bits.Once that lot is fixed, these patches expand
serial_coreto permit hardware assisted flow control by:
- adding throttle/unthrottle callbacks into low level serial drivers, which allow them to take whatever action is necessary with hardware assisted flow control to throttle the remote end. In the case of OMAP serial, this means disabling the RX interrupts so that the FIFO fills to the watermark.We then have a number of cleanups to the OMAP serial code to make the
set_termios()function clearer and less prone to the kinds of mistakes identified above. This results in a great simplification of the flow control configuration code.The OMAP serial driver hacks around with the transmit buffer allocation; lets clean that up so that drivers can cleanly allocate their transmitter buffer using coherent memory if that's what they desire.
Finally, the last few patches clean up the
plat/omap-serial.hheader file, moving most of its contents into the OMAP serial driver itself. Most of this is private to the OMAP serial driver and should never have been shared with anything else.I have omitted to include the conversion of the transmit paths to DMA engine. Even with all the above fixed, it has issues when DMA transmit is in progress, and a program issues a TCSETS call (as
less' does after it has written its prompt.) At the moment, this causes lots of junk to be emitted from the serial port when issuingdmesg | less' which sometimes brings the port to a complete halt.As the OMAP DMA hardware does not have a clean pause when performing a
MEM->DEVtransfer (it discards its FIFO) I do not see a solution to this, which probably means that we can _not_ ever support transmit DMA on OMAP platforms.This means the xmit buffer allocation patches are not that useful unless a solution to that can be found.
Now, the remaining question is, how much of this patch set do we think about merging, and when. Given that flow control in this driver has been broken for a very long time, and no one has apparantly noticed, I don't think there's any urgency to this, so given its size, my preference would be to queue it up for the next merge window. The thing that would worry me about applying some of the initial patches is that they may change the behaviour today and make any problems here more visible.
Quoted 4 lines
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Boot Tested this patch series against v3.6 tag(applied cleanly) on panda board and
PM tested(hitting off in Idle and suspend) on omap3630 based beagle board.
Note, I also tested the patches against the current master but only after rebasing, since the current master includes serial patches from Felipe Balbi[1].
[1] https://lkml.org/lkml/2012/8/24/139
Thanks,
Sourav
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
Hi Sourav,
Sourav <[email protected]> writes:
[...]
Quoted 10 lines
Boot Tested this patch series against v3.6 tag(applied cleanly) on panda board and
PM tested(hitting off in Idle and suspend) on omap3630 based beagle board.Note, I also tested the patches against the current master but only after rebasing, since the current master includes serial patches from Felipe Balbi[1].
[1] https://lkml.org/lkml/2012/8/24/139Tested-by Sourav Poddar <[email protected]>
Did you test flow control after off-mode transitons?
Russell indicated that the context save/restore is not saving important bits related to HW flow control, so I suspect some more testing, specifically of flow control after off-mode is needed.
Kevin
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
Hi Kevin,
Quoted 15 lines · Kevin Hilman
Hi Sourav,
Sourav <[email protected]> writes:
[...]
Quoted 9 lines
Boot Tested this patch series against v3.6 tag(applied cleanly) on panda board and
PM tested(hitting off in Idle and suspend) on omap3630 based beagle board.Note, I also tested the patches against the current master but only after rebasing, since the current master includes serial patches from Felipe Balbi[1].
[1] https://lkml.org/lkml/2012/8/24/139
Quoted 8 lines
Quoted 1 line
Tested-by Sourav Poddar <[email protected]>Did you test flow control after off-mode transitons?
Russell indicated that the context save/restore is not saving important bits related to HW flow control, so I suspect some more testing, specifically of flow control after off-mode is needed.
Kevin
The testing done was without any flow control enabled. I will try to figure out how to do a hardware flow control testing and see the status after the off mode.
~Sourav
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
Quoted 19 lines · Sourav
Hi Kevin,
Quoted 16 lines · Kevin Hilman
Hi Sourav,
Sourav <[email protected]> writes:
[...]
Quoted 10 lines
Boot Tested this patch series against v3.6 tag(applied cleanly) on panda board and
PM tested(hitting off in Idle and suspend) on omap3630 based beagle board.Note, I also tested the patches against the current master but only after rebasing, since the current master includes serial patches from Felipe Balbi[1].
[1] https://lkml.org/lkml/2012/8/24/139Tested-by Sourav Poddar <[email protected]>
Quoted 8 lines
Quoted 7 lines
Did you test flow control after off-mode transitons?
Russell indicated that the context save/restore is not saving important bits related to HW flow control, so I suspect some more testing, specifically of flow control after off-mode is needed.
Kevin
The testing done was without any flow control enabled.
So, as the patch set is about fixing the flow control stuff, would you say that your testing without flow control enabled has much value?
Quoted 2 lines
I will try to figure out how to do a hardware flow control testing and see the status after the off mode.
It's software flow control which isn't properly restored...
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
Hi Russell,
Quoted 21 lines · Russell King - ARM Linux
Quoted 20 lines · Sourav
Hi Kevin,
Quoted 18 lines · Kevin Hilman
Hi Sourav,
Sourav <[email protected]> writes:
[...]
Quoted 10 lines
Boot Tested this patch series against v3.6 tag(applied cleanly) on panda board and
PM tested(hitting off in Idle and suspend) on omap3630 based beagle board.Note, I also tested the patches against the current master but only after rebasing, since the current master includes serial patches from Felipe Balbi[1].
[1] https://lkml.org/lkml/2012/8/24/139Tested-by Sourav Poddar <[email protected]>Did you test flow control after off-mode transitons?
Quoted 8 lines
Quoted 6 lines
Quoted 5 lines
Russell indicated that the context save/restore is not saving important bits related to HW flow control, so I suspect some more testing, specifically of flow control after off-mode is needed.
Kevin
The testing done was without any flow control enabled.
So, as the patch set is about fixing the flow control stuff, would you say that your testing without flow control enabled has much value?
True. I missed that point while doing the testing. Sorry for that. I further looked into it and saw some two options in my minicom settings(Hardware Flow Control/ Software Flow Control) Which I am thinking are the ones used to enable the flow control ? and they are both set to NO.
I already enable software flow control and did the testing on beagle, where things are working fine
after off mode.
But if I enable hardware flow control, the teraterm does not allow me to load my fs and uImage from mmc.
If you have any pointers on how to test hardware flow control, I will like to do that on my beagle board.
Thanks,
Sourav
Quoted 3 lines
Quoted 2 lines
I will try to figure out how to do a hardware flow control testing and see the status after the off mode.
It's software flow control which isn't properly restored...
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
Quoted 6 lines · Sourav
True. I missed that point while doing the testing. Sorry for that. I further looked into it and saw some two options in my minicom settings(Hardware Flow Control/ Software Flow Control) Which I am thinking are the ones used to enable the flow control ? and they are both set to NO.
Quoted 7 lines
I already enable software flow control and did the testing on beagle, where things are working fine
after off mode.
But if I enable hardware flow control, the teraterm does not allow me to load my fs and uImage from mmc.
If you have any pointers on how to test hardware flow control, I will like to do that on my beagle board.
Okay, it sounds like I need to do a teach-in on flow control...
First, hardware flow control. Hardware flow control is operated by two
signals: RTS and CTS.
In conventional setups, CTS is an input to the transmitter, and controls whether the transmitter may start the transmission of a new character. If CTS is deasserted, the transmitter will stop after the completion of the previous character. When hardware flow control is disabled, the transmitter ignores this signal.
RTS is an output, and is generally used to control the remote transmitter. (There are setups where RTS means something else, but the kernel doesn't support other schemes directly.) RTS is asserted when either hardware flow control is disabled, or there is sufficient space to receive more characters from the remote end.
This is a symetrical setup, so that two UARTs connected together using this scheme will have the RTS of one connected to the CTS of the other. This way, each can signal whether characters should be transmitted.
So, in minicom, when hardware flow control is disabled, your hosts transmitter will ignore the state of the CTS signal, and will hold its RTS asserted.
If hardware flow control in minicom is enabled, then that tells the kernel (and possibly hardware) to take note of the CTS signal, and pause transmission when CTS is deasserted. It will also cause the RTS signal to be manipulated according to available buffer space on the receive side.
Obviously minicom will try to ensure that any characters received are displayed as quickly as possible, so it's unlikely that the receive side will fill up.
When you're logged into a system via a serial line, the hardware flow control state is controlled by the CRTSCTS termios flag. That can be seen and manipulated by stty. stty -a to see all flags. stty -crtscts to disable, stty crtscts to enable.
Now, for software flow control. It operates in the same way as above, but instead of a hardware signal reporting the state, characters are embedded into the stream.
In normal situations, these characters are the standard ^Q (noramlly XON) and ^S (XOFF) characters. You'll find that works in gnome-terminals, xterms, and many places because it's part of the standard terminal interface. You can type these characters into minicom with or without software flow control disabled; it just passes them through unmodified.
When software flow control is enabled, and the tty receive buffers start to fill up, the kernel will queue a high-priority XOFF character for the UART to transmit to the remote end. Once the tty buffers have emptied sufficiently, it will queue a high-priority XON character. If software flow control is disabled, it will ignore this.
When hardware assisted software flow control is enabled, this will be done by the hardware itself in response to the UART FIFO filling up and emptying.
For the target, software flow control has more configuration options:
ixon: controls whether the transmitter starts/stops on reception
of xon/xoff characters
ixoff: controls the generation of xon/xoff characters
ixany: permits any received character (including xon) to restart
transmission
stop <char>: sets the xoff character to the specified character start <char>: sets the xon character to the specified character
xon and xoff default to ^Q and ^S respectively, there's no need to 'initialize' them prior to use. So, to enable software flow control (which is probably already enabled on the target):
stty ixon ixoff
and then you can type ^S and ^Q into minicom to stop/start the target's transmit output.
Finally, to make the target's input buffer fill up, arrange for the target not to read from the controlling tty at all. sleep 120 will do that for two minutes, after which the input will be gobbled up by the shell (which it'll try to interpret as commands.) So, probably better to do:
sleep 120; echo Finished; cat >/dev/null
instead, and then send lots of data, and check whether the transmission stops, whether the right xon/xoff characters are transmitted, and whether any overrun errors are reported.
Going the other way, you can suspend minicom (^a z) and then arrange for the target to send lots of data, and again check what happens.
There's a gotcha there though: with standard 8250-based serial ports, we have /proc/tty/driver/serial which gives easy access to the port statistics. With USB stuff, those statistics are not available, so it becomes much harder to test. You have to arrange for the target to send a known pattern, and find some way to check at the host end that it was correctly received, including over the flow control events.
No characters should be lost when flow control is being used; after all, that's the whole point of the facility.
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
In reply to SouravHi Sourav,
On 10/11/2012 05:21 AM, Sourav wrote:
[snip]
Quoted 7 lines
I already enable software flow control and did the testing on beagle, where things are working fine
after off mode.
But if I enable hardware flow control, the teraterm does not allow me to load my fs and uImage from mmc.
If you have any pointers on how to test hardware flow control, I will like to do that on my beagle board.
For h/w flow control, you need to check if the CTS and RTS signals are being brought out to the serial header on the beagle-board. Looking at the schematics for the 3430 beagle board, it appears that only the UART RX and TX signals are available on the serial header. Therefore, I don't believe you can test h/w flow control using the console with that board.
I do see that you can access all the UART2 signals (RX/TX/CTS/RTS) via the expansion connector on the beagle. However, to connect to a serial port on a PC you need to have a voltage level-shifter in-between. There are some companies out there that make them [1], but you need to make sure you have one that can support a 1.8V input.
Cheers
Jon
cover OMAP serial driver flow control fixes, and preparation for DMA engine conversion
In reply to SouravQuoted 2 lines · Sourav
If you have any pointers on how to test hardware flow control, I will like to do that on my beagle board.
If you can find some board with BT chip connected to UART, that would be a good test as they usually have flow control lines connected and can run at high bitrates. I think some EVM or Zoom boards have BT chips.
Signature · 1 line
Gra?vydas