[PATCH v2 0/3] serial: max310x: honour per-channel DT RS485 properties

[PATCH v2 0/3] serial: max310x: honour per-channel DT RS485 properties

[Tapio Reijonen]

The MAX310x DT binding pulls in /schemas/serial/rs485.yaml via its allOf
list, but the driver has never actually called uart_get_rs485_mode(), so
none of the advertised rs485-* properties take effect at runtime.

This series wires per-channel RS485 DT configuration end to end:

Patch 1 reorders the probe so the gpio_chip is registered before
uart_add_one_port(). A port can then reference one of the chip's own
GPIOs (e.g. rs485-term-gpios = <&max310x ...>) without -EPROBE_DEFER
from its own provider - prerequisite for patch 3.

Patch 2 describes each UART channel as a "serial@N" subnode carrying the
standard rs485.yaml properties. The channels are constrained per
compatible: the single-channel max3107/max3108 have none (RS485 stays on
the chip node), max3109 has channels 0-1 and max14830 has 0-3. Being a
serial node, a channel may also host a serial slave device.

Patch 3 reads each channel's RS485 properties from its own subnode by
temporarily retargeting dev->fwnode while uart_get_rs485_mode() runs.
For single-channel variants, falls back to the chip's own fwnode when no
subnode is present, so existing top-level rs485 DTs keep working.

Note for maintainers: patch 3 mutates the parent SPI/I2C device's
fwnode around the uart_get_rs485_mode() call so the underlying
property/GPIO lookups resolve against the per-channel DT subnode. Probe
is serialised, so the swap is locally safe, but I'd appreciate feedback
on whether this idiom is acceptable. If a cleaner shape is preferred (a
serial_core helper that takes a fwnode directly, or one struct device
per port), I'll respin accordingly.

Tested on max14830 (SPI, 4 ports): each ttyMAXn port comes up with the
rs485 flags and delays configured in its serial@N subnode, and the
termination GPIO sourced from the MAX310x's own gpio_chip is resolved
without probe deferral.

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
Changes in v2:
- dt-bindings: rename the per-port subnode "port@N" -> "serial@N" so each
  channel is a proper serial node (serial.yaml) that can also host a
  serial slave device; "port" is reserved for the graph binding. (Krzysztof)
- dt-bindings: constrain channels per compatible - max3107/max3108 take no
  subnodes (nor #address-cells/#size-cells), max3109 allows 0-1, max14830
  allows 0-3; out-of-range channels now fail dt_binding_check. (Krzysztof)
- serial: max310x: match the "serial" child node name accordingly.
- No change to patch 1; patch 3 still reads rs485 via the temporary fwnode
  retarget (idiom flagged above).
- Link to v1: https://lore.kernel.org/r/20260525-b4-max310x-rs485-dt-v1-0-e6c19b4d5592@vaisala.com

---
Tapio Reijonen (3):
      serial: max310x: register GPIO controller before adding UART ports
      dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes
      serial: max310x: honour rs485 properties from per-channel DT subnode

 .../devicetree/bindings/serial/maxim,max310x.yaml  | 90 +++++++++++++++++++++
 drivers/tty/serial/max310x.c                       | 91 +++++++++++++++++-----
 2 files changed, 160 insertions(+), 21 deletions(-)
---
base-commit: 79bd2dded182b1d458b18e62684b7f82ffc682e5
change-id: 20260525-b4-max310x-rs485-dt-ebff12af9976

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

[PATCH v2 1/3] serial: max310x: register GPIO controller before adding UART ports

[Tapio Reijonen]

The MAX310x exposes four GPIOs per UART port via an in-driver
gpio_chip. devm_gpiochip_add_data() used to run after the per-port
uart_add_one_port() loop, so a device-tree consumer referencing one of
the chip's own GPIOs (for example rs485-term-gpios = <&max310x 0 ...>)
could not resolve it during port registration: the GPIO provider it
waits for is the very driver still trying to register, and the lookup
returns -EPROBE_DEFER on its own provider, deferring probe forever.

Split the per-port setup into two passes around the gpio_chip
registration:

  1. Initialise per-port state - port struct fields, regmap binding,
     IRQ disable, work queues. The gpio_chip callbacks dereference
     s->p[i].regmap via to_max310x_port() and become callable as soon
     as the chip is visible to gpiolib, so every entry must be
     populated first.
  2. devm_gpiochip_add_data() - register the gpio_chip.
  3. Allocate a line, uart_add_one_port(), set_bit(), max310x_power().
     Keeping line allocation, registration and set_bit() together
     preserves the existing "bit set <=> port registered" rollback
     invariant that out_uart relies on.

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

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index ac7d3f197c3a5ce3531d5607f48e21a807314021..5cb7d01e404663dc25b88bc7b4f8df61be2135ec 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1364,17 +1364,12 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 
 	dev_dbg(dev, "Reference clock set to %i Hz\n", uartclk);
 
+	/*
+	 * Set up each port's state before registering the gpiochip,
+	 * since the gpiochip callbacks will read s->p[i].regmap as
+	 * soon as gpiolib exposes the controller.
+	 */
 	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;
-		}
-
-		/* Initialize port data */
-		s->p[i].port.line	= line;
 		s->p[i].port.dev	= dev;
 		s->p[i].port.irq	= irq;
 		s->p[i].port.type	= PORT_MAX310X;
@@ -1404,20 +1399,16 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 		INIT_WORK(&s->p[i].md_work, max310x_md_proc);
 		/* Initialize queue for changing RS485 mode */
 		INIT_WORK(&s->p[i].rs_work, max310x_rs_proc);
-
-		/* Register port */
-		ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
-		if (ret)
-			goto out_uart;
-
-		set_bit(line, max310x_lines);
-
-		/* Go to suspend mode */
-		max310x_power(&s->p[i].port, 0);
 	}
 
 #ifdef CONFIG_GPIOLIB
-	/* Setup GPIO controller */
+	/*
+	 * Register the GPIO controller before adding the UART ports so
+	 * that consumers referencing the chip's own GPIOs from device
+	 * tree (for example rs485-term-gpios = <&max310x ...>) can
+	 * resolve them at uart_add_one_port() time instead of receiving
+	 * -EPROBE_DEFER from their own provider.
+	 */
 	s->gpio.owner		= THIS_MODULE;
 	s->gpio.parent		= dev;
 	s->gpio.label		= devtype->name;
@@ -1434,6 +1425,27 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 		goto out_uart;
 #endif
 
+	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);
+		if (ret)
+			goto out_uart;
+
+		set_bit(line, max310x_lines);
+
+		/* Go to suspend mode */
+		max310x_power(&s->p[i].port, 0);
+	}
+
 	/* Setup interrupt */
 	ret = devm_request_threaded_irq(dev, irq, NULL, max310x_ist,
 					IRQF_ONESHOT | IRQF_SHARED, dev_name(dev), s);

-- 
2.47.3

[PATCH v2 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes

[Tapio Reijonen]

The MAX310x is a family of one- (max3107, max3108), two- (max3109) and
four-channel (max14830) UARTs. The binding pulls in
/schemas/serial/rs485.yaml at the chip level, describing a single set of
RS-485 properties - enough for the single-channel parts, but a
multi-channel chip can wire RS-485 differently on each channel.

Describe each channel as a "serial@N" subnode. Being a serial node (per
serial.yaml) the channel carries the standard rs485.yaml properties and
may also host a serial slave device. Constrain the channels per
compatible: max3107 and max3108 have none and keep RS-485 on the chip
node, max3109 has channels 0-1, and max14830 has 0-3. Chip-level rs485
properties remain accepted and are the only form for the single-channel
parts.

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

diff --git a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
index 889eeaca64a027b4d9e8ec87bcf63fcc8fd9d55b..988864e7957416caea2d86c38957a894ce57c6fb 100644
--- a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
+++ b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
@@ -40,6 +40,36 @@ properties:
     minItems: 1
     maxItems: 16
 
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^serial@[0-3]$":
+    type: object
+    description:
+      A single UART channel of a multi-channel variant. Describe the
+      channel's RS-485 wiring here with the standard properties from
+      /schemas/serial/rs485.yaml#; being a serial node, the channel may
+      also host a serial slave device. Single-channel variants have no
+      such subnode - their settings stay on the chip node.
+
+    allOf:
+      - $ref: /schemas/serial/serial.yaml#
+      - $ref: /schemas/serial/rs485.yaml#
+
+    properties:
+      reg:
+        description: UART channel number on the chip.
+        maximum: 3
+
+    required:
+      - reg
+
+    unevaluatedProperties: false
+
 required:
   - compatible
   - reg
@@ -52,6 +82,32 @@ allOf:
   - $ref: /schemas/serial/serial.yaml#
   - $ref: /schemas/serial/rs485.yaml#
 
+  # max3107 and max3108 are single-channel parts: there are no
+  # addressable channel subnodes, so RS-485 stays on the chip node.
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - maxim,max3107
+              - maxim,max3108
+    then:
+      properties:
+        "#address-cells": false
+        "#size-cells": false
+      patternProperties:
+        "^serial@[0-3]$": false
+
+  # max3109 has two UART channels: 0 and 1.
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: maxim,max3109
+    then:
+      patternProperties:
+        "^serial@[23]$": false
+
 unevaluatedProperties: false
 
 examples:
@@ -70,5 +126,39 @@ examples:
             interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
             gpio-controller;
             #gpio-cells = <2>;
+            rs485-rts-active-low;
+            linux,rs485-enabled-at-boot-time;
+        };
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        serial@0 {
+            compatible = "maxim,max14830";
+            reg = <0>;
+            spi-max-frequency = <26000000>;
+            clocks = <&xtal4m>;
+            clock-names = "xtal";
+            interrupt-parent = <&gpio3>;
+            interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            serial@0 {
+                reg = <0>;
+                rs485-rts-active-low;
+                linux,rs485-enabled-at-boot-time;
+            };
+
+            serial@2 {
+                reg = <2>;
+                rs485-rts-active-low;
+            };
         };
     };

-- 
2.47.3

[PATCH v2 3/3] serial: max310x: honour rs485 properties from per-channel DT subnode

[Tapio Reijonen]

The MAX310x DT binding pulls in /schemas/serial/rs485.yaml via its allOf
list, advertising the rs485-* properties defined there - none of which
were honoured at runtime, because the driver never called
uart_get_rs485_mode().

All channels share the parent SPI/I2C device, so uart_get_rs485_mode()
called directly on each port would read the same chip-level fwnode for
every call. Walk dev->of_node's children for the "serial@N" subnode
with matching reg, and temporarily retarget the parent device's fwnode
while uart_get_rs485_mode() runs, so each channel picks up its own
subnode's properties. Probe is serialised, so the swap is safe.

For single-channel variants (max3107, max3108), fall back to the chip's
own fwnode when no subnode is present, so existing DTs that declare
rs485 properties at the top level keep working.

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

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 5cb7d01e404663dc25b88bc7b4f8df61be2135ec..aee3b75fff000385a7543f099663c8a0e4a7d014 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1426,6 +1426,9 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 #endif
 
 	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);
@@ -1435,6 +1438,40 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 		}
 		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) {
+			u32 reg;
+
+			if (!of_node_name_eq(child, "serial"))
+				continue;
+			if (of_property_read_u32(child, "reg", &reg))
+				continue;
+			if (reg == i) {
+				port_np = child;
+				break;
+			}
+		}
+
+		/*
+		 * 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.
+		 */
+		if (port_np) {
+			device_set_node(dev, of_fwnode_handle(port_np));
+			ret = uart_get_rs485_mode(&s->p[i].port);
+			device_set_node(dev, saved_fwnode);
+			of_node_put(port_np);
+			if (ret)
+				goto out_uart;
+		} else if (devtype->nr == 1) {
+			ret = uart_get_rs485_mode(&s->p[i].port);
+			if (ret)
+				goto out_uart;
+		}
+
 		/* Register port */
 		ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
 		if (ret)

-- 
2.47.3

Re: [PATCH v2 1/3] serial: max310x: register GPIO controller before adding UART ports

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] The `out_uart` error path erroneously unregisters other devices' UART ports and causes a NULL pointer dereference due to zero-initialized `port.line` values.
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition in UART line allocation allows concurrent probes to acquire the same line, leading to a NULL pointer dereference in the error path.
- [High] Pending workqueues are not cancelled on the probe error path, leading to use-after-free when the device structure is freed.
- [High] The UART port is exposed to userspace before its power state is finalized and before its IRQ handler is registered.
--

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

serial: max310x: register GPIO controller before adding UART ports

The MAX310x exposes four GPIOs per UART port via an in-driver gpio_chip.
This patch splits the per-port setup into two passes around the gpio_chip
registration to prevent -EPROBE_DEFER issues during port registration.

> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> index ac7d3f197c3a5..5cb7d01e40466 100644
> --- a/drivers/tty/serial/max310x.c
> +++ b/drivers/tty/serial/max310x.c

[ ... ]

> @@ -1434,6 +1425,27 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
>  		goto out_uart;
>  #endif
>  
> +	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;

[Severity: High]
This is a pre-existing issue, but does this error path cause a NULL pointer
dereference?

Because devm_kzalloc() zeroes s, s->p[i].port.line is initialized to 0 for
all ports. If we jump to out_uart here (or if devm_gpiochip_add_data()
failed earlier), the cleanup loop unconditionally tests bit 0 for the
uninitialized ports:

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);
    }
    ...
}

Could this clear the line 0 bit correctly owned by another device, and
attempt to unregister it using this failing device's partially-initialized
uport?

> +		}
> +		s->p[i].port.line = line;
> +
> +		/* Register port */
> +		ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
> +		if (ret)
> +			goto out_uart;

[Severity: High]
This is a pre-existing issue, but does this error path leak pending workqueues?

During uart_add_one_port(), the serial core can invoke callbacks like
rs485_config() or set_mctrl(), which synchronously schedule rs_work or md_work.

If a subsequent port registration fails and we jump to out_uart, the ports
are unregistered but cancel_work_sync() is never called for these workqueues.
When devres subsequently frees the device memory (s), could a delayed execution
of the scheduled work dereference the freed memory leading to a use-after-free?

> +
> +		set_bit(line, max310x_lines);

[Severity: High]
This is a pre-existing issue, but does this create a Time-of-Check to
Time-of-Use (TOCTOU) race condition during line allocation?

The find_first_zero_bit() function is called before uart_add_one_port(),
but the line isn't actually reserved with set_bit() until after
uart_add_one_port() returns.

Could concurrent probes of multiple max310x devices find the same free line,
leading to the second one failing uart_add_one_port() with -EINVAL,
jumping to out_uart, and erroneously clearing the bit correctly set by the
first device?

> +
> +		/* Go to suspend mode */
> +		max310x_power(&s->p[i].port, 0);
> +	}
> +
>  	/* Setup interrupt */
>  	ret = devm_request_threaded_irq(dev, irq, NULL, max310x_ist,
>  					IRQF_ONESHOT | IRQF_SHARED, dev_name(dev), s);

[Severity: High]
This is a pre-existing issue, but does exposing the port before finalized
setup cause missed interrupts or clobbered power states?

Because uart_add_one_port() makes the port visible to userspace, an
immediate open of the port will invoke the .startup callback and turn the
device power on.

However, the probe function immediately calls max310x_power(..., 0), which
would force the active session back into suspend mode. Furthermore, since
devm_request_threaded_irq() happens after all ports are added, could early
userspace activity trigger interrupts that are completely missed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-b4-max310x-rs485-dt-v2-0-a105105f8e70@vaisala.com?part=1

Re: [PATCH v2 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Kernel panic in error path due to uninitialized port structures in `max310x_probe()`
--

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

dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes

The MAX310x is a family of one- (max3107, max3108), two- (max3109) and
four-channel (max14830) UARTs. The binding pulls in
/schemas/serial/rs485.yaml at the chip level, describing a single set of
RS-485 properties - enough for the single-channel parts, but a
multi-channel chip can wire RS-485 differently on each channel.

> diff --git a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
> index 889eeaca64a02..988864e795741 100644
> --- a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
> +++ b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
> @@ -40,6 +40,36 @@ properties:
>      minItems: 1
>      maxItems: 16
>  
> +  "#address-cells":
> +    const: 1

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but could the error path in
max310x_probe() cause a NULL pointer dereference?

In drivers/tty/serial/max310x.c, the driver allocates the port state structure
using devm_kzalloc(), meaning s->p[i].port.line is zero-initialized.

If the probe fails early before any channels are registered, the function
jumps to the out_uart cleanup label:

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);
    }
    ...
}

Because s->p[i].port.line is still 0, this tests bit 0 in max310x_lines.

If another max310x device on the system has already successfully probed, it
will have claimed line 0, so bit 0 in max310x_lines will be set.

Would test_and_clear_bit(0, max310x_lines) then succeed, incorrectly clear
the bit owned by the other device, and call uart_remove_one_port() with an
uninitialized port?

This appears to lead to a NULL pointer dereference when
serial_core_unregister_port() calls serial_core_get_ctrl_dev(port_dev),
dereferencing port_dev->dev.parent since port_dev is NULL for the uninitialized
port.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-b4-max310x-rs485-dt-v2-0-a105105f8e70@vaisala.com?part=2

Re: [PATCH v2 3/3] serial: max310x: honour rs485 properties from per-channel DT subnode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] Temporarily retargeting `dev->of_node` on a registered device causes TOCTOU race

Pre-existing issues:
- [Critical] Error path blindly processes uninitialized ports, causing NULL pointer dereference
- [High] Unprotected concurrent access to global `max310x_lines` causes race condition during probe
- [High] Missing `cancel_work_sync()` in error path leads to Use-After-Free
- [High] Interrupt handler requested after port registration causes unhandled interrupt storm
--

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

serial: max310x: honour rs485 properties from per-channel DT subnode

The MAX310x driver is modified to read rs485 properties from per-channel DT
subnodes for multi-port variants. It walks the child nodes for a matching
"serial@N" node and temporarily retargets the parent device's fwnode so
uart_get_rs485_mode() picks up the subnode properties.

> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> index 5cb7d01e40466..aee3b75fff000 100644
> --- a/drivers/tty/serial/max310x.c
> +++ b/drivers/tty/serial/max310x.c
> @@ -1426,6 +1426,9 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
[ ... ]
>  	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);

[Severity: High]
This is a pre-existing issue, but does this code leave max310x_lines open to
a race condition?

The line number is allocated here using find_first_zero_bit(), but set_bit()
is not called until after uart_add_one_port() completes successfully later in
the function.

If two max310x devices probe concurrently, could they observe the same zero
bit, assign the same line, and attempt to register it?

>  		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. */
[ ... ]
> +		/*
> +		 * 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.
> +		 */
> +		if (port_np) {
> +			device_set_node(dev, of_fwnode_handle(port_np));

[Severity: High]
The commit message states "Probe is serialised, so the swap is safe."
Is this swap safe against lockless readers in other subsystems?

Since the parent device is already registered and globally visible to the
device model, could a concurrent lockless lookup via of_find_device_by_node()
or bus_find_device_by_of_node() observe the temporarily swapped dev->of_node?

If so, this might cause a concurrent lookup for the parent device's phandle
to spuriously fail, or a lookup for the child node to erroneously return the
parent device.

Would it be better to read subnode properties by explicitly passing the target
fwnode rather than temporarily replacing the parent device's identity?

> +			ret = uart_get_rs485_mode(&s->p[i].port);
> +			device_set_node(dev, saved_fwnode);
> +			of_node_put(port_np);
> +			if (ret)
> +				goto out_uart;

[Severity: Critical]
This isn't a bug introduced by this patch, but does the out_uart cleanup
path handle partially initialized devices correctly if we goto out_uart here?

The out_uart cleanup loop unconditionally iterates over all devtype->nr ports.
For ports that haven't been initialized yet, s->p[i].port.line evaluates to 0.
Could test_and_clear_bit(0, max310x_lines) return true if line 0 is currently
in use by another instance?

If it does, this would call uart_remove_one_port() on an unregistered port.
Could this lead to a NULL pointer dereference in serial_core_unregister_port()
since port->port_dev would be NULL?

[Severity: High]
This is also a pre-existing issue, but is it safe to let devres free the
device context without canceling pending work if we take this goto?

The driver initializes md_work, tx_work, and rs_work for each port. During
uart_add_one_port(), the serial core synchronously invokes callbacks like
set_mctrl() and rs485_config(), which can schedule these work items.

If an error occurs and the ports are removed in out_uart, there is no call to
cancel_work_sync(). Could asynchronously executing work items dereference the
freed memory?

> +		} else if (devtype->nr == 1) {
> +			ret = uart_get_rs485_mode(&s->p[i].port);
> +			if (ret)
> +				goto out_uart;
> +		}
>  
>  		/* Register port */
>  		ret = uart_add_one_port(&max310x_uart, &s->p[i].port);

[Severity: High]
This isn't a bug introduced by this patch, but could this registration order
cause an unhandled interrupt storm?

Calling uart_add_one_port() exposes the TTY device to userspace. If a
userspace program opens it immediately, max310x_startup() can enable TX/RX
interrupts on the device.

Because devm_request_threaded_irq() is called outside and after this loop,
there is a window where the device can generate interrupts without a
registered handler.

Since the interrupt line is shared, would the kernel forcefully disable the
IRQ line if an interrupt fires during this window?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-b4-max310x-rs485-dt-v2-0-a105105f8e70@vaisala.com?part=3