diff mbox series

[22/36] serial: Sanitize rs485_struct

Message ID 20220606100433.13793-23-ilpo.jarvinen@linux.intel.com
State New
Headers show
Series RS485 serial_rs485 sanitization | expand

Commit Message

Ilpo Järvinen June 6, 2022, 10:04 a.m. UTC
Sanitize serial_rs485 struct before calling into rs485_setup. The
drivers provide supported_rs485 to help sanitization of the fields.

If neither of SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
supported, don't pretend they can be set to sane settings but clear
them both instead. If only one of them is supported it may look
tempting to use the one driver supports to set the other, however, the
userspace does not have that information readily available so it
wouldn't be helpful.

While adjusting the documentation, remove also the claim that
TIOCGRS485 would call driver specific code. In reality, it does nothing
else than copies the stored serial_rs485 structure from uart_port to
userspace.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 .../driver-api/serial/serial-rs485.rst        | 12 ++++---
 drivers/tty/serial/serial_core.c              | 33 ++++++++++++++++---
 2 files changed, 37 insertions(+), 8 deletions(-)

Comments

Lino Sanfilippo June 26, 2022, 12:39 p.m. UTC | #1
On 25.06.22 at 22:12, Lukas Wunner wrote:
> On Mon, Jun 06, 2022 at 01:04:19PM +0300, Ilpo Järvinen wrote:
>> -	if (rs485->delay_rts_before_send > RS485_MAX_RTS_DELAY) {
>> +	if (!port->rs485_supported->delay_rts_before_send) {
>> +		if (rs485->delay_rts_before_send) {
>> +			dev_warn_ratelimited(port->dev,
>> +				"%s (%d): RTS delay before sending not supported\n",
>> +				port->name, port->line);
>> +		}
>> +		rs485->delay_rts_before_send = 0;
>> +	} else if (rs485->delay_rts_before_send > RS485_MAX_RTS_DELAY) {
>>  		rs485->delay_rts_before_send = RS485_MAX_RTS_DELAY;
>>  		dev_warn_ratelimited(port->dev,
>>  			"%s (%d): RTS delay before sending clamped to %u ms\n",
>>  			port->name, port->line, rs485->delay_rts_before_send);
>>  	}
>
> This series seems to set rs485_supported->delay_rts_before_send to 1
> in all drivers to indicate that a delay is supported.
>
> It would probably be smarter to define it as a maximum, i.e. drivers
> declare the supported maximum delay in their rs485_supported struct
> and the core can use that to clamp the value.  Initially, all drivers
> may use RS485_MAX_RTS_DELAY.  Some chips only support specific delays
> (multiples of the UART clock or baud clock).  We can amend their
> drivers later according to their capabilities.

Agreed.

Regards,
Lino
diff mbox series

Patch

diff --git a/Documentation/driver-api/serial/serial-rs485.rst b/Documentation/driver-api/serial/serial-rs485.rst
index 6bc824f948f9..00b5d333acba 100644
--- a/Documentation/driver-api/serial/serial-rs485.rst
+++ b/Documentation/driver-api/serial/serial-rs485.rst
@@ -38,10 +38,14 @@  RS485 Serial Communications
    the values given by the device tree.
 
    Any driver for devices capable of working both as RS232 and RS485 should
-   implement the rs485_config callback in the uart_port structure. The
-   serial_core calls rs485_config to do the device specific part in response
-   to TIOCSRS485 and TIOCGRS485 ioctls (see below). The rs485_config callback
-   receives a pointer to struct serial_rs485.
+   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
+   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
+   serial_rs485 structure matching to the current configuration.
 
 4. Usage from user-level
 ========================
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 44a50158552d..f0d7b3d20731 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1278,36 +1278,61 @@  static int uart_get_icount(struct tty_struct *tty,
 
 static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs485 *rs485)
 {
+	u32 supported_flags = port->rs485_supported->flags;
+
 	/* pick sane settings if the user hasn't */
-	if (!(rs485->flags & SER_RS485_RTS_ON_SEND) ==
+	if ((supported_flags & (SER_RS485_RTS_ON_SEND|SER_RS485_RTS_AFTER_SEND)) &&
+	    !(rs485->flags & SER_RS485_RTS_ON_SEND) ==
 	    !(rs485->flags & SER_RS485_RTS_AFTER_SEND)) {
 		dev_warn_ratelimited(port->dev,
 			"%s (%d): invalid RTS setting, using RTS_ON_SEND instead\n",
 			port->name, port->line);
 		rs485->flags |= SER_RS485_RTS_ON_SEND;
 		rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
+		supported_flags |= SER_RS485_RTS_ON_SEND|SER_RS485_RTS_AFTER_SEND;
 	}
 
-	if (rs485->delay_rts_before_send > RS485_MAX_RTS_DELAY) {
+	if (!port->rs485_supported->delay_rts_before_send) {
+		if (rs485->delay_rts_before_send) {
+			dev_warn_ratelimited(port->dev,
+				"%s (%d): RTS delay before sending not supported\n",
+				port->name, port->line);
+		}
+		rs485->delay_rts_before_send = 0;
+	} else if (rs485->delay_rts_before_send > RS485_MAX_RTS_DELAY) {
 		rs485->delay_rts_before_send = RS485_MAX_RTS_DELAY;
 		dev_warn_ratelimited(port->dev,
 			"%s (%d): RTS delay before sending clamped to %u ms\n",
 			port->name, port->line, rs485->delay_rts_before_send);
 	}
 
-	if (rs485->delay_rts_after_send > RS485_MAX_RTS_DELAY) {
+	if (!port->rs485_supported->delay_rts_after_send) {
+		if (rs485->delay_rts_after_send) {
+			dev_warn_ratelimited(port->dev,
+				"%s (%d): RTS delay after sending not supported\n",
+				port->name, port->line);
+		}
+		rs485->delay_rts_after_send = 0;
+	} else if (rs485->delay_rts_after_send > RS485_MAX_RTS_DELAY) {
 		rs485->delay_rts_after_send = RS485_MAX_RTS_DELAY;
 		dev_warn_ratelimited(port->dev,
 			"%s (%d): RTS delay after sending clamped to %u ms\n",
 			port->name, port->line, rs485->delay_rts_after_send);
 	}
+
+	rs485->flags &= supported_flags;
+
 	/* Return clean padding area to userspace */
 	memset(rs485->padding, 0, sizeof(rs485->padding));
 }
 
 int uart_rs485_config(struct uart_port *port)
 {
-	return port->rs485_config(port, &port->rs485);
+	struct serial_rs485 *rs485 = &port->rs485;
+
+	uart_sanitize_serial_rs485(port, rs485);
+
+	return port->rs485_config(port, rs485);
 }
 EXPORT_SYMBOL_GPL(uart_rs485_config);