Skip to content

Commit

Permalink
serial: Deassert Transmit Enable on probe in driver-specific way
Browse files Browse the repository at this point in the history
commit 7c7f9bc upstream.

When a UART port is newly registered, uart_configure_port() seeks to
deassert RS485 Transmit Enable by setting the RTS bit in port->mctrl.
However a number of UART drivers interpret a set RTS bit as *assertion*
instead of deassertion:  Affected drivers include those using
serial8250_em485_config() (except 8250_bcm2835aux.c) and some using
mctrl_gpio (e.g. imx.c).

Since the interpretation of the RTS bit is driver-specific, it is not
suitable as a means to centrally deassert Transmit Enable in the serial
core.  Instead, the serial core must call on drivers to deassert it in
their driver-specific way.  One way to achieve that is to call
->rs485_config().  It implicitly deasserts Transmit Enable.

So amend uart_configure_port() and uart_resume_port() to invoke
uart_rs485_config().  That allows removing calls to uart_rs485_config()
from drivers' ->probe() hooks and declaring the function static.

Skip any invocation of ->set_mctrl() if RS485 is enabled.  RS485 has no
hardware flow control, so the modem control lines are irrelevant and
need not be touched.  When leaving RS485 mode, reset the modem control
lines to the state stored in port->mctrl.  That way, UARTs which are
muxed between RS485 and RS232 transceivers drive the lines correctly
when switched to RS232.  (serial8250_do_startup() historically raises
the OUT1 modem signal because otherwise interrupts are not signaled on
ancient PC UARTs, but I believe that no longer applies to modern,
RS485-capable UARTs and is thus safe to be skipped.)

imx.c modifies port->mctrl whenever Transmit Enable is asserted and
deasserted.  Stop it from doing that so port->mctrl reflects the RS232
line state.

8250_omap.c deasserts Transmit Enable on ->runtime_resume() by calling
->set_mctrl().  Because that is now a no-op in RS485 mode, amend the
function to call serial8250_em485_stop_tx().

fsl_lpuart.c retrieves and applies the RS485 device tree properties
after registering the UART port.  Because applying now happens on
registration in uart_configure_port(), move retrieval of the properties
ahead of uart_add_one_port().

Link: https://lore.kernel.org/all/20220329085050.311408-1-matthias.schiffer@ew.tq-group.com/
Link: https://lore.kernel.org/all/8f538a8903795f22f9acc94a9a31b03c9c4ccacb.camel@ginzinger.com/
Fixes: d3b3404 ("serial: Fix incorrect rs485 polarity on uart open")
Cc: stable@vger.kernel.org # v4.14+
Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Reported-by: Roosen Henri <Henri.Roosen@ginzinger.com>
Tested-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Link: https://lore.kernel.org/r/2de36eba3fbe11278d5002e4e501afe0ceaca039.1663863805.git.lukas@wunner.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
l1k authored and gregkh committed Oct 21, 2022
1 parent 21763d1 commit 1f00783
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 42 deletions.
3 changes: 3 additions & 0 deletions drivers/tty/serial/8250/8250_omap.c
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,9 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
omap8250_update_mdr1(up, priv);

up->port.ops->set_mctrl(&up->port, up->port.mctrl);

if (up->port.rs485.flags & SER_RS485_ENABLED)
serial8250_em485_stop_tx(up);
}

/*
Expand Down
9 changes: 1 addition & 8 deletions drivers/tty/serial/8250/8250_pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -1627,7 +1627,6 @@ static int pci_fintek_init(struct pci_dev *dev)
resource_size_t bar_data[3];
u8 config_base;
struct serial_private *priv = pci_get_drvdata(dev);
struct uart_8250_port *port;

if (!(pci_resource_flags(dev, 5) & IORESOURCE_IO) ||
!(pci_resource_flags(dev, 4) & IORESOURCE_IO) ||
Expand Down Expand Up @@ -1674,13 +1673,7 @@ static int pci_fintek_init(struct pci_dev *dev)

pci_write_config_byte(dev, config_base + 0x06, dev->irq);

if (priv) {
/* re-apply RS232/485 mode when
* pciserial_resume_ports()
*/
port = serial8250_get_port(priv->line[i]);
uart_rs485_config(&port->port);
} else {
if (!priv) {
/* First init without port data
* force init to RS232 Mode
*/
Expand Down
12 changes: 7 additions & 5 deletions drivers/tty/serial/8250/8250_port.c
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
static int serial8250_em485_init(struct uart_8250_port *p)
{
if (p->em485)
return 0;
goto deassert_rts;

p->em485 = kmalloc(sizeof(struct uart_8250_em485), GFP_ATOMIC);
if (!p->em485)
Expand All @@ -616,7 +616,9 @@ static int serial8250_em485_init(struct uart_8250_port *p)
p->em485->active_timer = NULL;
p->em485->tx_stopped = true;

p->rs485_stop_tx(p);
deassert_rts:
if (p->em485->tx_stopped)
p->rs485_stop_tx(p);

return 0;
}
Expand Down Expand Up @@ -2042,6 +2044,9 @@ EXPORT_SYMBOL_GPL(serial8250_do_set_mctrl);

static void serial8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
if (port->rs485.flags & SER_RS485_ENABLED)
return;

if (port->set_mctrl)
port->set_mctrl(port, mctrl);
else
Expand Down Expand Up @@ -3187,9 +3192,6 @@ static void serial8250_config_port(struct uart_port *port, int flags)
if (flags & UART_CONFIG_TYPE)
autoconfig(up);

if (port->rs485.flags & SER_RS485_ENABLED)
uart_rs485_config(port);

/* if access method is AU, it is a 16550 with a quirk */
if (port->type == PORT_16550A && port->iotype == UPIO_AU)
up->bugs |= UART_BUG_NOMSR;
Expand Down
10 changes: 4 additions & 6 deletions drivers/tty/serial/fsl_lpuart.c
Original file line number Diff line number Diff line change
Expand Up @@ -2729,15 +2729,13 @@ static int lpuart_probe(struct platform_device *pdev)
if (ret)
goto failed_reset;

ret = uart_add_one_port(&lpuart_reg, &sport->port);
if (ret)
goto failed_attach_port;

ret = uart_get_rs485_mode(&sport->port);
if (ret)
goto failed_get_rs485;

uart_rs485_config(&sport->port);
ret = uart_add_one_port(&lpuart_reg, &sport->port);
if (ret)
goto failed_attach_port;

ret = devm_request_irq(&pdev->dev, sport->port.irq, handler, 0,
DRIVER_NAME, sport);
Expand All @@ -2747,9 +2745,9 @@ static int lpuart_probe(struct platform_device *pdev)
return 0;

failed_irq_request:
failed_get_rs485:
uart_remove_one_port(&lpuart_reg, &sport->port);
failed_attach_port:
failed_get_rs485:
failed_reset:
lpuart_disable_clks(sport);
return ret;
Expand Down
8 changes: 2 additions & 6 deletions drivers/tty/serial/imx.c
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,7 @@ static void imx_uart_rts_active(struct imx_port *sport, u32 *ucr2)
{
*ucr2 &= ~(UCR2_CTSC | UCR2_CTS);

sport->port.mctrl |= TIOCM_RTS;
mctrl_gpio_set(sport->gpios, sport->port.mctrl);
mctrl_gpio_set(sport->gpios, sport->port.mctrl | TIOCM_RTS);
}

/* called with port.lock taken and irqs caller dependent */
Expand All @@ -390,8 +389,7 @@ static void imx_uart_rts_inactive(struct imx_port *sport, u32 *ucr2)
*ucr2 &= ~UCR2_CTSC;
*ucr2 |= UCR2_CTS;

sport->port.mctrl &= ~TIOCM_RTS;
mctrl_gpio_set(sport->gpios, sport->port.mctrl);
mctrl_gpio_set(sport->gpios, sport->port.mctrl & ~TIOCM_RTS);
}

static void start_hrtimer_ms(struct hrtimer *hrt, unsigned long msec)
Expand Down Expand Up @@ -2347,8 +2345,6 @@ static int imx_uart_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"low-active RTS not possible when receiver is off, enabling receiver\n");

uart_rs485_config(&sport->port);

/* Disable interrupts before requesting them */
ucr1 = imx_uart_readl(sport, UCR1);
ucr1 &= ~(UCR1_ADEN | UCR1_TRDYEN | UCR1_IDEN | UCR1_RRDYEN | UCR1_RTSDEN);
Expand Down
36 changes: 20 additions & 16 deletions drivers/tty/serial/serial_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,10 @@ uart_update_mctrl(struct uart_port *port, unsigned int set, unsigned int clear)
unsigned long flags;
unsigned int old;

if (port->rs485.flags & SER_RS485_ENABLED) {
set &= ~TIOCM_RTS;
clear &= ~TIOCM_RTS;
}

spin_lock_irqsave(&port->lock, flags);
old = port->mctrl;
port->mctrl = (old & ~clear) | set;
if (old != port->mctrl)
if (old != port->mctrl && !(port->rs485.flags & SER_RS485_ENABLED))
port->ops->set_mctrl(port, port->mctrl);
spin_unlock_irqrestore(&port->lock, flags);
}
Expand Down Expand Up @@ -1391,7 +1386,7 @@ static void uart_set_rs485_termination(struct uart_port *port,
!!(rs485->flags & SER_RS485_TERMINATE_BUS));
}

int uart_rs485_config(struct uart_port *port)
static int uart_rs485_config(struct uart_port *port)
{
struct serial_rs485 *rs485 = &port->rs485;
int ret;
Expand All @@ -1405,7 +1400,6 @@ int uart_rs485_config(struct uart_port *port)

return ret;
}
EXPORT_SYMBOL_GPL(uart_rs485_config);

static int uart_get_rs485_config(struct uart_port *port,
struct serial_rs485 __user *rs485)
Expand Down Expand Up @@ -1444,8 +1438,13 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,

spin_lock_irqsave(&port->lock, flags);
ret = port->rs485_config(port, &tty->termios, &rs485);
if (!ret)
if (!ret) {
port->rs485 = rs485;

/* Reset RTS and other mctrl lines when disabling RS485 */
if (!(rs485.flags & SER_RS485_ENABLED))
port->ops->set_mctrl(port, port->mctrl);
}
spin_unlock_irqrestore(&port->lock, flags);
if (ret)
return ret;
Expand Down Expand Up @@ -2352,7 +2351,8 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)

spin_lock_irq(&uport->lock);
ops->stop_tx(uport);
ops->set_mctrl(uport, 0);
if (!(uport->rs485.flags & SER_RS485_ENABLED))
ops->set_mctrl(uport, 0);
/* save mctrl so it can be restored on resume */
mctrl = uport->mctrl;
uport->mctrl = 0;
Expand Down Expand Up @@ -2440,7 +2440,8 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)

uart_change_pm(state, UART_PM_STATE_ON);
spin_lock_irq(&uport->lock);
ops->set_mctrl(uport, 0);
if (!(uport->rs485.flags & SER_RS485_ENABLED))
ops->set_mctrl(uport, 0);
spin_unlock_irq(&uport->lock);
if (console_suspend_enabled || !uart_console(uport)) {
/* Protected by port mutex for now */
Expand All @@ -2451,7 +2452,10 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
if (tty)
uart_change_speed(tty, state, NULL);
spin_lock_irq(&uport->lock);
ops->set_mctrl(uport, uport->mctrl);
if (!(uport->rs485.flags & SER_RS485_ENABLED))
ops->set_mctrl(uport, uport->mctrl);
else
uart_rs485_config(uport);
ops->start_tx(uport);
spin_unlock_irq(&uport->lock);
tty_port_set_initialized(port, 1);
Expand Down Expand Up @@ -2558,10 +2562,10 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
*/
spin_lock_irqsave(&port->lock, flags);
port->mctrl &= TIOCM_DTR;
if (port->rs485.flags & SER_RS485_ENABLED &&
!(port->rs485.flags & SER_RS485_RTS_AFTER_SEND))
port->mctrl |= TIOCM_RTS;
port->ops->set_mctrl(port, port->mctrl);
if (!(port->rs485.flags & SER_RS485_ENABLED))
port->ops->set_mctrl(port, port->mctrl);
else
uart_rs485_config(port);
spin_unlock_irqrestore(&port->lock, flags);

/*
Expand Down
1 change: 0 additions & 1 deletion include/linux/serial_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -950,5 +950,4 @@ static inline int uart_handle_break(struct uart_port *port)
!((cflag) & CLOCAL))

int uart_get_rs485_mode(struct uart_port *port);
int uart_rs485_config(struct uart_port *port);
#endif /* LINUX_SERIAL_CORE_H */

0 comments on commit 1f00783

Please sign in to comment.