Give fsl_lpuart some love and add break, loopback and sysrq support. While
at it, some errors were noticed, which are also fixed in this series.
The sysrq support was tested on both interrupt driven and DMA based
transfers on the 32bit LPUART.
Changes since v1:
- rephrased commit message of
serial: fsl_lpuart: don't restore interrupt state in ISR
Sorry again, I didn't want to take credit of that sentence.
- rephrased commit message and split into two patches:
serial: fsl_lpuart: handle break and make sysrq work
Esp. drop the mention of a possible deadlock as it turns out
it isn't one.
- remove the spin_trylock_irqsave() special case for sysrq handling
Michael Walle (9):
serial: fsl_lpuart: don't modify arbitrary data on lpuart32
serial: fsl_lpuart: use UARTDATA_MASK macro
serial: fsl_lpuart: don't restore interrupt state in ISR
serial: fsl_lpuart: split sysrq handling
serial: fsl_lpuart: handle break and make sysrq work
serial: fsl_lpuart: remove RTSCTS handling from get_mctrl()
serial: fsl_lpuart: remove manual RTSCTS control from 8-bit LPUART
serial: fsl_lpuart: add loopback support
serial: fsl_lpuart: disable DMA for console and fix sysrq
drivers/tty/serial/fsl_lpuart.c | 132 +++++++++++++++++---------------
1 file changed, 72 insertions(+), 60 deletions(-)
--
2.20.1
Use the corresponding macro instead of the magic number. While at it,
drop the useless cast to "unsigned char".
Signed-off-by: Michael Walle <[email protected]>
---
drivers/tty/serial/fsl_lpuart.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index fbf2e4d2d22b..b76ddc0d8edc 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -928,9 +928,9 @@ static void lpuart32_rxint(struct lpuart_port *sport)
*/
sr = lpuart32_read(&sport->port, UARTSTAT);
rx = lpuart32_read(&sport->port, UARTDATA);
- rx &= 0x3ff;
+ rx &= UARTDATA_MASK;
- if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx))
+ if (uart_handle_sysrq_char(&sport->port, rx))
continue;
if (sr & (UARTSTAT_PE | UARTSTAT_OR | UARTSTAT_FE)) {
--
2.20.1
From commit 75f4e830fa9c ("serial: do not restore interrupt state in
sysrq helper"):
Since commit 81e2073c175b ("genirq: Disable interrupts for force
threaded handlers") interrupt handlers that are not explicitly
requested as threaded are always called with interrupts disabled and
there is no need to save the interrupt state when taking the port
lock.
Apply this also to fsl_lpuart in prepartion for sysrq handling with
uart_unlock_and_check_sysrq().
Signed-off-by: Michael Walle <[email protected]>
---
drivers/tty/serial/fsl_lpuart.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index b76ddc0d8edc..37e02d992c0b 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -824,21 +824,18 @@ static unsigned int lpuart32_tx_empty(struct uart_port *port)
static void lpuart_txint(struct lpuart_port *sport)
{
- unsigned long flags;
-
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
lpuart_transmit_buffer(sport);
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
}
static void lpuart_rxint(struct lpuart_port *sport)
{
unsigned int flg, ignored = 0, overrun = 0;
struct tty_port *port = &sport->port.state->port;
- unsigned long flags;
unsigned char rx, sr;
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
while (!(readb(sport->port.membase + UARTSFIFO) & UARTSFIFO_RXEMPT)) {
flg = TTY_NORMAL;
@@ -896,28 +893,25 @@ static void lpuart_rxint(struct lpuart_port *sport)
writeb(UARTSFIFO_RXOF, sport->port.membase + UARTSFIFO);
}
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
tty_flip_buffer_push(port);
}
static void lpuart32_txint(struct lpuart_port *sport)
{
- unsigned long flags;
-
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
lpuart32_transmit_buffer(sport);
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
}
static void lpuart32_rxint(struct lpuart_port *sport)
{
unsigned int flg, ignored = 0;
struct tty_port *port = &sport->port.state->port;
- unsigned long flags;
unsigned long rx, sr;
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
while (!(lpuart32_read(&sport->port, UARTFIFO) & UARTFIFO_RXEMPT)) {
flg = TTY_NORMAL;
@@ -965,7 +959,7 @@ static void lpuart32_rxint(struct lpuart_port *sport)
}
out:
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
tty_flip_buffer_push(port);
}
--
2.20.1
Instead of uart_handle_sysrq_char() use uart_prepare_sysrq_char() and
uart_unlock_and_check_sysrq(). This will call handle_sysrq() without
holding the port lock, which in turn let us drop the spin_trylock hack.
Suggested-by: Johan Hovold <[email protected]>
Signed-off-by: Michael Walle <[email protected]>
---
drivers/tty/serial/fsl_lpuart.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 37e02d992c0b..63a1dac7c3aa 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -847,7 +847,7 @@ static void lpuart_rxint(struct lpuart_port *sport)
sr = readb(sport->port.membase + UARTSR1);
rx = readb(sport->port.membase + UARTDR);
- if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx))
+ if (uart_prepare_sysrq_char(&sport->port, rx))
continue;
if (sr & (UARTSR1_PE | UARTSR1_OR | UARTSR1_FE)) {
@@ -893,7 +893,7 @@ static void lpuart_rxint(struct lpuart_port *sport)
writeb(UARTSFIFO_RXOF, sport->port.membase + UARTSFIFO);
}
- spin_unlock(&sport->port.lock);
+ uart_unlock_and_check_sysrq(&sport->port);
tty_flip_buffer_push(port);
}
@@ -924,7 +924,7 @@ static void lpuart32_rxint(struct lpuart_port *sport)
rx = lpuart32_read(&sport->port, UARTDATA);
rx &= UARTDATA_MASK;
- if (uart_handle_sysrq_char(&sport->port, rx))
+ if (uart_prepare_sysrq_char(&sport->port, rx))
continue;
if (sr & (UARTSTAT_PE | UARTSTAT_OR | UARTSTAT_FE)) {
@@ -959,7 +959,7 @@ static void lpuart32_rxint(struct lpuart_port *sport)
}
out:
- spin_unlock(&sport->port.lock);
+ uart_unlock_and_check_sysrq(&sport->port);
tty_flip_buffer_push(port);
}
@@ -2272,7 +2272,7 @@ lpuart_console_write(struct console *co, const char *s, unsigned int count)
unsigned long flags;
int locked = 1;
- if (sport->port.sysrq || oops_in_progress)
+ if (oops_in_progress)
locked = spin_trylock_irqsave(&sport->port.lock, flags);
else
spin_lock_irqsave(&sport->port.lock, flags);
@@ -2302,7 +2302,7 @@ lpuart32_console_write(struct console *co, const char *s, unsigned int count)
unsigned long flags;
int locked = 1;
- if (sport->port.sysrq || oops_in_progress)
+ if (oops_in_progress)
locked = spin_trylock_irqsave(&sport->port.lock, flags);
else
spin_lock_irqsave(&sport->port.lock, flags);
--
2.20.1
The LPUART doesn't have the ability to control the RTS or CTS line
manually. Instead it will set it automatically when data is send or
handle it when data is received. Thus drop the wrong code in set_mctrl.
For the 32 bit version this was already done in the commit 2b30efe2e88a
("tty: serial: lpuart: Remove unnecessary code from set_mctrl"). Keep
the 8-bit version in sync and remove it there, too.
Signed-off-by: Michael Walle <[email protected]>
---
drivers/tty/serial/fsl_lpuart.c | 28 +---------------------------
1 file changed, 1 insertion(+), 27 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 5b32d48bee56..ad1fe8cf63f2 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1403,17 +1403,7 @@ static int lpuart32_config_rs485(struct uart_port *port,
static unsigned int lpuart_get_mctrl(struct uart_port *port)
{
- unsigned int temp = 0;
- unsigned char reg;
-
- reg = readb(port->membase + UARTMODEM);
- if (reg & UARTMODEM_TXCTSE)
- temp |= TIOCM_CTS;
-
- if (reg & UARTMODEM_RXRTSE)
- temp |= TIOCM_RTS;
-
- return temp;
+ return 0;
}
static unsigned int lpuart32_get_mctrl(struct uart_port *port)
@@ -1423,23 +1413,7 @@ static unsigned int lpuart32_get_mctrl(struct uart_port *port)
static void lpuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
- unsigned char temp;
- struct lpuart_port *sport = container_of(port,
- struct lpuart_port, port);
-
- /* Make sure RXRTSE bit is not set when RS485 is enabled */
- if (!(sport->port.rs485.flags & SER_RS485_ENABLED)) {
- temp = readb(sport->port.membase + UARTMODEM) &
- ~(UARTMODEM_RXRTSE | UARTMODEM_TXCTSE);
-
- if (mctrl & TIOCM_RTS)
- temp |= UARTMODEM_RXRTSE;
- if (mctrl & TIOCM_CTS)
- temp |= UARTMODEM_TXCTSE;
-
- writeb(temp, port->membase + UARTMODEM);
- }
}
static void lpuart32_set_mctrl(struct uart_port *port, unsigned int mctrl)
--
2.20.1
SYSRQ doesn't work with DMA. This is because there is no error
indication whether a symbol had a framing error or not. Actually,
this is not completely correct, there is a bit in the data register
which is set in this case, but we'd have to read change the DMA access
to 16 bit and we'd need to post process the data, thus make the DMA
pointless in the first place.
Signed-off-by: Michael Walle <[email protected]>
---
Please note, that there is already sysrq/break support in the 8 bit
version. But I think there is a race between the hardware DMA controller
and the ISR in this driver. I'm not sure though and can't test it.
Angelo, maybe you could test it, I'd presume with this patch you don't need
the special handling in the ISR anymore.
drivers/tty/serial/fsl_lpuart.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 1ed019d91177..0185bb8b6000 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1587,6 +1587,9 @@ static void lpuart_tx_dma_startup(struct lpuart_port *sport)
u32 uartbaud;
int ret;
+ if (uart_console(&sport->port))
+ goto err;
+
if (!sport->dma_tx_chan)
goto err;
@@ -1616,6 +1619,9 @@ static void lpuart_rx_dma_startup(struct lpuart_port *sport)
int ret;
unsigned char cr3;
+ if (uart_console(&sport->port))
+ goto err;
+
if (!sport->dma_rx_chan)
goto err;
--
2.20.1
The LPUART can loop the RX and TX signal. Add support for it.
Please note, this was only tested on the 32 bit version of the LPUART.
Signed-off-by: Michael Walle <[email protected]>
---
drivers/tty/serial/fsl_lpuart.c | 36 +++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index ad1fe8cf63f2..1ed019d91177 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1403,22 +1403,54 @@ static int lpuart32_config_rs485(struct uart_port *port,
static unsigned int lpuart_get_mctrl(struct uart_port *port)
{
- return 0;
+ unsigned int mctrl = 0;
+ u8 reg;
+
+ reg = readb(port->membase + UARTCR1);
+ if (reg & UARTCR1_LOOPS)
+ mctrl |= TIOCM_LOOP;
+
+ return mctrl;
}
static unsigned int lpuart32_get_mctrl(struct uart_port *port)
{
- return 0;
+ unsigned int mctrl = 0;
+ u32 reg;
+
+ reg = lpuart32_read(port, UARTCTRL);
+ if (reg & UARTCTRL_LOOPS)
+ mctrl |= TIOCM_LOOP;
+
+ return mctrl;
}
static void lpuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
+ u8 reg;
+
+ reg = readb(port->membase + UARTCR1);
+
+ /* for internal loopback we need LOOPS=1 and RSRC=0 */
+ reg &= ~(UARTCR1_LOOPS | UARTCR1_RSRC);
+ if (mctrl & TIOCM_LOOP)
+ reg |= UARTCR1_LOOPS;
+ writeb(reg, port->membase + UARTCR1);
}
static void lpuart32_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
+ u32 reg;
+
+ reg = lpuart32_read(port, UARTCTRL);
+
+ /* for internal loopback we need LOOPS=1 and RSRC=0 */
+ reg &= ~(UARTCTRL_LOOPS | UARTCTRL_RSRC);
+ if (mctrl & TIOCM_LOOP)
+ reg |= UARTCTRL_LOOPS;
+ lpuart32_write(port, reg, UARTCTRL);
}
static void lpuart_break_ctl(struct uart_port *port, int break_state)
--
2.20.1
Although there is already sysrq characters handling, a break condition
was never detected. Add support for it.
The LPUART can't distinguish between a framing error and a break
condition. We assume it is a break if the received data is all zero.
Signed-off-by: Michael Walle <[email protected]>
---
drivers/tty/serial/fsl_lpuart.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 63a1dac7c3aa..c76bdb855ba9 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -910,6 +910,7 @@ static void lpuart32_rxint(struct lpuart_port *sport)
unsigned int flg, ignored = 0;
struct tty_port *port = &sport->port.state->port;
unsigned long rx, sr;
+ bool is_break;
spin_lock(&sport->port.lock);
@@ -924,14 +925,27 @@ static void lpuart32_rxint(struct lpuart_port *sport)
rx = lpuart32_read(&sport->port, UARTDATA);
rx &= UARTDATA_MASK;
+ /*
+ * The LPUART can't distinguish between a break and a framing error,
+ * thus we assume it is a break if the received data is zero.
+ */
+ is_break = (sr & UARTSTAT_FE) && !rx;
+
+ if (is_break && uart_handle_break(&sport->port))
+ continue;
+
if (uart_prepare_sysrq_char(&sport->port, rx))
continue;
if (sr & (UARTSTAT_PE | UARTSTAT_OR | UARTSTAT_FE)) {
- if (sr & UARTSTAT_PE)
- sport->port.icount.parity++;
- else if (sr & UARTSTAT_FE)
+ if (sr & UARTSTAT_PE) {
+ if (is_break)
+ sport->port.icount.brk++;
+ else
+ sport->port.icount.parity++;
+ } else if (sr & UARTSTAT_FE) {
sport->port.icount.frame++;
+ }
if (sr & UARTSTAT_OR)
sport->port.icount.overrun++;
@@ -944,15 +958,17 @@ static void lpuart32_rxint(struct lpuart_port *sport)
sr &= sport->port.read_status_mask;
- if (sr & UARTSTAT_PE)
- flg = TTY_PARITY;
- else if (sr & UARTSTAT_FE)
+ if (sr & UARTSTAT_PE) {
+ if (is_break)
+ flg = TTY_BREAK;
+ else
+ flg = TTY_PARITY;
+ } else if (sr & UARTSTAT_FE) {
flg = TTY_FRAME;
+ }
if (sr & UARTSTAT_OR)
flg = TTY_OVERRUN;
-
- sport->port.sysrq = 0;
}
tty_insert_flip_char(port, rx, flg);
--
2.20.1
The wrong code in set_mctrl() was already removed in commit 2b30efe2e88a
("tty: serial: lpuart: Remove unnecessary code from set_mctrl"), but the
code in get_mctrl() wasn't removed. It will not return the state of the
RTS or CTS line but whether automatic flow control is enabled, which is
wrong for the get_mctrl(). Thus remove it.
Fixes: 2b30efe2e88a ("tty: serial: lpuart: Remove unnecessary code from set_mctrl")
Signed-off-by: Michael Walle <[email protected]>
---
drivers/tty/serial/fsl_lpuart.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index c76bdb855ba9..5b32d48bee56 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1418,17 +1418,7 @@ static unsigned int lpuart_get_mctrl(struct uart_port *port)
static unsigned int lpuart32_get_mctrl(struct uart_port *port)
{
- unsigned int temp = 0;
- unsigned long reg;
-
- reg = lpuart32_read(port, UARTMODIR);
- if (reg & UARTMODIR_TXCTSE)
- temp |= TIOCM_CTS;
-
- if (reg & UARTMODIR_RXRTSE)
- temp |= TIOCM_RTS;
-
- return temp;
+ return 0;
}
static void lpuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
--
2.20.1