2023-11-20 15:13:30

by Rasmus Villemoes

[permalink] [raw]
Subject: [PATCH 0/2] serial: add rs485-mux-gpio dt binding and support

Some boards are wired so that they support both rs232 and rs485, with
a gpio controlling a mux connecting the SOC's pins to the appropriate
external circuitry. Instead of requiring the application to know about
such details and handle the gpio appropriately, allow it to be
described in device tree and let the serial core code handle it if
present.

Tested on a board based on imx8mp.

Rasmus Villemoes (2):
dt-bindings: serial: rs485: add rs485-mux-gpios binding
serial: core: implement support for rs485-mux-gpios

.../devicetree/bindings/serial/rs485.yaml | 5 +++
drivers/tty/serial/serial_core.c | 35 +++++++++++++++++--
include/linux/serial_core.h | 1 +
3 files changed, 39 insertions(+), 2 deletions(-)

--
2.40.1.1.g1c60b9335d


2023-11-20 15:13:34

by Rasmus Villemoes

[permalink] [raw]
Subject: [PATCH 2/2] serial: core: implement support for rs485-mux-gpios

Add code for handling a rs485-mux-gpio specified in device tree.

Signed-off-by: Rasmus Villemoes <[email protected]>
---
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 */
};
--
2.40.1.1.g1c60b9335d

2023-11-20 15:13:36

by Rasmus Villemoes

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

Some boards are capable of both rs232 and rs485, and control which
external terminals are active via a gpio-controlled mux. Allow
describing that gpio in DT so that the kernel can transparently handle
the proper setting when the uart is switched between rs232 and rs485
modes.

Signed-off-by: Rasmus Villemoes <[email protected]>
---
Documentation/devicetree/bindings/serial/rs485.yaml | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml
index 9418fd66a8e9..e8136c7d22ed 100644
--- a/Documentation/devicetree/bindings/serial/rs485.yaml
+++ b/Documentation/devicetree/bindings/serial/rs485.yaml
@@ -61,6 +61,11 @@ properties:
the active state enables RX during TX.
maxItems: 1

+ rs485-mux-gpios:
+ description: GPIO pin to control muxing of the SOC signals to the RS485
+ transceiver.
+ maxItems: 1
+
additionalProperties: true

...
--
2.40.1.1.g1c60b9335d

2023-11-20 23:28:28

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH 2/2] serial: core: implement support for rs485-mux-gpios

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 <[email protected]>
> ---
> 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 <[email protected]>

Regards,
Lino

2023-11-21 07:53:17

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On 20/11/2023 16:10, Rasmus Villemoes wrote:
> Some boards are capable of both rs232 and rs485, and control which
> external terminals are active via a gpio-controlled mux. Allow
> describing that gpio in DT so that the kernel can transparently handle
> the proper setting when the uart is switched between rs232 and rs485
> modes.
>
> Signed-off-by: Rasmus Villemoes <[email protected]>
> ---
> Documentation/devicetree/bindings/serial/rs485.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml
> index 9418fd66a8e9..e8136c7d22ed 100644
> --- a/Documentation/devicetree/bindings/serial/rs485.yaml
> +++ b/Documentation/devicetree/bindings/serial/rs485.yaml
> @@ -61,6 +61,11 @@ properties:
> the active state enables RX during TX.
> maxItems: 1
>
> + rs485-mux-gpios:
> + description: GPIO pin to control muxing of the SOC signals to the RS485
> + transceiver.
> + maxItems: 1

Aren't you duplicating
https://lore.kernel.org/all/[email protected]/ ?

Anyway, similar comments: this does not look like generic RS485
property. Are you saying that standard defines such GPIO?


Best regards,
Krzysztof

2023-11-21 08:29:30

by Rasmus Villemoes

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On 21/11/2023 08.52, Krzysztof Kozlowski wrote:
> On 20/11/2023 16:10, Rasmus Villemoes wrote:
>> Some boards are capable of both rs232 and rs485, and control which
>> external terminals are active via a gpio-controlled mux. Allow
>> describing that gpio in DT so that the kernel can transparently handle
>> the proper setting when the uart is switched between rs232 and rs485
>> modes.
>>
>> Signed-off-by: Rasmus Villemoes <[email protected]>
>> ---
>> Documentation/devicetree/bindings/serial/rs485.yaml | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml
>> index 9418fd66a8e9..e8136c7d22ed 100644
>> --- a/Documentation/devicetree/bindings/serial/rs485.yaml
>> +++ b/Documentation/devicetree/bindings/serial/rs485.yaml
>> @@ -61,6 +61,11 @@ properties:
>> the active state enables RX during TX.
>> maxItems: 1
>>
>> + rs485-mux-gpios:
>> + description: GPIO pin to control muxing of the SOC signals to the RS485
>> + transceiver.
>> + maxItems: 1
>
> Aren't you duplicating
> https://lore.kernel.org/all/[email protected]/ ?

Hadn't seen that, but no, this is not at all the same. That patch seems
to define an input pin to tell whether to enable rs485 mode or not (sort
of early run-time version of the linux,rs485-enabled-at-boot-time).

> Anyway, similar comments: this does not look like generic RS485
> property. Are you saying that standard defines such GPIO?

No, I'm saying that several boards that exist in the wild have the
RX/TX/CTS etc. pins routed to a multiplexer, which in turn routes those
signals to either a rs485 transceiver or an rs232 driver (and those in
turn are connected to different screw terminals). So no, it's not a
property of the rs485 protocol itself, but very much related to making
use of rs485 (and rs232, though of course not simultaneously) on such
boards.

Would a link to a schematic help?

Rasmus

2023-11-21 08:34:32

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On 21/11/2023 09:27, Rasmus Villemoes wrote:
> On 21/11/2023 08.52, Krzysztof Kozlowski wrote:
>> On 20/11/2023 16:10, Rasmus Villemoes wrote:
>>> Some boards are capable of both rs232 and rs485, and control which
>>> external terminals are active via a gpio-controlled mux. Allow
>>> describing that gpio in DT so that the kernel can transparently handle
>>> the proper setting when the uart is switched between rs232 and rs485
>>> modes.
>>>
>>> Signed-off-by: Rasmus Villemoes <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/serial/rs485.yaml | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml
>>> index 9418fd66a8e9..e8136c7d22ed 100644
>>> --- a/Documentation/devicetree/bindings/serial/rs485.yaml
>>> +++ b/Documentation/devicetree/bindings/serial/rs485.yaml
>>> @@ -61,6 +61,11 @@ properties:
>>> the active state enables RX during TX.
>>> maxItems: 1
>>>
>>> + rs485-mux-gpios:
>>> + description: GPIO pin to control muxing of the SOC signals to the RS485
>>> + transceiver.
>>> + maxItems: 1
>>
>> Aren't you duplicating
>> https://lore.kernel.org/all/[email protected]/ ?
>
> Hadn't seen that, but no, this is not at all the same. That patch seems
> to define an input pin to tell whether to enable rs485 mode or not (sort
> of early run-time version of the linux,rs485-enabled-at-boot-time).
>
>> Anyway, similar comments: this does not look like generic RS485
>> property. Are you saying that standard defines such GPIO?
>
> No, I'm saying that several boards that exist in the wild have the
> RX/TX/CTS etc. pins routed to a multiplexer, which in turn routes those
> signals to either a rs485 transceiver or an rs232 driver (and those in
> turn are connected to different screw terminals). So no, it's not a
> property of the rs485 protocol itself, but very much related to making
> use of rs485 (and rs232, though of course not simultaneously) on such
> boards.

Which upstream boards use it? To me it looks like specific to each
controller, not to RS485.

>
> Would a link to a schematic help?

Yes, always :)

Best regards,
Krzysztof

2023-11-21 09:29:19

by Rasmus Villemoes

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On 21/11/2023 09.34, Krzysztof Kozlowski wrote:
> On 21/11/2023 09:27, Rasmus Villemoes wrote:
>> On 21/11/2023 08.52, Krzysztof Kozlowski wrote:

>>> Anyway, similar comments: this does not look like generic RS485
>>> property. Are you saying that standard defines such GPIO?
>>
>> No, I'm saying that several boards that exist in the wild have the
>> RX/TX/CTS etc. pins routed to a multiplexer, which in turn routes those
>> signals to either a rs485 transceiver or an rs232 driver (and those in
>> turn are connected to different screw terminals). So no, it's not a
>> property of the rs485 protocol itself, but very much related to making
>> use of rs485 (and rs232, though of course not simultaneously) on such
>> boards.
>
> Which upstream boards use it?

None, because the binding doesn't exist.

> To me it looks like specific to each controller, not to RS485.

What do you mean "controller"? It's not specific to one particular
SOC/IP, any uart IP capable of both rs232 and rs485 could be wired to
circuitry like this.

>> Would a link to a schematic help?
>
> Yes, always :)

https://ibb.co/B3gzySt

The UART1.* signals on the right are from the SOC (in this case an
imx8mp, but I know of other boards e.g. based on powerpc that use a
similar scheme), and the COM1_Sel is just some gpio. The multiplexer is
roughly in the middle (U2103).

As you can see, if one wants to talk rs485, one must set COM1_Sel low
(and that works just fine by describing the rs485-mux-gpio as
ACTIVE_LOW), and if one wants to talk rs232, it must be set high. While
userspace could be tasked with knowing about and handling that gpio on
top of the ioctl() for switching mode, this really seems like the kind
of thing that DT is supposed to describe and the kernel is supposed to
handle.

Rasmus

2023-11-21 10:49:33

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 2/2] serial: core: implement support for rs485-mux-gpios

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/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20231121/[email protected]/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 <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

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 }

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-11-22 14:54:00

by Lukas Wunner

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On Mon, Nov 20, 2023 at 04:10:54PM +0100, Rasmus Villemoes wrote:
> Some boards are capable of both rs232 and rs485, and control which
> external terminals are active via a gpio-controlled mux. Allow
> describing that gpio in DT so that the kernel can transparently handle
> the proper setting when the uart is switched between rs232 and rs485
> modes.

Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
to struct serial_rs485:

https://lore.kernel.org/all/[email protected]/

I don't know whether that makes sense at all (I had thought RS-422 is
the same as RS-485 with full-duplex, i.e. SER_RS485_ENABLED plus
SER_RS485_RX_DURING_TX).

But if that patch gets accepted, we'd have *three* different modes:
RS-232, RS-485, RS-422. A single GPIO seems insufficient to handle that.
You'd need at least two GPIOs.


> --- a/Documentation/devicetree/bindings/serial/rs485.yaml
> +++ b/Documentation/devicetree/bindings/serial/rs485.yaml
> @@ -61,6 +61,11 @@ properties:
> the active state enables RX during TX.
> maxItems: 1
>
> + rs485-mux-gpios:
> + description: GPIO pin to control muxing of the SOC signals to the RS485
> + transceiver.
> + maxItems: 1

The description doesn't really add much to the name "rs485-mux-gpios".

Suggestion:

description: selects whether the UART is connect to an RS-232 driver (low)
or an RS-485 transceiver (high)

Thanks,

Lukas

2023-11-22 14:57:50

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 0/2] serial: add rs485-mux-gpio dt binding and support

On Mon, Nov 20, 2023 at 04:10:53PM +0100, Rasmus Villemoes wrote:
> Some boards are wired so that they support both rs232 and rs485, with
> a gpio controlling a mux connecting the SOC's pins to the appropriate
> external circuitry. Instead of requiring the application to know about
> such details and handle the gpio appropriately, allow it to be
> described in device tree and let the serial core code handle it if
> present.
>
> Tested on a board based on imx8mp.

Why mux framework is not (may not) used?

--
With Best Regards,
Andy Shevchenko


2023-11-22 15:11:42

by Lukas Wunner

[permalink] [raw]
Subject: Re: [PATCH 2/2] serial: core: implement support for rs485-mux-gpios

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

2023-11-22 18:03:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On 21/11/2023 10:28, Rasmus Villemoes wrote:
> On 21/11/2023 09.34, Krzysztof Kozlowski wrote:
>> On 21/11/2023 09:27, Rasmus Villemoes wrote:
>>> On 21/11/2023 08.52, Krzysztof Kozlowski wrote:
>
>>>> Anyway, similar comments: this does not look like generic RS485
>>>> property. Are you saying that standard defines such GPIO?
>>>
>>> No, I'm saying that several boards that exist in the wild have the
>>> RX/TX/CTS etc. pins routed to a multiplexer, which in turn routes those
>>> signals to either a rs485 transceiver or an rs232 driver (and those in
>>> turn are connected to different screw terminals). So no, it's not a
>>> property of the rs485 protocol itself, but very much related to making
>>> use of rs485 (and rs232, though of course not simultaneously) on such
>>> boards.
>>
>> Which upstream boards use it?
>
> None, because the binding doesn't exist.
>
>> To me it looks like specific to each controller, not to RS485.
>
> What do you mean "controller"? It's not specific to one particular
> SOC/IP, any uart IP capable of both rs232 and rs485 could be wired to
> circuitry like this.
>
>>> Would a link to a schematic help?
>>
>> Yes, always :)
>
> https://ibb.co/B3gzySt
>
> The UART1.* signals on the right are from the SOC (in this case an
> imx8mp, but I know of other boards e.g. based on powerpc that use a
> similar scheme), and the COM1_Sel is just some gpio. The multiplexer is
> roughly in the middle (U2103).
>
> As you can see, if one wants to talk rs485, one must set COM1_Sel low
> (and that works just fine by describing the rs485-mux-gpio as
> ACTIVE_LOW), and if one wants to talk rs232, it must be set high. While
> userspace could be tasked with knowing about and handling that gpio on
> top of the ioctl() for switching mode, this really seems like the kind
> of thing that DT is supposed to describe and the kernel is supposed to
> handle.

Yep, the trouble is only the placement. This GPIO mux is neither part of
the UART controller nor connector. Usually such pins in static
configuration are described either as GPIO hogs or as pinctrl. I guess
this matches other GPIOs in that binding, so I think it is fine. You
still though should answer to comments from other folks, including
multiple-GPIO case and mux.

Best regards,
Krzysztof

2023-11-23 10:08:09

by Rasmus Villemoes

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On 22/11/2023 15.53, Lukas Wunner wrote:
> On Mon, Nov 20, 2023 at 04:10:54PM +0100, Rasmus Villemoes wrote:
>> Some boards are capable of both rs232 and rs485, and control which
>> external terminals are active via a gpio-controlled mux. Allow
>> describing that gpio in DT so that the kernel can transparently handle
>> the proper setting when the uart is switched between rs232 and rs485
>> modes.
>
> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
> to struct serial_rs485:
>
> https://lore.kernel.org/all/[email protected]/
>
> I don't know whether that makes sense at all (I had thought RS-422 is
> the same as RS-485 with full-duplex, i.e. SER_RS485_ENABLED plus
> SER_RS485_RX_DURING_TX).

No, that latter case is as I understand it usually called "4-wire
rs485", while rs-422 is an entirely different animal, and the wiring is
in some sense actually closer to rs-232. rs-422 is full-duplex, with all
the slave device's tx-lines connected to the master's rx, and the
master's tx connected to the slaves' rx (ok, it uses differential
signalling, so there are four wires involved and not two as in rs-232).
But I'm no expert, and there doesn't seem to be entirely consistent
terminology.

>
> But if that patch gets accepted, we'd have *three* different modes:
> RS-232, RS-485, RS-422. A single GPIO seems insufficient to handle that.
> You'd need at least two GPIOs.

I don't see Crescent introducing any new gpio that needs to be handled.
In fact, I can't even see why from the perspective of the software that
rs422 isn't just rs232; there's no transmit enable pin that needs to be
handled. But maybe the uart driver does something different in rs422
mode; I assume he must have some update of some driver, since otherwise
the new rs422 bit should be rejected by the core. So I can't really see
the whole picture of that rs422 story.

>> --- a/Documentation/devicetree/bindings/serial/rs485.yaml
>> +++ b/Documentation/devicetree/bindings/serial/rs485.yaml
>> @@ -61,6 +61,11 @@ properties:
>> the active state enables RX during TX.
>> maxItems: 1
>>
>> + rs485-mux-gpios:
>> + description: GPIO pin to control muxing of the SOC signals to the RS485
>> + transceiver.
>> + maxItems: 1
>
> The description doesn't really add much to the name "rs485-mux-gpios".
>
> Suggestion:
>
> description: selects whether the UART is connect to an RS-232 driver (low)
> or an RS-485 transceiver (high)

Indeed, I wasn't really able to come up with a good description. Thanks,
that's much better.

Rasmus

2023-11-23 10:38:14

by Lukas Wunner

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On Thu, Nov 23, 2023 at 11:07:16AM +0100, Rasmus Villemoes wrote:
> On 22/11/2023 15.53, Lukas Wunner wrote:
> > But if that patch gets accepted, we'd have *three* different modes:
> > RS-232, RS-485, RS-422. A single GPIO seems insufficient to handle that.
> > You'd need at least two GPIOs.
>
> I don't see Crescent introducing any new gpio that needs to be handled.
> In fact, I can't even see why from the perspective of the software that
> rs422 isn't just rs232; there's no transmit enable pin that needs to be
> handled. But maybe the uart driver does something different in rs422
> mode; I assume he must have some update of some driver, since otherwise
> the new rs422 bit should be rejected by the core. So I can't really see
> the whole picture of that rs422 story.

The question is, could we conceivably have the need to support
switching between the three modes RS-232, RS-485, RS-422.
If yes, then the GPIO mux interface should probably allow for that.

As a case in point, the Siemens IOT 2040 has two serial ports
which can be set to either of those three modes. The signals
are routed to the same D-sub socket, but the pins used are
different. See page 46 and 47 of this document:

https://cache.industry.siemens.com/dl/files/658/109741658/att_899623/v1/iot2000_operating_instructions_enUS_en-US.pdf

The driver for this product is 8250_exar.c. It's an Intel-based
product, so no devicetree, but it shows that such use cases exist.

Thanks,

Lukas

2023-11-23 13:49:12

by Rasmus Villemoes

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On 23/11/2023 11.38, Lukas Wunner wrote:
> On Thu, Nov 23, 2023 at 11:07:16AM +0100, Rasmus Villemoes wrote:
>> On 22/11/2023 15.53, Lukas Wunner wrote:
>>> But if that patch gets accepted, we'd have *three* different modes:
>>> RS-232, RS-485, RS-422. A single GPIO seems insufficient to handle that.
>>> You'd need at least two GPIOs.
>>
>> I don't see Crescent introducing any new gpio that needs to be handled.
>> In fact, I can't even see why from the perspective of the software that
>> rs422 isn't just rs232; there's no transmit enable pin that needs to be
>> handled. But maybe the uart driver does something different in rs422
>> mode; I assume he must have some update of some driver, since otherwise
>> the new rs422 bit should be rejected by the core. So I can't really see
>> the whole picture of that rs422 story.
>
> The question is, could we conceivably have the need to support
> switching between the three modes RS-232, RS-485, RS-422.
> If yes, then the GPIO mux interface should probably allow for that.
>
> As a case in point, the Siemens IOT 2040 has two serial ports
> which can be set to either of those three modes. The signals
> are routed to the same D-sub socket, but the pins used are
> different. See page 46 and 47 of this document:
>
> https://cache.industry.siemens.com/dl/files/658/109741658/att_899623/v1/iot2000_operating_instructions_enUS_en-US.pdf
>
> The driver for this product is 8250_exar.c. It's an Intel-based
> product, so no devicetree, but it shows that such use cases exist.

OK. I did look at the mux-controller/mux-consumer bindings, but couldn't
really make heads or tails of it, and there aren't a whole lot of
examples in-tree. Also, the C API seems ... not quite what is needed
here. I realize that's not really anything to do with the best way to
describe the hardware, but that, plus the fact that the serial core
already handles a number of gpios controlling circuitry related to
rs485, was what made me go for one extra gpio.

How would a mux-consumer description look?

mux-states = <&mux 0>, <&mux 1>;
mux-state-names = "rs485", "rs232";

or should that be mux-controls? Would that be enough so that we're sure
that if and when a rs422 state is needed that could easily be
represented here?

Now implementation-wise, there's the complication that switching the mux
to/from rs485 mode must be done after/before the driver's ->rs485_config
is called, to avoid the transceiver temporarily being activated (thus
blocking/disturbing other traffic). That plus the need to mux_*_deselect
the old mode means the consumer (serial core in this case) ends up with
quite a lot of bookkeeping, and even more so taking error path into
consideration.

Rasmus

2023-11-25 23:40:51

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

Hi,

On 22.11.23 at 15:53, Lukas Wunner wrote:
> On Mon, Nov 20, 2023 at 04:10:54PM +0100, Rasmus Villemoes wrote:
>> Some boards are capable of both rs232 and rs485, and control which
>> external terminals are active via a gpio-controlled mux. Allow
>> describing that gpio in DT so that the kernel can transparently handle
>> the proper setting when the uart is switched between rs232 and rs485
>> modes.
>
> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
> to struct serial_rs485:
>
> https://lore.kernel.org/all/[email protected]/
>

That new flag was suggested by me instead of using SER_RS422_ENABLED, which
would mostly be redundant to SER_RS485_ENABLED.
I dont know if it is a good choice in the long term to handle both modes within
the RS485 configuration. It would be cleaner to have an own RS422 structure with
its own flags and properties. And until now the only flag that seems to make sense
for both RS422 and RS485 is AFAICS SER_RS485_TERMINATE_BUS.

On the other hand the bus termination is at least a property that both modes have
in common. And handling RS422 in its own structure would require another ioctl
to set and get the the RS422 settings.

But maybe there are more or better possibilities to handle RS4822 support. I would like to
hear other ideas.



> I don't know whether that makes sense at all (I had thought RS-422 is
> the same as RS-485 with full-duplex, i.e. SER_RS485_ENABLED plus
> SER_RS485_RX_DURING_TX)
>
> But if that patch gets accepted, we'd have *three* different modes:
> RS-232, RS-485, RS-422.

Actually we would have four (as Brenda already wrote,
see https://lore.kernel.org/all/[email protected]/),
and with the propose SER_RS485_MODE_RS422 flag these modes would be used like

RS-232: rs485->flags = 0
RS-422: rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_RS422
RS-485 (2-wire half-duplex): rs485->flags = SER_RS485_ENABLED
RS-485 (4-wire full-duplex): rs485->flags = SER_RS485_ENABLED|SER_RS485_RX_DURING_TX


> A single GPIO seems insufficient to handle that.

GPIOs for RS485 is another thing.

I mean, currently we have a GPIO for RS485 termination (I introduced it with commit 44b27aec9d9680875).
Christoph introduced support for a rx-during-tx GPIO (see commit 163f080eb717). Tomas intends
to add a GPIO which enables RS485 if asserted
(see https://lore.kernel.org/all/3Za.ZZs%[email protected]/) and with Rasmus patches
we are about to add a MUX-GPIO which is to be asserted if RS485 is enabled.

I wonder where this will end and if we really have to support every possible GPIO
in the serial core.

Regards,
Lino

2023-11-27 12:21:31

by Christoph Niedermaier

[permalink] [raw]
Subject: RE: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

From: Lino Sanfilippo [mailto:[email protected]]
Sent: Sunday, November 26, 2023 12:40 AM

Hi,

> On 22.11.23 at 15:53, Lukas Wunner wrote:
>> On Mon, Nov 20, 2023 at 04:10:54PM +0100, Rasmus Villemoes wrote:
>>> Some boards are capable of both rs232 and rs485, and control which
>>> external terminals are active via a gpio-controlled mux. Allow
>>> describing that gpio in DT so that the kernel can transparently handle
>>> the proper setting when the uart is switched between rs232 and rs485
>>> modes.
>>
>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
>> to struct serial_rs485:
>>
>> https://lore.kernel.org/all/[email protected]/
>>
>
> That new flag was suggested by me instead of using SER_RS422_ENABLED, which
> would mostly be redundant to SER_RS485_ENABLED.
> I dont know if it is a good choice in the long term to handle both modes within
> the RS485 configuration. It would be cleaner to have an own RS422 structure with
> its own flags and properties. And until now the only flag that seems to make sense
> for both RS422 and RS485 is AFAICS SER_RS485_TERMINATE_BUS.
>
> On the other hand the bus termination is at least a property that both modes have
> in common. And handling RS422 in its own structure would require another ioctl
> to set and get the the RS422 settings.
>
> But maybe there are more or better possibilities to handle RS4822 support. I would like to
> hear other ideas.
>
>
>
>> I don't know whether that makes sense at all (I had thought RS-422 is
>> the same as RS-485 with full-duplex, i.e. SER_RS485_ENABLED plus
>> SER_RS485_RX_DURING_TX)

With RS-485 full duplex, SER_RS485_RX_DURING_TX makes no sense to me.
See below.

>>
>> But if that patch gets accepted, we'd have *three* different modes:
>> RS-232, RS-485, RS-422.
>
> Actually we would have four (as Brenda already wrote,
> see https://lore.kernel.org/all/[email protected]/),
> and with the propose SER_RS485_MODE_RS422 flag these modes would be used like
>
> RS-232: rs485->flags = 0
> RS-422: rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_RS422
> RS-485 (2-wire half-duplex): rs485->flags = SER_RS485_ENABLED
> RS-485 (4-wire full-duplex): rs485->flags = SER_RS485_ENABLED|SER_RS485_RX_DURING_TX

In my point of view there are also two different modes for the RS-485 2-wire
half-duplex bus depending on the flag SER_RS485_RX_DURING_TX.
- SER_RS485_RX_DURING_TX is not set: The device doesn't see the bus during sending
(RX is off during sending).
- SER_RS485_RX_DURING_TX is set: The device see want is on bus during sending
(RX is also on during sending), so you can
see your transmission and also if another bus
device is transmitting at the same time.

On RS-485 4-wire TX and RX are separated by wires. So the definition of
SER_RS485_RX_DURING_TX above makes no sense, because you can receive all the time
without worrying about TX. On the software side RS-485 4-wire full duplex it behaves
like RS-232. So we don't need transceiver controlling by the RTS pin.

Basically for me the SER_RS485_ENABLED flag is to enable the RTS control for the
transceiver. Maybe on software side we can distinguish between half and full duplex
mode and whether RX is enabled during sending by the flag SER_RS485_RX_DURING_TX:
RS-232: rs485->flags = 0
RS-422 / RS-485 (4-wire): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_FULL_DUPLEX
RS-485 (2-wire NO RX_DURING_TX): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_HALF_DUPLEX
RS-485 (2-wire RX_DURING_TX): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_HALF_DUPLEX|SER_RS485_RX_DURING_TX

SER_RS485_MODE_FULL_DUPLEX and SER_RS485_MODE_HALF_DUPLEX can be defined at the
same bit. If SER_RS485_MODE_HALF_DUPLEX will be defined as 0 it breaks nothing.
With SER_RS485_MODE_FULL_DUPLEX, the RTS pin does not need to be controlled.

>
>> A single GPIO seems insufficient to handle that.
>
> GPIOs for RS485 is another thing.
>
> I mean, currently we have a GPIO for RS485 termination (I introduced it with commit
> 44b27aec9d9680875).
> Christoph introduced support for a rx-during-tx GPIO (see commit 163f080eb717). Tomas
> intends
> to add a GPIO which enables RS485 if asserted
> (see https://lore.kernel.org/all/3Za.ZZs%[email protected]/) and with Rasmus
> patches
> we are about to add a MUX-GPIO which is to be asserted if RS485 is enabled.
>
> I wonder where this will end and if we really have to support every possible GPIO
> in the serial core.

I think the GPIOs reflect the flag states and are meaningful:
- SER_RS485_TERMINATE_BUS: Switch bus termination on/off by GPIO
- SER_RS485_RX_DURING_TX: Used to stop RX during TX in hardware by GPIO (for 2-wire)
- SER_RS485_ENABLED: Muxing between RS-232 and RS-485 by GPIO

Switching RS-485 on during boot could also be handled by a devicetree overlay. Evaluate the
GPIO and load a DTO accordingly before booting.


Regards
Christoph

2023-12-04 05:01:16

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 2/2] serial: core: implement support for rs485-mux-gpios

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/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20231203/[email protected]/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 <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

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 }

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-12-06 15:43:31

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding


Hi,

On 27.11.23 13:14, Christoph Niedermaier wrote:
> From: Lino Sanfilippo [mailto:[email protected]]
> Sent: Sunday, November 26, 2023 12:40 AM
>
> Hi,
>
>> On 22.11.23 at 15:53, Lukas Wunner wrote:
>>> On Mon, Nov 20, 2023 at 04:10:54PM +0100, Rasmus Villemoes wrote:
>>>> Some boards are capable of both rs232 and rs485, and control which
>>>> external terminals are active via a gpio-controlled mux. Allow
>>>> describing that gpio in DT so that the kernel can transparently handle
>>>> the proper setting when the uart is switched between rs232 and rs485
>>>> modes.
>>>
>>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
>>> to struct serial_rs485:
>>>
>>> https://lore.kernel.org/all/[email protected]/
>>>
>>
>> That new flag was suggested by me instead of using SER_RS422_ENABLED, which
>> would mostly be redundant to SER_RS485_ENABLED.
>> I dont know if it is a good choice in the long term to handle both modes within
>> the RS485 configuration. It would be cleaner to have an own RS422 structure with
>> its own flags and properties. And until now the only flag that seems to make sense
>> for both RS422 and RS485 is AFAICS SER_RS485_TERMINATE_BUS.
>>
>> On the other hand the bus termination is at least a property that both modes have
>> in common. And handling RS422 in its own structure would require another ioctl
>> to set and get the the RS422 settings.
>>
>> But maybe there are more or better possibilities to handle RS4822 support. I would like to
>> hear other ideas.
>>
>>
>>
>>> I don't know whether that makes sense at all (I had thought RS-422 is
>>> the same as RS-485 with full-duplex, i.e. SER_RS485_ENABLED plus
>>> SER_RS485_RX_DURING_TX)
>
> With RS-485 full duplex, SER_RS485_RX_DURING_TX makes no sense to me.
> See below.
>
>>>
>>> But if that patch gets accepted, we'd have *three* different modes:
>>> RS-232, RS-485, RS-422.
>>
>> Actually we would have four (as Brenda already wrote,
>> see https://lore.kernel.org/all/[email protected]/),
>> and with the propose SER_RS485_MODE_RS422 flag these modes would be used like
>>
>> RS-232: rs485->flags = 0
>> RS-422: rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_RS422
>> RS-485 (2-wire half-duplex): rs485->flags = SER_RS485_ENABLED
>> RS-485 (4-wire full-duplex): rs485->flags = SER_RS485_ENABLED|SER_RS485_RX_DURING_TX
>
> In my point of view there are also two different modes for the RS-485 2-wire
> half-duplex bus depending on the flag SER_RS485_RX_DURING_TX.
> - SER_RS485_RX_DURING_TX is not set: The device doesn't see the bus during sending
> (RX is off during sending).
> - SER_RS485_RX_DURING_TX is set: The device see want is on bus during sending
> (RX is also on during sending), so you can
> see your transmission and also if another bus
> device is transmitting at the same time.
>
> On RS-485 4-wire TX and RX are separated by wires. So the definition of
> SER_RS485_RX_DURING_TX above makes no sense, because you can receive all the time
> without worrying about TX. On the software side RS-485 4-wire full duplex it behaves
> like RS-232. So we don't need transceiver controlling by the RTS pin.
>> Basically for me the SER_RS485_ENABLED flag is to enable the RTS control for the
> transceiver. Maybe on software side we can distinguish between half and full duplex
> mode and whether RX is enabled during sending by the flag SER_RS485_RX_DURING_TX:
> RS-232: rs485->flags = 0
> RS-422 / RS-485 (4-wire): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_FULL_DUPLEX

How can we switch between RS485 (4-wire) and RS422 then? AFAIU they are not the same. And even
if a driver behaves the same in both modes it needs to know when to switch from one mode to the
other.

> RS-485 (2-wire NO RX_DURING_TX): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_HALF_DUPLEX
> RS-485 (2-wire RX_DURING_TX): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_HALF_DUPLEX|SER_RS485_RX_DURING_TX

I think we can omit the SER_RS485_MODE_HALF_DUPLEX flag if we assume that a missing
SER_RS485_MODE_FULL_DUPLEX means half duplex (i.e. controlling the RTS line).

> SER_RS485_MODE_FULL_DUPLEX and SER_RS485_MODE_HALF_DUPLEX can be defined at the
> same bit. If SER_RS485_MODE_HALF_DUPLEX will be defined as 0 it breaks nothing.
> With SER_RS485_MODE_FULL_DUPLEX, the RTS pin does not need to be controlled
>
>>
>>> A single GPIO seems insufficient to handle that.
>>
>> GPIOs for RS485 is another thing.
>>
>> I mean, currently we have a GPIO for RS485 termination (I introduced it with commit
>> 44b27aec9d9680875).
>> Christoph introduced support for a rx-during-tx GPIO (see commit 163f080eb717). Tomas
>> intends
>> to add a GPIO which enables RS485 if asserted
>> (see https://lore.kernel.org/all/3Za.ZZs%[email protected]/) and with Rasmus
>> patches
>> we are about to add a MUX-GPIO which is to be asserted if RS485 is enabled.
>>
>> I wonder where this will end and if we really have to support every possible GPIO
>> in the serial core.
>
> I think the GPIOs reflect the flag states and are meaningful:
> - SER_RS485_TERMINATE_BUS: Switch bus termination on/off by GPIO
> - SER_RS485_RX_DURING_TX: Used to stop RX during TX in hardware by GPIO (for 2-wire)
> - SER_RS485_ENABLED: Muxing between RS-232 and RS-485 by GPIO
>
> Switching RS-485 on during boot could also be handled by a devicetree overlay. Evaluate the
> GPIO and load a DTO accordingly before booting.
>

Regards,
Lino

2023-12-07 12:36:09

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On Wed, Dec 06, 2023 at 04:42:53PM +0100, Lino Sanfilippo wrote:
> On 27.11.23 13:14, Christoph Niedermaier wrote:
> > From: Lino Sanfilippo [mailto:[email protected]]
> > Sent: Sunday, November 26, 2023 12:40 AM

...

> > RS-485 (2-wire NO RX_DURING_TX): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_HALF_DUPLEX
> > RS-485 (2-wire RX_DURING_TX): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_HALF_DUPLEX|SER_RS485_RX_DURING_TX
>
> I think we can omit the SER_RS485_MODE_HALF_DUPLEX flag if we assume that
> a missing SER_RS485_MODE_FULL_DUPLEX means half duplex (i.e. controlling
> the RTS line).

You should be very careful on these assumptions, i.e. one must to check _all_
existing user space tools (at least that are in use / supplied by main distros)
on how they behave.

--
With Best Regards,
Andy Shevchenko


2023-12-09 11:26:47

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

Hi Andy,

On 07.12.23 13:35, Andy Shevchenko wrote:
> On Wed, Dec 06, 2023 at 04:42:53PM +0100, Lino Sanfilippo wrote:
>> On 27.11.23 13:14, Christoph Niedermaier wrote:
>>> From: Lino Sanfilippo [mailto:[email protected]]
>>> Sent: Sunday, November 26, 2023 12:40 AM
>
> ...
>
>>> RS-485 (2-wire NO RX_DURING_TX): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_HALF_DUPLEX
>>> RS-485 (2-wire RX_DURING_TX): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_HALF_DUPLEX|SER_RS485_RX_DURING_TX
>>
>> I think we can omit the SER_RS485_MODE_HALF_DUPLEX flag if we assume that
>> a missing SER_RS485_MODE_FULL_DUPLEX means half duplex (i.e. controlling
>> the RTS line).
>
> You should be very careful on these assumptions, i.e. one must to check _all_
> existing user space tools (at least that are in use / supplied by main distros)
> on how they behave.
>

Until now the DUPLEX flags do not yet exist, so existing userspace applications are not
concerned. Christoph suggested to introduce two flags to distinguish between
a FULL duplex and a half duplex RS485 mode. My point was that we do not need the flag
for half duplex, since this would be the default.

BR,
Lino

2023-12-09 11:48:39

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding



On 06.12.23 16:42, Lino Sanfilippo wrote:
>

>>>>
>>>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
>>>> to struct serial_rs485:
>>>>
>>>> https://lore.kernel.org/all/[email protected]/
>>>>
>>>
>>> That new flag was suggested by me instead of using SER_RS422_ENABLED, which
>>> would mostly be redundant to SER_RS485_ENABLED.

A cleaner solution would probably be to not handle RS422 with the RS485 settings at
all, but to introduce another set of ioctls to set and read it.

An own RS422 structure like

struct serial_rs422 {
__u32 flags;
#define SER_RS422_ENABLED (1 << 0)
#define SER_RS422_TERMINATE_BUS (1 << 1)
};


could be used as the parameter for these new ioctls.

Any comments on this?


Regards,
Lino

2023-12-11 13:08:24

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On Sat, Dec 09, 2023 at 12:47:47PM +0100, Lino Sanfilippo wrote:
> On 06.12.23 16:42, Lino Sanfilippo wrote:

> >>>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
> >>>> to struct serial_rs485:
> >>>>
> >>>> https://lore.kernel.org/all/[email protected]/
> >>>>
> >>>
> >>> That new flag was suggested by me instead of using SER_RS422_ENABLED, which
> >>> would mostly be redundant to SER_RS485_ENABLED.
>
> A cleaner solution would probably be to not handle RS422 with the RS485 settings at
> all, but to introduce another set of ioctls to set and read it.
>
> An own RS422 structure like
>
> struct serial_rs422 {
> __u32 flags;
> #define SER_RS422_ENABLED (1 << 0)
> #define SER_RS422_TERMINATE_BUS (1 << 1)
> };
>
>
> could be used as the parameter for these new ioctls.
>
> Any comments on this?

I have (maybe not so constructive) a comment. Please, at all means try to not
extend the existing serial data structures, we have too many ones with too many
fields already. For user space, though, one may use unions and flags, but for
internal ones it might be better ways, I think.

--
With Best Regards,
Andy Shevchenko


2023-12-14 08:52:57

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

Hi,

On 11.12.23 14:07, Andy Shevchenko wrote:
> On Sat, Dec 09, 2023 at 12:47:47PM +0100, Lino Sanfilippo wrote:
>> On 06.12.23 16:42, Lino Sanfilippo wrote:
>
>>>>>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
>>>>>> to struct serial_rs485:
>>>>>>
>>>>>> https://lore.kernel.org/all/[email protected]/
>>>>>>
>>>>>
>>>>> That new flag was suggested by me instead of using SER_RS422_ENABLED, which
>>>>> would mostly be redundant to SER_RS485_ENABLED.
>>
>> A cleaner solution would probably be to not handle RS422 with the RS485 settings at
>> all, but to introduce another set of ioctls to set and read it.
>>
>> An own RS422 structure like
>>
>> struct serial_rs422 {
>> __u32 flags;
>> #define SER_RS422_ENABLED (1 << 0)
>> #define SER_RS422_TERMINATE_BUS (1 << 1)
>> };
>>
>>
>> could be used as the parameter for these new ioctls.
>>
>> Any comments on this?
>
> I have (maybe not so constructive) a comment. Please, at all means try to not
> extend the existing serial data structures, we have too many ones with too many
> fields already. For user space, though, one may use unions and flags, but for
> internal ones it might be better ways, I think.
>

Ok, thanks. This is still a valuable information. So what if the above structure (serial_rs422)
is only used as a parameter of a new TIOCSRS422 ioctl and only internally we set a SER_RS485_MODE_RS422
flag in the serial_rs485 struct?
So we do not have to add something new to uart_port but also do not expose the mixture of RS485 and RS422
settings within the serial_rs485 structure to userspace.

Regards,
Lino

2023-12-14 10:25:03

by Crescent CY Hsieh

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On Mon, Dec 11, 2023 at 03:07:59PM +0200, Andy Shevchenko wrote:
> On Sat, Dec 09, 2023 at 12:47:47PM +0100, Lino Sanfilippo wrote:
> > On 06.12.23 16:42, Lino Sanfilippo wrote:
>
> > >>>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
> > >>>> to struct serial_rs485:
> > >>>>
> > >>>> https://lore.kernel.org/all/[email protected]/
> > >>>>
> > >>>
> > >>> That new flag was suggested by me instead of using SER_RS422_ENABLED, which
> > >>> would mostly be redundant to SER_RS485_ENABLED.
> >
> > A cleaner solution would probably be to not handle RS422 with the RS485 settings at
> > all, but to introduce another set of ioctls to set and read it.
> >
> > An own RS422 structure like
> >
> > struct serial_rs422 {
> > __u32 flags;
> > #define SER_RS422_ENABLED (1 << 0)
> > #define SER_RS422_TERMINATE_BUS (1 << 1)
> > };
> >
> >
> > could be used as the parameter for these new ioctls.
> >
> > Any comments on this?
>
> I have (maybe not so constructive) a comment. Please, at all means try to not
> extend the existing serial data structures, we have too many ones with too many
> fields already. For user space, though, one may use unions and flags, but for
> internal ones it might be better ways, I think.

How about revising the name of 'TIOCSRS485' and 'serial_rs485' to a
general one, and put RS422 and RS485 configuration flags into that
structure?

So that in userspace it could set RS422 or RS485 configurations using a
single ioctl command and one structure.

In this way, it won't be confused in userspace and won't add new data
structure internally as well.

---
Sincerely,
Crescent Hsieh.

2023-12-14 13:43:27

by Christoph Niedermaier

[permalink] [raw]
Subject: RE: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

From: Crescent CY Hsieh <[email protected]>
Sent: Thursday, December 14, 2023 11:25 AM
> On Mon, Dec 11, 2023 at 03:07:59PM +0200, Andy Shevchenko wrote:
>> On Sat, Dec 09, 2023 at 12:47:47PM +0100, Lino Sanfilippo wrote:
>>> On 06.12.23 16:42, Lino Sanfilippo wrote:
>>
>>>>>>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
>>>>>>> to struct serial_rs485:
>>>>>>>
>>>>>>> https://lore.kernel.org/all/[email protected]/
>>>>>>>
>>>>>>
>>>>>> That new flag was suggested by me instead of using SER_RS422_ENABLED, which
>>>>>> would mostly be redundant to SER_RS485_ENABLED.
>>>
>>> A cleaner solution would probably be to not handle RS422 with the RS485 settings at
>>> all, but to introduce another set of ioctls to set and read it.
>>>
>>> An own RS422 structure like
>>>
>>> struct serial_rs422 {
>>> __u32 flags;
>>> #define SER_RS422_ENABLED (1 << 0)
>>> #define SER_RS422_TERMINATE_BUS (1 << 1)
>>> };
>>>
>>>
>>> could be used as the parameter for these new ioctls.
>>>
>>> Any comments on this?
>>
>> I have (maybe not so constructive) a comment. Please, at all means try to not
>> extend the existing serial data structures, we have too many ones with too many
>> fields already. For user space, though, one may use unions and flags, but for
>> internal ones it might be better ways, I think.
>
> How about revising the name of 'TIOCSRS485' and 'serial_rs485' to a
> general one, and put RS422 and RS485 configuration flags into that
> structure?
>
> So that in userspace it could set RS422 or RS485 configurations using a
> single ioctl command and one structure.
>
> In this way, it won't be confused in userspace and won't add new data
> structure internally as well.
>

I will summarize the current situation from my point of view, maybe it helps:

RS-232:
- Full Duplex Point-to-Point connection
- No transceiver control with RTS
- No termination
- No extra struct in use

RS-422:
- Full Duplex Point-to-Point connection
- No transceiver control with RTS needed
- Termination possible
- Extra struct serial_rs485 needed if termination is used
=> RS-422 can be used in RS-232 operation, but if a termination should be
switchable the RS485 flag has to be enabled. But then also transceiver
control will be enabled. Not a very satisfying situation.

RS-485 (2-wire) very common:
- Half Duplex RS-485 bus
- Transceiver control with RTS is needed
- Termination possible
- Extra struct serial_rs485 is needed
=> RS-485 has to be enabled and configured:
- Set SER_RS485_ENABLED
- Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
- Set/clear SER_RS485_RX_DURING_TX depending on whether
the receiver path should be on or off during sending.
If it's set it allows to monitor the sending on the bus
and detect whether another bus device is transmitting
at the same time.
- Set/clear SER_RS485_TERMINATE_BUS for bus termination.

RS-485 (4-wire) little used:
- Full Duplex RS-485 bus
- Transceiver control with RTS is needed
- Termination possible
- Extra struct serial_rs485 is needed
=> RS-485 has to be enabled and configured:
- Set SER_RS485_ENABLED
- Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
- Set SER_RS485_RX_DURING_TX, as the receiver should always
be enabled independently of TX, because TX and RX are
separated from each other by their own wires.
- Set/clear SER_RS485_TERMINATE_BUS for bus termination.

I think the GPIOs reflect the flag states and are meaningful:
- SER_RS485_TERMINATE_BUS: Switch bus termination on/off by GPIO
- SER_RS485_RX_DURING_TX: Used to enable/disable RX during TX
in hardware by GPIO (for 2-wire)
- SER_RS485_ENABLED: Muxing between RS-232 and RS-485 by GPIO

Switching RS-485 on during boot could also be handled by a devicetree
overlay. Evaluate the GPIO and load a DTO accordingly before booting.

Please correct me if I have misrepresented something...

If I looked at it in this new way, I would discard my idea with the
FULL_DUPLEX and HALF_DUPLEX. For a better use of RS-422 it would be
good to disable transceiver control via RTS. It can be done by clearing
the existing flags SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND
at the same time, but I think it is confusing. Better would be a flag
for RS-422:

RS-422: Set SER_RS422_MODE for disabling
transceiver control via RTS.
RS-485 (2-wire and 4-wire): Clear SER_RS422_MODE for enabling
transceiver control via RTS.

Finally, at present it is also not possible to distinguish between RS485
2-wire and 4-wire operation. I think it isn't necessary, as different
hardware has to be used anyway. The hardware then determines the
configuration, see above.


Regards
Christoph

2023-12-14 14:05:24

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding


Hi,

On 14.12.23 14:41, Christoph Niedermaier wrote:
> From: Crescent CY Hsieh <[email protected]>
> Sent: Thursday, December 14, 2023 11:25 AM
>> On Mon, Dec 11, 2023 at 03:07:59PM +0200, Andy Shevchenko wrote:
>>> On Sat, Dec 09, 2023 at 12:47:47PM +0100, Lino Sanfilippo wrote:
>>>> On 06.12.23 16:42, Lino Sanfilippo wrote:
>>>
>>>>>>>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
>>>>>>>> to struct serial_rs485:
>>>>>>>>
>>>>>>>> https://lore.kernel.org/all/[email protected]/
>>>>>>>>
>>>>>>>
>>>>>>> That new flag was suggested by me instead of using SER_RS422_ENABLED, which
>>>>>>> would mostly be redundant to SER_RS485_ENABLED.
>>>>
>>>> A cleaner solution would probably be to not handle RS422 with the RS485 settings at
>>>> all, but to introduce another set of ioctls to set and read it.
>>>>
>>>> An own RS422 structure like
>>>>
>>>> struct serial_rs422 {
>>>> __u32 flags;
>>>> #define SER_RS422_ENABLED (1 << 0)
>>>> #define SER_RS422_TERMINATE_BUS (1 << 1)
>>>> };
>>>>
>>>>
>>>> could be used as the parameter for these new ioctls.
>>>>
>>>> Any comments on this?
>>>
>>> I have (maybe not so constructive) a comment. Please, at all means try to not
>>> extend the existing serial data structures, we have too many ones with too many
>>> fields already. For user space, though, one may use unions and flags, but for
>>> internal ones it might be better ways, I think.
>>
>> How about revising the name of 'TIOCSRS485' and 'serial_rs485' to a
>> general one, and put RS422 and RS485 configuration flags into that
>> structure?
>>
>> So that in userspace it could set RS422 or RS485 configurations using a
>> single ioctl command and one structure.
>>
>> In this way, it won't be confused in userspace and won't add new data
>> structure internally as well.
>>
>
> I will summarize the current situation from my point of view, maybe it helps:
>
> RS-232:
> - Full Duplex Point-to-Point connection
> - No transceiver control with RTS
> - No termination
> - No extra struct in use
>
> RS-422:
> - Full Duplex Point-to-Point connection
> - No transceiver control with RTS needed
> - Termination possible
> - Extra struct serial_rs485 needed if termination is used
> => RS-422 can be used in RS-232 operation, but if a termination should be
> switchable the RS485 flag has to be enabled. But then also transceiver
> control will be enabled. Not a very satisfying situation.
>

Thats why I vote for a RS422 mode.

> RS-485 (2-wire) very common:
> - Half Duplex RS-485 bus
> - Transceiver control with RTS is needed
> - Termination possible
> - Extra struct serial_rs485 is needed
> => RS-485 has to be enabled and configured:
> - Set SER_RS485_ENABLED
> - Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
> - Set/clear SER_RS485_RX_DURING_TX depending on whether
> the receiver path should be on or off during sending.
> If it's set it allows to monitor the sending on the bus
> and detect whether another bus device is transmitting
> at the same time.
> - Set/clear SER_RS485_TERMINATE_BUS for bus termination.
>
> RS-485 (4-wire) little used:
> - Full Duplex RS-485 bus
> - Transceiver control with RTS is needed
> - Termination possible
> - Extra struct serial_rs485 is needed
> => RS-485 has to be enabled and configured:
> - Set SER_RS485_ENABLED
> - Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
> - Set SER_RS485_RX_DURING_TX, as the receiver should always
> be enabled independently of TX, because TX and RX are
> separated from each other by their own wires.
> - Set/clear SER_RS485_TERMINATE_BUS for bus termination.

How can the driver distinguish between RS485 full duplex and half duplex then?
In full duplex RTS control is not needed AFAIU.

>
> I think the GPIOs reflect the flag states and are meaningful:
> - SER_RS485_TERMINATE_BUS: Switch bus termination on/off by GPIO
> - SER_RS485_RX_DURING_TX: Used to enable/disable RX during TX
> in hardware by GPIO (for 2-wire)
> - SER_RS485_ENABLED: Muxing between RS-232 and RS-485 by GPIO
>
> Switching RS-485 on during boot could also be handled by a devicetree
> overlay. Evaluate the GPIO and load a DTO accordingly before booting.
>
> Please correct me if I have misrepresented something...
>
> If I looked at it in this new way, I would discard my idea with the
> FULL_DUPLEX and HALF_DUPLEX. For a better use of RS-422 it would be
> good to disable transceiver control via RTS. It can be done by clearing
> the existing flags SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND
> at the same time, but I think it is confusing. Better would be a flag
> for RS-422:
>
> RS-422: Set SER_RS422_MODE for disabling
> transceiver control via RTS.
> RS-485 (2-wire and 4-wire): Clear SER_RS422_MODE for enabling
> transceiver control via RTS.
>
> Finally, at present it is also not possible to distinguish between RS485
> 2-wire and 4-wire operation. I think it isn't necessary, as different
> hardware has to be used anyway. The hardware then determines the
> configuration, see above.

But the driver should somehow be informed that there exists a full
duplex hardware setup, so that it does not need to control the RTS line.
Maybe by means of a device tree property?

Regards,
Lino


>
>
> Regards
> Christoph

2023-12-14 14:52:36

by Christoph Niedermaier

[permalink] [raw]
Subject: RE: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

From: Lino Sanfilippo <[email protected]>
Sent: Thursday, December 14, 2023 3:04 PM
>
> Hi,
>
> On 14.12.23 14:41, Christoph Niedermaier wrote:
>> From: Crescent CY Hsieh <[email protected]>
>> Sent: Thursday, December 14, 2023 11:25 AM
>>> On Mon, Dec 11, 2023 at 03:07:59PM +0200, Andy Shevchenko wrote:
>>>> On Sat, Dec 09, 2023 at 12:47:47PM +0100, Lino Sanfilippo wrote:
>>>>> On 06.12.23 16:42, Lino Sanfilippo wrote:
>>>>
>>>>>>>>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
>>>>>>>>> to struct serial_rs485:
>>>>>>>>>
>>>>>>>>> https://lore.kernel.org/all/[email protected]/
>>>>>>>>>
>>>>>>>>
>>>>>>>> That new flag was suggested by me instead of using SER_RS422_ENABLED, which
>>>>>>>> would mostly be redundant to SER_RS485_ENABLED.
>>>>>
>>>>> A cleaner solution would probably be to not handle RS422 with the RS485 settings at
>>>>> all, but to introduce another set of ioctls to set and read it.
>>>>>
>>>>> An own RS422 structure like
>>>>>
>>>>> struct serial_rs422 {
>>>>> __u32 flags;
>>>>> #define SER_RS422_ENABLED (1 << 0)
>>>>> #define SER_RS422_TERMINATE_BUS (1 << 1)
>>>>> };
>>>>>
>>>>>
>>>>> could be used as the parameter for these new ioctls.
>>>>>
>>>>> Any comments on this?
>>>>
>>>> I have (maybe not so constructive) a comment. Please, at all means try to not
>>>> extend the existing serial data structures, we have too many ones with too many
>>>> fields already. For user space, though, one may use unions and flags, but for
>>>> internal ones it might be better ways, I think.
>>>
>>> How about revising the name of 'TIOCSRS485' and 'serial_rs485' to a
>>> general one, and put RS422 and RS485 configuration flags into that
>>> structure?
>>>
>>> So that in userspace it could set RS422 or RS485 configurations using a
>>> single ioctl command and one structure.
>>>
>>> In this way, it won't be confused in userspace and won't add new data
>>> structure internally as well.
>>>
>>
>> I will summarize the current situation from my point of view, maybe it helps:
>>
>> RS-232:
>> - Full Duplex Point-to-Point connection
>> - No transceiver control with RTS
>> - No termination
>> - No extra struct in use
>>
>> RS-422:
>> - Full Duplex Point-to-Point connection
>> - No transceiver control with RTS needed
>> - Termination possible
>> - Extra struct serial_rs485 needed if termination is used
>> => RS-422 can be used in RS-232 operation, but if a termination should be
>> switchable the RS485 flag has to be enabled. But then also transceiver
>> control will be enabled. Not a very satisfying situation.
>>
>
> Thats why I vote for a RS422 mode.
>
>> RS-485 (2-wire) very common:
>> - Half Duplex RS-485 bus
>> - Transceiver control with RTS is needed
>> - Termination possible
>> - Extra struct serial_rs485 is needed
>> => RS-485 has to be enabled and configured:
>> - Set SER_RS485_ENABLED
>> - Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
>> - Set/clear SER_RS485_RX_DURING_TX depending on whether
>> the receiver path should be on or off during sending.
>> If it's set it allows to monitor the sending on the bus
>> and detect whether another bus device is transmitting
>> at the same time.
>> - Set/clear SER_RS485_TERMINATE_BUS for bus termination.
>>
>> RS-485 (4-wire) little used:
>> - Full Duplex RS-485 bus
>> - Transceiver control with RTS is needed
>> - Termination possible
>> - Extra struct serial_rs485 is needed
>> => RS-485 has to be enabled and configured:
>> - Set SER_RS485_ENABLED
>> - Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
>> - Set SER_RS485_RX_DURING_TX, as the receiver should always
>> be enabled independently of TX, because TX and RX are
>> separated from each other by their own wires.
>> - Set/clear SER_RS485_TERMINATE_BUS for bus termination.
>
> How can the driver distinguish between RS485 full duplex and half duplex then?
> In full duplex RTS control is not needed AFAIU.

I think we don't need to distinguish, because for a full duplex RS-485
transceiver also needs RTS control. For example look at the full duplex
RS-485 transceiver ADM3491E [1]. It's a full duplex transceiver (A/B and Z/Y)
that has DE (Driver enable) and DI (Driver Input) pins for controlling TX. I
think the RS-485 master doesn't need it. The DE pin could also be set
permanently high. But if we have more than one RS-485 slaves it's needed to
avoid blocking of each other on the receiving wires of the RS-485 master.

[1] https://www.analog.com/en/products/adm3491e.html

>> I think the GPIOs reflect the flag states and are meaningful:
>> - SER_RS485_TERMINATE_BUS: Switch bus termination on/off by GPIO
>> - SER_RS485_RX_DURING_TX: Used to enable/disable RX during TX
>> in hardware by GPIO (for 2-wire)
>> - SER_RS485_ENABLED: Muxing between RS-232 and RS-485 by GPIO
>>
>> Switching RS-485 on during boot could also be handled by a devicetree
>> overlay. Evaluate the GPIO and load a DTO accordingly before booting.
>>
>> Please correct me if I have misrepresented something...
>>
>> If I looked at it in this new way, I would discard my idea with the
>> FULL_DUPLEX and HALF_DUPLEX. For a better use of RS-422 it would be
>> good to disable transceiver control via RTS. It can be done by clearing
>> the existing flags SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND
>> at the same time, but I think it is confusing. Better would be a flag
>> for RS-422:
>>
>> RS-422: Set SER_RS422_MODE for disabling
>> transceiver control via RTS.
>> RS-485 (2-wire and 4-wire): Clear SER_RS422_MODE for enabling
>> transceiver control via RTS.
>>
>> Finally, at present it is also not possible to distinguish between RS485
>> 2-wire and 4-wire operation. I think it isn't necessary, as different
>> hardware has to be used anyway. The hardware then determines the
>> configuration, see above.
>
> But the driver should somehow be informed that there exists a full
> duplex hardware setup, so that it does not need to control the RTS line.
> Maybe by means of a device tree property?

See above.

Regards
Christoph

2023-12-15 22:14:36

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

Hi Christoph,

On 14.12.23 15:50, Christoph Niedermaier wrote:

>
> I think we don't need to distinguish, because for a full duplex RS-485
> transceiver also needs RTS control. For example look at the full duplex
> RS-485 transceiver ADM3491E [1]. It's a full duplex transceiver (A/B and Z/Y)
> that has DE (Driver enable) and DI (Driver Input) pins for controlling TX. I
> think the RS-485 master doesn't need it. The DE pin could also be set
> permanently high. But if we have more than one RS-485 slaves it's needed to
> avoid blocking of each other on the receiving wires of the RS-485 master.
>

Thanks for the explanation. So while still needed for the slaves, in case of the
RS485 master the RTS control is not needed, right? Is this something that userspace
should be able to configure? It could be set by clearing both the RTS_ON_SEND and
RTS_AFTER_SEND flag (only if the driver supports this special RS485 mode, of course).

Regards,
Lino



2023-12-18 09:17:47

by Christoph Niedermaier

[permalink] [raw]
Subject: RE: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

From: Lino Sanfilippo [mailto:[email protected]]
Sent: Friday, December 15, 2023 11:14 PM
>
> Hi Christoph,
>

Hi Lino,

> On 14.12.23 15:50, Christoph Niedermaier wrote:
>
>>
>> I think we don't need to distinguish, because for a full duplex RS-485
>> transceiver also needs RTS control. For example look at the full duplex
>> RS-485 transceiver ADM3491E [1]. It's a full duplex transceiver (A/B and Z/Y)
>> that has DE (Driver enable) and DI (Driver Input) pins for controlling TX. I
>> think the RS-485 master doesn't need it. The DE pin could also be set
>> permanently high. But if we have more than one RS-485 slaves it's needed to
>> avoid blocking of each other on the receiving wires of the RS-485 master.
>>
>
> Thanks for the explanation. So while still needed for the slaves, in case of the
> RS485 master the RTS control is not needed, right?

Yes, for the RS-485 (4-wire) master it isn't needed, but I think on the driver
level it is better not to distinguish between master and salve. So use also RTS
control for enabling sending on a RS-485 (4-wire) master device.

> Is this something that userspace should be able to configure? It could be set
> by clearing both the RTS_ON_SEND and RTS_AFTER_SEND flag (only if the driver
> supports this special RS485 mode, of course).

No, I think it shouldn't configure in userspace. Treating a RS-485 (4-wire) master
device in the driver as a slave (with RTS control) has the following advantages:
- Less confusion in user space (no additional setting available).
- Only bus topology determines who is master and salve.
- Save energy, because DE is only driven during sending.


Regards
Christoph

2023-12-21 15:53:20

by Lukas Wunner

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On Thu, Dec 14, 2023 at 01:41:47PM +0000, Christoph Niedermaier wrote:
> I will summarize the current situation from my point of view, maybe it helps:
>
> RS-232:
> - Full Duplex Point-to-Point connection
> - No transceiver control with RTS
> - No termination
> - No extra struct in use
>
> RS-422:
> - Full Duplex Point-to-Point connection
> - No transceiver control with RTS needed
> - Termination possible
> - Extra struct serial_rs485 needed if termination is used
> => RS-422 can be used in RS-232 operation, but if a termination should be
> switchable the RS485 flag has to be enabled. But then also transceiver
> control will be enabled. Not a very satisfying situation.

Well why don't we just allow enabling or disabling RS-485 termination
independently from the SER_RS485_ENABLED bit in struct serial_rs485?

Just let the user issue a TIOCSRS485 ioctl to toggle termination even
if in RS-232 mode and use that mode for RS-422.

Looks like the simplest solution to me.


> RS-485 (2-wire) very common:
> - Half Duplex RS-485 bus
> - Transceiver control with RTS is needed
> - Termination possible
> - Extra struct serial_rs485 is needed
> => RS-485 has to be enabled and configured:
> - Set SER_RS485_ENABLED
> - Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
> - Set/clear SER_RS485_RX_DURING_TX depending on whether
> the receiver path should be on or off during sending.
> If it's set it allows to monitor the sending on the bus
> and detect whether another bus device is transmitting
> at the same time.
> - Set/clear SER_RS485_TERMINATE_BUS for bus termination.
>
> RS-485 (4-wire) little used:
> - Full Duplex RS-485 bus
> - Transceiver control with RTS is needed
> - Termination possible
> - Extra struct serial_rs485 is needed
> => RS-485 has to be enabled and configured:
> - Set SER_RS485_ENABLED
> - Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
> - Set SER_RS485_RX_DURING_TX, as the receiver should always
> be enabled independently of TX, because TX and RX are
> separated from each other by their own wires.
> - Set/clear SER_RS485_TERMINATE_BUS for bus termination.

Thanks for that overview, I found it very helpful.

One small addendum: Hardware flow control. Only possible with
RS-232. Doesn't work in any of the other modes, right?

Thanks,

Lukas

2023-12-23 13:00:40

by Christoph Niedermaier

[permalink] [raw]
Subject: RE: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

From: Lukas Wunner [mailto:[email protected]]
Sent: Thursday, December 21, 2023 4:53 PM
>
> On Thu, Dec 14, 2023 at 01:41:47PM +0000, Christoph Niedermaier wrote:
>> I will summarize the current situation from my point of view, maybe it helps:
>>
>> RS-232:
>> - Full Duplex Point-to-Point connection
>> - No transceiver control with RTS
>> - No termination
>> - No extra struct in use
>>
>> RS-422:
>> - Full Duplex Point-to-Point connection
>> - No transceiver control with RTS needed
>> - Termination possible
>> - Extra struct serial_rs485 needed if termination is used
>> => RS-422 can be used in RS-232 operation, but if a termination should be
>> switchable the RS485 flag has to be enabled. But then also transceiver
>> control will be enabled. Not a very satisfying situation.
>
> Well why don't we just allow enabling or disabling RS-485 termination
> independently from the SER_RS485_ENABLED bit in struct serial_rs485?
>
> Just let the user issue a TIOCSRS485 ioctl to toggle termination even
> if in RS-232 mode and use that mode for RS-422.
>
> Looks like the simplest solution to me.

Sounds not bad. The termination should only depend on whether the GPIO is
given or not.

Irrespective of this, I think the Linos idea of an RS-422 mode is not bad.
This allows you to take care of special features that were previously
overlooked. For example, hardware flow control can be switched off so that
this does not cause any problems.

>> RS-485 (2-wire) very common:
>> - Half Duplex RS-485 bus
>> - Transceiver control with RTS is needed
>> - Termination possible
>> - Extra struct serial_rs485 is needed
>> => RS-485 has to be enabled and configured:
>> - Set SER_RS485_ENABLED
>> - Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
>> - Set/clear SER_RS485_RX_DURING_TX depending on whether
>> the receiver path should be on or off during sending.
>> If it's set it allows to monitor the sending on the bus
>> and detect whether another bus device is transmitting
>> at the same time.
>> - Set/clear SER_RS485_TERMINATE_BUS for bus termination.
>>
>> RS-485 (4-wire) little used:
>> - Full Duplex RS-485 bus
>> - Transceiver control with RTS is needed
>> - Termination possible
>> - Extra struct serial_rs485 is needed
>> => RS-485 has to be enabled and configured:
>> - Set SER_RS485_ENABLED
>> - Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
>> - Set SER_RS485_RX_DURING_TX, as the receiver should always
>> be enabled independently of TX, because TX and RX are
>> separated from each other by their own wires.
>> - Set/clear SER_RS485_TERMINATE_BUS for bus termination.
>
> Thanks for that overview, I found it very helpful.
>
> One small addendum: Hardware flow control. Only possible with
> RS-232. Doesn't work in any of the other modes, right?

You are right I forgot to mention it.

Regards and Merry Christmas
Christoph

2023-12-23 13:41:45

by Lino Sanfilippo

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding



On 23.12.23 13:49, Christoph Niedermaier wrote:
> From: Lukas Wunner [mailto:[email protected]]
> Sent: Thursday, December 21, 2023 4:53 PM
>>
>> On Thu, Dec 14, 2023 at 01:41:47PM +0000, Christoph Niedermaier wrote:
>>> I will summarize the current situation from my point of view, maybe it helps:
>>>
>>> RS-232:
>>> - Full Duplex Point-to-Point connection
>>> - No transceiver control with RTS
>>> - No termination
>>> - No extra struct in use
>>>
>>> RS-422:
>>> - Full Duplex Point-to-Point connection
>>> - No transceiver control with RTS needed
>>> - Termination possible
>>> - Extra struct serial_rs485 needed if termination is used
>>> => RS-422 can be used in RS-232 operation, but if a termination should be
>>> switchable the RS485 flag has to be enabled. But then also transceiver
>>> control will be enabled. Not a very satisfying situation.
>>
>> Well why don't we just allow enabling or disabling RS-485 termination
>> independently from the SER_RS485_ENABLED bit in struct serial_rs485?
>>
>> Just let the user issue a TIOCSRS485 ioctl to toggle termination even
>> if in RS-232 mode and use that mode for RS-422.
>>
>> Looks like the simplest solution to me.
>
> Sounds not bad. The termination should only depend on whether the GPIO is
> given or not.
>
> Irrespective of this, I think the Linos idea of an RS-422 mode is not bad.
> This allows you to take care of special features that were previously
> overlooked. For example, hardware flow control can be switched off so that
> this does not cause any problems.
>

Also note, that RS232 and RS422 may NOT always be the same from driver perspective.
Take a look at 8250_excar.c for example. That driver has to configure the hardware
accordingly when switching from RS232 to RS422 (see iot2040_rs485_config()).

While a SER_RS485_MODE_RS422 flag set by userspace could work to switch to RS422, I
still think that the cleanest solution would be another ioctl TIOCSRS422 with a
parameter like

struct serial_rs422 {
__u32 flags;
#define SER_RS422_ENABLED (1 << 0)
#define SER_RS422_TERMINATE_BUS (1 << 1)
__u32 padding[7]
};

The SER_RS485_MODE_RS422 flag could still be used internally as a hint to the driver.
That solution would also be expandable if needed. I planned to send a patch that implements this
as a RFC to the mailing list (maybe in the next few days).

Regards,
Lino



2023-12-24 10:20:58

by Christoph Niedermaier

[permalink] [raw]
Subject: RE: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

From: Lino Sanfilippo [mailto:[email protected]]
Sent: Saturday, December 23, 2023 2:41 PM
> On 23.12.23 13:49, Christoph Niedermaier wrote:
>> From: Lukas Wunner [mailto:[email protected]]
>> Sent: Thursday, December 21, 2023 4:53 PM
>>>
>>> On Thu, Dec 14, 2023 at 01:41:47PM +0000, Christoph Niedermaier wrote:
>>>> I will summarize the current situation from my point of view, maybe it helps:
>>>>
>>>> RS-232:
>>>> - Full Duplex Point-to-Point connection
>>>> - No transceiver control with RTS
>>>> - No termination
>>>> - No extra struct in use
>>>>
>>>> RS-422:
>>>> - Full Duplex Point-to-Point connection
>>>> - No transceiver control with RTS needed
>>>> - Termination possible
>>>> - Extra struct serial_rs485 needed if termination is used
>>>> => RS-422 can be used in RS-232 operation, but if a termination should be
>>>> switchable the RS485 flag has to be enabled. But then also transceiver
>>>> control will be enabled. Not a very satisfying situation.
>>>
>>> Well why don't we just allow enabling or disabling RS-485 termination
>>> independently from the SER_RS485_ENABLED bit in struct serial_rs485?
>>>
>>> Just let the user issue a TIOCSRS485 ioctl to toggle termination even
>>> if in RS-232 mode and use that mode for RS-422.
>>>
>>> Looks like the simplest solution to me.
>>
>> Sounds not bad. The termination should only depend on whether the GPIO is
>> given or not.
>>
>> Irrespective of this, I think the Linos idea of an RS-422 mode is not bad.
>> This allows you to take care of special features that were previously
>> overlooked. For example, hardware flow control can be switched off so that
>> this does not cause any problems.
>>
>
> Also note, that RS232 and RS422 may NOT always be the same from driver perspective.
> Take a look at 8250_excar.c for example. That driver has to configure the hardware
> accordingly when switching from RS232 to RS422 (see iot2040_rs485_config()).
>
> While a SER_RS485_MODE_RS422 flag set by userspace could work to switch to RS422, I
> still think that the cleanest solution would be another ioctl TIOCSRS422 with a
> parameter like
>
> struct serial_rs422 {
> __u32 flags;
> #define SER_RS422_ENABLED (1 << 0)
> #define SER_RS422_TERMINATE_BUS (1 << 1)
> __u32 padding[7]
> };
>
> The SER_RS485_MODE_RS422 flag could still be used internally as a hint to the driver.
> That solution would also be expandable if needed. I planned to send a patch that
> implements this
> as a RFC to the mailing list (maybe in the next few days).

Having your own ioctl is a nice distinction, but when I think about it for a moment,
questions come to mind:

From userspace perspective then there are two termination flags
SER_RS485_TERMINATE_BUS and SER_RS422_TERMINATE_BUS?
How will they interact internally?

What about the devicetree property?
Will there be rs422-term-gpios as well?

Could the user enable RS422 and RS485 at the same time?


Regards and Merry Christmas
Christoph

2023-12-27 17:40:35

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On Sun, Dec 24, 2023 at 10:11:17AM +0000, Christoph Niedermaier wrote:
> From: Lino Sanfilippo [mailto:[email protected]]
> Sent: Saturday, December 23, 2023 2:41 PM
> > On 23.12.23 13:49, Christoph Niedermaier wrote:
> >> From: Lukas Wunner [mailto:[email protected]]
> >> Sent: Thursday, December 21, 2023 4:53 PM
> >>> On Thu, Dec 14, 2023 at 01:41:47PM +0000, Christoph Niedermaier wrote:
> >>>> I will summarize the current situation from my point of view, maybe it helps:
> >>>>
> >>>> RS-232:
> >>>> - Full Duplex Point-to-Point connection
> >>>> - No transceiver control with RTS
> >>>> - No termination
> >>>> - No extra struct in use
> >>>>
> >>>> RS-422:
> >>>> - Full Duplex Point-to-Point connection
> >>>> - No transceiver control with RTS needed
> >>>> - Termination possible
> >>>> - Extra struct serial_rs485 needed if termination is used
> >>>> => RS-422 can be used in RS-232 operation, but if a termination should be
> >>>> switchable the RS485 flag has to be enabled. But then also transceiver
> >>>> control will be enabled. Not a very satisfying situation.
> >>>
> >>> Well why don't we just allow enabling or disabling RS-485 termination
> >>> independently from the SER_RS485_ENABLED bit in struct serial_rs485?
> >>>
> >>> Just let the user issue a TIOCSRS485 ioctl to toggle termination even
> >>> if in RS-232 mode and use that mode for RS-422.
> >>>
> >>> Looks like the simplest solution to me.
> >>
> >> Sounds not bad. The termination should only depend on whether the GPIO is
> >> given or not.
> >>
> >> Irrespective of this, I think the Linos idea of an RS-422 mode is not bad.
> >> This allows you to take care of special features that were previously
> >> overlooked. For example, hardware flow control can be switched off so that
> >> this does not cause any problems.
> >>
> >
> > Also note, that RS232 and RS422 may NOT always be the same from driver perspective.
> > Take a look at 8250_excar.c for example. That driver has to configure the hardware
> > accordingly when switching from RS232 to RS422 (see iot2040_rs485_config()).
> >
> > While a SER_RS485_MODE_RS422 flag set by userspace could work to switch to RS422, I
> > still think that the cleanest solution would be another ioctl TIOCSRS422 with a
> > parameter like
> >
> > struct serial_rs422 {
> > __u32 flags;
> > #define SER_RS422_ENABLED (1 << 0)
> > #define SER_RS422_TERMINATE_BUS (1 << 1)
> > __u32 padding[7]
> > };
> >
> > The SER_RS485_MODE_RS422 flag could still be used internally as a hint to the driver.
> > That solution would also be expandable if needed. I planned to send a patch that
> > implements this
> > as a RFC to the mailing list (maybe in the next few days).
>
> Having your own ioctl is a nice distinction, but when I think about it for a moment,
> questions come to mind:
>
> From userspace perspective then there are two termination flags
> SER_RS485_TERMINATE_BUS and SER_RS422_TERMINATE_BUS?
> How will they interact internally?
>
> What about the devicetree property?
> Will there be rs422-term-gpios as well?
>
> Could the user enable RS422 and RS485 at the same time?

Exactly, if you are going for this, just make a new entry into union, and
use flags for that. So, you probably will have the same IOCTL, but which
will serve RS422/RS385 purposes excluding odds of the use of the pieces.

--
With Best Regards,
Andy Shevchenko


2023-12-28 23:25:38

by Lukas Wunner

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

On Sat, Dec 23, 2023 at 02:40:58PM +0100, Lino Sanfilippo wrote:
> On 23.12.23 13:49, Christoph Niedermaier wrote:
> > From: Lukas Wunner [mailto:[email protected]]
> > Sent: Thursday, December 21, 2023 4:53 PM
> > > Well why don't we just allow enabling or disabling RS-485 termination
> > > independently from the SER_RS485_ENABLED bit in struct serial_rs485?
> > >
> > > Just let the user issue a TIOCSRS485 ioctl to toggle termination even
> > > if in RS-232 mode and use that mode for RS-422.
> >
> > Sounds not bad. The termination should only depend on whether the GPIO is
> > given or not.
> >
> > Irrespective of this, I think the Linos idea of an RS-422 mode is not bad.
> > This allows you to take care of special features that were previously
> > overlooked. For example, hardware flow control can be switched off so that
> > this does not cause any problems.
>
> Also note, that RS232 and RS422 may NOT always be the same from driver
> perspective.
> Take a look at 8250_excar.c for example. That driver has to configure
> the hardware accordingly when switching from RS232 to RS422
> (see iot2040_rs485_config()).

Actually it seems there are a bunch of GPIOs on the IOT2040 board
(called MPIO instead of GPIO by the driver). See the documentation
of the wiring at line 87 in drivers/tty/serial/8250/8250_exar.c.

So "configure the hardware" seems to just boil down to toggling the
right GPIO (aka MPIO) pins to mux the UART signals to the right
(RS232/RS485/RS422) transceiver.

IOT2040 is an ACPI-based platform, so no devicetree to describe
the RS232/RS485/RS422 mux GPIOs, but the underlying concept is the same.

Thanks,

Lukas