Re: [PATCH 2/3] dt-bindings: serial: maxim,max310x: allow per-port subnodes for rs485

[Tapio Reijonen]

Hi,
On 5/30/26 14:26, Krzysztof Kozlowski wrote:
> On Mon, May 25, 2026 at 09:43:38AM +0000, Tapio Reijonen wrote:
> > The MAX310x is a multi-port UART (up to four ports). The existing
> > binding pulls in /schemas/serial/rs485.yaml at the top level, which
> > only describes a single port - sufficient for max3107 but ambiguous
> > for max14830 where each port can have its own RS485 wiring.
> >
> > Add a "port@N" pattern (N = 0..3) carrying rs485 properties on a
> > per-port basis. When port@N subnodes are present, the chip node also
> > needs #address-cells = <1> and #size-cells = <0>; allow both. Top-
> > level rs485 properties remain accepted for compatibility.
> >
> > Signed-off-by: Tapio Reijonen <tapio.reijonen@xxxxxxxxxxx>
> > ---
> >   .../devicetree/bindings/serial/maxim,max310x.yaml  | 60 ++++++++++++++++++++++
> >   1 file changed, 60 insertions(+)
>
> That's a total mess now in the binding.
> 1. maxim,max3107 does not have ports, but you add there. You need to
> constrain (see writing bindings) or split.

You're right, the port count is part of the ABI and I left it
unconstrained. In v2 I'll constrain the subnodes per-compatible with
allOf/if-then in the one file: max3107 and max3108 are single-port and
take only the top-level rs485 properties (no subnodes); max3109 allows
indices 0-1 and max14830 allows 0-3. An out-of-range port will then
fail validation instead of being silently accepted.

> 2. So where do you place serial devices? Did you validate any of this?

Good point, and it made me reconsider the node name. I'll rename the
subnodes from "port@N" to "serial@N" so each channel is a proper serial
node per serial.yaml -- that matches the device class, frees the "port"
name reserved for the graph binding, and gives an attached serial slave
device a place to live under the correct channel, which "port@N" did
not. The driver match in patch 3 changes to of_node_name_eq(np,
"serial") accordingly.

I also looked at whether subnodes are the right shape at all, since the
sibling sc16is7xx (dual-UART, same driver author) does per-port config
with top-level index arrays (irda-mode-ports,
nxp,modem-control-line-ports) rather than child nodes. That idiom works
for simple per-port flags, but it doesn't extend to RS485: rs485.yaml is
a rich standard schema (several flags, the rts delays, rs485-term-gpios)
and flattening it into vendor index-arrays would mean redefining common
properties instead of referencing them. So a per-channel node that
$refs rs485.yaml seems the only clean carrier; I just had the node name
and the per-compatible constraints wrong.

On validation: the schema and the example do pass
  make dt_binding_check DT_SCHEMA_FILES=.../serial/maxim,max310x.yaml
and the series was tested on max14830 hardware with per-port rs485 and
termination GPIOs sourced from the chip's own gpio-controller. But I
take the structural point -- passing the check didn't make the layout
right. I'll respin patch 2 (constraints + serial@N) and the patch 3
match for v2, with an example that exercises a multi-port variant.

> Best regards,
> Krzysztof

Thanks for the review.

Tapio