Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 0/3] serial: max310x: honour per-channel DT RS485 properties
@ 2026-06-01 10:25 Tapio Reijonen
  2026-06-01 10:25 ` [PATCH v2 1/3] serial: max310x: register GPIO controller before adding UART ports Tapio Reijonen
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Tapio Reijonen @ 2026-06-01 10:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Hugo Villeneuve
  Cc: linux-kernel, linux-serial, devicetree, 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>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/3] serial: max310x: register GPIO controller before adding UART ports
  2026-06-01 10:25 [PATCH v2 0/3] serial: max310x: honour per-channel DT RS485 properties Tapio Reijonen
@ 2026-06-01 10:25 ` Tapio Reijonen
  2026-06-01 10:25 ` [PATCH v2 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes Tapio Reijonen
  2026-06-01 10:25 ` [PATCH v2 3/3] serial: max310x: honour rs485 properties from per-channel DT subnode Tapio Reijonen
  2 siblings, 0 replies; 5+ messages in thread
From: Tapio Reijonen @ 2026-06-01 10:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Hugo Villeneuve
  Cc: linux-kernel, linux-serial, devicetree, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes
  2026-06-01 10:25 [PATCH v2 0/3] serial: max310x: honour per-channel DT RS485 properties Tapio Reijonen
  2026-06-01 10:25 ` [PATCH v2 1/3] serial: max310x: register GPIO controller before adding UART ports Tapio Reijonen
@ 2026-06-01 10:25 ` Tapio Reijonen
  2026-06-07  8:41   ` Krzysztof Kozlowski
  2026-06-01 10:25 ` [PATCH v2 3/3] serial: max310x: honour rs485 properties from per-channel DT subnode Tapio Reijonen
  2 siblings, 1 reply; 5+ messages in thread
From: Tapio Reijonen @ 2026-06-01 10:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Hugo Villeneuve
  Cc: linux-kernel, linux-serial, devicetree, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 3/3] serial: max310x: honour rs485 properties from per-channel DT subnode
  2026-06-01 10:25 [PATCH v2 0/3] serial: max310x: honour per-channel DT RS485 properties Tapio Reijonen
  2026-06-01 10:25 ` [PATCH v2 1/3] serial: max310x: register GPIO controller before adding UART ports Tapio Reijonen
  2026-06-01 10:25 ` [PATCH v2 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes Tapio Reijonen
@ 2026-06-01 10:25 ` Tapio Reijonen
  2 siblings, 0 replies; 5+ messages in thread
From: Tapio Reijonen @ 2026-06-01 10:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Hugo Villeneuve
  Cc: linux-kernel, linux-serial, devicetree, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes
  2026-06-01 10:25 ` [PATCH v2 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes Tapio Reijonen
@ 2026-06-07  8:41   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2026-06-07  8:41 UTC (permalink / raw)
  To: Tapio Reijonen
  Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Hugo Villeneuve, linux-kernel, linux-serial,
	devicetree

On Mon, Jun 01, 2026 at 10:25:32AM +0000, Tapio Reijonen wrote:
> 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#

So now both top-level node is serial and its children are. I don't think
that's correct. This binding should be split for two cases - single
channel and multi-channel, while multi-channels will require subnodes
(in the binding, keep driver backwards compatible).

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-07  8:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-01 10:25 [PATCH v2 0/3] serial: max310x: honour per-channel DT RS485 properties Tapio Reijonen
2026-06-01 10:25 ` [PATCH v2 1/3] serial: max310x: register GPIO controller before adding UART ports Tapio Reijonen
2026-06-01 10:25 ` [PATCH v2 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes Tapio Reijonen
2026-06-07  8:41   ` Krzysztof Kozlowski
2026-06-01 10:25 ` [PATCH v2 3/3] serial: max310x: honour rs485 properties from per-channel DT subnode Tapio Reijonen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox