[PATCH v2 0/4] serial: Create kserial_rs485 to get rid of padding

[PATCH v2 0/4] serial: Create kserial_rs485 to get rid of padding

[Ilpo Järvinen]

serial_rs485 contains padding at the end to make extending it easier.
Create kserial_rs485 counterpart w/o padding for kernel use leaving
serial_rs485 only to be used for uapi.

v2:
- Address style comments (from Andy)
- Convert most of the serial_rs485 & kserial_rs485 inline comments to
  kernel doc

Ilpo Järvinen (4):
  serial: Rename vars in uart_get_rs485_config()
  serial: add helpers to copy serial_rs485 from/to userspace
  serial: Convert serial_rs485 to kernel doc
  serial: Add kserial_rs485 to avoid wasted space due to .padding

 .../driver-api/serial/serial-rs485.rst        |  2 +-
 drivers/tty/serial/8250/8250.h                |  4 +-
 drivers/tty/serial/8250/8250_dwlib.c          |  6 +-
 drivers/tty/serial/8250/8250_exar.c           | 12 +--
 drivers/tty/serial/8250/8250_fintek.c         |  6 +-
 drivers/tty/serial/8250/8250_lpc18xx.c        |  4 +-
 drivers/tty/serial/8250/8250_pci.c            |  4 +-
 drivers/tty/serial/8250/8250_port.c           |  4 +-
 drivers/tty/serial/amba-pl011.c               |  4 +-
 drivers/tty/serial/ar933x_uart.c              |  8 +-
 drivers/tty/serial/atmel_serial.c             |  4 +-
 drivers/tty/serial/fsl_lpuart.c               |  6 +-
 drivers/tty/serial/imx.c                      |  6 +-
 drivers/tty/serial/max310x.c                  |  4 +-
 drivers/tty/serial/mcf.c                      |  4 +-
 drivers/tty/serial/omap-serial.c              |  6 +-
 drivers/tty/serial/sc16is7xx.c                |  6 +-
 drivers/tty/serial/serial_core.c              | 87 +++++++++++++------
 drivers/tty/serial/stm32-usart.c              | 12 +--
 include/linux/serial_core.h                   | 29 ++++++-
 include/uapi/linux/serial.h                   | 63 +++++++++-----
 21 files changed, 181 insertions(+), 100 deletions(-)

-- 
2.30.2

[PATCH v2 1/4] serial: Rename vars in uart_get_rs485_config()

[Ilpo Järvinen]

Make variable names to match uart_set_rs485_config() ones:
	- rs485 -> rs485_user
	- aux -> rs485

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/serial_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 12c87cd201a7..8834414a0b2f 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1408,16 +1408,16 @@ int uart_rs485_config(struct uart_port *port)
 EXPORT_SYMBOL_GPL(uart_rs485_config);
 
 static int uart_get_rs485_config(struct uart_port *port,
-			 struct serial_rs485 __user *rs485)
+			 struct serial_rs485 __user *rs485_user)
 {
+	struct serial_rs485 rs485;
 	unsigned long flags;
-	struct serial_rs485 aux;
 
 	spin_lock_irqsave(&port->lock, flags);
-	aux = port->rs485;
+	rs485 = port->rs485;
 	spin_unlock_irqrestore(&port->lock, flags);
 
-	if (copy_to_user(rs485, &aux, sizeof(aux)))
+	if (copy_to_user(rs485_user, &rs485, sizeof(rs485)))
 		return -EFAULT;
 
 	return 0;
-- 
2.30.2

Re: [PATCH v2 1/4] serial: Rename vars in uart_get_rs485_config()

[Greg Kroah-Hartman]

On Tue, Aug 30, 2022 at 10:29:53AM +0300, Ilpo Järvinen wrote:
> Make variable names to match uart_set_rs485_config() ones:
> 	- rs485 -> rs485_user
> 	- aux -> rs485
> 
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/tty/serial/serial_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 12c87cd201a7..8834414a0b2f 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1408,16 +1408,16 @@ int uart_rs485_config(struct uart_port *port)
>  EXPORT_SYMBOL_GPL(uart_rs485_config);
>  
>  static int uart_get_rs485_config(struct uart_port *port,
> -			 struct serial_rs485 __user *rs485)
> +			 struct serial_rs485 __user *rs485_user)
>  {
> +	struct serial_rs485 rs485;
>  	unsigned long flags;
> -	struct serial_rs485 aux;
>  
>  	spin_lock_irqsave(&port->lock, flags);
> -	aux = port->rs485;
> +	rs485 = port->rs485;
>  	spin_unlock_irqrestore(&port->lock, flags);

I missed this originally, but why does the lock matter here at all?  You
are just copying all data out of the structure into an on-stack one, why
the extra step at all?

As the structure can change instantly after you release the lock, I
don't see what the lock is protecting.

thanks,

greg k-h

Re: [PATCH v2 1/4] serial: Rename vars in uart_get_rs485_config()

[Ilpo Järvinen]

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

On Tue, 30 Aug 2022, Greg Kroah-Hartman wrote:

> On Tue, Aug 30, 2022 at 10:29:53AM +0300, Ilpo Järvinen wrote:
> > Make variable names to match uart_set_rs485_config() ones:
> > 	- rs485 -> rs485_user
> > 	- aux -> rs485
> > 
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> >  drivers/tty/serial/serial_core.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > index 12c87cd201a7..8834414a0b2f 100644
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@ -1408,16 +1408,16 @@ int uart_rs485_config(struct uart_port *port)
> >  EXPORT_SYMBOL_GPL(uart_rs485_config);
> >  
> >  static int uart_get_rs485_config(struct uart_port *port,
> > -			 struct serial_rs485 __user *rs485)
> > +			 struct serial_rs485 __user *rs485_user)
> >  {
> > +	struct serial_rs485 rs485;
> >  	unsigned long flags;
> > -	struct serial_rs485 aux;
> >  
> >  	spin_lock_irqsave(&port->lock, flags);
> > -	aux = port->rs485;
> > +	rs485 = port->rs485;
> >  	spin_unlock_irqrestore(&port->lock, flags);
> 
> I missed this originally, but why does the lock matter here at all?  You
> are just copying all data out of the structure into an on-stack one, why
> the extra step at all?
> 
> As the structure can change instantly after you release the lock, I
> don't see what the lock is protecting.

At least it cannot return inconsistent serial_rs485 because of the lock. 
Probably not an end of the world if the lock wouldn't be taken and a 
concurrent update of port->rs485 would be allowed to mess up a getted 
serial_rs485 as it seems kind of a high-level/userland concurrency issue 
on the tty. Anyway, that seems to be the only reason really.

It's orthogonal to the patch series though.

-- 
 i.

[PATCH v2 2/4] serial: add helpers to copy serial_rs485 from/to userspace

[Ilpo Järvinen]

Due to the padding fields, the copying will need to do a bit more than
usual.

Move padding clearing into the helper that copies back to userspace.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/serial_core.c | 41 +++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 8834414a0b2f..4a4ed619ee71 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1375,10 +1375,6 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
 	rs485->flags &= supported_flags;
 
 	uart_sanitize_serial_rs485_delays(port, rs485);
-
-	/* Return clean padding area to userspace */
-	memset(rs485->padding0, 0, sizeof(rs485->padding0));
-	memset(rs485->padding1, 0, sizeof(rs485->padding1));
 }
 
 static void uart_set_rs485_termination(struct uart_port *port,
@@ -1407,6 +1403,28 @@ int uart_rs485_config(struct uart_port *port)
 }
 EXPORT_SYMBOL_GPL(uart_rs485_config);
 
+static int serial_rs485_from_user(struct serial_rs485 *rs485,
+				  const struct serial_rs485 __user *rs485_user)
+{
+	if (copy_from_user(rs485, rs485_user, sizeof(*rs485)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int serial_rs485_to_user(struct serial_rs485 __user *rs485_user,
+				struct serial_rs485 *rs485)
+{
+	/* Return clean padding area to userspace */
+	memset(rs485->padding0, 0, sizeof(rs485->padding0));
+	memset(rs485->padding1, 0, sizeof(rs485->padding1));
+
+	if (copy_to_user(rs485_user, rs485, sizeof(*rs485)))
+		return -EFAULT;
+
+	return 0;
+}
+
 static int uart_get_rs485_config(struct uart_port *port,
 			 struct serial_rs485 __user *rs485_user)
 {
@@ -1417,10 +1435,7 @@ static int uart_get_rs485_config(struct uart_port *port,
 	rs485 = port->rs485;
 	spin_unlock_irqrestore(&port->lock, flags);
 
-	if (copy_to_user(rs485_user, &rs485, sizeof(rs485)))
-		return -EFAULT;
-
-	return 0;
+	return serial_rs485_to_user(rs485_user, &rs485);
 }
 
 static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
@@ -1433,8 +1448,9 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
 	if (!port->rs485_config)
 		return -ENOTTY;
 
-	if (copy_from_user(&rs485, rs485_user, sizeof(*rs485_user)))
-		return -EFAULT;
+	ret = serial_rs485_from_user(&rs485, rs485_user);
+	if (ret)
+		return ret;
 
 	ret = uart_check_rs485_flags(port, &rs485);
 	if (ret)
@@ -1450,10 +1466,7 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
 	if (ret)
 		return ret;
 
-	if (copy_to_user(rs485_user, &port->rs485, sizeof(port->rs485)))
-		return -EFAULT;
-
-	return 0;
+	return serial_rs485_to_user(rs485_user, &port->rs485);
 }
 
 static int uart_get_iso7816_config(struct uart_port *port,
-- 
2.30.2

[PATCH v2 3/4] serial: Convert serial_rs485 to kernel doc

[Ilpo Järvinen]

Convert struct serial_rs485 comments to kernel doc format.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 include/uapi/linux/serial.h | 61 ++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index cea06924b295..dbdc7865abdc 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -107,37 +107,56 @@ struct serial_icounter_struct {
 	int reserved[9];
 };
 
-/*
+/**
+ * struct serial_rs485 - serial interface for controlling RS485 settings.
+ * @flags:			RS485 feature flags
+ * @delay_rts_before_send:	Delay before send (milliseconds)
+ * @delay_rts_after_send:	Delay after send (milliseconds)
+ * @addr_recv:			Receive filter for RS485 addressing mode
+ *				(used only when %SER_RS485_ADDR_RECV is set).
+ * @addr_dest:			Destination address for RS485 addressing mode
+ *				(used only when %SER_RS485_ADDR_DEST is set).
+ * @padding0:			Padding (set to zero).
+ * @padding1:			Padding (set to zero).
+ * @padding:			Deprecated, use @padding0 and @padding1 instead.
+ *				Do not use with @addr_recv and @addr_dest (due to
+ *				overlap).
+ *
  * Serial interface for controlling RS485 settings on chips with suitable
  * support. Set with TIOCSRS485 and get with TIOCGRS485 if supported by your
  * platform. The set function returns the new state, with any unsupported bits
  * reverted appropriately.
+ *
+ * serial_rs485::flags bits are:
+ * * %SER_RS485_ENABLED		- RS485 enabled
+ * * %SER_RS485_RTS_ON_SEND	- Logical level for RTS pin when sending
+ * * %SER_RS485_RTS_AFTER_SEND	- Logical level for RTS pin after sent
+ * * %SER_RS485_RX_DURING_TX	- Full-duplex RS485 line
+ * * %SER_RS485_TERMINATE_BUS	- Enable bus termination (if supported)
+ * * %SER_RS485_ADDRB		- Enable RS485 addressing mode
+ * * %SER_RS485_ADDR_RECV	- Receive address filter (enables @addr_recv).
+ *				  Requires %SER_RS485_ADDRB.
+ * * %SER_RS485_ADDR_DEST	- Destination address (enables @addr_dest).
+ *				  Requires %SER_RS485_ADDRB.
  */
-
 struct serial_rs485 {
-	__u32	flags;			/* RS485 feature flags */
-#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
-#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Logical level for
-							   RTS pin when
-							   sending */
-#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
-							   RTS pin after sent*/
+	__u32	flags;
+#define SER_RS485_ENABLED		(1 << 0)
+#define SER_RS485_RTS_ON_SEND		(1 << 1)
+#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
 #define SER_RS485_RX_DURING_TX		(1 << 4)
-#define SER_RS485_TERMINATE_BUS		(1 << 5)	/* Enable bus
-							   termination
-							   (if supported) */
-
-/* RS-485 addressing mode */
-#define SER_RS485_ADDRB			(1 << 6)	/* Enable addressing mode */
-#define SER_RS485_ADDR_RECV		(1 << 7)	/* Receive address filter */
-#define SER_RS485_ADDR_DEST		(1 << 8)	/* Destination address */
+#define SER_RS485_TERMINATE_BUS		(1 << 5)
+#define SER_RS485_ADDRB			(1 << 6)
+#define SER_RS485_ADDR_RECV		(1 << 7)
+#define SER_RS485_ADDR_DEST		(1 << 8)
 
-	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
-	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
+	__u32	delay_rts_before_send;
+	__u32	delay_rts_after_send;
 
-	/* The fields below are defined by flags */
+	/* private: The fields below are defined by flags */
 	union {
-		__u32	padding[5];		/* Memory is cheap, new structs are a pain */
+		/* private: Memory is cheap, new structs are a pain. */
+		__u32	padding[5];
 
 		struct {
 			__u8	addr_recv;
-- 
2.30.2

[PATCH v2 4/4] serial: Add kserial_rs485 to avoid wasted space due to .padding

[Ilpo Järvinen]

The struct serial_rs485 has a .padding field to make uapi updates
easier. It wastes space, however. Create struct kserial_rs485 which is
a kerner counterpart w/o padding.

serial_rs485_to_user()'s rs485 can now become const as padding is dealt
within the local variable.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 .../driver-api/serial/serial-rs485.rst        |  2 +-
 drivers/tty/serial/8250/8250.h                |  4 +-
 drivers/tty/serial/8250/8250_dwlib.c          |  6 +--
 drivers/tty/serial/8250/8250_exar.c           | 12 ++---
 drivers/tty/serial/8250/8250_fintek.c         |  6 +--
 drivers/tty/serial/8250/8250_lpc18xx.c        |  4 +-
 drivers/tty/serial/8250/8250_pci.c            |  4 +-
 drivers/tty/serial/8250/8250_port.c           |  4 +-
 drivers/tty/serial/amba-pl011.c               |  4 +-
 drivers/tty/serial/ar933x_uart.c              |  8 +--
 drivers/tty/serial/atmel_serial.c             |  4 +-
 drivers/tty/serial/fsl_lpuart.c               |  6 +--
 drivers/tty/serial/imx.c                      |  6 +--
 drivers/tty/serial/max310x.c                  |  4 +-
 drivers/tty/serial/mcf.c                      |  4 +-
 drivers/tty/serial/omap-serial.c              |  6 +--
 drivers/tty/serial/sc16is7xx.c                |  6 +--
 drivers/tty/serial/serial_core.c              | 54 +++++++++++++------
 drivers/tty/serial/stm32-usart.c              | 12 ++---
 include/linux/serial_core.h                   | 29 ++++++++--
 include/uapi/linux/serial.h                   |  2 +
 21 files changed, 118 insertions(+), 69 deletions(-)

diff --git a/Documentation/driver-api/serial/serial-rs485.rst b/Documentation/driver-api/serial/serial-rs485.rst
index 6ebad75c74ed..7e496336a01b 100644
--- a/Documentation/driver-api/serial/serial-rs485.rst
+++ b/Documentation/driver-api/serial/serial-rs485.rst
@@ -41,7 +41,7 @@ RS485 Serial Communications
    implement the rs485_config callback and provide rs485_supported in the
    uart_port structure. The serial core calls rs485_config to do the device
    specific part in response to TIOCSRS485 ioctl (see below). The rs485_config
-   callback receives a pointer to a sanitizated serial_rs485 structure. The
+   callback receives a pointer to a sanitizated kserial_rs485 structure. The
    serial_rs485 userspace provides is sanitized before calling rs485_config
    using rs485_supported that indicates what RS485 features the driver supports
    for the uart_port. TIOCGRS485 ioctl can be used to read back the
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 287153d32536..4e22f8d50e91 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -204,11 +204,11 @@ void serial8250_rpm_get_tx(struct uart_8250_port *p);
 void serial8250_rpm_put_tx(struct uart_8250_port *p);
 
 int serial8250_em485_config(struct uart_port *port, struct ktermios *termios,
-			    struct serial_rs485 *rs485);
+			    struct kserial_rs485 *rs485);
 void serial8250_em485_start_tx(struct uart_8250_port *p);
 void serial8250_em485_stop_tx(struct uart_8250_port *p);
 void serial8250_em485_destroy(struct uart_8250_port *p);
-extern struct serial_rs485 serial8250_em485_supported;
+extern struct kserial_rs485 serial8250_em485_supported;
 
 /* MCR <-> TIOCM conversion */
 static inline int serial8250_TIOCM_to_MCR(int tiocm)
diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index dbe4d44f60d4..a628ed2c235b 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -134,7 +134,7 @@ static void dw8250_update_rar(struct uart_port *p, u32 addr)
 		dw8250_writel_ext(p, DW_UART_RE_EN, re_en);
 }
 
-static void dw8250_rs485_set_addr(struct uart_port *p, struct serial_rs485 *rs485,
+static void dw8250_rs485_set_addr(struct uart_port *p, struct kserial_rs485 *rs485,
 				  struct ktermios *termios)
 {
 	u32 lcr = dw8250_readl_ext(p, DW_UART_LCR_EXT);
@@ -177,7 +177,7 @@ static void dw8250_rs485_set_addr(struct uart_port *p, struct serial_rs485 *rs48
 }
 
 static int dw8250_rs485_config(struct uart_port *p, struct ktermios *termios,
-			       struct serial_rs485 *rs485)
+			       struct kserial_rs485 *rs485)
 {
 	u32 tcr;
 
@@ -232,7 +232,7 @@ static bool dw8250_detect_rs485_hw(struct uart_port *p)
 	return reg;
 }
 
-static const struct serial_rs485 dw8250_rs485_supported = {
+static const struct kserial_rs485 dw8250_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX | SER_RS485_RTS_ON_SEND |
 		 SER_RS485_RTS_AFTER_SEND | SER_RS485_ADDRB | SER_RS485_ADDR_RECV |
 		 SER_RS485_ADDR_DEST,
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 314a05e009df..9fef2056a9b5 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -113,8 +113,8 @@ struct exar8250;
 
 struct exar8250_platform {
 	int (*rs485_config)(struct uart_port *port, struct ktermios *termios,
-			    struct serial_rs485 *rs485);
-	const struct serial_rs485 *rs485_supported;
+			    struct kserial_rs485 *rs485);
+	const struct kserial_rs485 *rs485_supported;
 	int (*register_gpio)(struct pci_dev *, struct uart_8250_port *);
 	void (*unregister_gpio)(struct uart_8250_port *);
 };
@@ -411,7 +411,7 @@ static void xr17v35x_unregister_gpio(struct uart_8250_port *port)
 }
 
 static int generic_rs485_config(struct uart_port *port, struct ktermios *termios,
-				struct serial_rs485 *rs485)
+				struct kserial_rs485 *rs485)
 {
 	bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED);
 	u8 __iomem *p = port->membase;
@@ -431,7 +431,7 @@ static int generic_rs485_config(struct uart_port *port, struct ktermios *termios
 	return 0;
 }
 
-static const struct serial_rs485 generic_rs485_supported = {
+static const struct kserial_rs485 generic_rs485_supported = {
 	.flags = SER_RS485_ENABLED,
 };
 
@@ -443,7 +443,7 @@ static const struct exar8250_platform exar8250_default_platform = {
 };
 
 static int iot2040_rs485_config(struct uart_port *port, struct ktermios *termios,
-				struct serial_rs485 *rs485)
+				struct kserial_rs485 *rs485)
 {
 	bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED);
 	u8 __iomem *p = port->membase;
@@ -475,7 +475,7 @@ static int iot2040_rs485_config(struct uart_port *port, struct ktermios *termios
 	return generic_rs485_config(port, termios, rs485);
 }
 
-static const struct serial_rs485 iot2040_rs485_supported = {
+static const struct kserial_rs485 iot2040_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX | SER_RS485_TERMINATE_BUS,
 };
 
diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index 65b6b3cbaff6..d3d2e25dd06b 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -192,7 +192,7 @@ static int fintek_8250_get_ldn_range(struct fintek_8250 *pdata, int *min,
 }
 
 static int fintek_8250_rs485_config(struct uart_port *port, struct ktermios *termios,
-			      struct serial_rs485 *rs485)
+			      struct kserial_rs485 *rs485)
 {
 	uint8_t config = 0;
 	struct fintek_8250 *pdata = port->private_data;
@@ -411,13 +411,13 @@ static int probe_setup_port(struct fintek_8250 *pdata,
 }
 
 /* Only the first port supports delays */
-static const struct serial_rs485 fintek_8250_rs485_supported_port0 = {
+static const struct kserial_rs485 fintek_8250_rs485_supported_port0 = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND,
 	.delay_rts_before_send = 1,
 	.delay_rts_after_send = 1,
 };
 
-static const struct serial_rs485 fintek_8250_rs485_supported = {
+static const struct kserial_rs485 fintek_8250_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND,
 };
 
diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index 6dc85aaba5d0..0619427683cf 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -33,7 +33,7 @@ struct lpc18xx_uart_data {
 };
 
 static int lpc18xx_rs485_config(struct uart_port *port, struct ktermios *termios,
-				struct serial_rs485 *rs485)
+				struct kserial_rs485 *rs485)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 	u32 rs485_ctrl_reg = 0;
@@ -81,7 +81,7 @@ static void lpc18xx_uart_serial_out(struct uart_port *p, int offset, int value)
 	writel(value, p->membase + offset);
 }
 
-static const struct serial_rs485 lpc18xx_rs485_supported = {
+static const struct kserial_rs485 lpc18xx_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND,
 	.delay_rts_after_send = 1,
 	/* Delay RTS before send is not supported */
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 6f66dc2ebacc..8675b0d9658d 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1554,7 +1554,7 @@ pci_brcm_trumanage_setup(struct serial_private *priv,
 
 /* We should do proper H/W transceiver setting before change to RS485 mode */
 static int pci_fintek_rs485_config(struct uart_port *port, struct ktermios *termios,
-			       struct serial_rs485 *rs485)
+			       struct kserial_rs485 *rs485)
 {
 	struct pci_dev *pci_dev = to_pci_dev(port->dev);
 	u8 setting;
@@ -1583,7 +1583,7 @@ static int pci_fintek_rs485_config(struct uart_port *port, struct ktermios *term
 	return 0;
 }
 
-static const struct serial_rs485 pci_fintek_rs485_supported = {
+static const struct kserial_rs485 pci_fintek_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
 	/* F81504/508/512 does not support RTS delay before or after send */
 };
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 39b35a61958c..2345b6fbac6d 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -647,7 +647,7 @@ void serial8250_em485_destroy(struct uart_8250_port *p)
 }
 EXPORT_SYMBOL_GPL(serial8250_em485_destroy);
 
-struct serial_rs485 serial8250_em485_supported = {
+struct kserial_rs485 serial8250_em485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND |
 		 SER_RS485_TERMINATE_BUS | SER_RS485_RX_DURING_TX,
 	.delay_rts_before_send = 1,
@@ -665,7 +665,7 @@ EXPORT_SYMBOL_GPL(serial8250_em485_supported);
  * hardware, relying on software emulation instead.
  */
 int serial8250_em485_config(struct uart_port *port, struct ktermios *termios,
-			    struct serial_rs485 *rs485)
+			    struct kserial_rs485 *rs485)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 15f0e4d88c5a..e78cd0759dc1 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2215,7 +2215,7 @@ static int pl011_verify_port(struct uart_port *port, struct serial_struct *ser)
 }
 
 static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
-			      struct serial_rs485 *rs485)
+			      struct kserial_rs485 *rs485)
 {
 	struct uart_amba_port *uap =
 		container_of(port, struct uart_amba_port, port);
@@ -2765,7 +2765,7 @@ static int pl011_register_port(struct uart_amba_port *uap)
 	return ret;
 }
 
-static const struct serial_rs485 pl011_rs485_supported = {
+static const struct kserial_rs485 pl011_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND |
 		 SER_RS485_RX_DURING_TX,
 	.delay_rts_before_send = 1,
diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c
index 32caeac12985..30c716b4a105 100644
--- a/drivers/tty/serial/ar933x_uart.c
+++ b/drivers/tty/serial/ar933x_uart.c
@@ -391,7 +391,7 @@ static void ar933x_uart_rx_chars(struct ar933x_uart_port *up)
 static void ar933x_uart_tx_chars(struct ar933x_uart_port *up)
 {
 	struct circ_buf *xmit = &up->port.state->xmit;
-	struct serial_rs485 *rs485conf = &up->port.rs485;
+	struct kserial_rs485 *rs485conf = &up->port.rs485;
 	int count;
 	bool half_duplex_send = false;
 
@@ -581,7 +581,7 @@ static const struct uart_ops ar933x_uart_ops = {
 };
 
 static int ar933x_config_rs485(struct uart_port *port, struct ktermios *termios,
-				struct serial_rs485 *rs485conf)
+				struct kserial_rs485 *rs485conf)
 {
 	return 0;
 }
@@ -693,8 +693,8 @@ static struct uart_driver ar933x_uart_driver = {
 	.cons		= NULL, /* filled in runtime */
 };
 
-static const struct serial_rs485 ar933x_no_rs485 = {};
-static const struct serial_rs485 ar933x_rs485_supported = {
+static const struct kserial_rs485 ar933x_no_rs485 = {};
+static const struct kserial_rs485 ar933x_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND,
 };
 
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 30ba9eef7b39..be0296714585 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -284,7 +284,7 @@ static void atmel_tasklet_schedule(struct atmel_uart_port *atmel_port,
 
 /* Enable or disable the rs485 support */
 static int atmel_config_rs485(struct uart_port *port, struct ktermios *termios,
-			      struct serial_rs485 *rs485conf)
+			      struct kserial_rs485 *rs485conf)
 {
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 	unsigned int mode;
@@ -2471,7 +2471,7 @@ static const struct uart_ops atmel_pops = {
 #endif
 };
 
-static const struct serial_rs485 atmel_rs485_supported = {
+static const struct kserial_rs485 atmel_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX,
 	.delay_rts_before_send = 1,
 	.delay_rts_after_send = 1,
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index f6c33cd228c8..7b28cb8bee1e 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1374,7 +1374,7 @@ static void lpuart_dma_rx_free(struct uart_port *port)
 }
 
 static int lpuart_config_rs485(struct uart_port *port, struct ktermios *termios,
-			struct serial_rs485 *rs485)
+			struct kserial_rs485 *rs485)
 {
 	struct lpuart_port *sport = container_of(port,
 			struct lpuart_port, port);
@@ -1404,7 +1404,7 @@ static int lpuart_config_rs485(struct uart_port *port, struct ktermios *termios,
 }
 
 static int lpuart32_config_rs485(struct uart_port *port, struct ktermios *termios,
-			struct serial_rs485 *rs485)
+			struct kserial_rs485 *rs485)
 {
 	struct lpuart_port *sport = container_of(port,
 			struct lpuart_port, port);
@@ -2633,7 +2633,7 @@ static struct uart_driver lpuart_reg = {
 	.cons		= LPUART_CONSOLE,
 };
 
-static const struct serial_rs485 lpuart_rs485_supported = {
+static const struct kserial_rs485 lpuart_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND,
 	/* delay_rts_* and RX_DURING_TX are not supported */
 };
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 522445a8f666..0768e8270153 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1908,7 +1908,7 @@ static void imx_uart_poll_put_char(struct uart_port *port, unsigned char c)
 
 /* called with port.lock taken and irqs off or from .probe without locking */
 static int imx_uart_rs485_config(struct uart_port *port, struct ktermios *termios,
-				 struct serial_rs485 *rs485conf)
+				 struct kserial_rs485 *rs485conf)
 {
 	struct imx_port *sport = (struct imx_port *)port;
 	u32 ucr2;
@@ -2196,8 +2196,8 @@ static enum hrtimer_restart imx_trigger_stop_tx(struct hrtimer *t)
 	return HRTIMER_NORESTART;
 }
 
-static const struct serial_rs485 imx_no_rs485 = {};	/* No RS485 if no RTS */
-static const struct serial_rs485 imx_rs485_supported = {
+static const struct kserial_rs485 imx_no_rs485 = {};	/* No RS485 if no RTS */
+static const struct kserial_rs485 imx_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND |
 		 SER_RS485_RX_DURING_TX,
 	.delay_rts_before_send = 1,
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index ab10ca4a45b5..6a8ab63d1072 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1037,7 +1037,7 @@ static void max310x_rs_proc(struct work_struct *ws)
 }
 
 static int max310x_rs485_config(struct uart_port *port, struct ktermios *termios,
-				struct serial_rs485 *rs485)
+				struct kserial_rs485 *rs485)
 {
 	struct max310x_one *one = to_max310x_port(port);
 
@@ -1257,7 +1257,7 @@ static int max310x_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
 }
 #endif
 
-static const struct serial_rs485 max310x_rs485_supported = {
+static const struct kserial_rs485 max310x_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RX_DURING_TX,
 	.delay_rts_before_send = 1,
 	.delay_rts_after_send = 1,
diff --git a/drivers/tty/serial/mcf.c b/drivers/tty/serial/mcf.c
index f4aaaadd0742..9c86218ae29d 100644
--- a/drivers/tty/serial/mcf.c
+++ b/drivers/tty/serial/mcf.c
@@ -432,7 +432,7 @@ static int mcf_verify_port(struct uart_port *port, struct serial_struct *ser)
 
 /* Enable or disable the RS485 support */
 static int mcf_config_rs485(struct uart_port *port, struct ktermios *termios,
-			    struct serial_rs485 *rs485)
+			    struct kserial_rs485 *rs485)
 {
 	unsigned char mr1, mr2;
 
@@ -453,7 +453,7 @@ static int mcf_config_rs485(struct uart_port *port, struct ktermios *termios,
 	return 0;
 }
 
-static const struct serial_rs485 mcf_rs485_supported = {
+static const struct kserial_rs485 mcf_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND,
 };
 
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 0aa666e247d5..013d843833a9 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1325,7 +1325,7 @@ static inline void serial_omap_add_console_port(struct uart_omap_port *up)
 /* Enable or disable the rs485 support */
 static int
 serial_omap_config_rs485(struct uart_port *port, struct ktermios *termios,
-			 struct serial_rs485 *rs485)
+			 struct kserial_rs485 *rs485)
 {
 	struct uart_omap_port *up = to_uart_omap_port(port);
 	unsigned int mode;
@@ -1516,7 +1516,7 @@ static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
 static int serial_omap_probe_rs485(struct uart_omap_port *up,
 				   struct device *dev)
 {
-	struct serial_rs485 *rs485conf = &up->port.rs485;
+	struct kserial_rs485 *rs485conf = &up->port.rs485;
 	struct device_node *np = dev->of_node;
 	enum gpiod_flags gflags;
 	int ret;
@@ -1559,7 +1559,7 @@ static int serial_omap_probe_rs485(struct uart_omap_port *up,
 	return 0;
 }
 
-static const struct serial_rs485 serial_omap_rs485_supported = {
+static const struct kserial_rs485 serial_omap_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND |
 		 SER_RS485_RX_DURING_TX,
 	.delay_rts_before_send = 1,
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 259e08cc347c..e21f30d9fb2f 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -836,7 +836,7 @@ static void sc16is7xx_reconf_rs485(struct uart_port *port)
 	const u32 mask = SC16IS7XX_EFCR_AUTO_RS485_BIT |
 			 SC16IS7XX_EFCR_RTS_INVERT_BIT;
 	u32 efcr = 0;
-	struct serial_rs485 *rs485 = &port->rs485;
+	struct kserial_rs485 *rs485 = &port->rs485;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&port->lock, irqflags);
@@ -1128,7 +1128,7 @@ static void sc16is7xx_set_termios(struct uart_port *port,
 }
 
 static int sc16is7xx_config_rs485(struct uart_port *port, struct ktermios *termios,
-				  struct serial_rs485 *rs485)
+				  struct kserial_rs485 *rs485)
 {
 	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
 	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
@@ -1353,7 +1353,7 @@ static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip,
 }
 #endif
 
-static const struct serial_rs485 sc16is7xx_rs485_supported = {
+static const struct kserial_rs485 sc16is7xx_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND,
 	.delay_rts_before_send = 1,
 	.delay_rts_after_send = 1,	/* Not supported but keep returning -EINVAL */
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 4a4ed619ee71..62c188490988 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1291,7 +1291,7 @@ static int uart_get_icount(struct tty_struct *tty,
 				 SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX | \
 				 SER_RS485_TERMINATE_BUS)
 
-static int uart_check_rs485_flags(struct uart_port *port, struct serial_rs485 *rs485)
+static int uart_check_rs485_flags(struct uart_port *port, struct kserial_rs485 *rs485)
 {
 	u32 flags = rs485->flags;
 
@@ -1320,7 +1320,7 @@ static int uart_check_rs485_flags(struct uart_port *port, struct serial_rs485 *r
 }
 
 static void uart_sanitize_serial_rs485_delays(struct uart_port *port,
-					      struct serial_rs485 *rs485)
+					      struct kserial_rs485 *rs485)
 {
 	if (!port->rs485_supported.delay_rts_before_send) {
 		if (rs485->delay_rts_before_send) {
@@ -1351,7 +1351,7 @@ static void uart_sanitize_serial_rs485_delays(struct uart_port *port,
 	}
 }
 
-static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs485 *rs485)
+static void uart_sanitize_serial_rs485(struct uart_port *port, struct kserial_rs485 *rs485)
 {
 	u32 supported_flags = port->rs485_supported.flags;
 
@@ -1378,7 +1378,7 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
 }
 
 static void uart_set_rs485_termination(struct uart_port *port,
-				       const struct serial_rs485 *rs485)
+				       const struct kserial_rs485 *rs485)
 {
 	if (!(rs485->flags & SER_RS485_ENABLED))
 		return;
@@ -1389,7 +1389,7 @@ static void uart_set_rs485_termination(struct uart_port *port,
 
 int uart_rs485_config(struct uart_port *port)
 {
-	struct serial_rs485 *rs485 = &port->rs485;
+	struct kserial_rs485 *rs485 = &port->rs485;
 	int ret;
 
 	uart_sanitize_serial_rs485(port, rs485);
@@ -1403,23 +1403,31 @@ int uart_rs485_config(struct uart_port *port)
 }
 EXPORT_SYMBOL_GPL(uart_rs485_config);
 
-static int serial_rs485_from_user(struct serial_rs485 *rs485,
+static int serial_rs485_from_user(struct kserial_rs485 *rs485,
 				  const struct serial_rs485 __user *rs485_user)
 {
-	if (copy_from_user(rs485, rs485_user, sizeof(*rs485)))
+	struct serial_rs485 rs485_uapi;
+
+	if (copy_from_user(&rs485_uapi, rs485_user, sizeof(*rs485)))
 		return -EFAULT;
 
+	*rs485 = *((struct kserial_rs485 *)&rs485_uapi);
+
 	return 0;
 }
 
 static int serial_rs485_to_user(struct serial_rs485 __user *rs485_user,
-				struct serial_rs485 *rs485)
+				const struct kserial_rs485 *rs485)
 {
+	struct serial_rs485 rs485_uapi;
+
+	*((struct kserial_rs485 *)&rs485_uapi) = *rs485;
+
 	/* Return clean padding area to userspace */
-	memset(rs485->padding0, 0, sizeof(rs485->padding0));
-	memset(rs485->padding1, 0, sizeof(rs485->padding1));
+	memset(rs485_uapi.padding0, 0, sizeof(rs485_uapi.padding0));
+	memset(rs485_uapi.padding1, 0, sizeof(rs485_uapi.padding1));
 
-	if (copy_to_user(rs485_user, rs485, sizeof(*rs485)))
+	if (copy_to_user(rs485_user, &rs485_uapi, sizeof(rs485_uapi)))
 		return -EFAULT;
 
 	return 0;
@@ -1428,7 +1436,7 @@ static int serial_rs485_to_user(struct serial_rs485 __user *rs485_user,
 static int uart_get_rs485_config(struct uart_port *port,
 			 struct serial_rs485 __user *rs485_user)
 {
-	struct serial_rs485 rs485;
+	struct kserial_rs485 rs485;
 	unsigned long flags;
 
 	spin_lock_irqsave(&port->lock, flags);
@@ -1441,7 +1449,7 @@ static int uart_get_rs485_config(struct uart_port *port,
 static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
 			 struct serial_rs485 __user *rs485_user)
 {
-	struct serial_rs485 rs485;
+	struct kserial_rs485 rs485;
 	int ret;
 	unsigned long flags;
 
@@ -3415,7 +3423,7 @@ EXPORT_SYMBOL_GPL(uart_try_toggle_sysrq);
  */
 int uart_get_rs485_mode(struct uart_port *port)
 {
-	struct serial_rs485 *rs485conf = &port->rs485;
+	struct kserial_rs485 *rs485conf = &port->rs485;
 	struct device *dev = port->dev;
 	u32 rs485_delay[2];
 	int ret;
@@ -3471,7 +3479,23 @@ int uart_get_rs485_mode(struct uart_port *port)
 }
 EXPORT_SYMBOL_GPL(uart_get_rs485_mode);
 
-/* Compile-time assertions for serial_rs485 layout */
+/*
+ * Compile-time asserts for struct kserial_rs485 and struct serial_rs485 equality
+ * (except padding).
+ */
+static_assert(offsetof(struct kserial_rs485, flags) ==
+	      offsetof(struct serial_rs485, flags));
+static_assert(offsetof(struct kserial_rs485, delay_rts_before_send) ==
+	      offsetof(struct serial_rs485, delay_rts_before_send));
+static_assert(offsetof(struct kserial_rs485, delay_rts_after_send) ==
+	      offsetof(struct serial_rs485, delay_rts_after_send));
+static_assert(offsetof(struct kserial_rs485, addr_recv) ==
+	      offsetof(struct serial_rs485, addr_recv));
+static_assert(offsetof(struct kserial_rs485, addr_dest) ==
+	      offsetof(struct serial_rs485, addr_dest));
+static_assert(sizeof(struct kserial_rs485) <= sizeof(struct serial_rs485));
+
+/* Compile-time assertions for struct serial_rs485 layout */
 static_assert(offsetof(struct serial_rs485, padding) ==
               (offsetof(struct serial_rs485, delay_rts_after_send) + sizeof(__u32)));
 static_assert(offsetof(struct serial_rs485, padding1) ==
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 2c85dbf165c4..8606bda9f7fa 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -169,7 +169,7 @@ static void stm32_usart_config_reg_rs485(u32 *cr1, u32 *cr3, u32 delay_ADE,
 }
 
 static int stm32_usart_config_rs485(struct uart_port *port, struct ktermios *termios,
-				    struct serial_rs485 *rs485conf)
+				    struct kserial_rs485 *rs485conf)
 {
 	struct stm32_port *stm32_port = to_stm32_port(port);
 	const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
@@ -220,7 +220,7 @@ static int stm32_usart_config_rs485(struct uart_port *port, struct ktermios *ter
 static int stm32_usart_init_rs485(struct uart_port *port,
 				  struct platform_device *pdev)
 {
-	struct serial_rs485 *rs485conf = &port->rs485;
+	struct kserial_rs485 *rs485conf = &port->rs485;
 
 	rs485conf->flags = 0;
 	rs485conf->delay_rts_before_send = 0;
@@ -532,7 +532,7 @@ static void stm32_usart_tc_interrupt_disable(struct uart_port *port)
 static void stm32_usart_rs485_rts_enable(struct uart_port *port)
 {
 	struct stm32_port *stm32_port = to_stm32_port(port);
-	struct serial_rs485 *rs485conf = &port->rs485;
+	struct kserial_rs485 *rs485conf = &port->rs485;
 
 	if (stm32_port->hw_flow_control ||
 	    !(rs485conf->flags & SER_RS485_ENABLED))
@@ -550,7 +550,7 @@ static void stm32_usart_rs485_rts_enable(struct uart_port *port)
 static void stm32_usart_rs485_rts_disable(struct uart_port *port)
 {
 	struct stm32_port *stm32_port = to_stm32_port(port);
-	struct serial_rs485 *rs485conf = &port->rs485;
+	struct kserial_rs485 *rs485conf = &port->rs485;
 
 	if (stm32_port->hw_flow_control ||
 	    !(rs485conf->flags & SER_RS485_ENABLED))
@@ -1094,7 +1094,7 @@ static void stm32_usart_set_termios(struct uart_port *port,
 	struct stm32_port *stm32_port = to_stm32_port(port);
 	const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
 	const struct stm32_usart_config *cfg = &stm32_port->info->cfg;
-	struct serial_rs485 *rs485conf = &port->rs485;
+	struct kserial_rs485 *rs485conf = &port->rs485;
 	unsigned int baud, bits;
 	u32 usartdiv, mantissa, fraction, oversampling;
 	tcflag_t cflag = termios->c_cflag;
@@ -1446,7 +1446,7 @@ static void stm32_usart_deinit_port(struct stm32_port *stm32port)
 	clk_disable_unprepare(stm32port->clk);
 }
 
-static const struct serial_rs485 stm32_rs485_supported = {
+static const struct kserial_rs485 stm32_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND |
 		 SER_RS485_RX_DURING_TX,
 	.delay_rts_before_send = 1,
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index aef3145f2032..3a987af88ffe 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -31,6 +31,29 @@ struct serial_struct;
 struct device;
 struct gpio_desc;
 
+/**
+ * struct kserial_rs485 - kernel-side struct for controlling RS485 settings.
+ * @flags:			RS485 feature flags
+ * @delay_rts_before_send:	Delay before send (milliseconds)
+ * @delay_rts_after_send:	Delay after send (milliseconds)
+ * @addr_recv:			Receive filter for RS485 addressing mode
+ *				(used only when %SER_RS485_ADDR_RECV is set).
+ * @addr_dest:			Destination address for RS485 addressing mode
+ *				(used only when %SER_RS485_ADDR_DEST is set).
+ *
+ * Must match with struct serial_rs485 in include/uapi/linux/serial.h excluding
+ * the padding.
+ */
+struct kserial_rs485 {
+	__u32	flags;
+	__u32	delay_rts_before_send;
+	__u32	delay_rts_after_send;
+	struct {
+		__u8    addr_recv;
+		__u8    addr_dest;
+	};
+};
+
 /**
  * struct uart_ops -- interface between serial_core and the driver
  *
@@ -447,7 +470,7 @@ struct uart_port {
 	void			(*handle_break)(struct uart_port *);
 	int			(*rs485_config)(struct uart_port *,
 						struct ktermios *termios,
-						struct serial_rs485 *rs485);
+						struct kserial_rs485 *rs485);
 	int			(*iso7816_config)(struct uart_port *,
 						  struct serial_iso7816 *iso7816);
 	unsigned int		irq;			/* irq number */
@@ -567,8 +590,8 @@ struct uart_port {
 	const char		*name;			/* port name */
 	struct attribute_group	*attr_group;		/* port specific attributes */
 	const struct attribute_group **tty_groups;	/* all attributes (serial core use only) */
-	struct serial_rs485     rs485;
-	struct serial_rs485	rs485_supported;	/* Supported mask for serial_rs485 */
+	struct kserial_rs485	rs485;
+	struct kserial_rs485	rs485_supported;	/* Supported mask for serial_rs485 */
 	struct gpio_desc	*rs485_term_gpio;	/* enable RS485 bus termination */
 	struct serial_iso7816   iso7816;
 	void			*private_data;		/* generic platform data pointer */
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index dbdc7865abdc..7063eb6b1617 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -138,6 +138,8 @@ struct serial_icounter_struct {
  *				  Requires %SER_RS485_ADDRB.
  * * %SER_RS485_ADDR_DEST	- Destination address (enables @addr_dest).
  *				  Requires %SER_RS485_ADDRB.
+ *
+ * There's kernel counterpart struct kserial_rs485 of this struct without padding.
  */
 struct serial_rs485 {
 	__u32	flags;
-- 
2.30.2

Re: [PATCH v2 4/4] serial: Add kserial_rs485 to avoid wasted space due to .padding

[Jiri Slaby]

On 30. 08. 22, 9:29, Ilpo Järvinen wrote:
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -31,6 +31,29 @@ struct serial_struct;
...
> + * Must match with struct serial_rs485 in include/uapi/linux/serial.h excluding
> + * the padding.

Have you considered BUILD_BUG_ON() checks with few offset_of()s to 
enforce the above?

> +struct kserial_rs485 {
> +	__u32	flags;
> +	__u32	delay_rts_before_send;
> +	__u32	delay_rts_after_send;
> +	struct {
> +		__u8    addr_recv;
> +		__u8    addr_dest;
> +	};
> +};

thanks,
-- 
js
suse labs

Re: [PATCH v2 4/4] serial: Add kserial_rs485 to avoid wasted space due to .padding

[Ilpo Järvinen]

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

On Tue, 30 Aug 2022, Jiri Slaby wrote:

> On 30. 08. 22, 9:29, Ilpo Järvinen wrote:
> > --- a/include/linux/serial_core.h
> > +++ b/include/linux/serial_core.h
> > @@ -31,6 +31,29 @@ struct serial_struct;
> ...
> > + * Must match with struct serial_rs485 in include/uapi/linux/serial.h
> > excluding
> > + * the padding.
> 
> Have you considered BUILD_BUG_ON() checks with few offset_of()s to enforce the
> above?

It's all enforced at the end of serial_core.c already. Would you perhaps 
prefer I'd put them elsewhere or is it fine as is?

/*
 * Compile-time asserts for struct kserial_rs485 and struct serial_rs485 equality
 * (except padding).
 */
static_assert(offsetof(struct kserial_rs485, flags) ==
              offsetof(struct serial_rs485, flags));
static_assert(offsetof(struct kserial_rs485, delay_rts_before_send) ==
              offsetof(struct serial_rs485, delay_rts_before_send));
static_assert(offsetof(struct kserial_rs485, delay_rts_after_send) ==
              offsetof(struct serial_rs485, delay_rts_after_send));
static_assert(offsetof(struct kserial_rs485, addr_recv) ==
              offsetof(struct serial_rs485, addr_recv));
static_assert(offsetof(struct kserial_rs485, addr_dest) ==
              offsetof(struct serial_rs485, addr_dest));
static_assert(sizeof(struct kserial_rs485) <= sizeof(struct serial_rs485));


-- 
 i.

> > +struct kserial_rs485 {
> > +	__u32	flags;
> > +	__u32	delay_rts_before_send;
> > +	__u32	delay_rts_after_send;
> > +	struct {
> > +		__u8    addr_recv;
> > +		__u8    addr_dest;
> > +	};
> > +};

Re: [PATCH v2 4/4] serial: Add kserial_rs485 to avoid wasted space due to .padding

[Greg Kroah-Hartman]

On Tue, Aug 30, 2022 at 10:29:56AM +0300, Ilpo Järvinen wrote:
> The struct serial_rs485 has a .padding field to make uapi updates
> easier. It wastes space, however. Create struct kserial_rs485 which is
> a kerner counterpart w/o padding.

"kernel"?

And what is the size difference now?

> +/**
> + * struct kserial_rs485 - kernel-side struct for controlling RS485 settings.
> + * @flags:			RS485 feature flags
> + * @delay_rts_before_send:	Delay before send (milliseconds)
> + * @delay_rts_after_send:	Delay after send (milliseconds)
> + * @addr_recv:			Receive filter for RS485 addressing mode
> + *				(used only when %SER_RS485_ADDR_RECV is set).
> + * @addr_dest:			Destination address for RS485 addressing mode
> + *				(used only when %SER_RS485_ADDR_DEST is set).
> + *
> + * Must match with struct serial_rs485 in include/uapi/linux/serial.h excluding
> + * the padding.

Why must this match?  And how is that going to be enforced?

> + */
> +struct kserial_rs485 {
> +	__u32	flags;
> +	__u32	delay_rts_before_send;
> +	__u32	delay_rts_after_send;
> +	struct {
> +		__u8    addr_recv;
> +		__u8    addr_dest;
> +	};

As this is an in-kernel structure, this should be "u32" and "u8" now.

thanks,

greg k-h

Re: [PATCH v2 4/4] serial: Add kserial_rs485 to avoid wasted space due to .padding

[Ilpo Järvinen]

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

On Tue, 30 Aug 2022, Greg Kroah-Hartman wrote:

> On Tue, Aug 30, 2022 at 10:29:56AM +0300, Ilpo Järvinen wrote:
> > The struct serial_rs485 has a .padding field to make uapi updates
> > easier. It wastes space, however. Create struct kserial_rs485 which is
> > a kerner counterpart w/o padding.
> 
> "kernel"?
> 
> And what is the size difference now?

Roughly 16B of padding is eliminated. That saving is then multiplied by 
the times it appears in in-kernel structs (2x per uart_port, 1x per rs485 
supporting driver). As said in my other reply, if you feel it's too 
little gained by eliminating the padding, I can drop this patch, just let 
me know.

> > +/**
> > + * struct kserial_rs485 - kernel-side struct for controlling RS485 settings.
> > + * @flags:			RS485 feature flags
> > + * @delay_rts_before_send:	Delay before send (milliseconds)
> > + * @delay_rts_after_send:	Delay after send (milliseconds)
> > + * @addr_recv:			Receive filter for RS485 addressing mode
> > + *				(used only when %SER_RS485_ADDR_RECV is set).
> > + * @addr_dest:			Destination address for RS485 addressing mode
> > + *				(used only when %SER_RS485_ADDR_DEST is set).
> > + *
> > + * Must match with struct serial_rs485 in include/uapi/linux/serial.h excluding
> > + * the padding.
> 
> Why must this match?

Because serial_rs485_from_user() and serial_rs485_to_user() just copy 
things over from one struct type to another w/o considering the fields 
individually. If that's not acceptable, I could make it copy field by 
field but it didn't feel necessary to allow "real" fields to differ to 
achieve padding elimination...

> And how is that going to be enforced?

With static_assert()s in serial_core.c. I'll add a note about that into 
the comment.

> > + */
> > +struct kserial_rs485 {
> > +	__u32	flags;
> > +	__u32	delay_rts_before_send;
> > +	__u32	delay_rts_after_send;
> > +	struct {
> > +		__u8    addr_recv;
> > +		__u8    addr_dest;
> > +	};
> 
> As this is an in-kernel structure, this should be "u32" and "u8" now.

Right, I'll change those.

-- 
 i.

Re: [PATCH v2 4/4] serial: Add kserial_rs485 to avoid wasted space due to .padding

[Greg Kroah-Hartman]

On Tue, Aug 30, 2022 at 10:29:56AM +0300, Ilpo Järvinen wrote:
> -static int serial_rs485_from_user(struct serial_rs485 *rs485,
> +static int serial_rs485_from_user(struct kserial_rs485 *rs485,
>  				  const struct serial_rs485 __user *rs485_user)
>  {
> -	if (copy_from_user(rs485, rs485_user, sizeof(*rs485)))
> +	struct serial_rs485 rs485_uapi;
> +
> +	if (copy_from_user(&rs485_uapi, rs485_user, sizeof(*rs485)))
>  		return -EFAULT;
>  
> +	*rs485 = *((struct kserial_rs485 *)&rs485_uapi);

Ah, you are mapping this on top of the existing structure, so there was
no padding in the original one, why say that?

> +/*
> + * Compile-time asserts for struct kserial_rs485 and struct serial_rs485 equality
> + * (except padding).

This does not take into account any padding, in fact it's the opposite
as all of this:

> + */
> +static_assert(offsetof(struct kserial_rs485, flags) ==
> +	      offsetof(struct serial_rs485, flags));
> +static_assert(offsetof(struct kserial_rs485, delay_rts_before_send) ==
> +	      offsetof(struct serial_rs485, delay_rts_before_send));
> +static_assert(offsetof(struct kserial_rs485, delay_rts_after_send) ==
> +	      offsetof(struct serial_rs485, delay_rts_after_send));
> +static_assert(offsetof(struct kserial_rs485, addr_recv) ==
> +	      offsetof(struct serial_rs485, addr_recv));
> +static_assert(offsetof(struct kserial_rs485, addr_dest) ==
> +	      offsetof(struct serial_rs485, addr_dest));
> +static_assert(sizeof(struct kserial_rs485) <= sizeof(struct serial_rs485));

Is there to ensure that the offsets are exactly the same, no padding
involved anywhere.

So I don't understand the problem you are trying to solve here,

greg k-h

Re: [PATCH v2 4/4] serial: Add kserial_rs485 to avoid wasted space due to .padding

[Ilpo Järvinen]

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

On Tue, 30 Aug 2022, Greg Kroah-Hartman wrote:

> On Tue, Aug 30, 2022 at 10:29:56AM +0300, Ilpo Järvinen wrote:
> > -static int serial_rs485_from_user(struct serial_rs485 *rs485,
> > +static int serial_rs485_from_user(struct kserial_rs485 *rs485,
> >  				  const struct serial_rs485 __user *rs485_user)
> >  {
> > -	if (copy_from_user(rs485, rs485_user, sizeof(*rs485)))
> > +	struct serial_rs485 rs485_uapi;
> > +
> > +	if (copy_from_user(&rs485_uapi, rs485_user, sizeof(*rs485)))
> >  		return -EFAULT;
> >  
> > +	*rs485 = *((struct kserial_rs485 *)&rs485_uapi);
> 
> Ah, you are mapping this on top of the existing structure, so there was
> no padding in the original one, why say that?

While I'm not exactly sure what you tried to say with this, I'll try to 
answer regardless.

It's the opposite, there's padding in rs485_user, and therefore also in 
rs485_uapi. Struct serial_rs485 has padding and is part of uapi so it 
cannot be changed to remove the extra padding.

I cannot directly copy_from_user into *rs485 because it lacks the padding. 
Thus, the immediate rs485_uapi and then assign to rs485.

> > +/*
> > + * Compile-time asserts for struct kserial_rs485 and struct serial_rs485 equality
> > + * (except padding).
> 
> This does not take into account any padding, in fact it's the opposite
> as all of this:

?? I said: "(except padding)" which I thought implies that padding is 
intentionally excluded (it doesn't exist in kserial_rs485).

> > + */
> > +static_assert(offsetof(struct kserial_rs485, flags) ==
> > +	      offsetof(struct serial_rs485, flags));
> > +static_assert(offsetof(struct kserial_rs485, delay_rts_before_send) ==
> > +	      offsetof(struct serial_rs485, delay_rts_before_send));
> > +static_assert(offsetof(struct kserial_rs485, delay_rts_after_send) ==
> > +	      offsetof(struct serial_rs485, delay_rts_after_send));
> > +static_assert(offsetof(struct kserial_rs485, addr_recv) ==
> > +	      offsetof(struct serial_rs485, addr_recv));
> > +static_assert(offsetof(struct kserial_rs485, addr_dest) ==
> > +	      offsetof(struct serial_rs485, addr_dest));
> > +static_assert(sizeof(struct kserial_rs485) <= sizeof(struct serial_rs485));
> 
> Is there to ensure that the offsets are exactly the same, no padding
> involved anywhere.

That's because for kernel padding "doesn't matter", it doesn't want it,
it would be just wasted space. After this series, padding is used only for 
uapi, no longer for the in-kernel structs.

> So I don't understand the problem you are trying to solve here,

struct serial_rs485 has padding that is ~16B long currently. serial_rs485 
is currently used for a few things:
- Keep track of rs485 state (per port)
- Record what rs485 options the port supports (per port)
- Record rs485 options a driver supports (per driver with rs485 support)
- Exchange rs485 config/state information with userspace

Only the last of those requires the padding (because it has been part of 
uapi since day 1). With kserial_rs485, the padding can eliminated for the 
first 3 cases.

If you feel ~32B per uart_port too little to be useful (and a little 
more per driver), I can just drop this patch.

-- 
 i.

Re: [PATCH v2 4/4] serial: Add kserial_rs485 to avoid wasted space due to .padding

[Greg Kroah-Hartman]

On Tue, Aug 30, 2022 at 12:26:29PM +0300, Ilpo Järvinen wrote:
> On Tue, 30 Aug 2022, Greg Kroah-Hartman wrote:
> 
> > On Tue, Aug 30, 2022 at 10:29:56AM +0300, Ilpo Järvinen wrote:
> > > -static int serial_rs485_from_user(struct serial_rs485 *rs485,
> > > +static int serial_rs485_from_user(struct kserial_rs485 *rs485,
> > >  				  const struct serial_rs485 __user *rs485_user)
> > >  {
> > > -	if (copy_from_user(rs485, rs485_user, sizeof(*rs485)))
> > > +	struct serial_rs485 rs485_uapi;
> > > +
> > > +	if (copy_from_user(&rs485_uapi, rs485_user, sizeof(*rs485)))
> > >  		return -EFAULT;
> > >  
> > > +	*rs485 = *((struct kserial_rs485 *)&rs485_uapi);
> > 
> > Ah, you are mapping this on top of the existing structure, so there was
> > no padding in the original one, why say that?
> 
> While I'm not exactly sure what you tried to say with this, I'll try to 
> answer regardless.
> 
> It's the opposite, there's padding in rs485_user, and therefore also in 
> rs485_uapi. Struct serial_rs485 has padding and is part of uapi so it 
> cannot be changed to remove the extra padding.
> 
> I cannot directly copy_from_user into *rs485 because it lacks the padding. 
> Thus, the immediate rs485_uapi and then assign to rs485.

Padding could be in the middle of the structure, it's not obvious that
it is not there.  You are just trying to drop the trailing "unused
space", while all of the fields are identical otherwise.

So be specific about that, as padding is often in the middle of a
structure.

> > > +/*
> > > + * Compile-time asserts for struct kserial_rs485 and struct serial_rs485 equality
> > > + * (except padding).
> > 
> > This does not take into account any padding, in fact it's the opposite
> > as all of this:
> 
> ?? I said: "(except padding)" which I thought implies that padding is 
> intentionally excluded (it doesn't exist in kserial_rs485).
> 
> > > + */
> > > +static_assert(offsetof(struct kserial_rs485, flags) ==
> > > +	      offsetof(struct serial_rs485, flags));
> > > +static_assert(offsetof(struct kserial_rs485, delay_rts_before_send) ==
> > > +	      offsetof(struct serial_rs485, delay_rts_before_send));
> > > +static_assert(offsetof(struct kserial_rs485, delay_rts_after_send) ==
> > > +	      offsetof(struct serial_rs485, delay_rts_after_send));
> > > +static_assert(offsetof(struct kserial_rs485, addr_recv) ==
> > > +	      offsetof(struct serial_rs485, addr_recv));
> > > +static_assert(offsetof(struct kserial_rs485, addr_dest) ==
> > > +	      offsetof(struct serial_rs485, addr_dest));
> > > +static_assert(sizeof(struct kserial_rs485) <= sizeof(struct serial_rs485));
> > 
> > Is there to ensure that the offsets are exactly the same, no padding
> > involved anywhere.
> 
> That's because for kernel padding "doesn't matter", it doesn't want it,
> it would be just wasted space. After this series, padding is used only for 
> uapi, no longer for the in-kernel structs.

Again, you are talking about padding at the end, not in the middle,
hence my confusion.

> > So I don't understand the problem you are trying to solve here,
> 
> struct serial_rs485 has padding that is ~16B long currently. serial_rs485 
> is currently used for a few things:
> - Keep track of rs485 state (per port)
> - Record what rs485 options the port supports (per port)
> - Record rs485 options a driver supports (per driver with rs485 support)
> - Exchange rs485 config/state information with userspace
> 
> Only the last of those requires the padding (because it has been part of 
> uapi since day 1). With kserial_rs485, the padding can eliminated for the 
> first 3 cases.
> 
> If you feel ~32B per uart_port too little to be useful (and a little 
> more per driver), I can just drop this patch.

I think 32 bytes per serial port is totally lost in the noise and would
not even be able to be measured at all due to how slabs are aligned
(meaning you are not actually saving any memory at all.)

Can you notice any measurable savings on your systems?

And what is the code increase overall with this patch series?  :)

I'm all for making things const, to prevent errors, but that could
probably be done without this type of change, right?

thanks,

greg k-h

Re: [PATCH v2 4/4] serial: Add kserial_rs485 to avoid wasted space due to .padding

[Ilpo Järvinen]

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

On Tue, 30 Aug 2022, Greg Kroah-Hartman wrote:

> On Tue, Aug 30, 2022 at 12:26:29PM +0300, Ilpo Järvinen wrote:
> > On Tue, 30 Aug 2022, Greg Kroah-Hartman wrote:

> > > Ah, you are mapping this on top of the existing structure, so there was
> > > no padding in the original one, why say that?
> > 
> > While I'm not exactly sure what you tried to say with this, I'll try to 
> > answer regardless.
> > 
> > It's the opposite, there's padding in rs485_user, and therefore also in 
> > rs485_uapi. Struct serial_rs485 has padding and is part of uapi so it 
> > cannot be changed to remove the extra padding.
> > 
> > I cannot directly copy_from_user into *rs485 because it lacks the padding. 
> > Thus, the immediate rs485_uapi and then assign to rs485.
> 
> Padding could be in the middle of the structure, it's not obvious that
> it is not there.  You are just trying to drop the trailing "unused
> space", while all of the fields are identical otherwise.
> 
> So be specific about that, as padding is often in the middle of a
> structure.

Ah, sorry. I didn't realize there would be such a way to misunderstand
the message because I knew too well where the padding with this particular 
struct is.

> > If you feel ~32B per uart_port too little to be useful (and a little 
> > more per driver), I can just drop this patch.
> 
> I think 32 bytes per serial port is totally lost in the noise and would
> not even be able to be measured at all due to how slabs are aligned
> (meaning you are not actually saving any memory at all.)
>
> Can you notice any measurable savings on your systems?

It's not that straightforward. Many uart_ports are embedded into arrays
like this:

static struct ...[N];

...But then one could again say that, e.g., module alignment eats up all 
potential benefits, etc.

Obviously with big systems and small number of ports, this would never 
matter much so while I believe likely could get some small looking number 
for you I don't feel the effort needed to be anymore justified.

> And what is the code increase overall with this patch series?  :)

The series was mostly shuffling existing code around, the only thing added 
was that those struct copies so probably less than it looked.

> I'm all for making things const, to prevent errors, but that could
> probably be done without this type of change, right?

OK, I'll drop this last patch. For the first three though, there's useful 
stuff in them making some things more cleaner/consistent, I'll sort that 
out.

Thanks for your comments.


-- 
 i.