[PATCH v5 06/10] serial: General support for multipoint addresses

[PATCH v5 06/10] serial: General support for multipoint addresses

[Ilpo Järvinen]

Add 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: linux-api@vger.kernel.org
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: linux-alpha@vger.kernel.org
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: linux-mips@vger.kernel.org
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-parisc@vger.kernel.org
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: linux-sh@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: linux-xtensa@linux-xtensa.org
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arch@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 .../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 ++++++++++++++++++-
 drivers/tty/tty_io.c                          |  2 +
 include/linux/serial_core.h                   |  6 ++
 include/uapi/asm-generic/ioctls.h             |  3 +
 include/uapi/linux/serial.h                   |  8 +++
 14 files changed, 125 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 0f397e67eeb0..5762987bf176 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1397,6 +1397,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 (!port->set_addr)
+		return -ENOTTY;
+
+	if (copy_from_user(&addr, serial_addr_user, sizeof(*serial_addr_user)))
+		return -EFAULT;
+
+	spin_lock_irqsave(&port->lock, flags);
+	ret = port->set_addr(port, &addr);
+	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 (!port->get_addr)
+		return -ENOTTY;
+
+	if (copy_from_user(&addr, serial_addr_user, sizeof(*serial_addr_user)))
+		return -EFAULT;
+
+	spin_lock_irqsave(&port->lock, flags);
+	ret = port->get_addr(port, &addr);
+	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.
  */
@@ -1474,6 +1524,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);
@@ -1540,7 +1599,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/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 8fec1d8648f5..ef22b95e9c79 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2885,6 +2885,8 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd,
 	case TIOCSERGETLSR:
 	case TIOCGRS485:
 	case TIOCSRS485:
+	case TIOCSADDR:
+	case TIOCGADDR:
 #ifdef TIOCGETP
 	case TIOCGETP:
 	case TIOCSETP:
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cbd5070bc87f..2dd4c4d01c59 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 fa6b16e5fdd8..8cb785ea7087 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -149,4 +149,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

Re: [PATCH v5 06/10] serial: General support for multipoint addresses

[Greg KH]

On Tue, Apr 26, 2022 at 03:24:44PM +0300, Ilpo Järvinen wrote:
> Add 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: linux-api@vger.kernel.org
> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> Cc: Matt Turner <mattst88@gmail.com>
> Cc: linux-alpha@vger.kernel.org
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: linux-mips@vger.kernel.org
> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: linux-parisc@vger.kernel.org
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: linux-sh@vger.kernel.org
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: sparclinux@vger.kernel.org
> Cc: Chris Zankel <chris@zankel.net>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Cc: linux-xtensa@linux-xtensa.org
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-arch@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  .../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 ++++++++++++++++++-
>  drivers/tty/tty_io.c                          |  2 +
>  include/linux/serial_core.h                   |  6 ++
>  include/uapi/asm-generic/ioctls.h             |  3 +
>  include/uapi/linux/serial.h                   |  8 +++
>  14 files changed, 125 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 0f397e67eeb0..5762987bf176 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1397,6 +1397,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 (!port->set_addr)
> +		return -ENOTTY;
> +
> +	if (copy_from_user(&addr, serial_addr_user, sizeof(*serial_addr_user)))
> +		return -EFAULT;
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +	ret = port->set_addr(port, &addr);
> +	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 (!port->get_addr)
> +		return -ENOTTY;
> +
> +	if (copy_from_user(&addr, serial_addr_user, sizeof(*serial_addr_user)))
> +		return -EFAULT;
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +	ret = port->get_addr(port, &addr);
> +	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.
>   */
> @@ -1474,6 +1524,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);
> @@ -1540,7 +1599,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/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 8fec1d8648f5..ef22b95e9c79 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -2885,6 +2885,8 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd,
>  	case TIOCSERGETLSR:
>  	case TIOCGRS485:
>  	case TIOCSRS485:
> +	case TIOCSADDR:
> +	case TIOCGADDR:
>  #ifdef TIOCGETP
>  	case TIOCGETP:
>  	case TIOCSETP:
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index cbd5070bc87f..2dd4c4d01c59 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 fa6b16e5fdd8..8cb785ea7087 100644
> --- a/include/uapi/linux/serial.h
> +++ b/include/uapi/linux/serial.h
> @@ -149,4 +149,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)

You never check for invalid flags being sent to the kernel, which means
this api can never change in the future to add new flags :(

And what about struct serial_rs485?  Shouldn't that be used here
instead?  Why do we need a new ioctl and structure?

thanks,

greg k-h

Re: [PATCH v5 06/10] serial: General support for multipoint addresses

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4705 bytes --]

On Tue, 26 Apr 2022, Greg KH wrote:

> On Tue, Apr 26, 2022 at 03:24:44PM +0300, Ilpo Järvinen wrote:
> > Add 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: linux-api@vger.kernel.org
> > Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> > Cc: Matt Turner <mattst88@gmail.com>
> > Cc: linux-alpha@vger.kernel.org
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: linux-mips@vger.kernel.org
> > Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> > Cc: Helge Deller <deller@gmx.de>
> > Cc: linux-parisc@vger.kernel.org
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> > Cc: Rich Felker <dalias@libc.org>
> > Cc: linux-sh@vger.kernel.org
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: sparclinux@vger.kernel.org
> > Cc: Chris Zankel <chris@zankel.net>
> > Cc: Max Filippov <jcmvbkbc@gmail.com>
> > Cc: linux-xtensa@linux-xtensa.org
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: linux-arch@vger.kernel.org
> > Cc: linux-doc@vger.kernel.org
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---

> > diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
> > index fa6b16e5fdd8..8cb785ea7087 100644
> > --- a/include/uapi/linux/serial.h
> > +++ b/include/uapi/linux/serial.h
> > @@ -149,4 +149,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)
> 
> You never check for invalid flags being sent to the kernel, which means
> this api can never change in the future to add new flags :(

Ok, so you mean the general level should to check
if (...->flags & ~(SER_ADDR_FLAGS_ALL))
	return -EINVAL;
?

There's some code in the driver that detects invalid flag combinations 
(in 10/10) but I guess it doesn't satisfies what you're after. It is 
similar to how serial_rs485 flags is handled, that is, clearing flags it 
didn't handle (when it can) and returning -EINVAL for impossible 
combinations such as getting both RECV and DEST addr at the same time.
I don't know if serial_rs485 flags is a good example at all, it certainly 
doesn't check whether bits are set where there's no flag defined.

> And what about struct serial_rs485?  Shouldn't that be used here
> instead?  Why do we need a new ioctl and structure?

It is possible (Lukas already mentioned that option too). It just means
this will be available only on RS485 which could well be enough but Andy 
mentioned he has in the past come across addressing mode also with some 
RS232 thing (he didn't remember details anymore and it could be 
insignificant for the real world of today).


-- 
 i.

Re: [PATCH v5 06/10] serial: General support for multipoint addresses

[Greg KH]

On Tue, Apr 26, 2022 at 04:36:49PM +0300, Ilpo Järvinen wrote:
> On Tue, 26 Apr 2022, Greg KH wrote:
> 
> > On Tue, Apr 26, 2022 at 03:24:44PM +0300, Ilpo Järvinen wrote:
> > > Add 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: linux-api@vger.kernel.org
> > > Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> > > Cc: Matt Turner <mattst88@gmail.com>
> > > Cc: linux-alpha@vger.kernel.org
> > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > Cc: linux-mips@vger.kernel.org
> > > Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> > > Cc: Helge Deller <deller@gmx.de>
> > > Cc: linux-parisc@vger.kernel.org
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> > > Cc: Rich Felker <dalias@libc.org>
> > > Cc: linux-sh@vger.kernel.org
> > > Cc: "David S. Miller" <davem@davemloft.net>
> > > Cc: sparclinux@vger.kernel.org
> > > Cc: Chris Zankel <chris@zankel.net>
> > > Cc: Max Filippov <jcmvbkbc@gmail.com>
> > > Cc: linux-xtensa@linux-xtensa.org
> > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > Cc: linux-arch@vger.kernel.org
> > > Cc: linux-doc@vger.kernel.org
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > ---
> 
> > > diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
> > > index fa6b16e5fdd8..8cb785ea7087 100644
> > > --- a/include/uapi/linux/serial.h
> > > +++ b/include/uapi/linux/serial.h
> > > @@ -149,4 +149,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)
> > 
> > You never check for invalid flags being sent to the kernel, which means
> > this api can never change in the future to add new flags :(
> 
> Ok, so you mean the general level should to check
> if (...->flags & ~(SER_ADDR_FLAGS_ALL))
> 	return -EINVAL;
> ?

For any new kernel api you always have to ensure that no "extra" flags
or bits are set and reject it otherwise you can never add any more bits
or flags in the future.  This should be in the Documentation/ directory
for how to add new ioctls somewhere.

> There's some code in the driver that detects invalid flag combinations 
> (in 10/10) but I guess it doesn't satisfies what you're after. It is 
> similar to how serial_rs485 flags is handled, that is, clearing flags it 
> didn't handle (when it can) and returning -EINVAL for impossible 
> combinations such as getting both RECV and DEST addr at the same time.
> I don't know if serial_rs485 flags is a good example at all, it certainly 
> doesn't check whether bits are set where there's no flag defined.
> 
> > And what about struct serial_rs485?  Shouldn't that be used here
> > instead?  Why do we need a new ioctl and structure?
> 
> It is possible (Lukas already mentioned that option too). It just means
> this will be available only on RS485 which could well be enough but Andy 
> mentioned he has in the past come across addressing mode also with some 
> RS232 thing (he didn't remember details anymore and it could be 
> insignificant for the real world of today).

This is rs485 so let's keep it attached to that.  Lots of people do
their own custom addressing schemes on top of 232 but that's up to them
to support in userspace or as a line discipline.

thanks,

greg k-h