From: Lino Sanfilippo LinoSanfilippo@gmx.de
Several drivers that support setting the RS485 configuration via userspace implement one or more of the following tasks:
- in case of an invalid RTS configuration (both RTS after send and RTS on send set or both unset) fall back to enable RTS on send and disable RTS after send
- nullify the padding field of the returned serial_rs485 struct
- copy the configuration into the uart port struct
- limit RTS delays to 100 ms
Move these tasks into the serial core to make them generic and to provide a consistent behaviour among all drivers.
Signed-off-by: Lino Sanfilippo LinoSanfilippo@gmx.de Link: https://lore.kernel.org/r/20220410104642.32195-2-LinoSanfilippo@gmx.de Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [ Upstream commit 0ed12afa5655512ee418047fb3546d229df20aa1 ] Signed-off-by: Daisuke Mizobuchi mizo@atmark-techno.com Signed-off-by: Dominique Martinet dominique.martinet@atmark-techno.com --- Follow-up of https://lkml.kernel.org/r/20221017013807.34614-1-dominique.martinet@atmark-t...
v2: identical to v1
drivers/tty/serial/serial_core.c | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index b578f7090b63..6cc909d44a81 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -42,6 +42,11 @@ static struct lock_class_key port_lock_key;
#define HIGH_BITS_OFFSET ((sizeof(long)-sizeof(int))*8)
+/* + * Max time with active RTS before/after data is sent. + */ +#define RS485_MAX_RTS_DELAY 100 /* msecs */ + static void uart_change_speed(struct tty_struct *tty, struct uart_state *state, struct ktermios *old_termios); static void uart_wait_until_sent(struct tty_struct *tty, int timeout); @@ -1326,8 +1331,36 @@ static int uart_set_rs485_config(struct uart_port *port, if (copy_from_user(&rs485, rs485_user, sizeof(*rs485_user))) return -EFAULT;
+ /* pick sane settings if the user hasn't */ + if (!(rs485.flags & SER_RS485_RTS_ON_SEND) == + !(rs485.flags & SER_RS485_RTS_AFTER_SEND)) { + dev_warn_ratelimited(port->dev, + "%s (%d): invalid RTS setting, using RTS_ON_SEND instead\n", + port->name, port->line); + rs485.flags |= SER_RS485_RTS_ON_SEND; + rs485.flags &= ~SER_RS485_RTS_AFTER_SEND; + } + + if (rs485.delay_rts_before_send > RS485_MAX_RTS_DELAY) { + rs485.delay_rts_before_send = RS485_MAX_RTS_DELAY; + dev_warn_ratelimited(port->dev, + "%s (%d): RTS delay before sending clamped to %u ms\n", + port->name, port->line, rs485.delay_rts_before_send); + } + + if (rs485.delay_rts_after_send > RS485_MAX_RTS_DELAY) { + rs485.delay_rts_after_send = RS485_MAX_RTS_DELAY; + dev_warn_ratelimited(port->dev, + "%s (%d): RTS delay after sending clamped to %u ms\n", + port->name, port->line, rs485.delay_rts_after_send); + } + /* Return clean padding area to userspace */ + memset(rs485.padding, 0, sizeof(rs485.padding)); + spin_lock_irqsave(&port->lock, flags); ret = port->rs485_config(port, &rs485); + if (!ret) + port->rs485 = rs485; spin_unlock_irqrestore(&port->lock, flags); if (ret) return ret;
From: Lukas Wunner lukas@wunner.de
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-... Link: https://lore.kernel.org/all/8f538a8903795f22f9acc94a9a31b03c9c4ccacb.camel@g... Fixes: d3b3404df318 ("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.166386380... Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [ Upstream commit 7c7f9bc986e698873b489c371a08f206979d06b7 ] Signed-off-by: Daisuke Mizobuchi mizo@atmark-techno.com Signed-off-by: Dominique Martinet dominique.martinet@atmark-techno.com --- Follow-up of https://lkml.kernel.org/r/20221017013807.34614-1-dominique.martinet@atmark-t...
v1 -> v2: remove port.rs485_config() in fsl lpuart driver
drivers/tty/serial/8250/8250_omap.c | 3 +++ drivers/tty/serial/8250/8250_pci.c | 9 +-------- drivers/tty/serial/8250/8250_port.c | 12 +++++++----- drivers/tty/serial/fsl_lpuart.c | 8 +++----- drivers/tty/serial/imx.c | 8 ++------ drivers/tty/serial/serial_core.c | 30 ++++++++++++++++------------- 6 files changed, 33 insertions(+), 37 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 537bee8d2258..f3744ac805ec 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -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); }
/* diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c index df10cc606582..b6656898699d 100644 --- a/drivers/tty/serial/8250/8250_pci.c +++ b/drivers/tty/serial/8250/8250_pci.c @@ -1531,7 +1531,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) || @@ -1578,13 +1577,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]); - pci_fintek_rs485_config(&port->port, NULL); - } else { + if (!priv) { /* First init without port data * force init to RS232 Mode */ diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 9d60418e4adb..e6cadb23850a 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -592,7 +592,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) @@ -608,7 +608,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; } @@ -2030,6 +2032,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 @@ -3159,9 +3164,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) - port->rs485_config(port, &port->rs485); - /* 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; diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index a2c4eab0b470..698e7b5e1f05 100644 --- a/drivers/tty/serial/fsl_lpuart.c +++ b/drivers/tty/serial/fsl_lpuart.c @@ -2667,10 +2667,6 @@ static int lpuart_probe(struct platform_device *pdev) if (ret) goto failed_irq_request;
- 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; @@ -2682,7 +2678,9 @@ static int lpuart_probe(struct platform_device *pdev) sport->port.rs485.delay_rts_after_send) dev_err(&pdev->dev, "driver doesn't support RTS delays\n");
- sport->port.rs485_config(&sport->port, &sport->port.rs485); + ret = uart_add_one_port(&lpuart_reg, &sport->port); + if (ret) + goto failed_attach_port;
return 0;
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index bfbca711bbf9..cf3d53165776 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -398,8 +398,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 */ @@ -408,8 +407,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) @@ -2381,8 +2379,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");
- imx_uart_rs485_config(&sport->port, &sport->port.rs485); - /* Disable interrupts before requesting them */ ucr1 = imx_uart_readl(sport, UCR1); ucr1 &= ~(UCR1_ADEN | UCR1_TRDYEN | UCR1_IDEN | UCR1_RRDYEN | UCR1_RTSDEN); diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 6cc909d44a81..605f928f0636 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -149,15 +149,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); } @@ -1359,8 +1354,13 @@ static int uart_set_rs485_config(struct uart_port *port,
spin_lock_irqsave(&port->lock, flags); ret = port->rs485_config(port, &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; @@ -2335,7 +2335,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 */ @@ -2346,7 +2347,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 + uport->rs485_config(uport, &uport->rs485); ops->start_tx(uport); spin_unlock_irq(&uport->lock); tty_port_set_initialized(port, 1); @@ -2444,10 +2448,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 + port->rs485_config(port, &port->rs485); spin_unlock_irqrestore(&port->lock, flags);
/*
On Mon, Oct 17, 2022 at 02:17:36PM +0900, Dominique Martinet wrote:
From: Lino Sanfilippo LinoSanfilippo@gmx.de
Several drivers that support setting the RS485 configuration via userspace implement one or more of the following tasks:
in case of an invalid RTS configuration (both RTS after send and RTS on send set or both unset) fall back to enable RTS on send and disable RTS after send
nullify the padding field of the returned serial_rs485 struct
copy the configuration into the uart port struct
limit RTS delays to 100 ms
Move these tasks into the serial core to make them generic and to provide a consistent behaviour among all drivers.
Signed-off-by: Lino Sanfilippo LinoSanfilippo@gmx.de Link: https://lore.kernel.org/r/20220410104642.32195-2-LinoSanfilippo@gmx.de Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [ Upstream commit 0ed12afa5655512ee418047fb3546d229df20aa1 ] Signed-off-by: Daisuke Mizobuchi mizo@atmark-techno.com Signed-off-by: Dominique Martinet dominique.martinet@atmark-techno.com
Follow-up of https://lkml.kernel.org/r/20221017013807.34614-1-dominique.martinet@atmark-t...
I need a 5.15.y version of this series before I can take the 5.10.y version.
thanks,
greg k-h
Greg Kroah-Hartman wrote on Wed, Oct 26, 2022 at 06:54:11PM +0200:
On Mon, Oct 17, 2022 at 02:17:36PM +0900, Dominique Martinet wrote:
From: Lino Sanfilippo LinoSanfilippo@gmx.de
Several drivers that support setting the RS485 configuration via userspace implement one or more of the following tasks:
in case of an invalid RTS configuration (both RTS after send and RTS on send set or both unset) fall back to enable RTS on send and disable RTS after send
nullify the padding field of the returned serial_rs485 struct
copy the configuration into the uart port struct
limit RTS delays to 100 ms
Move these tasks into the serial core to make them generic and to provide a consistent behaviour among all drivers.
Signed-off-by: Lino Sanfilippo LinoSanfilippo@gmx.de Link: https://lore.kernel.org/r/20220410104642.32195-2-LinoSanfilippo@gmx.de Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [ Upstream commit 0ed12afa5655512ee418047fb3546d229df20aa1 ] Signed-off-by: Daisuke Mizobuchi mizo@atmark-techno.com Signed-off-by: Dominique Martinet dominique.martinet@atmark-techno.com
Follow-up of https://lkml.kernel.org/r/20221017013807.34614-1-dominique.martinet@atmark-t...
I need a 5.15.y version of this series before I can take the 5.10.y version.
Thanks for the probing, I did not know about this rule (but it makes sense); I've just sent a 5.15 version: https://lkml.kernel.org/r/20221027001943.637449-1-dominique.martinet@atmark-...
I'd really appreciate if Lino could take a look and confirm we didn't botch this too much -- we've tested the 5.10 version and it looks ok, but this is different enough from the original patch to warrant a check from the author.
Thanks!
On Thu, Oct 27, 2022 at 09:21:41AM +0900, Dominique Martinet wrote:
Greg Kroah-Hartman wrote on Wed, Oct 26, 2022 at 06:54:11PM +0200:
On Mon, Oct 17, 2022 at 02:17:36PM +0900, Dominique Martinet wrote:
From: Lino Sanfilippo LinoSanfilippo@gmx.de
Several drivers that support setting the RS485 configuration via userspace implement one or more of the following tasks:
in case of an invalid RTS configuration (both RTS after send and RTS on send set or both unset) fall back to enable RTS on send and disable RTS after send
nullify the padding field of the returned serial_rs485 struct
copy the configuration into the uart port struct
limit RTS delays to 100 ms
Move these tasks into the serial core to make them generic and to provide a consistent behaviour among all drivers.
Signed-off-by: Lino Sanfilippo LinoSanfilippo@gmx.de Link: https://lore.kernel.org/r/20220410104642.32195-2-LinoSanfilippo@gmx.de Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [ Upstream commit 0ed12afa5655512ee418047fb3546d229df20aa1 ] Signed-off-by: Daisuke Mizobuchi mizo@atmark-techno.com Signed-off-by: Dominique Martinet dominique.martinet@atmark-techno.com
Follow-up of https://lkml.kernel.org/r/20221017013807.34614-1-dominique.martinet@atmark-t...
I need a 5.15.y version of this series before I can take the 5.10.y version.
Thanks for the probing, I did not know about this rule (but it makes sense); I've just sent a 5.15 version: https://lkml.kernel.org/r/20221027001943.637449-1-dominique.martinet@atmark-...
I'd really appreciate if Lino could take a look and confirm we didn't botch this too much -- we've tested the 5.10 version and it looks ok, but this is different enough from the original patch to warrant a check from the author.
Ok, I'll wait for an ACK from Lino for both of these series before taking them.
thanks,
greg k-h
On 27.10.22 02:21, Dominique Martinet wrote:
Greg Kroah-Hartman wrote on Wed, Oct 26, 2022 at 06:54:11PM +0200:
On Mon, Oct 17, 2022 at 02:17:36PM +0900, Dominique Martinet wrote:
From: Lino Sanfilippo LinoSanfilippo@gmx.de
Several drivers that support setting the RS485 configuration via userspace implement one or more of the following tasks:
in case of an invalid RTS configuration (both RTS after send and RTS on send set or both unset) fall back to enable RTS on send and disable RTS after send
nullify the padding field of the returned serial_rs485 struct
copy the configuration into the uart port struct
limit RTS delays to 100 ms
Move these tasks into the serial core to make them generic and to provide a consistent behaviour among all drivers.
Signed-off-by: Lino Sanfilippo LinoSanfilippo@gmx.de Link: https://lore.kernel.org/r/20220410104642.32195-2-LinoSanfilippo@gmx.de Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [ Upstream commit 0ed12afa5655512ee418047fb3546d229df20aa1 ] Signed-off-by: Daisuke Mizobuchi mizo@atmark-techno.com Signed-off-by: Dominique Martinet dominique.martinet@atmark-techno.com
Follow-up of https://lkml.kernel.org/r/20221017013807.34614-1-dominique.martinet@atmark-t...
I need a 5.15.y version of this series before I can take the 5.10.y version.
Thanks for the probing, I did not know about this rule (but it makes sense); I've just sent a 5.15 version: https://lkml.kernel.org/r/20221027001943.637449-1-dominique.martinet@atmark-...
I'd really appreciate if Lino could take a look and confirm we didn't botch this too much -- we've tested the 5.10 version and it looks ok, but this is different enough from the original patch to warrant a check from the author.
Concerning the part I authored (patch 1) I do not see any changes, so
Acked-by: Lino Sanfilippo LinoSanfilippo@gmx.de
However the part Lukas authored (patch 2) seems to be the one that has been adjusted a lot, so maybe you rather/also want to have his ack?
Best Regards, Lino
Lino Sanfilippo wrote on Fri, Oct 28, 2022 at 10:06:51AM +0200:
I'd really appreciate if Lino could take a look and confirm we didn't botch this too much -- we've tested the 5.10 version and it looks ok, but this is different enough from the original patch to warrant a check from the author.
However the part Lukas authored (patch 2) seems to be the one that has been adjusted a lot, so maybe you rather/also want to have his ack?
Gah, sorry -- I'm juggling with too many things at once here and got the two patches mixed up.
Yes, I meant Lukas here for patch 2/2, sorry. Promoted him to To. And thank you Lino for having a look and pointing it out!
linux-stable-mirror@lists.linaro.org
-
Dominique Martinet -
Greg Kroah-Hartman -
Lino Sanfilippo