2021-09-22 08:02:57

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 1/7] mxser: restore baud rate if its setting fails

If a user tries to set a too high rate, it fails due to check in
mxser_set_baud(). But the high rate remains set in termios, so the user
might think everything went smooth. Restore the baud rate from the
old_termios if this happens, so that user knows nothing was changed in
fact.

It used to behave the correct way many years ago, but somehow the
restoration vanished with commit 1c45607ad3eb (Char: mxser, remove it)
-- the commit removed mxser's older clone.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/mxser.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 1216f3985e18..b9cc41782ce1 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -559,14 +559,20 @@ static void mxser_handle_cts(struct tty_struct *tty, struct mxser_port *info,
* This routine is called to set the UART divisor registers to match
* the specified baud rate for a serial port.
*/
-static void mxser_change_speed(struct tty_struct *tty)
+static void mxser_change_speed(struct tty_struct *tty, struct ktermios *old_termios)
{
struct mxser_port *info = tty->driver_data;
unsigned cflag, cval, fcr;

cflag = tty->termios.c_cflag;

- mxser_set_baud(tty, tty_get_baud_rate(tty));
+ if (mxser_set_baud(tty, tty_get_baud_rate(tty))) {
+ /* Use previous rate on a failure */
+ if (old_termios) {
+ speed_t baud = tty_termios_baud_rate(old_termios);
+ tty_encode_baud_rate(tty, baud, baud);
+ }
+ }

/* byte size and parity */
switch (cflag & CSIZE) {
@@ -791,7 +797,7 @@ static int mxser_activate(struct tty_port *port, struct tty_struct *tty)
/*
* and set the speed of the serial port
*/
- mxser_change_speed(tty);
+ mxser_change_speed(tty, NULL);
spin_unlock_irqrestore(&info->slock, flags);

return 0;
@@ -1119,7 +1125,7 @@ static int mxser_set_serial_info(struct tty_struct *tty,
if (tty_port_initialized(port)) {
if (old_speed != (port->flags & ASYNC_SPD_MASK)) {
spin_lock_irqsave(&info->slock, sl_flags);
- mxser_change_speed(tty);
+ mxser_change_speed(tty, NULL);
spin_unlock_irqrestore(&info->slock, sl_flags);
}
} else {
@@ -1425,7 +1431,7 @@ static void mxser_set_termios(struct tty_struct *tty, struct ktermios *old_termi
unsigned long flags;

spin_lock_irqsave(&info->slock, flags);
- mxser_change_speed(tty);
+ mxser_change_speed(tty, old_termios);
spin_unlock_irqrestore(&info->slock, flags);

if ((old_termios->c_cflag & CRTSCTS) && !C_CRTSCTS(tty)) {
--
2.33.0


2021-09-22 08:02:57

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 4/7] mxser: simplify FCR computation in mxser_change_speed()

Provided FIFO is always enabled for MUST chips, move its FCR setting out
of PORT_8250/PORT_16450 special case in mxser_change_speed(). Now, we
can pre-set fcr to zero and invert the condition of the 'if'.

This makes the code more readable (no functional change intended).

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/mxser.c | 47 +++++++++++++++++++--------------------------
1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 309acf3f1ec3..c194a96bb14e 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -600,33 +600,26 @@ static void mxser_change_speed(struct tty_struct *tty, struct ktermios *old_term
if (cflag & CMSPAR)
cval |= UART_LCR_SPAR;

- if ((info->type == PORT_8250) || (info->type == PORT_16450)) {
- if (info->board->must_hwid) {
- fcr = UART_FCR_ENABLE_FIFO;
- fcr |= MOXA_MUST_FCR_GDA_MODE_ENABLE;
- mxser_set_must_fifo_value(info);
- } else
- fcr = 0;
- } else {
- fcr = UART_FCR_ENABLE_FIFO;
- if (info->board->must_hwid) {
- fcr |= MOXA_MUST_FCR_GDA_MODE_ENABLE;
- mxser_set_must_fifo_value(info);
- } else {
- switch (info->rx_high_water) {
- case 1:
- fcr |= UART_FCR_TRIGGER_1;
- break;
- case 4:
- fcr |= UART_FCR_TRIGGER_4;
- break;
- case 8:
- fcr |= UART_FCR_TRIGGER_8;
- break;
- default:
- fcr |= UART_FCR_TRIGGER_14;
- break;
- }
+ fcr = 0;
+ if (info->board->must_hwid) {
+ fcr |= UART_FCR_ENABLE_FIFO |
+ MOXA_MUST_FCR_GDA_MODE_ENABLE;
+ mxser_set_must_fifo_value(info);
+ } else if (info->type != PORT_8250 && info->type != PORT_16450) {
+ fcr |= UART_FCR_ENABLE_FIFO;
+ switch (info->rx_high_water) {
+ case 1:
+ fcr |= UART_FCR_TRIGGER_1;
+ break;
+ case 4:
+ fcr |= UART_FCR_TRIGGER_4;
+ break;
+ case 8:
+ fcr |= UART_FCR_TRIGGER_8;
+ break;
+ default:
+ fcr |= UART_FCR_TRIGGER_14;
+ break;
}
}

--
2.33.0

2021-09-22 08:03:26

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 7/7] mxser: store FCR state in mxser_port::FCR

We force the FCR contents on many places in the code instead of writing
what was actually set in mxser_change_speed() (by ->activate() or
->set_serial_info()).

So introduce mxser_port::FCR to hold the proper contents and bitwise-OR
the value to what needs to be set on all those locations. That is,
clearing RX and/or TX FIFOs. Those flags are self-clearing, so no need
to set them to mxser_port::FCR.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/mxser.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 4ce21cdb1ea5..93a95a135a71 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -249,6 +249,7 @@ struct mxser_port {
unsigned char x_char; /* xon/xoff character */
u8 IER; /* Interrupt Enable Register */
u8 MCR; /* Modem control register */
+ u8 FCR; /* FIFO control register */

bool ldisc_stop_rx;

@@ -562,7 +563,7 @@ static void mxser_handle_cts(struct tty_struct *tty, struct mxser_port *info,
static void mxser_change_speed(struct tty_struct *tty, struct ktermios *old_termios)
{
struct mxser_port *info = tty->driver_data;
- unsigned cflag, cval, fcr;
+ unsigned cflag, cval;

cflag = tty->termios.c_cflag;

@@ -600,25 +601,25 @@ static void mxser_change_speed(struct tty_struct *tty, struct ktermios *old_term
if (cflag & CMSPAR)
cval |= UART_LCR_SPAR;

- fcr = 0;
+ info->FCR = 0;
if (info->board->must_hwid) {
- fcr |= UART_FCR_ENABLE_FIFO |
+ info->FCR |= UART_FCR_ENABLE_FIFO |
MOXA_MUST_FCR_GDA_MODE_ENABLE;
mxser_set_must_fifo_value(info);
} else if (info->type != PORT_8250 && info->type != PORT_16450) {
- fcr |= UART_FCR_ENABLE_FIFO;
+ info->FCR |= UART_FCR_ENABLE_FIFO;
switch (info->rx_high_water) {
case 1:
- fcr |= UART_FCR_TRIGGER_1;
+ info->FCR |= UART_FCR_TRIGGER_1;
break;
case 4:
- fcr |= UART_FCR_TRIGGER_4;
+ info->FCR |= UART_FCR_TRIGGER_4;
break;
case 8:
- fcr |= UART_FCR_TRIGGER_8;
+ info->FCR |= UART_FCR_TRIGGER_8;
break;
default:
- fcr |= UART_FCR_TRIGGER_14;
+ info->FCR |= UART_FCR_TRIGGER_14;
break;
}
}
@@ -679,7 +680,7 @@ static void mxser_change_speed(struct tty_struct *tty, struct ktermios *old_term
}


- outb(fcr, info->ioaddr + UART_FCR); /* set fcr */
+ outb(info->FCR, info->ioaddr + UART_FCR);
outb(cval, info->ioaddr + UART_LCR);
}

@@ -865,7 +866,7 @@ static void mxser_flush_buffer(struct tty_struct *tty)
spin_lock_irqsave(&info->slock, flags);
info->xmit_cnt = info->xmit_head = info->xmit_tail = 0;

- outb(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
+ outb(info->FCR | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
info->ioaddr + UART_FCR);

spin_unlock_irqrestore(&info->slock, flags);
@@ -1572,8 +1573,7 @@ static u8 mxser_receive_chars_old(struct tty_struct *tty,

ch = inb(port->ioaddr + UART_RX);
if (hwid && (status & UART_LSR_OE))
- outb(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
- MOXA_MUST_FCR_GDA_MODE_ENABLE,
+ outb(port->FCR | UART_FCR_CLEAR_RCVR,
port->ioaddr + UART_FCR);
status &= port->read_status_mask;
if (status & port->ignore_status_mask) {
@@ -1685,8 +1685,7 @@ static bool mxser_port_isr(struct mxser_port *port)
tty = tty_port_tty_get(&port->port);
if (!tty || port->closing || !tty_port_initialized(&port->port)) {
status = inb(port->ioaddr + UART_LSR);
- outb(MOXA_MUST_FCR_GDA_MODE_ENABLE | UART_FCR_ENABLE_FIFO |
- UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
+ outb(port->FCR | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
port->ioaddr + UART_FCR);
inb(port->ioaddr + UART_MSR);

--
2.33.0

2021-09-22 08:03:26

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 6/7] mxser: don't read from UART_FCR

The UART_FCR register is write-only. When reading it, one gets contents
of (read-only) UART_IIR instead as they are shared. This mistake was
performed in mxser_flush_buffer() to clear FIFOs.

Actually FCR handling throughout the driver is completely broken. On
many places, it respects neither mu860 settings, nor FIFO (16450 vs
16550) setting. This patch doesn't help to fix this, it actually does
the same. We will introduce a mxser_port::FCR in the next patch to fix
this issue.

Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/mxser.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 6913a1caa6f0..4ce21cdb1ea5 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -859,17 +859,14 @@ static int mxser_open(struct tty_struct *tty, struct file *filp)
static void mxser_flush_buffer(struct tty_struct *tty)
{
struct mxser_port *info = tty->driver_data;
- char fcr;
unsigned long flags;


spin_lock_irqsave(&info->slock, flags);
info->xmit_cnt = info->xmit_head = info->xmit_tail = 0;

- fcr = inb(info->ioaddr + UART_FCR);
- outb((fcr | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT),
+ outb(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
info->ioaddr + UART_FCR);
- outb(fcr, info->ioaddr + UART_FCR);

spin_unlock_irqrestore(&info->slock, flags);

--
2.33.0