2022-03-02 11:01:42

by Ilpo Järvinen

[permalink] [raw]
Subject: [RFC PATCH 6/7] serial: General support for multipoint addresses

This patch adds generic support for serial multipoint
addressing. Two new ioctls are added. TIOCSADDR is used to
indicate the destination/receive address. TIOCGADDR returns
the current address in use. The driver should implement
set_addr and get_addr to support addressing mode.

Adjust ADDRB clearing to happen only if driver does not provide
set_addr (=the driver doesn't support address mode).

This change is necessary for supporting devices with RS485
multipoint addressing [*]. A following patch in the patch series
adds support for Synopsys Designware UART capable for 9th bit
addressing mode. In this mode, 9th bit is used to indicate an
address (byte) within the communication line. The 9th bit
addressing mode is selected using ADDRB introduced by the
previous patch.

Transmit addresses / receiver filter are specified by setting
the flags SER_ADDR_DEST and/or SER_ADDR_RECV. When the user
supplies the transmit address, in the 9bit addressing mode it is
sent out immediately with the 9th bit set to 1. After that, the
subsequent normal data bytes are sent with 9th bit as 0 and they
are intended to the device with the given address. It is up to
receiver to enforce the filter using SER_ADDR_RECV. When userspace
has supplied the receive address, the driver is expected to handle
the matching of the address and only data with that address is
forwarded to the user. Both SER_ADDR_DEST and SER_ADDR_RECV can
be given at the same time in a single call if the addresses are
the same.

The user can clear the receive filter with SER_ADDR_RECV_CLEAR.

[*] Technically, RS485 is just an electronic spec and does not
itself specify the 9th bit addressing mode but 9th bit seems
at least "semi-standard" way to do addressing with RS485.

Cc: [email protected]
Cc: Richard Henderson <[email protected]>
Cc: Ivan Kokshaysky <[email protected]>
Cc: Matt Turner <[email protected]>
Cc: [email protected]
Cc: Thomas Bogendoerfer <[email protected]>
Cc: [email protected]
Cc: "James E.J. Bottomley" <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: [email protected]
Cc: Michael Ellerman <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: [email protected]
Cc: Yoshinori Sato <[email protected]>
Cc: Rich Felker <[email protected]>
Cc: [email protected]
Cc: "David S. Miller" <[email protected]>
Cc: [email protected]
Cc: Chris Zankel <[email protected]>
Cc: Max Filippov <[email protected]>
Cc: [email protected]
Cc: Arnd Bergmann <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Ilpo Järvinen <[email protected]>
---
.../driver-api/serial/serial-rs485.rst | 23 ++++++-
arch/alpha/include/uapi/asm/ioctls.h | 3 +
arch/mips/include/uapi/asm/ioctls.h | 3 +
arch/parisc/include/uapi/asm/ioctls.h | 3 +
arch/powerpc/include/uapi/asm/ioctls.h | 3 +
arch/sh/include/uapi/asm/ioctls.h | 3 +
arch/sparc/include/uapi/asm/ioctls.h | 3 +
arch/xtensa/include/uapi/asm/ioctls.h | 3 +
drivers/tty/serial/8250/8250_core.c | 2 +
drivers/tty/serial/serial_core.c | 62 ++++++++++++++++++-
include/linux/serial_core.h | 6 ++
include/uapi/asm-generic/ioctls.h | 3 +
include/uapi/linux/serial.h | 8 +++
13 files changed, 123 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/serial/serial-rs485.rst b/Documentation/driver-api/serial/serial-rs485.rst
index 6bc824f948f9..2f45f007fa5b 100644
--- a/Documentation/driver-api/serial/serial-rs485.rst
+++ b/Documentation/driver-api/serial/serial-rs485.rst
@@ -95,7 +95,28 @@ RS485 Serial Communications
/* Error handling. See errno. */
}

-5. References
+5. Multipoint Addressing
+========================
+
+ The Linux kernel provides serial_addr structure to handle addressing within
+ multipoint serial communications line such as RS485. 9th bit addressiong mode
+ is enabled by adding ADDRB flag in termios c_cflag.
+
+ Serial core calls device specific set/get_addr in response to TIOCSADDR and
+ TIOCGADDR ioctls with a pointer to serial_addr. Destination and receive
+ address can be specified using serial_addr flags field. Receive address may
+ also be cleared using flags. Once an address is set, the communication
+ can occur only with the particular device and other peers are filtered out.
+ It is left up to the receiver side to enforce the filtering.
+
+ Address flags:
+ - SER_ADDR_RECV: Receive (filter) address.
+ - SER_ADDR_RECV_CLEAR: Clear receive filter (only for TIOCSADDR).
+ - SER_ADDR_DEST: Destination address.
+
+ Note: not all devices supporting RS485 support multipoint addressing.
+
+6. References
=============

[1] include/uapi/linux/serial.h
diff --git a/arch/alpha/include/uapi/asm/ioctls.h b/arch/alpha/include/uapi/asm/ioctls.h
index 971311605288..500cab3e1d6b 100644
--- a/arch/alpha/include/uapi/asm/ioctls.h
+++ b/arch/alpha/include/uapi/asm/ioctls.h
@@ -125,4 +125,7 @@
#define TIOCMIWAIT 0x545C /* wait for a change on serial input line(s) */
#define TIOCGICOUNT 0x545D /* read serial port inline interrupt counts */

+#define TIOCSADDR _IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR _IOWR('T', 0x64, struct serial_addr)
+
#endif /* _ASM_ALPHA_IOCTLS_H */
diff --git a/arch/mips/include/uapi/asm/ioctls.h b/arch/mips/include/uapi/asm/ioctls.h
index 16aa8a766aec..3859dc46857e 100644
--- a/arch/mips/include/uapi/asm/ioctls.h
+++ b/arch/mips/include/uapi/asm/ioctls.h
@@ -96,6 +96,9 @@
#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)

+#define TIOCSADDR _IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR _IOWR('T', 0x64, struct serial_addr)
+
/* I hope the range from 0x5480 on is free ... */
#define TIOCSCTTY 0x5480 /* become controlling tty */
#define TIOCGSOFTCAR 0x5481
diff --git a/arch/parisc/include/uapi/asm/ioctls.h b/arch/parisc/include/uapi/asm/ioctls.h
index 82d1148c6379..62337743db64 100644
--- a/arch/parisc/include/uapi/asm/ioctls.h
+++ b/arch/parisc/include/uapi/asm/ioctls.h
@@ -86,6 +86,9 @@
#define TIOCSTOP 0x5462
#define TIOCSLTC 0x5462

+#define TIOCSADDR _IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR _IOWR('T', 0x64, struct serial_addr)
+
/* Used for packet mode */
#define TIOCPKT_DATA 0
#define TIOCPKT_FLUSHREAD 1
diff --git a/arch/powerpc/include/uapi/asm/ioctls.h b/arch/powerpc/include/uapi/asm/ioctls.h
index 2c145da3b774..84fd69ac366a 100644
--- a/arch/powerpc/include/uapi/asm/ioctls.h
+++ b/arch/powerpc/include/uapi/asm/ioctls.h
@@ -120,4 +120,7 @@
#define TIOCMIWAIT 0x545C /* wait for a change on serial input line(s) */
#define TIOCGICOUNT 0x545D /* read serial port inline interrupt counts */

+#define TIOCSADDR _IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR _IOWR('T', 0x64, struct serial_addr)
+
#endif /* _ASM_POWERPC_IOCTLS_H */
diff --git a/arch/sh/include/uapi/asm/ioctls.h b/arch/sh/include/uapi/asm/ioctls.h
index 11866d4f60e1..f82966b7dba2 100644
--- a/arch/sh/include/uapi/asm/ioctls.h
+++ b/arch/sh/include/uapi/asm/ioctls.h
@@ -113,4 +113,7 @@
#define TIOCMIWAIT _IO('T', 92) /* 0x545C */ /* wait for a change on serial input line(s) */
#define TIOCGICOUNT 0x545D /* read serial port inline interrupt counts */

+#define TIOCSADDR _IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR _IOWR('T', 0x64, struct serial_addr)
+
#endif /* __ASM_SH_IOCTLS_H */
diff --git a/arch/sparc/include/uapi/asm/ioctls.h b/arch/sparc/include/uapi/asm/ioctls.h
index 7fd2f5873c9e..e44624c67c79 100644
--- a/arch/sparc/include/uapi/asm/ioctls.h
+++ b/arch/sparc/include/uapi/asm/ioctls.h
@@ -125,6 +125,9 @@
#define TIOCMIWAIT 0x545C /* Wait for change on serial input line(s) */
#define TIOCGICOUNT 0x545D /* Read serial port inline interrupt counts */

+#define TIOCSADDR _IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR _IOWR('T', 0x64, struct serial_addr)
+
/* Kernel definitions */

/* Used for packet mode */
diff --git a/arch/xtensa/include/uapi/asm/ioctls.h b/arch/xtensa/include/uapi/asm/ioctls.h
index 6d4a87296c95..759ca9377f2a 100644
--- a/arch/xtensa/include/uapi/asm/ioctls.h
+++ b/arch/xtensa/include/uapi/asm/ioctls.h
@@ -127,4 +127,7 @@
#define TIOCMIWAIT _IO('T', 92) /* wait for a change on serial input line(s) */
#define TIOCGICOUNT 0x545D /* read serial port inline interrupt counts */

+#define TIOCSADDR _IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR _IOWR('T', 0x64, struct serial_addr)
+
#endif /* _XTENSA_IOCTLS_H */
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 01d30f6ed8fb..f67bc3b76f65 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1008,6 +1008,8 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
uart->port.rs485 = up->port.rs485;
uart->rs485_start_tx = up->rs485_start_tx;
uart->rs485_stop_tx = up->rs485_stop_tx;
+ uart->port.set_addr = up->port.set_addr;
+ uart->port.get_addr = up->port.get_addr;
uart->dma = up->dma;

/* Take tx_loadsz from fifosize if it wasn't set separately */
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 8ab88293c917..8235f696d073 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1346,6 +1346,56 @@ static int uart_set_iso7816_config(struct uart_port *port,
return 0;
}

+static int uart_set_addr(struct uart_port *port,
+ struct serial_addr __user *serial_addr_user)
+{
+ struct serial_addr addr;
+ unsigned long flags;
+ int ret;
+
+ if (copy_from_user(&addr, serial_addr_user, sizeof(*serial_addr_user)))
+ return -EFAULT;
+
+ spin_lock_irqsave(&port->lock, flags);
+ if (port->set_addr)
+ ret = port->set_addr(port, &addr);
+ else
+ ret = -EINVAL;
+ spin_unlock_irqrestore(&port->lock, flags);
+ if (ret)
+ return ret;
+
+ if (copy_to_user(serial_addr_user, &addr, sizeof(addr)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int uart_get_addr(struct uart_port *port,
+ struct serial_addr __user *serial_addr_user)
+{
+ struct serial_addr addr;
+ unsigned long flags;
+ int ret;
+
+ if (copy_from_user(&addr, serial_addr_user, sizeof(*serial_addr_user)))
+ return -EFAULT;
+
+ spin_lock_irqsave(&port->lock, flags);
+ if (port->get_addr)
+ ret = port->get_addr(port, &addr);
+ else
+ ret = -EINVAL;
+ spin_unlock_irqrestore(&port->lock, flags);
+ if (ret)
+ return ret;
+
+ if (copy_to_user(serial_addr_user, &addr, sizeof(addr)))
+ return -EFAULT;
+
+ return 0;
+}
+
/*
* Called via sys_ioctl. We can use spin_lock_irq() here.
*/
@@ -1423,6 +1473,15 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
case TIOCGISO7816:
ret = uart_get_iso7816_config(state->uart_port, uarg);
break;
+
+ case TIOCSADDR:
+ ret = uart_set_addr(uport, uarg);
+ break;
+
+ case TIOCGADDR:
+ ret = uart_get_addr(uport, uarg);
+ break;
+
default:
if (uport->ops->ioctl)
ret = uport->ops->ioctl(uport, cmd, arg);
@@ -1489,7 +1548,8 @@ static void uart_set_termios(struct tty_struct *tty,
goto out;
}

- tty->termios.c_cflag &= ~ADDRB;
+ if (!uport->set_addr)
+ tty->termios.c_cflag &= ~ADDRB;

uart_change_speed(tty, state, old_termios);
/* reload cflag from termios; port driver may have overridden flags */
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 31f7fe527395..ebe1d0ec11d8 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -135,6 +135,12 @@ struct uart_port {
struct serial_rs485 *rs485);
int (*iso7816_config)(struct uart_port *,
struct serial_iso7816 *iso7816);
+
+ int (*set_addr)(struct uart_port *p,
+ struct serial_addr *addr);
+ int (*get_addr)(struct uart_port *p,
+ struct serial_addr *addr);
+
unsigned int irq; /* irq number */
unsigned long irqflags; /* irq flags */
unsigned int uartclk; /* base uart clock */
diff --git a/include/uapi/asm-generic/ioctls.h b/include/uapi/asm-generic/ioctls.h
index cdc9f4ca8c27..689743366091 100644
--- a/include/uapi/asm-generic/ioctls.h
+++ b/include/uapi/asm-generic/ioctls.h
@@ -106,6 +106,9 @@
# define FIOQSIZE 0x5460
#endif

+#define TIOCSADDR _IOWR('T', 0x63, struct serial_addr)
+#define TIOCGADDR _IOWR('T', 0x64, struct serial_addr)
+
/* Used for packet mode */
#define TIOCPKT_DATA 0
#define TIOCPKT_FLUSHREAD 1
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index f868685b35a0..7b198fd5d390 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -151,4 +151,12 @@ struct serial_iso7816 {
__u32 reserved[5];
};

+struct serial_addr {
+ __u32 flags;
+#define SER_ADDR_RECV (1 << 0)
+#define SER_ADDR_RECV_CLEAR (1 << 1)
+#define SER_ADDR_DEST (1 << 2)
+ __u32 addr;
+};
+
#endif /* _UAPI_LINUX_SERIAL_H */
--
2.30.2


2022-03-07 02:13:23

by Lukas Wunner

[permalink] [raw]
Subject: Re: [RFC PATCH 6/7] serial: General support for multipoint addresses

On Wed, Mar 02, 2022 at 11:56:05AM +0200, Ilpo J?rvinen wrote:
> This patch adds generic support for serial multipoint
> addressing. Two new ioctls are added. TIOCSADDR is used to

Nit: "This patch adds..." is superfluous. Just write "Add ..."
in imperative mood.


> This change is necessary for supporting devices with RS485
> multipoint addressing [*].

If this is only used with RS485, why can't we just store the
addresses in struct serial_rs485 and use the existing TIOCSRS485
and TIOCGRS485 ioctls? There's 20 bytes of padding left in
struct serial_rs485 which you could use. No need to add more
user-space ABI.


> [*] Technically, RS485 is just an electronic spec and does not
> itself specify the 9th bit addressing mode but 9th bit seems
> at least "semi-standard" way to do addressing with RS485.

Is 9th bit addressing actually used by an Intel customer or was
it implemented just for feature completeness? I think this mode
isn't used often (I've never seen a use case myself), primarily
because it requires disabling parity.

Thanks,

Lukas

2022-03-07 16:19:41

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [RFC PATCH 6/7] serial: General support for multipoint addresses

On Sun, 6 Mar 2022, Lukas Wunner wrote:

> On Wed, Mar 02, 2022 at 11:56:05AM +0200, Ilpo J?rvinen wrote:
>
> > This change is necessary for supporting devices with RS485
> > multipoint addressing [*].
>
> If this is only used with RS485, why can't we just store the
> addresses in struct serial_rs485 and use the existing TIOCSRS485
> and TIOCGRS485 ioctls? There's 20 bytes of padding left in
> struct serial_rs485 which you could use. No need to add more
> user-space ABI.

It could if it is agreed that serial multipoint addressing is just
a thing in RS-485 and nowhere else? In that case, there is no point
in adding more generic support for it.

> > [*] Technically, RS485 is just an electronic spec and does not
> > itself specify the 9th bit addressing mode but 9th bit seems
> > at least "semi-standard" way to do addressing with RS485.
>
> Is 9th bit addressing actually used by an Intel customer or was
> it implemented just for feature completeness? I think this mode
> isn't used often (I've never seen a use case myself), primarily
> because it requires disabling parity.

On what basis? ...The datasheet I'm looking at has a timing diagram
with both D8 (9th bit) and parity so I think your information must be
incorrect. I don't have direct contacts with customers but I'm told
it's important for other org's customers.


--
i.

2022-03-10 01:06:58

by Lukas Wunner

[permalink] [raw]
Subject: Re: [RFC PATCH 6/7] serial: General support for multipoint addresses

On Mon, Mar 07, 2022 at 11:48:01AM +0200, Ilpo J?rvinen wrote:
> On Sun, 6 Mar 2022, Lukas Wunner wrote:
> > On Wed, Mar 02, 2022 at 11:56:05AM +0200, Ilpo J?rvinen wrote:
> > > This change is necessary for supporting devices with RS485
> > > multipoint addressing [*].
> >
> > If this is only used with RS485, why can't we just store the
> > addresses in struct serial_rs485 and use the existing TIOCSRS485
> > and TIOCGRS485 ioctls? There's 20 bytes of padding left in
> > struct serial_rs485 which you could use. No need to add more
> > user-space ABI.
>
> It could if it is agreed that serial multipoint addressing is just
> a thing in RS-485 and nowhere else? In that case, there is no point
> in adding more generic support for it.

It's just that the above-quoted sentence in the commit message
specifically mentions RS485. If you intend to use it with RS232
as well, that should be made explicit, otherwise one wonders why
it wasn't integrated into struct serial_rs485.

I have no idea how common 9th bit addressing mode is with RS232.
Goggle turns up links saying it's mainly used with RS485, "but also
RS232". Since RS232 isn't a bus but a point-to-point link,
9th bit addressing doesn't seem to make as much sense.


> > > [*] Technically, RS485 is just an electronic spec and does not
> > > itself specify the 9th bit addressing mode but 9th bit seems
> > > at least "semi-standard" way to do addressing with RS485.
> >
> > Is 9th bit addressing actually used by an Intel customer or was
> > it implemented just for feature completeness? I think this mode
> > isn't used often (I've never seen a use case myself), primarily
> > because it requires disabling parity.
>
> On what basis? ...The datasheet I'm looking at has a timing diagram
> with both D8 (9th bit) and parity so I think your information must be
> incorrect.

E.g. the discussion here says that 9th bit addressing requires that
parity is disabled or the character size is reduced to 7-bit:

https://www.microchip.com/forums/m299904.aspx

I guess that applies only to some UARTs, the Synopsys databook doesn't
mention any such constraints.

Thanks,

Lukas

2022-03-10 12:49:24

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [RFC PATCH 6/7] serial: General support for multipoint addresses

On Wed, 9 Mar 2022, Lukas Wunner wrote:

> On Mon, Mar 07, 2022 at 11:48:01AM +0200, Ilpo J?rvinen wrote:
> > On Sun, 6 Mar 2022, Lukas Wunner wrote:
> > > On Wed, Mar 02, 2022 at 11:56:05AM +0200, Ilpo J?rvinen wrote:
> > > > This change is necessary for supporting devices with RS485
> > > > multipoint addressing [*].
> > >
> > > If this is only used with RS485, why can't we just store the
> > > addresses in struct serial_rs485 and use the existing TIOCSRS485
> > > and TIOCGRS485 ioctls? There's 20 bytes of padding left in
> > > struct serial_rs485 which you could use. No need to add more
> > > user-space ABI.
> >
> > It could if it is agreed that serial multipoint addressing is just
> > a thing in RS-485 and nowhere else? In that case, there is no point
> > in adding more generic support for it.
>
> It's just that the above-quoted sentence in the commit message
> specifically mentions RS485.

That sentence is just to justify why addressing mode is needed,
not to take a stance on whether it is only used with RS485 or not.

> If you intend to use it with RS232
> as well, that should be made explicit, otherwise one wonders why
> it wasn't integrated into struct serial_rs485.
>
> I have no idea how common 9th bit addressing mode is with RS232.
> Goggle turns up links saying it's mainly used with RS485, "but also
> RS232". Since RS232 isn't a bus but a point-to-point link,
> 9th bit addressing doesn't seem to make as much sense.

While I don't know any better, I can image though that with an
RS232-to-RS485 converter, it could make some sense.

If I put them back to serial_rs485 / rs485 config, it's basically just
where I initially started from with this patchset (offlist).


--
i.

2022-03-10 16:05:48

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC PATCH 6/7] serial: General support for multipoint addresses

On Wed, Mar 09, 2022 at 08:05:21PM +0100, Lukas Wunner wrote:
> On Mon, Mar 07, 2022 at 11:48:01AM +0200, Ilpo J?rvinen wrote:
> > On Sun, 6 Mar 2022, Lukas Wunner wrote:
> > > On Wed, Mar 02, 2022 at 11:56:05AM +0200, Ilpo J?rvinen wrote:
> > > > This change is necessary for supporting devices with RS485
> > > > multipoint addressing [*].
> > >
> > > If this is only used with RS485, why can't we just store the
> > > addresses in struct serial_rs485 and use the existing TIOCSRS485
> > > and TIOCGRS485 ioctls? There's 20 bytes of padding left in
> > > struct serial_rs485 which you could use. No need to add more
> > > user-space ABI.
> >
> > It could if it is agreed that serial multipoint addressing is just
> > a thing in RS-485 and nowhere else? In that case, there is no point
> > in adding more generic support for it.
>
> It's just that the above-quoted sentence in the commit message
> specifically mentions RS485. If you intend to use it with RS232
> as well, that should be made explicit, otherwise one wonders why
> it wasn't integrated into struct serial_rs485.
>
> I have no idea how common 9th bit addressing mode is with RS232.
> Goggle turns up links saying it's mainly used with RS485, "but also
> RS232". Since RS232 isn't a bus but a point-to-point link,
> 9th bit addressing doesn't seem to make as much sense.

In my student years I have an exercise to use 9-bit addressing mode on RS232.
Obviously I forgot all of the details, but I remember that that has a practical
application.

--
With Best Regards,
Andy Shevchenko