[2/2] serial: core: implement support for rs485-mux-gpios
Commit Message
Add code for handling a rs485-mux-gpio specified in device tree.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
drivers/tty/serial/serial_core.c | 35 ++++++++++++++++++++++++++++++--
include/linux/serial_core.h | 1 +
2 files changed, 34 insertions(+), 2 deletions(-)
Comments
Hi,
On 20.11.23 16:10, Rasmus Villemoes wrote:
> Add code for handling a rs485-mux-gpio specified in device tree.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> drivers/tty/serial/serial_core.c | 35 ++++++++++++++++++++++++++++++--
> include/linux/serial_core.h | 1 +
> 2 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index f1348a509552..410b17ea7444 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1402,14 +1402,20 @@ static void uart_set_rs485_termination(struct uart_port *port,
> !!(rs485->flags & SER_RS485_TERMINATE_BUS));
> }
>
> +static void uart_set_rs485_mux(struct uart_port *port, const struct serial_rs485 *rs485)
> +{
> + gpiod_set_value_cansleep(port->rs485_mux_gpio,
> + !!(rs485->flags & SER_RS485_ENABLED));
> +}
> +
> static int uart_rs485_config(struct uart_port *port)
> {
> struct serial_rs485 *rs485 = &port->rs485;
> unsigned long flags;
> - int ret;
> + int ret = 0;
>
> if (!(rs485->flags & SER_RS485_ENABLED))
> - return 0;
> + goto out;
>
> uart_sanitize_serial_rs485(port, rs485);
> uart_set_rs485_termination(port, rs485);
> @@ -1420,6 +1426,9 @@ static int uart_rs485_config(struct uart_port *port)
> if (ret)
> memset(rs485, 0, sizeof(*rs485));
>
> +out:
> + uart_set_rs485_mux(port, rs485);
> +
> return ret;
> }
>
> @@ -1457,6 +1466,14 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
> return ret;
> uart_sanitize_serial_rs485(port, &rs485);
> uart_set_rs485_termination(port, &rs485);
> + /*
> + * To avoid glitches on the transmit enable pin, the mux must
> + * be set before calling the driver's ->rs485_config when
> + * disabling rs485 mode, but after when enabling rs485
> + * mode.
> + */
> + if (!(rs485.flags & SER_RS485_ENABLED))
> + uart_set_rs485_mux(port, &rs485);
>
> uart_port_lock_irqsave(port, &flags);
> ret = port->rs485_config(port, &tty->termios, &rs485);
> @@ -1468,6 +1485,13 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
> port->ops->set_mctrl(port, port->mctrl);
> }
> uart_port_unlock_irqrestore(port, flags);
> +
> + /*
> + * The ->rs485_config might have failed. Regardless, set the
> + * mux according to the port's effective rs485 config.
> + */
> + uart_set_rs485_mux(port, &port->rs485);
> +
> if (ret)
> return ret;
>
> @@ -3621,6 +3645,13 @@ int uart_get_rs485_mode(struct uart_port *port)
> return dev_err_probe(dev, PTR_ERR(desc), "Cannot get rs485-rx-during-tx-gpios\n");
> port->rs485_rx_during_tx_gpio = desc;
>
> + dflags = (rs485conf->flags & SER_RS485_ENABLED) ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
> + desc = devm_gpiod_get_optional(dev, "rs485-mux", dflags);
> + if (IS_ERR(desc))
> + return dev_err_probe(dev, PTR_ERR(port->rs485_mux_gpio),
> + "Cannot get rs485-mux-gpios\n");
> + port->rs485_mux_gpio = desc;
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(uart_get_rs485_mode);
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 89f7b6c63598..943818209c49 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -584,6 +584,7 @@ struct uart_port {
> struct serial_rs485 rs485_supported; /* Supported mask for serial_rs485 */
> struct gpio_desc *rs485_term_gpio; /* enable RS485 bus termination */
> struct gpio_desc *rs485_rx_during_tx_gpio; /* Output GPIO that sets the state of RS485 RX during TX */
> + struct gpio_desc *rs485_mux_gpio; /* gpio for selecting RS485 mode */
> struct serial_iso7816 iso7816;
> void *private_data; /* generic platform data pointer */
> };
FWIW
Reviewed-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Regards,
Lino
Hi Rasmus,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Rasmus-Villemoes/dt-bindings-serial-rs485-add-rs485-mux-gpios-binding/20231120-231551
base: v6.7-rc2
patch link: https://lore.kernel.org/r/20231120151056.148450-3-linux%40rasmusvillemoes.dk
patch subject: [PATCH 2/2] serial: core: implement support for rs485-mux-gpios
config: hexagon-randconfig-r071-20231121 (https://download.01.org/0day-ci/archive/20231121/202311211751.MgZLovko-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20231121/202311211751.MgZLovko-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202311211751.MgZLovko-lkp@intel.com/
New smatch warnings:
drivers/tty/serial/serial_core.c:3651 uart_get_rs485_mode() warn: passing zero to 'PTR_ERR'
Old smatch warnings:
drivers/tty/serial/serial_core.c:2996 iomem_base_show() warn: argument 3 to %lX specifier is cast from pointer
vim +/PTR_ERR +3651 drivers/tty/serial/serial_core.c
d58a2df3d8877b Lukas Wunner 2020-05-18 3629 /*
d58a2df3d8877b Lukas Wunner 2020-05-18 3630 * Disabling termination by default is the safe choice: Else if many
d58a2df3d8877b Lukas Wunner 2020-05-18 3631 * bus participants enable it, no communication is possible at all.
d58a2df3d8877b Lukas Wunner 2020-05-18 3632 * Works fine for short cables and users may enable for longer cables.
d58a2df3d8877b Lukas Wunner 2020-05-18 3633 */
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3634 desc = devm_gpiod_get_optional(dev, "rs485-term", GPIOD_OUT_LOW);
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3635 if (IS_ERR(desc))
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3636 return dev_err_probe(dev, PTR_ERR(desc), "Cannot get rs485-term-gpios\n");
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3637 port->rs485_term_gpio = desc;
8bec874f84d826 Ilpo Järvinen 2022-07-04 3638 if (port->rs485_term_gpio)
8bec874f84d826 Ilpo Järvinen 2022-07-04 3639 port->rs485_supported.flags |= SER_RS485_TERMINATE_BUS;
d58a2df3d8877b Lukas Wunner 2020-05-18 3640
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3641 dflags = (rs485conf->flags & SER_RS485_RX_DURING_TX) ?
163f080eb717d2 Christoph Niedermaier 2022-12-02 3642 GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3643 desc = devm_gpiod_get_optional(dev, "rs485-rx-during-tx", dflags);
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3644 if (IS_ERR(desc))
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3645 return dev_err_probe(dev, PTR_ERR(desc), "Cannot get rs485-rx-during-tx-gpios\n");
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3646 port->rs485_rx_during_tx_gpio = desc;
163f080eb717d2 Christoph Niedermaier 2022-12-02 3647
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3648 dflags = (rs485conf->flags & SER_RS485_ENABLED) ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3649 desc = devm_gpiod_get_optional(dev, "rs485-mux", dflags);
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3650 if (IS_ERR(desc))
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 @3651 return dev_err_probe(dev, PTR_ERR(port->rs485_mux_gpio),
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Should be PTR_ERR(desc).
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3652 "Cannot get rs485-mux-gpios\n");
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3653 port->rs485_mux_gpio = desc;
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3654
c150c0f362c1e5 Lukas Wunner 2020-05-12 3655 return 0;
ef838a81dd4de1 Uwe Kleine-König 2017-09-13 3656 }
On Mon, Nov 20, 2023 at 04:10:55PM +0100, Rasmus Villemoes wrote:
> Add code for handling a rs485-mux-gpio specified in device tree.
Hm, that's a bit terse as a commit message.
> @@ -1457,6 +1466,14 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
> return ret;
> uart_sanitize_serial_rs485(port, &rs485);
> uart_set_rs485_termination(port, &rs485);
> + /*
> + * To avoid glitches on the transmit enable pin, the mux must
> + * be set before calling the driver's ->rs485_config when
> + * disabling rs485 mode, but after when enabling rs485
> + * mode.
> + */
> + if (!(rs485.flags & SER_RS485_ENABLED))
> + uart_set_rs485_mux(port, &rs485);
Can it happen that the UART's FIFO contains characters such that
suddenly switching the mux causes some of them to appear on the
RS-485 transceiver and some on the RS-232 driver?
Shouldn't we wait for the FIFO to drain before making the switch?
I think that would be closer to the behavior expected by user space.
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -584,6 +584,7 @@ struct uart_port {
> struct serial_rs485 rs485_supported; /* Supported mask for serial_rs485 */
> struct gpio_desc *rs485_term_gpio; /* enable RS485 bus termination */
> struct gpio_desc *rs485_rx_during_tx_gpio; /* Output GPIO that sets the state of RS485 RX during TX */
> + struct gpio_desc *rs485_mux_gpio; /* gpio for selecting RS485 mode */
Again, the code comment isn't really helpful as it doesn't add a whole
lot of information to the variable name "rs485_mux_gpio". How about:
"select between RS-232 and RS-485 transceiver" ?
(I realize I made a typo in my previous e-mail about the DT-binding,
sorry about that: s/connect/connected/)
Thanks,
Lukas
Hi Rasmus,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Rasmus-Villemoes/dt-bindings-serial-rs485-add-rs485-mux-gpios-binding/20231120-231551
base: v6.7-rc2
patch link: https://lore.kernel.org/r/20231120151056.148450-3-linux%40rasmusvillemoes.dk
patch subject: [PATCH 2/2] serial: core: implement support for rs485-mux-gpios
config: hexagon-randconfig-r071-20231121 (https://download.01.org/0day-ci/archive/20231203/202312031811.pmLZJIf5-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20231203/202312031811.pmLZJIf5-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202312031811.pmLZJIf5-lkp@intel.com/
New smatch warnings:
drivers/tty/serial/serial_core.c:3651 uart_get_rs485_mode() warn: passing zero to 'PTR_ERR'
Old smatch warnings:
drivers/tty/serial/serial_core.c:2996 iomem_base_show() warn: argument 3 to %lX specifier is cast from pointer
vim +/PTR_ERR +3651 drivers/tty/serial/serial_core.c
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3641 dflags = (rs485conf->flags & SER_RS485_RX_DURING_TX) ?
163f080eb717d2 Christoph Niedermaier 2022-12-02 3642 GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3643 desc = devm_gpiod_get_optional(dev, "rs485-rx-during-tx", dflags);
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3644 if (IS_ERR(desc))
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3645 return dev_err_probe(dev, PTR_ERR(desc), "Cannot get rs485-rx-during-tx-gpios\n");
7cda0b9eb6eb9e Andy Shevchenko 2023-10-03 3646 port->rs485_rx_during_tx_gpio = desc;
163f080eb717d2 Christoph Niedermaier 2022-12-02 3647
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3648 dflags = (rs485conf->flags & SER_RS485_ENABLED) ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3649 desc = devm_gpiod_get_optional(dev, "rs485-mux", dflags);
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3650 if (IS_ERR(desc))
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 @3651 return dev_err_probe(dev, PTR_ERR(port->rs485_mux_gpio),
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
s/port->rs485_mux_gpio/desc/
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3652 "Cannot get rs485-mux-gpios\n");
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3653 port->rs485_mux_gpio = desc;
da0ccd117da1e4 Rasmus Villemoes 2023-11-20 3654
c150c0f362c1e5 Lukas Wunner 2020-05-12 3655 return 0;
ef838a81dd4de1 Uwe Kleine-König 2017-09-13 3656 }
@@ -1402,14 +1402,20 @@ static void uart_set_rs485_termination(struct uart_port *port,
!!(rs485->flags & SER_RS485_TERMINATE_BUS));
}
+static void uart_set_rs485_mux(struct uart_port *port, const struct serial_rs485 *rs485)
+{
+ gpiod_set_value_cansleep(port->rs485_mux_gpio,
+ !!(rs485->flags & SER_RS485_ENABLED));
+}
+
static int uart_rs485_config(struct uart_port *port)
{
struct serial_rs485 *rs485 = &port->rs485;
unsigned long flags;
- int ret;
+ int ret = 0;
if (!(rs485->flags & SER_RS485_ENABLED))
- return 0;
+ goto out;
uart_sanitize_serial_rs485(port, rs485);
uart_set_rs485_termination(port, rs485);
@@ -1420,6 +1426,9 @@ static int uart_rs485_config(struct uart_port *port)
if (ret)
memset(rs485, 0, sizeof(*rs485));
+out:
+ uart_set_rs485_mux(port, rs485);
+
return ret;
}
@@ -1457,6 +1466,14 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
return ret;
uart_sanitize_serial_rs485(port, &rs485);
uart_set_rs485_termination(port, &rs485);
+ /*
+ * To avoid glitches on the transmit enable pin, the mux must
+ * be set before calling the driver's ->rs485_config when
+ * disabling rs485 mode, but after when enabling rs485
+ * mode.
+ */
+ if (!(rs485.flags & SER_RS485_ENABLED))
+ uart_set_rs485_mux(port, &rs485);
uart_port_lock_irqsave(port, &flags);
ret = port->rs485_config(port, &tty->termios, &rs485);
@@ -1468,6 +1485,13 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
port->ops->set_mctrl(port, port->mctrl);
}
uart_port_unlock_irqrestore(port, flags);
+
+ /*
+ * The ->rs485_config might have failed. Regardless, set the
+ * mux according to the port's effective rs485 config.
+ */
+ uart_set_rs485_mux(port, &port->rs485);
+
if (ret)
return ret;
@@ -3621,6 +3645,13 @@ int uart_get_rs485_mode(struct uart_port *port)
return dev_err_probe(dev, PTR_ERR(desc), "Cannot get rs485-rx-during-tx-gpios\n");
port->rs485_rx_during_tx_gpio = desc;
+ dflags = (rs485conf->flags & SER_RS485_ENABLED) ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
+ desc = devm_gpiod_get_optional(dev, "rs485-mux", dflags);
+ if (IS_ERR(desc))
+ return dev_err_probe(dev, PTR_ERR(port->rs485_mux_gpio),
+ "Cannot get rs485-mux-gpios\n");
+ port->rs485_mux_gpio = desc;
+
return 0;
}
EXPORT_SYMBOL_GPL(uart_get_rs485_mode);
@@ -584,6 +584,7 @@ struct uart_port {
struct serial_rs485 rs485_supported; /* Supported mask for serial_rs485 */
struct gpio_desc *rs485_term_gpio; /* enable RS485 bus termination */
struct gpio_desc *rs485_rx_during_tx_gpio; /* Output GPIO that sets the state of RS485 RX during TX */
+ struct gpio_desc *rs485_mux_gpio; /* gpio for selecting RS485 mode */
struct serial_iso7816 iso7816;
void *private_data; /* generic platform data pointer */
};