* [PATCH v2 0/4] rs485 bus termination GPIO
@ 2020-05-12 12:40 Lukas Wunner
2020-05-12 12:40 ` [PATCH v2 1/4] serial: 8250: Avoid error message on reprobe Lukas Wunner
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Lukas Wunner @ 2020-05-12 12:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring
Cc: Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner,
Andy Shevchenko, Christoph Muellner, linux-serial, devicetree
Define a device tree binding for an rs485 bus termination GPIO
(patch [3/4]), amend the serial core to retrieve the GPIO from
the device tree and amend the default ->rs485_config() callback
for 8250 drivers to change the GPIO on request from user space
(patch [4/4]).
Retrieving the GPIO from the device tree may fail, so allow
uart_get_rs485_mode() to return an errno and change all callers
to check for failure (patch [2/4]).
Fix a bug in the 8250 core if retrieval of the GPIO initially
fails with -EPROBE_DEFER and is later retried (patch [1/4]).
Changes v1 -> v2:
Patch [1/4]:
* Drop unnecessary "else" after "if ... goto" statement. (Andy)
* Document applicability to older kernels in commit message. (Andy)
* Add Fixes tag. (Andy)
Patch [4/4]:
* Drop unnecessary devm_gpiod_put(). (Andy)
* Use GPIOD_OUT_LOW macro for brevity. (Andy)
* Document the rationale for disabling termination by default.
* Drop nonsensical read of GPIO after setting its direction to out.
Link to v1:
https://lore.kernel.org/linux-serial/cover.1588505407.git.lukas@wunner.de/
Lukas Wunner (4):
serial: 8250: Avoid error message on reprobe
serial: Allow uart_get_rs485_mode() to return errno
dt-bindings: serial: Add binding for rs485 bus termination GPIO
serial: 8250: Support rs485 bus termination GPIO
.../devicetree/bindings/serial/rs485.yaml | 4 ++++
drivers/tty/serial/8250/8250_core.c | 18 ++++++++++-----
drivers/tty/serial/8250/8250_port.c | 4 ++++
drivers/tty/serial/ar933x_uart.c | 6 +++--
drivers/tty/serial/atmel_serial.c | 6 +++--
drivers/tty/serial/fsl_lpuart.c | 5 ++++-
drivers/tty/serial/imx.c | 6 ++++-
drivers/tty/serial/omap-serial.c | 4 +++-
drivers/tty/serial/serial_core.c | 22 ++++++++++++++++++-
drivers/tty/serial/stm32-usart.c | 8 +++----
include/linux/serial_core.h | 4 +++-
11 files changed, 69 insertions(+), 18 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2 1/4] serial: 8250: Avoid error message on reprobe 2020-05-12 12:40 [PATCH v2 0/4] rs485 bus termination GPIO Lukas Wunner @ 2020-05-12 12:40 ` Lukas Wunner 2020-05-12 13:45 ` Andy Shevchenko 2020-05-12 12:40 ` [PATCH v2 2/4] serial: Allow uart_get_rs485_mode() to return errno Lukas Wunner ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Lukas Wunner @ 2020-05-12 12:40 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner, Andy Shevchenko, Christoph Muellner, linux-serial If the call to uart_add_one_port() in serial8250_register_8250_port() fails, a half-initialized entry in the serial_8250ports[] array is left behind. A subsequent reprobe of the same serial port causes that entry to be reused. Because uart->port.dev is set, uart_remove_one_port() is called for the half-initialized entry and bails out with an error message: bcm2835-aux-uart 3f215040.serial: Removing wrong port: (null) != (ptrval) The same happens on failure of mctrl_gpio_init() since commit 4a96895f74c9 ("tty/serial/8250: use mctrl_gpio helpers"). Fix by zeroing the uart->port.dev pointer in the probe error path. The bug was introduced in v2.6.10 by historical commit befff6f5bf5f ("[SERIAL] Add new port registration/unregistration functions."): https://git.kernel.org/tglx/history/c/befff6f5bf5f The commit added an unconditional call to uart_remove_one_port() in serial8250_register_port(). In v3.7, commit 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe") made that call conditional on uart->port.dev which allows me to fix the issue by zeroing that pointer in the error path. Thus, the present commit will fix the problem as far back as v3.7 whereas still older versions need to also cherry-pick 835d844d1a28. Fixes: 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe") Signed-off-by: Lukas Wunner <lukas@wunner.de> Cc: stable@vger.kernel.org # v2.6.10 Cc: stable@vger.kernel.org # v2.6.10: 835d844d1a28: 8250_pnp: do pnp probe before legacy --- Changes v1 -> v2: * Drop unnecessary "else" after "if ... goto" statement. (Andy) * Document applicability to older kernels in commit message. (Andy) * Add Fixes tag. (Andy) drivers/tty/serial/8250/8250_core.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 45d9117cab68..9548d3f8fc8e 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -1040,7 +1040,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up) gpios = mctrl_gpio_init(&uart->port, 0); if (IS_ERR(gpios)) { ret = PTR_ERR(gpios); - goto out_unlock; + goto err; } else { uart->gpios = gpios; } @@ -1089,8 +1089,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up) serial8250_apply_quirks(uart); ret = uart_add_one_port(&serial8250_reg, &uart->port); - if (ret == 0) - ret = uart->port.line; + if (ret) + goto err; + + ret = uart->port.line; } else { dev_info(uart->port.dev, "skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n", @@ -1112,10 +1114,14 @@ int serial8250_register_8250_port(struct uart_8250_port *up) } } -out_unlock: mutex_unlock(&serial_mutex); return ret; + +err: + uart->port.dev = NULL; + mutex_unlock(&serial_mutex); + return ret; } EXPORT_SYMBOL(serial8250_register_8250_port); -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/4] serial: 8250: Avoid error message on reprobe 2020-05-12 12:40 ` [PATCH v2 1/4] serial: 8250: Avoid error message on reprobe Lukas Wunner @ 2020-05-12 13:45 ` Andy Shevchenko 0 siblings, 0 replies; 8+ messages in thread From: Andy Shevchenko @ 2020-05-12 13:45 UTC (permalink / raw) To: Lukas Wunner Cc: Greg Kroah-Hartman, Jiri Slaby, Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner, Christoph Muellner, linux-serial On Tue, May 12, 2020 at 02:40:01PM +0200, Lukas Wunner wrote: > If the call to uart_add_one_port() in serial8250_register_8250_port() > fails, a half-initialized entry in the serial_8250ports[] array is left > behind. > > A subsequent reprobe of the same serial port causes that entry to be > reused. Because uart->port.dev is set, uart_remove_one_port() is called > for the half-initialized entry and bails out with an error message: > > bcm2835-aux-uart 3f215040.serial: Removing wrong port: (null) != (ptrval) > > The same happens on failure of mctrl_gpio_init() since commit > 4a96895f74c9 ("tty/serial/8250: use mctrl_gpio helpers"). > > Fix by zeroing the uart->port.dev pointer in the probe error path. > > The bug was introduced in v2.6.10 by historical commit befff6f5bf5f > ("[SERIAL] Add new port registration/unregistration functions."): > https://git.kernel.org/tglx/history/c/befff6f5bf5f > > The commit added an unconditional call to uart_remove_one_port() in > serial8250_register_port(). In v3.7, commit 835d844d1a28 ("8250_pnp: > do pnp probe before legacy probe") made that call conditional on > uart->port.dev which allows me to fix the issue by zeroing that pointer > in the error path. Thus, the present commit will fix the problem as far > back as v3.7 whereas still older versions need to also cherry-pick > 835d844d1a28. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Fixes: 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe") > Signed-off-by: Lukas Wunner <lukas@wunner.de> > Cc: stable@vger.kernel.org # v2.6.10 > Cc: stable@vger.kernel.org # v2.6.10: 835d844d1a28: 8250_pnp: do pnp probe before legacy > --- > Changes v1 -> v2: > * Drop unnecessary "else" after "if ... goto" statement. (Andy) > * Document applicability to older kernels in commit message. (Andy) > * Add Fixes tag. (Andy) > > drivers/tty/serial/8250/8250_core.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c > index 45d9117cab68..9548d3f8fc8e 100644 > --- a/drivers/tty/serial/8250/8250_core.c > +++ b/drivers/tty/serial/8250/8250_core.c > @@ -1040,7 +1040,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up) > gpios = mctrl_gpio_init(&uart->port, 0); > if (IS_ERR(gpios)) { > ret = PTR_ERR(gpios); > - goto out_unlock; > + goto err; > } else { > uart->gpios = gpios; > } > @@ -1089,8 +1089,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up) > serial8250_apply_quirks(uart); > ret = uart_add_one_port(&serial8250_reg, > &uart->port); > - if (ret == 0) > - ret = uart->port.line; > + if (ret) > + goto err; > + > + ret = uart->port.line; > } else { > dev_info(uart->port.dev, > "skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n", > @@ -1112,10 +1114,14 @@ int serial8250_register_8250_port(struct uart_8250_port *up) > } > } > > -out_unlock: > mutex_unlock(&serial_mutex); > > return ret; > + > +err: > + uart->port.dev = NULL; > + mutex_unlock(&serial_mutex); > + return ret; > } > EXPORT_SYMBOL(serial8250_register_8250_port); > > -- > 2.26.2 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/4] serial: Allow uart_get_rs485_mode() to return errno 2020-05-12 12:40 [PATCH v2 0/4] rs485 bus termination GPIO Lukas Wunner 2020-05-12 12:40 ` [PATCH v2 1/4] serial: 8250: Avoid error message on reprobe Lukas Wunner @ 2020-05-12 12:40 ` Lukas Wunner 2020-05-12 13:46 ` Andy Shevchenko 2020-05-12 12:40 ` [PATCH v2 3/4] dt-bindings: serial: Add binding for rs485 bus termination GPIO Lukas Wunner 2020-05-12 12:40 ` [PATCH v2 4/4] serial: 8250: Support " Lukas Wunner 3 siblings, 1 reply; 8+ messages in thread From: Lukas Wunner @ 2020-05-12 12:40 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner, Andy Shevchenko, Christoph Muellner, linux-serial We're about to amend uart_get_rs485_mode() to support a GPIO pin for rs485 bus termination. Retrieving the GPIO descriptor may fail, so allow uart_get_rs485_mode() to return an errno and change all callers to check for failure. The GPIO descriptor is going to be stored in struct uart_port. Pass that struct to uart_get_rs485_mode() in lieu of a struct device and struct serial_rs485, both of which are directly accessible from struct uart_port. A few drivers call uart_get_rs485_mode() before setting the struct device pointer in struct uart_port. Shuffle those calls around where necessary. [Heiko Stuebner did the ar933x_uart.c portion, hence his Signed-off-by.] Signed-off-by: Heiko Stuebner <heiko@sntech.de> Signed-off-by: Lukas Wunner <lukas@wunner.de> --- drivers/tty/serial/8250/8250_core.c | 4 +++- drivers/tty/serial/ar933x_uart.c | 6 ++++-- drivers/tty/serial/atmel_serial.c | 6 ++++-- drivers/tty/serial/fsl_lpuart.c | 5 ++++- drivers/tty/serial/imx.c | 6 +++++- drivers/tty/serial/omap-serial.c | 4 +++- drivers/tty/serial/serial_core.c | 6 +++++- drivers/tty/serial/stm32-usart.c | 8 ++++---- include/linux/serial_core.h | 2 +- 9 files changed, 33 insertions(+), 14 deletions(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 9548d3f8fc8e..fc118f649887 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -1026,7 +1026,9 @@ int serial8250_register_8250_port(struct uart_8250_port *up) if (up->port.dev) { uart->port.dev = up->port.dev; - uart_get_rs485_mode(uart->port.dev, &uart->port.rs485); + ret = uart_get_rs485_mode(&uart->port); + if (ret) + goto err; } if (up->port.flags & UPF_FIXED_TYPE) diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c index 7e7f1398019f..0c80a79d7442 100644 --- a/drivers/tty/serial/ar933x_uart.c +++ b/drivers/tty/serial/ar933x_uart.c @@ -766,8 +766,6 @@ static int ar933x_uart_probe(struct platform_device *pdev) goto err_disable_clk; } - uart_get_rs485_mode(&pdev->dev, &port->rs485); - port->mapbase = mem_res->start; port->line = id; port->irq = irq_res->start; @@ -786,6 +784,10 @@ static int ar933x_uart_probe(struct platform_device *pdev) baud = ar933x_uart_get_baud(port->uartclk, 0, AR933X_UART_MAX_STEP); up->max_baud = min_t(unsigned int, baud, AR933X_UART_MAX_BAUD); + ret = uart_get_rs485_mode(port); + if (ret) + goto err_disable_clk; + up->gpios = mctrl_gpio_init(port, 0); if (IS_ERR(up->gpios) && PTR_ERR(up->gpios) != -ENOSYS) return PTR_ERR(up->gpios); diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c index 8d7080efad9b..e43471b33710 100644 --- a/drivers/tty/serial/atmel_serial.c +++ b/drivers/tty/serial/atmel_serial.c @@ -2491,8 +2491,6 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port, atmel_init_property(atmel_port, pdev); atmel_set_ops(port); - uart_get_rs485_mode(&mpdev->dev, &port->rs485); - port->iotype = UPIO_MEM; port->flags = UPF_BOOT_AUTOCONF | UPF_IOREMAP; port->ops = &atmel_pops; @@ -2506,6 +2504,10 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port, memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring)); + ret = uart_get_rs485_mode(port); + if (ret) + return ret; + /* for console, the clock could already be configured */ if (!atmel_port->clk) { atmel_port->clk = clk_get(&mpdev->dev, "usart"); diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index 6a9909e56449..029324c77cd7 100644 --- a/drivers/tty/serial/fsl_lpuart.c +++ b/drivers/tty/serial/fsl_lpuart.c @@ -2619,7 +2619,9 @@ static int lpuart_probe(struct platform_device *pdev) if (ret) goto failed_attach_port; - uart_get_rs485_mode(&pdev->dev, &sport->port.rs485); + ret = uart_get_rs485_mode(&sport->port); + if (ret) + goto failed_get_rs485; if (sport->port.rs485.flags & SER_RS485_RX_DURING_TX) dev_err(&pdev->dev, "driver doesn't support RX during TX\n"); @@ -2632,6 +2634,7 @@ static int lpuart_probe(struct platform_device *pdev) return 0; +failed_get_rs485: failed_attach_port: failed_irq_request: lpuart_disable_clks(sport); diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index f4d68109bc8b..91f3910d6c44 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -2302,7 +2302,11 @@ static int imx_uart_probe(struct platform_device *pdev) sport->ucr4 = readl(sport->port.membase + UCR4); sport->ufcr = readl(sport->port.membase + UFCR); - uart_get_rs485_mode(&pdev->dev, &sport->port.rs485); + ret = uart_get_rs485_mode(&sport->port); + if (ret) { + clk_disable_unprepare(sport->clk_ipg); + return ret; + } if (sport->port.rs485.flags & SER_RS485_ENABLED && (!sport->have_rtscts && !sport->have_rtsgpio)) diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index c71c1a2266dc..8573fc9cb0cd 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -1608,7 +1608,9 @@ static int serial_omap_probe_rs485(struct uart_omap_port *up, if (!np) return 0; - uart_get_rs485_mode(up->dev, rs485conf); + ret = uart_get_rs485_mode(&up->port); + if (ret) + return ret; if (of_property_read_bool(np, "rs485-rts-active-high")) { rs485conf->flags |= SER_RS485_RTS_ON_SEND; diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 66a5e2faf57e..43b6682877d5 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -3295,8 +3295,10 @@ EXPORT_SYMBOL(uart_remove_one_port); * This function implements the device tree binding described in * Documentation/devicetree/bindings/serial/rs485.txt. */ -void uart_get_rs485_mode(struct device *dev, struct serial_rs485 *rs485conf) +int uart_get_rs485_mode(struct uart_port *port) { + struct serial_rs485 *rs485conf = &port->rs485; + struct device *dev = port->dev; u32 rs485_delay[2]; int ret; @@ -3328,6 +3330,8 @@ void uart_get_rs485_mode(struct device *dev, struct serial_rs485 *rs485conf) rs485conf->flags &= ~SER_RS485_RTS_ON_SEND; rs485conf->flags |= SER_RS485_RTS_AFTER_SEND; } + + return 0; } EXPORT_SYMBOL_GPL(uart_get_rs485_mode); diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c index 5e93e8d40f59..e3db54398159 100644 --- a/drivers/tty/serial/stm32-usart.c +++ b/drivers/tty/serial/stm32-usart.c @@ -158,9 +158,7 @@ static int stm32_init_rs485(struct uart_port *port, if (!pdev->dev.of_node) return -ENODEV; - uart_get_rs485_mode(&pdev->dev, rs485conf); - - return 0; + return uart_get_rs485_mode(port); } static int stm32_pending_rx(struct uart_port *port, u32 *sr, int *last_res, @@ -931,7 +929,9 @@ static int stm32_init_port(struct stm32_port *stm32port, port->rs485_config = stm32_config_rs485; - stm32_init_rs485(port, pdev); + ret = stm32_init_rs485(port, pdev); + if (ret) + return ret; if (stm32port->info->cfg.has_wakeup) { stm32port->wakeirq = platform_get_irq(pdev, 1); diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index 92f5eba86052..b649a2b894e7 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -472,5 +472,5 @@ extern int uart_handle_break(struct uart_port *port); (cflag) & CRTSCTS || \ !((cflag) & CLOCAL)) -void uart_get_rs485_mode(struct device *dev, struct serial_rs485 *rs485conf); +int uart_get_rs485_mode(struct uart_port *port); #endif /* LINUX_SERIAL_CORE_H */ -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/4] serial: Allow uart_get_rs485_mode() to return errno 2020-05-12 12:40 ` [PATCH v2 2/4] serial: Allow uart_get_rs485_mode() to return errno Lukas Wunner @ 2020-05-12 13:46 ` Andy Shevchenko 0 siblings, 0 replies; 8+ messages in thread From: Andy Shevchenko @ 2020-05-12 13:46 UTC (permalink / raw) To: Lukas Wunner Cc: Greg Kroah-Hartman, Jiri Slaby, Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner, Christoph Muellner, linux-serial On Tue, May 12, 2020 at 02:40:02PM +0200, Lukas Wunner wrote: > We're about to amend uart_get_rs485_mode() to support a GPIO pin for > rs485 bus termination. Retrieving the GPIO descriptor may fail, so > allow uart_get_rs485_mode() to return an errno and change all callers > to check for failure. > > The GPIO descriptor is going to be stored in struct uart_port. Pass > that struct to uart_get_rs485_mode() in lieu of a struct device and > struct serial_rs485, both of which are directly accessible from struct > uart_port. > > A few drivers call uart_get_rs485_mode() before setting the struct > device pointer in struct uart_port. Shuffle those calls around where > necessary. > [Heiko Stuebner did the ar933x_uart.c portion, hence his Signed-off-by.] We also have Co-developed-by tag in addition to SoB to specify the credit. > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > drivers/tty/serial/8250/8250_core.c | 4 +++- > drivers/tty/serial/ar933x_uart.c | 6 ++++-- > drivers/tty/serial/atmel_serial.c | 6 ++++-- > drivers/tty/serial/fsl_lpuart.c | 5 ++++- > drivers/tty/serial/imx.c | 6 +++++- > drivers/tty/serial/omap-serial.c | 4 +++- > drivers/tty/serial/serial_core.c | 6 +++++- > drivers/tty/serial/stm32-usart.c | 8 ++++---- > include/linux/serial_core.h | 2 +- > 9 files changed, 33 insertions(+), 14 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c > index 9548d3f8fc8e..fc118f649887 100644 > --- a/drivers/tty/serial/8250/8250_core.c > +++ b/drivers/tty/serial/8250/8250_core.c > @@ -1026,7 +1026,9 @@ int serial8250_register_8250_port(struct uart_8250_port *up) > > if (up->port.dev) { > uart->port.dev = up->port.dev; > - uart_get_rs485_mode(uart->port.dev, &uart->port.rs485); > + ret = uart_get_rs485_mode(&uart->port); > + if (ret) > + goto err; > } > > if (up->port.flags & UPF_FIXED_TYPE) > diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c > index 7e7f1398019f..0c80a79d7442 100644 > --- a/drivers/tty/serial/ar933x_uart.c > +++ b/drivers/tty/serial/ar933x_uart.c > @@ -766,8 +766,6 @@ static int ar933x_uart_probe(struct platform_device *pdev) > goto err_disable_clk; > } > > - uart_get_rs485_mode(&pdev->dev, &port->rs485); > - > port->mapbase = mem_res->start; > port->line = id; > port->irq = irq_res->start; > @@ -786,6 +784,10 @@ static int ar933x_uart_probe(struct platform_device *pdev) > baud = ar933x_uart_get_baud(port->uartclk, 0, AR933X_UART_MAX_STEP); > up->max_baud = min_t(unsigned int, baud, AR933X_UART_MAX_BAUD); > > + ret = uart_get_rs485_mode(port); > + if (ret) > + goto err_disable_clk; > + > up->gpios = mctrl_gpio_init(port, 0); > if (IS_ERR(up->gpios) && PTR_ERR(up->gpios) != -ENOSYS) > return PTR_ERR(up->gpios); > diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c > index 8d7080efad9b..e43471b33710 100644 > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -2491,8 +2491,6 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port, > atmel_init_property(atmel_port, pdev); > atmel_set_ops(port); > > - uart_get_rs485_mode(&mpdev->dev, &port->rs485); > - > port->iotype = UPIO_MEM; > port->flags = UPF_BOOT_AUTOCONF | UPF_IOREMAP; > port->ops = &atmel_pops; > @@ -2506,6 +2504,10 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port, > > memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring)); > > + ret = uart_get_rs485_mode(port); > + if (ret) > + return ret; > + > /* for console, the clock could already be configured */ > if (!atmel_port->clk) { > atmel_port->clk = clk_get(&mpdev->dev, "usart"); > diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c > index 6a9909e56449..029324c77cd7 100644 > --- a/drivers/tty/serial/fsl_lpuart.c > +++ b/drivers/tty/serial/fsl_lpuart.c > @@ -2619,7 +2619,9 @@ static int lpuart_probe(struct platform_device *pdev) > if (ret) > goto failed_attach_port; > > - uart_get_rs485_mode(&pdev->dev, &sport->port.rs485); > + ret = uart_get_rs485_mode(&sport->port); > + if (ret) > + goto failed_get_rs485; > > if (sport->port.rs485.flags & SER_RS485_RX_DURING_TX) > dev_err(&pdev->dev, "driver doesn't support RX during TX\n"); > @@ -2632,6 +2634,7 @@ static int lpuart_probe(struct platform_device *pdev) > > return 0; > > +failed_get_rs485: > failed_attach_port: > failed_irq_request: > lpuart_disable_clks(sport); > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index f4d68109bc8b..91f3910d6c44 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -2302,7 +2302,11 @@ static int imx_uart_probe(struct platform_device *pdev) > sport->ucr4 = readl(sport->port.membase + UCR4); > sport->ufcr = readl(sport->port.membase + UFCR); > > - uart_get_rs485_mode(&pdev->dev, &sport->port.rs485); > + ret = uart_get_rs485_mode(&sport->port); > + if (ret) { > + clk_disable_unprepare(sport->clk_ipg); > + return ret; > + } > > if (sport->port.rs485.flags & SER_RS485_ENABLED && > (!sport->have_rtscts && !sport->have_rtsgpio)) > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c > index c71c1a2266dc..8573fc9cb0cd 100644 > --- a/drivers/tty/serial/omap-serial.c > +++ b/drivers/tty/serial/omap-serial.c > @@ -1608,7 +1608,9 @@ static int serial_omap_probe_rs485(struct uart_omap_port *up, > if (!np) > return 0; > > - uart_get_rs485_mode(up->dev, rs485conf); > + ret = uart_get_rs485_mode(&up->port); > + if (ret) > + return ret; > > if (of_property_read_bool(np, "rs485-rts-active-high")) { > rs485conf->flags |= SER_RS485_RTS_ON_SEND; > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index 66a5e2faf57e..43b6682877d5 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -3295,8 +3295,10 @@ EXPORT_SYMBOL(uart_remove_one_port); > * This function implements the device tree binding described in > * Documentation/devicetree/bindings/serial/rs485.txt. > */ > -void uart_get_rs485_mode(struct device *dev, struct serial_rs485 *rs485conf) > +int uart_get_rs485_mode(struct uart_port *port) > { > + struct serial_rs485 *rs485conf = &port->rs485; > + struct device *dev = port->dev; > u32 rs485_delay[2]; > int ret; > > @@ -3328,6 +3330,8 @@ void uart_get_rs485_mode(struct device *dev, struct serial_rs485 *rs485conf) > rs485conf->flags &= ~SER_RS485_RTS_ON_SEND; > rs485conf->flags |= SER_RS485_RTS_AFTER_SEND; > } > + > + return 0; > } > EXPORT_SYMBOL_GPL(uart_get_rs485_mode); > > diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c > index 5e93e8d40f59..e3db54398159 100644 > --- a/drivers/tty/serial/stm32-usart.c > +++ b/drivers/tty/serial/stm32-usart.c > @@ -158,9 +158,7 @@ static int stm32_init_rs485(struct uart_port *port, > if (!pdev->dev.of_node) > return -ENODEV; > > - uart_get_rs485_mode(&pdev->dev, rs485conf); > - > - return 0; > + return uart_get_rs485_mode(port); > } > > static int stm32_pending_rx(struct uart_port *port, u32 *sr, int *last_res, > @@ -931,7 +929,9 @@ static int stm32_init_port(struct stm32_port *stm32port, > > port->rs485_config = stm32_config_rs485; > > - stm32_init_rs485(port, pdev); > + ret = stm32_init_rs485(port, pdev); > + if (ret) > + return ret; > > if (stm32port->info->cfg.has_wakeup) { > stm32port->wakeirq = platform_get_irq(pdev, 1); > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h > index 92f5eba86052..b649a2b894e7 100644 > --- a/include/linux/serial_core.h > +++ b/include/linux/serial_core.h > @@ -472,5 +472,5 @@ extern int uart_handle_break(struct uart_port *port); > (cflag) & CRTSCTS || \ > !((cflag) & CLOCAL)) > > -void uart_get_rs485_mode(struct device *dev, struct serial_rs485 *rs485conf); > +int uart_get_rs485_mode(struct uart_port *port); > #endif /* LINUX_SERIAL_CORE_H */ > -- > 2.26.2 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/4] dt-bindings: serial: Add binding for rs485 bus termination GPIO 2020-05-12 12:40 [PATCH v2 0/4] rs485 bus termination GPIO Lukas Wunner 2020-05-12 12:40 ` [PATCH v2 1/4] serial: 8250: Avoid error message on reprobe Lukas Wunner 2020-05-12 12:40 ` [PATCH v2 2/4] serial: Allow uart_get_rs485_mode() to return errno Lukas Wunner @ 2020-05-12 12:40 ` Lukas Wunner 2020-05-12 12:40 ` [PATCH v2 4/4] serial: 8250: Support " Lukas Wunner 3 siblings, 0 replies; 8+ messages in thread From: Lukas Wunner @ 2020-05-12 12:40 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring Cc: Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner, Andy Shevchenko, Christoph Muellner, linux-serial, devicetree Commit e8759ad17d41 ("serial: uapi: Add support for bus termination") introduced the ability to enable rs485 bus termination from user space. So far the feature is only used by a single driver, 8250_exar.c, using a hardcoded GPIO pin specific to Siemens IOT2040 products. Provide for a more generic solution by allowing specification of an rs485 bus termination GPIO pin in the device tree. An upcoming commit implements support for this pin for any 8250 driver. The binding is used in device trees of the "Revolution Pi" PLCs offered by KUNBUS. [Heiko Stuebner converted the binding to YAML, hence his Signed-off-by.] Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com> Signed-off-by: Lukas Wunner <lukas@wunner.de> Cc: Jan Kiszka <jan.kiszka@siemens.com> --- Documentation/devicetree/bindings/serial/rs485.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml index d4beaf11222d..a9ad17864889 100644 --- a/Documentation/devicetree/bindings/serial/rs485.yaml +++ b/Documentation/devicetree/bindings/serial/rs485.yaml @@ -43,3 +43,7 @@ properties: rs485-rx-during-tx: description: enables the receiving of data even while sending data. $ref: /schemas/types.yaml#/definitions/flag + + rs485-term-gpios: + description: GPIO pin to enable RS485 bus termination. + maxItems: 1 -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 4/4] serial: 8250: Support rs485 bus termination GPIO 2020-05-12 12:40 [PATCH v2 0/4] rs485 bus termination GPIO Lukas Wunner ` (2 preceding siblings ...) 2020-05-12 12:40 ` [PATCH v2 3/4] dt-bindings: serial: Add binding for rs485 bus termination GPIO Lukas Wunner @ 2020-05-12 12:40 ` Lukas Wunner 2020-05-12 14:09 ` Andy Shevchenko 3 siblings, 1 reply; 8+ messages in thread From: Lukas Wunner @ 2020-05-12 12:40 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner, Andy Shevchenko, Christoph Muellner, linux-serial Commit e8759ad17d41 ("serial: uapi: Add support for bus termination") introduced the ability to enable rs485 bus termination from user space. So far the feature is only used by a single driver, 8250_exar.c, using a hardcoded GPIO pin specific to Siemens IOT2040 products. Provide for a more generic solution by allowing specification of an rs485 bus termination GPIO pin in the device tree: Amend the serial core to retrieve the GPIO from the device tree (or ACPI table) and amend the default ->rs485_config() callback for 8250 drivers to change the GPIO on request from user space. Signed-off-by: Lukas Wunner <lukas@wunner.de> --- Changes v1 -> v2: * Drop unnecessary devm_gpiod_put(). (Andy) * Use GPIOD_OUT_LOW macro for brevity. (Andy) * Document the rationale for disabling termination by default. * Drop nonsensical read of GPIO after setting its direction to out. drivers/tty/serial/8250/8250_port.c | 4 ++++ drivers/tty/serial/serial_core.c | 16 ++++++++++++++++ include/linux/serial_core.h | 2 ++ 3 files changed, 22 insertions(+) diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index f77bf820b7a3..b5b630d02110 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -681,6 +681,10 @@ int serial8250_em485_config(struct uart_port *port, struct serial_rs485 *rs485) memset(rs485->padding, 0, sizeof(rs485->padding)); port->rs485 = *rs485; + if (port->rs485_term_gpio) + gpiod_set_value(port->rs485_term_gpio, + rs485->flags & SER_RS485_TERMINATE_BUS); + /* * Both serial8250_em485_init() and serial8250_em485_destroy() * are idempotent. diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 43b6682877d5..57840cf90388 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -3317,6 +3317,7 @@ int uart_get_rs485_mode(struct uart_port *port) * to get to a defined state with the following properties: */ rs485conf->flags &= ~(SER_RS485_RX_DURING_TX | SER_RS485_ENABLED | + SER_RS485_TERMINATE_BUS | SER_RS485_RTS_AFTER_SEND); rs485conf->flags |= SER_RS485_RTS_ON_SEND; @@ -3331,6 +3332,21 @@ int uart_get_rs485_mode(struct uart_port *port) rs485conf->flags |= SER_RS485_RTS_AFTER_SEND; } + /* + * Disabling termination by default is the safe choice: Else if many + * bus participants enable it, no communication is possible at all. + * Works fine for short cables and users may enable for longer cables. + */ + port->rs485_term_gpio = devm_gpiod_get_optional(dev, "rs485-term", + GPIOD_OUT_LOW); + if (IS_ERR(port->rs485_term_gpio)) { + ret = PTR_ERR(port->rs485_term_gpio); + port->rs485_term_gpio = NULL; + if (ret != -EPROBE_DEFER) + dev_err(dev, "Cannot get rs485-term-gpios\n"); + return ret; + } + return 0; } EXPORT_SYMBOL_GPL(uart_get_rs485_mode); diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index b649a2b894e7..9fd550e7946a 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -10,6 +10,7 @@ #include <linux/bitops.h> #include <linux/compiler.h> #include <linux/console.h> +#include <linux/gpio/consumer.h> #include <linux/interrupt.h> #include <linux/circ_buf.h> #include <linux/spinlock.h> @@ -251,6 +252,7 @@ struct uart_port { struct attribute_group *attr_group; /* port specific attributes */ const struct attribute_group **tty_groups; /* all attributes (serial core use only) */ struct serial_rs485 rs485; + struct gpio_desc *rs485_term_gpio; /* enable RS485 bus termination */ struct serial_iso7816 iso7816; void *private_data; /* generic platform data pointer */ }; -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 4/4] serial: 8250: Support rs485 bus termination GPIO 2020-05-12 12:40 ` [PATCH v2 4/4] serial: 8250: Support " Lukas Wunner @ 2020-05-12 14:09 ` Andy Shevchenko 0 siblings, 0 replies; 8+ messages in thread From: Andy Shevchenko @ 2020-05-12 14:09 UTC (permalink / raw) To: Lukas Wunner Cc: Greg Kroah-Hartman, Jiri Slaby, Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner, Christoph Muellner, linux-serial On Tue, May 12, 2020 at 02:40:04PM +0200, Lukas Wunner wrote: > Commit e8759ad17d41 ("serial: uapi: Add support for bus termination") > introduced the ability to enable rs485 bus termination from user space. > So far the feature is only used by a single driver, 8250_exar.c, using a > hardcoded GPIO pin specific to Siemens IOT2040 products. Perhaps you need to Cc this to Jan Kiszka. > Provide for a more generic solution by allowing specification of an > rs485 bus termination GPIO pin in the device tree: Amend the serial > core to retrieve the GPIO from the device tree (or ACPI table) and amend > the default ->rs485_config() callback for 8250 drivers to change the > GPIO on request from user space. Perhaps the followup can be turning 8250_exar to request those GPIOs it provides (as hogs or regular ones, I don't know). FWIW, Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > Changes v1 -> v2: > * Drop unnecessary devm_gpiod_put(). (Andy) > * Use GPIOD_OUT_LOW macro for brevity. (Andy) > * Document the rationale for disabling termination by default. > * Drop nonsensical read of GPIO after setting its direction to out. > > drivers/tty/serial/8250/8250_port.c | 4 ++++ > drivers/tty/serial/serial_core.c | 16 ++++++++++++++++ > include/linux/serial_core.h | 2 ++ > 3 files changed, 22 insertions(+) > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > index f77bf820b7a3..b5b630d02110 100644 > --- a/drivers/tty/serial/8250/8250_port.c > +++ b/drivers/tty/serial/8250/8250_port.c > @@ -681,6 +681,10 @@ int serial8250_em485_config(struct uart_port *port, struct serial_rs485 *rs485) > memset(rs485->padding, 0, sizeof(rs485->padding)); > port->rs485 = *rs485; > > + if (port->rs485_term_gpio) > + gpiod_set_value(port->rs485_term_gpio, > + rs485->flags & SER_RS485_TERMINATE_BUS); > + > /* > * Both serial8250_em485_init() and serial8250_em485_destroy() > * are idempotent. > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index 43b6682877d5..57840cf90388 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -3317,6 +3317,7 @@ int uart_get_rs485_mode(struct uart_port *port) > * to get to a defined state with the following properties: > */ > rs485conf->flags &= ~(SER_RS485_RX_DURING_TX | SER_RS485_ENABLED | > + SER_RS485_TERMINATE_BUS | > SER_RS485_RTS_AFTER_SEND); > rs485conf->flags |= SER_RS485_RTS_ON_SEND; > > @@ -3331,6 +3332,21 @@ int uart_get_rs485_mode(struct uart_port *port) > rs485conf->flags |= SER_RS485_RTS_AFTER_SEND; > } > > + /* > + * Disabling termination by default is the safe choice: Else if many > + * bus participants enable it, no communication is possible at all. > + * Works fine for short cables and users may enable for longer cables. > + */ > + port->rs485_term_gpio = devm_gpiod_get_optional(dev, "rs485-term", > + GPIOD_OUT_LOW); > + if (IS_ERR(port->rs485_term_gpio)) { > + ret = PTR_ERR(port->rs485_term_gpio); > + port->rs485_term_gpio = NULL; > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Cannot get rs485-term-gpios\n"); > + return ret; > + } > + > return 0; > } > EXPORT_SYMBOL_GPL(uart_get_rs485_mode); > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h > index b649a2b894e7..9fd550e7946a 100644 > --- a/include/linux/serial_core.h > +++ b/include/linux/serial_core.h > @@ -10,6 +10,7 @@ > #include <linux/bitops.h> > #include <linux/compiler.h> > #include <linux/console.h> > +#include <linux/gpio/consumer.h> > #include <linux/interrupt.h> > #include <linux/circ_buf.h> > #include <linux/spinlock.h> > @@ -251,6 +252,7 @@ struct uart_port { > struct attribute_group *attr_group; /* port specific attributes */ > const struct attribute_group **tty_groups; /* all attributes (serial core use only) */ > struct serial_rs485 rs485; > + struct gpio_desc *rs485_term_gpio; /* enable RS485 bus termination */ > struct serial_iso7816 iso7816; > void *private_data; /* generic platform data pointer */ > }; > -- > 2.26.2 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-12 14:09 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-12 12:40 [PATCH v2 0/4] rs485 bus termination GPIO Lukas Wunner 2020-05-12 12:40 ` [PATCH v2 1/4] serial: 8250: Avoid error message on reprobe Lukas Wunner 2020-05-12 13:45 ` Andy Shevchenko 2020-05-12 12:40 ` [PATCH v2 2/4] serial: Allow uart_get_rs485_mode() to return errno Lukas Wunner 2020-05-12 13:46 ` Andy Shevchenko 2020-05-12 12:40 ` [PATCH v2 3/4] dt-bindings: serial: Add binding for rs485 bus termination GPIO Lukas Wunner 2020-05-12 12:40 ` [PATCH v2 4/4] serial: 8250: Support " Lukas Wunner 2020-05-12 14:09 ` Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).