[PATCH 0/7] serial: core: Power the line transceiver with the port

[PATCH 0/7] serial: core: Power the line transceiver with the port

[Tapio Reijonen]

A UART's line transceiver (RS-232, RS-485, isolated or multiprotocol)
has no driver of its own, so it stays powered for as long as the system
is up, whether or not the port is open. On many boards it is the
largest consumer behind the port and can be switched off, through a
shutdown input of the part or a switch in its supply.

Patch 1 is a max310x fix that stands on its own: a GPIO lookup that
defers for a later channel came after the earlier channels' ports were
added, and the probe then looped for as long as the GPIO provider was
missing. It carries Fixes and Cc stable and applies without the rest.

Patch 2 describes that switch in serial.yaml as
transceiver-enable-gpios, with transceiver-startup-delay-us for parts
that need time to come up. Patch 3 rejects the properties on the
dual-channel SC16IS7xx parts, whose two ports share one node. Patch 4
lets serial core request the GPIO when a port is added and switch it in
uart_change_pm(): on before the driver powers the port up, off after the
driver has powered it down, so the transceiver is never switched off
under a running port. That covers open and close, suspend and resume,
registration and the /proc/tty/driver read. A console port stays
powered, as it does today. Without the property nothing changes.

Patches 5 to 7 are for max310x: the transceiver is requested per channel
from the channel's subnode, and a new per-channel flag,
maxim,tristate-when-closed, three-states TX and RTS while the port is
closed, so that the pins do not back-feed an unpowered part behind them.

The series applies on top of the rs485-select series [1]: at
registration, serial core sets the rs485-select-gpios line before it
powers the transceiver up, so that a multiprotocol part comes up in the
configured mode.

Tested on an i.MX6SX UART with an LTC2873 behind its shutdown input and
on a MAX14830 with one LTC2873 and three ADM2484E isolated transceivers
behind switched supplies. On the bus and on the isolated supply: no
drive and no back-feed at open, close, registration, /proc read, hangup
and unbind of an open port; the last character of a write followed by
a close reaches the bus; data in both directions between the two UARTs;
a closed port's unpowered transceiver does not load a live bus. Suspend
and resume were not tested on hardware; they use the same
uart_change_pm() order as close and open.

Tool use: Claude Code (Anthropic) was used throughout. It drafted the
code and the commit messages, reviewed the series against serial core,
wrote the hardware test plans and analysed the logic-analyser captures.
I decided the design, in particular powering the transceiver in
uart_change_pm() and the max310x three-state, reviewed every change
against the code, and ran the hardware tests. The code and the messages
are mine to defend.

[1] https://lore.kernel.org/r/20261105-serial-rs485-mode-gpio-v2-0-027e5348892f@vaisala.com

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
Tapio Reijonen (7):
      serial: max310x: Look up all channels before adding any port
      dt-bindings: serial: Add the line transceiver enable GPIO
      dt-bindings: serial: sc16is7xx: Disallow transceiver GPIO on dual parts
      serial: core: Power the line transceiver with the port
      serial: max310x: Request the line transceiver per port
      dt-bindings: serial: max310x: Add maxim,tristate-when-closed
      serial: max310x: Three-state TX and RTS while the port is closed

 .../devicetree/bindings/serial/maxim,max310x.yaml  | 16 ++++
 .../devicetree/bindings/serial/nxp,sc16is7xx.yaml  | 11 +++
 .../devicetree/bindings/serial/serial.yaml         | 14 +++
 drivers/tty/serial/max310x.c                       | 62 +++++++++++---
 drivers/tty/serial/serial_core.c                   | 99 +++++++++++++++++++++-
 include/linux/serial_core.h                        |  3 +
 6 files changed, 189 insertions(+), 16 deletions(-)
---
base-commit: bc71431221f5ba8bb33918399552636e059c2c45
change-id: 20261022-serial-transceiver-supply-9e5b76fbf153
prerequisite-change-id: 20261019-serial-rs485-mode-gpio-e76bb72b6f21:v2
prerequisite-patch-id: cbd0696a89bd2d2c80a6a1767d30e1521a1365eb
prerequisite-patch-id: 7e8ceb778d3d7189c06b6a4730cb3ae30039c174
prerequisite-patch-id: f3cacd874a4d95dd58b19ebd5ce19db6489be5e2

Best regards,
-- 
Tapio Reijonen <tapio.reijonen@vaisala.com>

[PATCH 1/7] serial: max310x: Look up all channels before adding any port

[Tapio Reijonen]

The probe reads each channel's RS485 properties with
uart_get_rs485_mode() and adds the port in the same loop. The GPIO
lookups in uart_get_rs485_mode() can return -EPROBE_DEFER, and for
channel N > 0 that happens after channels 0..N-1 were added, with
their serial_base child devices bound. A probe must not defer after
creating child devices: the bound children re-trigger deferred probing
at once, so the probe runs in a loop for as long as the GPIO provider
is missing, see Documentation/driver-api/driver-model/driver.rst.

Read the properties of all channels first and add the ports in a
second loop. An error in the first loop goes to out_clk: no line is
assigned yet, so out_uart would test line 0 for every channel.

Fixes: bed094602ffb ("serial: max310x: honour rs485 properties from per-channel DT subnode")
Cc: stable@vger.kernel.org
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
 drivers/tty/serial/max310x.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index a0ed70185555eed745d905b36c8b8f162468dd6f..bdfdf4a9000bbd18509746bbfd8ed0ade8164598 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -2010,18 +2010,11 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 		goto out_uart;
 #endif
 
+	/* Look up every channel before registering any: lookups may defer */
 	for (i = 0; i < devtype->nr; i++) {
 		struct fwnode_handle *saved_fwnode = dev_fwnode(dev);
 		struct device_node *port_np = NULL;
 		struct device_node *child;
-		unsigned int line;
-
-		line = find_first_zero_bit(max310x_lines, MAX310X_UART_NRMAX);
-		if (line == MAX310X_UART_NRMAX) {
-			ret = -ERANGE;
-			goto out_uart;
-		}
-		s->p[i].port.line = line;
 
 		/* Locate the matching "serial@i" DT subnode, if any. */
 		for_each_available_child_of_node(dev->of_node, child) {
@@ -2050,12 +2043,23 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 			device_set_node(dev, saved_fwnode);
 			of_node_put(port_np);
 			if (ret)
-				goto out_uart;
+				goto out_clk;
 		} else if (devtype->nr == 1) {
 			ret = uart_get_rs485_mode(&s->p[i].port);
 			if (ret)
-				goto out_uart;
+				goto out_clk;
 		}
+	}
+
+	for (i = 0; i < devtype->nr; i++) {
+		unsigned int line;
+
+		line = find_first_zero_bit(max310x_lines, MAX310X_UART_NRMAX);
+		if (line == MAX310X_UART_NRMAX) {
+			ret = -ERANGE;
+			goto out_uart;
+		}
+		s->p[i].port.line = line;
 
 		/* Register port */
 		ret = uart_add_one_port(&max310x_uart, &s->p[i].port);

-- 
2.47.3

[PATCH 2/7] dt-bindings: serial: Add the line transceiver enable GPIO

[Tapio Reijonen]

A UART's line transceiver (RS-232, RS-485, an isolated or a
multiprotocol part) is a dumb device with no node of its own, yet on
many boards it is the largest consumer behind the port and can be
switched off, through a shutdown input of the part or a switch in its
supply. Device trees have no way to say so, and the transceiver stays
powered for as long as the system is up, whether or not the port is
open.

Add transceiver-enable-gpios, the GPIO that turns the transceiver on,
as a property of the UART node alongside the transceiver pins that
rs485.yaml already describes, and transceiver-startup-delay-us for the
time the part needs to become operational: a transceiver that
generates its own supplies behind a shutdown input can take a
millisecond until they are up. The GPIO belongs to one port; on a
multi-port controller it is a property of the port's child node. A
serial device with a driver of its own, such as a Bluetooth module,
remains a child node of the UART.

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
 Documentation/devicetree/bindings/serial/serial.yaml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/serial.yaml b/Documentation/devicetree/bindings/serial/serial.yaml
index 96eb1de8771e9b4fc659aab3dadc3ac9f8599d39..baf7733c720b42a79643e65696041ff0765b682f 100644
--- a/Documentation/devicetree/bindings/serial/serial.yaml
+++ b/Documentation/devicetree/bindings/serial/serial.yaml
@@ -87,9 +87,23 @@ properties:
     description:
       TX FIFO threshold configuration (in bytes).
 
+  transceiver-enable-gpios:
+    maxItems: 1
+    description:
+      GPIO that turns on the line transceiver, through its enable or
+      shutdown input or a switch in its supply.
+
+  transceiver-startup-delay-us:
+    description:
+      Time from the enable GPIO becoming active until the transceiver is
+      operational.
+
   port:
     $ref: /schemas/graph.yaml#/properties/port
 
+dependencies:
+  transceiver-startup-delay-us: [ transceiver-enable-gpios ]
+
 patternProperties:
   "^(bluetooth|bluetooth-gnss|embedded-controller|gnss|gps|mcu|onewire)$":
     if:

-- 
2.47.3

[PATCH 3/7] dt-bindings: serial: sc16is7xx: Disallow transceiver GPIO on dual parts

[Tapio Reijonen]

The SC16IS752 and SC16IS762 describe both of their channels in the one
node that references serial.yaml, so transceiver-enable-gpios validates
on it, while the GPIO belongs to one port: the driver gives both ports
the same device, serial core requests the line once per port, and the
second request fails with -EBUSY, which fails the probe of the whole
chip. Reject the transceiver properties on these parts until their
channels have nodes of their own.

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
index ab39b95dae4029974dce1a2a2425785f26588eac..cfb2250587f74c9e4fa2ffa3f9ddb7de9cb418d2 100644
--- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
+++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
@@ -83,6 +83,17 @@ allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
   - $ref: /schemas/serial/serial.yaml#
   - $ref: /schemas/serial/rs485.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nxp,sc16is752
+              - nxp,sc16is762
+    then:
+      properties:
+        transceiver-enable-gpios: false
+        transceiver-startup-delay-us: false
 
 oneOf:
   - required:

-- 
2.47.3

[PATCH 4/7] serial: core: Power the line transceiver with the port

[Tapio Reijonen]

A line transceiver that can be switched off stays powered for as long
as the system is up, because nothing ties it to the port: serial core
powers the port up when it is opened and down when it is closed, but
the transceiver behind it is unknown to the kernel.

Implement the transceiver properties of serial.yaml. Serial core
requests the enable GPIO when a port is added, so a driver needs no
change to support it, and from then on uart_change_pm() powers the
transceiver along with the port: on before the driver powers the port
up, waiting transceiver-startup-delay-us for the part to come up, off
after the driver has powered the port down, so the transceiver is
never switched off under a running port. That covers open and close,
suspend and resume, the probe-time power-down and the brief power-up
for /proc/tty/driver. Whether the pins stop driving while the
transceiver is off is up to the driver. A console port stays powered, as
it does today; its transceiver is switched off when the port is removed,
after the driver has powered the port down. A never-opened console gets
no shutdown() at removal, so a driver that three-states its pins only
there keeps driving until the power-off; a console on a removable UART
is left as that corner. The GPIO is requested driven inactive, so
console output between the request and the power-up in
uart_configure_port() is lost; earlycon output before probe depends on
how the bootloader left the line.

On a port with the transceiver GPIO, the rs485-select-gpios line is
set from the port's RS485 configuration at registration, before the
power-up, so the transceiver comes on in the configured mode instead
of the one the line was left in; RTS reaches its RS485 idle level in
uart_rs485_config() after that. A port without the GPIO keeps the
order of the rs485-select change.

The lookup goes through the port's device, which is the chip for a
multi-port device. A driver whose ports have a firmware node of their
own requests the GPIO from that node with uart_get_transceiver()
before adding the port; serial core looks at the port's device only
for a port that has no GPIO yet, which serves single-port devices. The
GPIO belongs to one port, so on a multi-port device it has to be
described per port.

The property is optional. A port without it gets no GPIO, and serial
core behaves as before.

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
 drivers/tty/serial/serial_core.c | 99 +++++++++++++++++++++++++++++++++++++++-
 include/linux/serial_core.h      |  3 ++
 2 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 6c0a6d9dcb851b6db0acd91143e651bcb33ce5ae..d36bffaa100b89b519e84c20df2f61f507e6e89f 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2348,12 +2348,32 @@ uart_set_options(struct uart_port *port, struct console *co,
 EXPORT_SYMBOL_GPL(uart_set_options);
 #endif /* CONFIG_SERIAL_CORE_CONSOLE */
 
+/*
+ * Power the line transceiver along with the port: on before the driver
+ * powers the port up, off after it has powered the port down.
+ */
+static void uart_transceiver_pm(struct uart_port *port,
+				enum uart_pm_state pm_state)
+{
+	bool powered = (pm_state == UART_PM_STATE_ON);
+
+	if (!port->transceiver_enable_gpio)
+		return;
+
+	gpiod_set_value_cansleep(port->transceiver_enable_gpio, powered);
+	if (powered && port->transceiver_startup_delay_us)
+		fsleep(port->transceiver_startup_delay_us);
+}
+
 /**
  * uart_change_pm - set power state of the port
  *
  * @state: port descriptor
  * @pm_state: new state
  *
+ * The line transceiver follows: powered on before the driver sees the
+ * new state, off after it.
+ *
  * Locking: port->mutex has to be held
  */
 static void uart_change_pm(struct uart_state *state,
@@ -2362,8 +2382,12 @@ static void uart_change_pm(struct uart_state *state,
 	struct uart_port *port = uart_port_check(state);
 
 	if (state->pm_state != pm_state) {
+		if (port && pm_state == UART_PM_STATE_ON)
+			uart_transceiver_pm(port, pm_state);
 		if (port && port->ops->pm)
 			port->ops->pm(port, pm_state, state->pm_state);
+		if (port && pm_state != UART_PM_STATE_ON)
+			uart_transceiver_pm(port, pm_state);
 		state->pm_state = pm_state;
 	}
 }
@@ -2638,6 +2662,10 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
 		if (uart_console(port))
 			console_lock();
 
+		/* Select the interface before the transceiver is powered */
+		if (port->transceiver_enable_gpio)
+			uart_set_rs485_select_gpio(port, &port->rs485);
+
 		/* Power up port for set_mctrl() */
 		uart_change_pm(state, UART_PM_STATE_ON);
 
@@ -3282,6 +3310,12 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 
 		if (tty_port_initialized(port))
 			uart_shutdown(NULL, state);
+
+		/* Only a console port is still powered here */
+		if (uport->transceiver_enable_gpio &&
+		    state->pm_state == UART_PM_STATE_ON)
+			uart_change_pm(state, UART_PM_STATE_OFF);
+		uport->transceiver_enable_gpio = NULL;
 	}
 
 	/*
@@ -3364,10 +3398,18 @@ static int serial_core_port_device_add(struct serial_ctrl_device *ctrl_dev,
 int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
 {
 	struct serial_ctrl_device *ctrl_dev, *new_ctrl_dev = NULL;
+	bool core_transceiver = false;
 	int ret;
 
 	guard(mutex)(&port_mutex);
 
+	if (port->dev && !port->transceiver_enable_gpio) {
+		ret = uart_get_transceiver(port);
+		if (ret)
+			return ret;
+		core_transceiver = true;
+	}
+
 	/*
 	 * Prevent serial_port_runtime_resume() from trying to use the port
 	 * until serial_core_add_one_port() has completed
@@ -3378,8 +3420,10 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
 	ctrl_dev = serial_core_ctrl_find(drv, port->dev, port->ctrl_id);
 	if (!ctrl_dev) {
 		new_ctrl_dev = serial_core_ctrl_device_add(port);
-		if (IS_ERR(new_ctrl_dev))
-			return PTR_ERR(new_ctrl_dev);
+		if (IS_ERR(new_ctrl_dev)) {
+			ret = PTR_ERR(new_ctrl_dev);
+			goto err_put_transceiver;
+		}
 		ctrl_dev = new_ctrl_dev;
 	}
 
@@ -3409,6 +3453,14 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
 err_unregister_ctrl_dev:
 	serial_base_ctrl_device_remove(new_ctrl_dev);
 
+err_put_transceiver:
+	/* The port may be registered again: forget the GPIO */
+	if (core_transceiver && port->transceiver_enable_gpio) {
+		devm_gpiod_put(port->dev, port->transceiver_enable_gpio);
+		port->transceiver_enable_gpio = NULL;
+		port->transceiver_startup_delay_us = 0;
+	}
+
 	return ret;
 }
 
@@ -3710,6 +3762,49 @@ int uart_get_rs485_mode(struct uart_port *port)
 }
 EXPORT_SYMBOL_GPL(uart_get_rs485_mode);
 
+/**
+ * uart_get_transceiver() - request the line transceiver GPIO of a port
+ * @port: uart device's target port
+ *
+ * Reads the transceiver properties of serial.yaml from the firmware
+ * node of @port->dev. Serial core calls it for a port added without a
+ * GPIO; a driver whose ports have firmware nodes of their own calls it
+ * per port, with the device's node retargeted, before adding the port.
+ */
+int uart_get_transceiver(struct uart_port *port)
+{
+	struct device *dev = port->dev;
+	struct gpio_desc *desc;
+	u32 delay_us = 0;
+	int ret;
+
+	port->transceiver_enable_gpio = NULL;
+	port->transceiver_startup_delay_us = 0;
+
+	desc = devm_gpiod_get_optional(dev, "transceiver-enable", GPIOD_OUT_LOW);
+	if (IS_ERR(desc))
+		return dev_err_probe(dev, PTR_ERR(desc),
+				     "Cannot get transceiver-enable-gpios\n");
+	if (!desc)
+		return 0;
+
+	if (device_property_present(dev, "transceiver-startup-delay-us")) {
+		ret = device_property_read_u32(dev, "transceiver-startup-delay-us",
+					       &delay_us);
+		if (ret) {
+			devm_gpiod_put(dev, desc);
+			return dev_err_probe(dev, ret,
+					     "Cannot read transceiver-startup-delay-us\n");
+		}
+	}
+
+	port->transceiver_enable_gpio = desc;
+	port->transceiver_startup_delay_us = delay_us;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(uart_get_transceiver);
+
 /* Compile-time assertions for serial_rs485 layout */
 static_assert(offsetof(struct serial_rs485, padding) ==
               (offsetof(struct serial_rs485, delay_rts_after_send) + sizeof(__u32)));
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index fe90a8aada98cacf5a80f20ec2a91e249c92d0ed..419d7b46302e5627f7bfbf0b3e658fde37e06a96 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -580,6 +580,7 @@ struct uart_port {
 	unsigned int		custom_divisor;
 	unsigned int		line;			/* port index */
 	unsigned int		minor;
+	u32			transceiver_startup_delay_us;
 	resource_size_t		mapbase;		/* for ioremap */
 	resource_size_t		mapsize;
 	struct device		*dev;			/* serial port physical parent device */
@@ -601,6 +602,7 @@ struct uart_port {
 	struct gpio_desc	*rs485_term_gpio;	/* enable RS485 bus termination */
 	struct gpio_desc	*rs485_rx_during_tx_gpio; /* Output GPIO that sets the state of RS485 RX during TX */
 	struct gpio_desc	*rs485_select_gpio;	/* select the RS485 interface */
+	struct gpio_desc	*transceiver_enable_gpio; /* turns the line transceiver on */
 	struct serial_iso7816   iso7816;
 	void			*private_data;		/* generic platform data pointer */
 };
@@ -1341,6 +1343,7 @@ static inline int uart_handle_break(struct uart_port *port)
 					 !((cflag) & CLOCAL))
 
 int uart_get_rs485_mode(struct uart_port *port);
+int uart_get_transceiver(struct uart_port *port);
 
 void uart_get_ioinfos(struct uart_port *port, char *buf, size_t size);
 bool uart_iotype_mmio(enum uart_iotype iotype);

-- 
2.47.3

[PATCH 5/7] serial: max310x: Request the line transceiver per port

[Tapio Reijonen]

Serial core requests the transceiver GPIO through the port's device
when the port is added, which for this driver is the chip: the
per-port "serial@N" subnodes, where the transceiver of a multi-channel
part is described, are out of its reach. Request it the way the RS485
properties are requested, while the device's firmware node is
temporarily retargeted at the subnode, so each channel gets its own.
Single-channel variants have no subnode; request it from the chip node
there, so the transceiver is known before uart_add_one_port() for
every variant.

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
 drivers/tty/serial/max310x.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index bdfdf4a9000bbd18509746bbfd8ed0ade8164598..876bdd1f5496dcf46fdc790a89892342fd85a9e4 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -2032,20 +2032,24 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 
 		/*
 		 * Temporarily retarget dev's fwnode to the per-port subnode
-		 * so uart_get_rs485_mode() picks up the per-port properties.
-		 * For single-port variants, fall back to the chip's own
-		 * fwnode so legacy DTs that declare rs485 properties at the
-		 * top level keep working.
+		 * so uart_get_rs485_mode() and uart_get_transceiver() pick up
+		 * the per-port properties. For single-port variants, fall
+		 * back to the chip's own fwnode so legacy DTs that declare
+		 * rs485 properties at the top level keep working.
 		 */
 		if (port_np) {
 			device_set_node(dev, of_fwnode_handle(port_np));
 			ret = uart_get_rs485_mode(&s->p[i].port);
+			if (!ret)
+				ret = uart_get_transceiver(&s->p[i].port);
 			device_set_node(dev, saved_fwnode);
 			of_node_put(port_np);
 			if (ret)
 				goto out_clk;
 		} else if (devtype->nr == 1) {
 			ret = uart_get_rs485_mode(&s->p[i].port);
+			if (!ret)
+				ret = uart_get_transceiver(&s->p[i].port);
 			if (ret)
 				goto out_clk;
 		}

-- 
2.47.3

[PATCH 6/7] dt-bindings: serial: max310x: Add maxim,tristate-when-closed

[Tapio Reijonen]

The MAX310x UARTs can three-state TX and RTS per channel. Add a
per-channel flag asking for that whenever the port is not open, for
boards where the outputs must stay undriven then: a driven output
would back-feed an unpowered part behind the pins, or fight another
device driving the line.

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
 .../devicetree/bindings/serial/maxim,max310x.yaml        | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
index e598dda4d13f514820ed8012f2fb2fc2aba3e669..f54296f6b6c85e3a6246e210af9ea8ee6af5118b 100644
--- a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
+++ b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
@@ -54,6 +54,16 @@ required:
   - clocks
   - clock-names
 
+$defs:
+  maxim,tristate-when-closed:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Three-state the channel's TX and RTS outputs whenever the port is
+      not open, from probe until the first open and after each close.
+      For boards where a driven output would back-feed an unpowered part
+      behind the pins, or where another device drives the line while the
+      port is closed. The board defines the levels the outputs float to.
+
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
 
@@ -68,6 +78,9 @@ allOf:
       allOf:
         - $ref: /schemas/serial/serial.yaml#
         - $ref: /schemas/serial/rs485.yaml#
+      properties:
+        maxim,tristate-when-closed:
+          $ref: "#/$defs/maxim,tristate-when-closed"
 
   - if:
       properties:
@@ -93,6 +106,8 @@ allOf:
             reg:
               description: UART channel number on the chip.
               maximum: 3
+            maxim,tristate-when-closed:
+              $ref: "#/$defs/maxim,tristate-when-closed"
           required:
             - reg
           unevaluatedProperties: false
@@ -156,6 +171,7 @@ examples:
 
             serial@2 {
                 reg = <2>;
+                maxim,tristate-when-closed;
                 rs485-rts-active-low;
             };
         };

-- 
2.47.3

[PATCH 7/7] serial: max310x: Three-state TX and RTS while the port is closed

[Tapio Reijonen]

Implement maxim,tristate-when-closed: keep TX and RTS undriven while
the port is closed, so that they do not back-feed an unpowered part
behind them.

The MODE1 register can three-state both pins. Do so on a channel with
the flag at probe time, before the port is registered, and as the last
thing in shutdown(); drive the pins again as the first thing in
startup(). What the part's inputs do while undriven is up to the part
and the board. The pins are driven from chip reset until this driver's
probe reaches each channel, after the MODE1 reset write, the clock
setup and the gpiochip registration.

Channels without the flag are left alone.

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
 drivers/tty/serial/max310x.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 876bdd1f5496dcf46fdc790a89892342fd85a9e4..d5884c870d8e082ea99486cb0d2bf906815e98b2 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -315,6 +315,7 @@ struct max310x_one {
 	unsigned int		char_time_us;
 	unsigned int		baud;
 	bool			sw_rts_during_tx;
+	bool			tristate_when_closed;
 	bool			cancel_tx_delay_tmr;
 	bool			tx_teardown;	/* envelope being torn down */
 	bool			rs485_pending;	/* rs_work not yet applied */
@@ -433,6 +434,20 @@ static void max310x_power(struct uart_port *port, int on)
 		msleep(50);
 }
 
+/*
+ * Three-state TX and RTS while the port is closed, on channels that ask
+ * for it: a driven pin would back-feed an unpowered part behind it.
+ */
+static void max310x_set_tristate(struct uart_port *port, bool tristate)
+{
+	unsigned int mask = MAX310X_MODE1_TXHIZ_BIT | MAX310X_MODE1_RTSHIZ_BIT;
+
+	if (!to_max310x_port(port)->tristate_when_closed)
+		return;
+
+	max310x_port_update(port, MAX310X_MODE1_REG, mask, tristate ? mask : 0);
+}
+
 static const struct max310x_devtype max3107_devtype = {
 	.name	= "MAX3107",
 	.nr	= 1,
@@ -1542,6 +1557,7 @@ static int max310x_startup(struct uart_port *port)
 	struct max310x_one *one = to_max310x_port(port);
 	unsigned int val;
 
+	max310x_set_tristate(port, false);
 	max310x_power(port, 1);
 
 	/*
@@ -1685,6 +1701,7 @@ static void max310x_shutdown(struct uart_port *port)
 		fsleep(max(one->char_time_us / 10, 100U));
 	}
 
+	max310x_set_tristate(port, true);
 	max310x_power(port, 0);
 }
 
@@ -2042,6 +2059,9 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 			ret = uart_get_rs485_mode(&s->p[i].port);
 			if (!ret)
 				ret = uart_get_transceiver(&s->p[i].port);
+			s->p[i].tristate_when_closed =
+				device_property_read_bool(dev,
+							  "maxim,tristate-when-closed");
 			device_set_node(dev, saved_fwnode);
 			of_node_put(port_np);
 			if (ret)
@@ -2050,6 +2070,9 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 			ret = uart_get_rs485_mode(&s->p[i].port);
 			if (!ret)
 				ret = uart_get_transceiver(&s->p[i].port);
+			s->p[i].tristate_when_closed =
+				device_property_read_bool(dev,
+							  "maxim,tristate-when-closed");
 			if (ret)
 				goto out_clk;
 		}
@@ -2065,6 +2088,9 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 		}
 		s->p[i].port.line = line;
 
+		/* Pins float until the first open */
+		max310x_set_tristate(&s->p[i].port, true);
+
 		/* Register port */
 		ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
 		if (ret)

-- 
2.47.3

Re: [PATCH 1/7] serial: max310x: Look up all channels before adding any port

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] serial: max310x: null pointer dereference in max310x_rs_proc()
  [1]
--

commit c564df97d56ed8d392fe2d422cb99ad90eb2aa67
Author: Tapio Reijonen <tapio.reijonen@vaisala.com>

serial: max310x: Look up all channels before adding any port

This commit splits the per-channel loop in max310x_probe() so that RS485
device tree properties and GPIOs are looked up for all channels before
registering any UART ports with uart_add_one_port().

> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> index a0ed70185555e..bdfdf4a9000bb 100644
> --- a/drivers/tty/serial/max310x.c
> +++ b/drivers/tty/serial/max310x.c
[ ... ]
> @@ -2050,12 +2043,23 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
>  			device_set_node(dev, saved_fwnode);
>  			of_node_put(port_np);
>  			if (ret)
> -				goto out_uart;
> +				goto out_clk;
>  		} else if (devtype->nr == 1) {
>  			ret = uart_get_rs485_mode(&s->p[i].port);
>  			if (ret)
> -				goto out_uart;
> +				goto out_clk;
>  		}
> +	}
> +
> +	for (i = 0; i < devtype->nr; i++) {
> +		unsigned int line;
> +
> +		line = find_first_zero_bit(max310x_lines, MAX310X_UART_NRMAX);
> +		if (line == MAX310X_UART_NRMAX) {
> +			ret = -ERANGE;
> +			goto out_uart;
> +		}
> +		s->p[i].port.line = line;
>  
>  		/* Register port */
>  		ret = uart_add_one_port(&max310x_uart, &s->p[i].port);

[Severity: High]
This problem wasn't introduced by this patch, but can jumping to out_uart
after uart_add_one_port() cause a NULL pointer dereference in
max310x_rs_proc() [1]?

When RS485 is enabled in the device tree, uart_add_one_port() calls
max310x_rs485_config(), which schedules s->p[i].rs_work.  If a later step in
max310x_probe() fails (such as registering a subsequent port or requesting the
IRQ), the out_uart cleanup loop calls uart_remove_one_port() without
cancelling rs_work first:

drivers/tty/serial/max310x.c:max310x_probe() {
    ...
out_uart:
	for (i = 0; i < devtype->nr; i++) {
		if (test_and_clear_bit(s->p[i].port.line, max310x_lines))
			uart_remove_one_port(&max310x_uart, &s->p[i].port);
	}
    ...
}

uart_remove_one_port() clears port.state to NULL, so when max310x_rs_proc()
runs, it dereferences NULL at &one->port.state->port.mutex (and before devres
frees s, or into freed memory afterward).  Should out_uart cancel rs_work
before calling uart_remove_one_port(), as max310x_remove() does?

[1] https://sashiko.dev/#/bug/linux-0992a296-a8ff-4396-990f-c73ea073185b

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-serial-transceiver-supply-v1-0-40bad0f912f2@vaisala.com?part=1

Re: [PATCH 4/7] serial: core: Power the line transceiver with the port

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 2 related pre-existing issues it does not
introduce:
- [Critical] serial: core: use-after-free in uart_line_info() [1]
- [Critical] serial: core: dangling pointer in
  serial_core_add_one_port() [2]
--

commit 54c2531a6ac957209b731d5dba93f7f0ba6a052f
Author: Tapio Reijonen <tapio.reijonen@vaisala.com>

serial: core: Power the line transceiver with the port

This commit requests the optional transceiver-enable GPIO and startup delay
for a serial port and toggles the transceiver power alongside the port in
uart_change_pm().

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 6c0a6d9dcb851..d36bffaa100b8 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c

[ ... ]

> @@ -3282,6 +3310,12 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
>  
>  		if (tty_port_initialized(port))
>  			uart_shutdown(NULL, state);
> +
> +		/* Only a console port is still powered here */
> +		if (uport->transceiver_enable_gpio &&
> +		    state->pm_state == UART_PM_STATE_ON)
> +			uart_change_pm(state, UART_PM_STATE_OFF);
> +		uport->transceiver_enable_gpio = NULL;
>  	}

[Severity: Critical]
This problem wasn't introduced by this patch, but can uart_line_info() race
with serial_core_remove_one_port() and cause a use-after-free [1]?

serial_core_remove_one_port() drops port->mutex at the end of this block and
calls uport->ops->release_port(uport) before setting uport->type to
PORT_UNKNOWN:

CPU0                              CPU1
serial_core_remove_one_port()
  // drops port->mutex
  uport->ops->release_port(uport)
                                  uart_line_info()
                                    guard(mutex)(&port->mutex)
                                    if (uport->type == PORT_UNKNOWN)
                                      return;
                                    uport->ops->get_mctrl(uport)  <-- use-after-free
  uport->type = PORT_UNKNOWN

A concurrent read of /proc/tty/driver/serial sees uport->type != PORT_UNKNOWN
and invokes get_mctrl() on unmapped IO resources.

[ ... ]

> @@ -3409,6 +3453,14 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
>  err_unregister_ctrl_dev:
>  	serial_base_ctrl_device_remove(new_ctrl_dev);
>  
> +err_put_transceiver:
> +	/* The port may be registered again: forget the GPIO */
> +	if (core_transceiver && port->transceiver_enable_gpio) {
> +		devm_gpiod_put(port->dev, port->transceiver_enable_gpio);
> +		port->transceiver_enable_gpio = NULL;
> +		port->transceiver_startup_delay_us = 0;
> +	}
> +
>  	return ret;
>  }

[Severity: Critical]
This problem wasn't introduced by this patch, but does
serial_core_add_one_port() leave a dangling pointer in uport->name when
allocating uport->tty_groups fails [2]?

drivers/tty/serial/serial_core.c:serial_core_add_one_port() {
	...
	uport->tty_groups = kzalloc_objs(*uport->tty_groups, 3);
	if (!uport->tty_groups) {
		kfree(uport->name);
		return -ENOMEM;
	}
	...
}

Because the caller owns uport across registration failure, leaving the freed
pointer in uport->name can lead to a use-after-free or double free during
driver cleanup or a subsequent registration retry.

[1] https://sashiko.dev/#/bug/linux-dccfb530-78ab-4c75-b86c-353ac639e159
[2] https://sashiko.dev/#/bug/linux-4946dfed-5de8-46a5-b799-8b71897faf37

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-serial-transceiver-supply-v1-0-40bad0f912f2@vaisala.com?part=4

Re: [PATCH 7/7] serial: max310x: Three-state TX and RTS while the port is closed

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] serial: max310x: phantom break on TX line in
  max310x_startup() [1]
--

commit e4e606eca28fdbe85b131fef6cbe0acf9223f301
Author: Tapio Reijonen <tapio.reijonen@vaisala.com>

serial: max310x: Three-state TX and RTS while the port is closed

This commit implements the maxim,tristate-when-closed property to keep TX and
RTS three-stated in MODE1 while the UART port is closed.

> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> index 876bdd1f5496d..d5884c870d8e0 100644
> --- a/drivers/tty/serial/max310x.c
> +++ b/drivers/tty/serial/max310x.c

[ ... ]

> @@ -1542,6 +1557,7 @@ static int max310x_startup(struct uart_port *port)
>  	struct max310x_one *one = to_max310x_port(port);
>  	unsigned int val;
>  
> +	max310x_set_tristate(port, false);
>  	max310x_power(port, 1);
>  
>  	/*

[Severity: Medium]
This problem wasn't introduced by this patch, but can max310x_startup()
transmit a 50ms phantom break on the TX line [1] when reopening a port
that was closed with a break condition still asserted?

MAX310X_LCR_TXBREAK_BIT is only cleared in MAX310X_LCR_REG after
max310x_power(port, 1) returns.  Because max310x_power(port, 1) sleeps for
50ms via msleep(50) while waking the device, the newly enabled TX pin drives
the latched break state across the entire 50ms settle window, which can cause
framing errors or data corruption on the remote receiver.

[1] https://sashiko.dev/#/bug/linux-48f66bf9-1d5f-4959-b6b9-bc3800762a8e

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-serial-transceiver-supply-v1-0-40bad0f912f2@vaisala.com?part=7