[PATCH net-next v2 1/8] dt-bindings: net: add Xilinx TSN Endpoint Ethernet MAC

[Neeli, Srinivas]

Hi,

On 9/13/2026 2:06 AM, netdev-bot+sashiko at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
>
> - [Medium] Inconsistent/incomplete port constraints in the new switch
>    `^(ethernet-)?ports$` container.
> - [Low] The endpoint MAC child's DMA/queue description is under-
>    constrained and, more importantly, duplicates information that is…
>
> --
>
> dt-bindings: net: add Xilinx TSN Endpoint Ethernet MAC
>
> Adds a new binding for the Xilinx TSN Endpoint Ethernet MAC IP in the
> Bridged Endpoint configuration.  The wrapper node owns the six IP clocks
> and xlnx,num-priorities, and has an ethernet@ child for the endpoint MAC
> and a switch@ child for the three-port switch plus its two per-MAC
> windows.
>
> Two questions below about the schema constraints.
>
>> diff --git a/Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-ethernet-mac.yaml b/Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-ethernet-mac.yaml
>> new file mode 100644
>> index 000000000000..4e65d1710da7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-ethernet-mac.yaml
>> @@ -0,0 +1,364 @@
> [ ... ]
>
>> +  xlnx,num-priorities:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 2
>> +    maximum: 8
>> +    description:
>> +      Number of traffic classes (priorities) configured in the IP.
>> +      This is an IP configuration parameter that determines the number of
>> +      priority queues available for QoS scheduling. Traffic classes map to
>> +      IEEE 802.1Q priority levels (0-7).
> [Severity: Low]
>
> Is xlnx,num-priorities derivable from the properties already present in
> the ethernet@ child?
>
> The driver added later in this series only accepts one value for it.  In
> xlnx_tsn_ep_probe() in drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c:
>
> 	if (num_tx != num_prio)
> 		return dev_err_probe(dev, -EINVAL,
> 				     "TX channel count %u must equal num-priorities %u\n",
> 				     num_tx, num_prio);
>
> so the only legal value is the number of tx_chanN entries in the child's
> dma-names.
>
> Since this is a required ABI property, would it be better to drop it and
> count the tx_chanN entries, or, if it must stay, to state in the
> description which of the two is authoritative?
>
> [ ... ]

No, xlnx,num-priorities is not derivable from the ethernet@ child 
properties.

xlnx,num-priorities is the number of egress priority queues the IP is
synthesised with. It is an IP configuration parameter and describes the
hardware, so it belongs in the binding independently of the host DMA 
wiring in
dma-names.

The num_tx == num-priorities check reflects this MCDMA only configuration,
where the driver sets up one MCDMA TX channel per priority queue. The driver
requires the tx_chanN count to equal num-priorities and rejects a device 
tree
where they differ. That requirement is a property of this configuration, not
the meaning of num-priorities. With the time-aware and per-stream 
schedulers a
priority queue can be served without an MCDMA TX channel, so 
num-priorities is
not derivable from the dma-names count in general.

Thanks
Srinivas Neeli
>> +      dma-names:
>> +        minItems: 2
>> +        maxItems: 16
>> +        items:
>> +          pattern: "^(tx|rx)_chan[0-7]$"
>> +        description: |
>> +          Names matching the entries in 'dmas', one per DMA channel and in
>> +          the same order. TX channel names use "tx_chan<N>" and RX channel
>> +          names use "rx_chan<N>", where N is 0..7. Example: "tx_chan0",
>> +          "tx_chan1", ..., "rx_chan0", "rx_chan1", ....
> [Severity: Low]
>
> A plain items pattern here also accepts lists the description rules out:
> a TX-only or RX-only list, the same name repeated twice, or RX names
> before TX names.  Could the entries be spelled out as an ordered items
> list (or split with minItems/maxItems per direction) so the "TX first,
> then RX" ordering is actually enforced?
>
>> +      tx-queues-config:
>> +        type: object
>> +        additionalProperties: false
> [ ... ]
>> +        patternProperties:
>> +          "^queue[0-7]$":
>> +            type: object
>> +            additionalProperties: false
> [ ... ]
>> +            properties:
>> +              xlnx,dma-channel-num:
>> +                $ref: /schemas/types.yaml#/definitions/uint32
>> +                description:
>> +                  DMA channel that transmits this priority queue, given as
>> +                  the N of the tx_chanN entry in dma-names.
>> +                minimum: 0
>> +                maximum: 7
> [Severity: Low]
>
> tx-queues-config is required but has no required queueN child, so an
> empty tx-queues-config node validates.  xlnx,dma-channel-num is also
> bounded only by 0..7, so a queue may point at a channel that is absent
> from dma-names, or at a channel another queue already claims.  Was at
> least a required queue0 intended here?
>
> [ ... ]
>
>> +    patternProperties:
>> +      "^(ethernet-)?ports$":
>> +        type: object
>> +        additionalProperties: false
>> +
>> +        properties:
>> +          '#address-cells':
>> +            const: 1
>> +
>> +          '#size-cells':
>> +            const: 0
>> +
>> +          port at 0:
>> +            type: object
>> +            unevaluatedProperties: true
>> +            description:
>> +              CPU port. Wired internally to the endpoint MAC sibling.
>> +
>> +            properties:
>> +              reg:
>> +                const: 0
>> +
>> +              phy-mode:
>> +                const: internal
>> +
>> +            required:
>> +              - ethernet
> [Severity: Medium]
>
> Should reg be required on port at 0 as well?
>
> The value is constrained with reg: const: 0, but the required list only
> names ethernet, and nothing in the inherited chain makes reg mandatory:
> dsa.yaml#/$defs/ethernet-ports -> dsa-port.yaml only defines it,
>
>    properties:
>      reg:
>        items:
>          - description: Port number
>
> and its if/then on ethernet/link requires phy-mode plus one of
> fixed-link/phy-handle/managed.  ethernet-switch.yaml's ports container
> requires only #address-cells/#size-cells.
>
> So this node passes dt_binding_check:
>
>      port at 0 {
>          ethernet = <&ep_mac>;
>          phy-mode = "internal";
>          fixed-link { speed = <1000>; full-duplex; };
>      };
>
> but dsa_switch_parse_ports_of() in net/dsa/dsa.c bails out on it:
>
> 	for_each_available_child_of_node(ports, port) {
> 		err = of_property_read_u32(port, "reg", &reg);
> 		if (err) {
> 			of_node_put(port);
> 			goto out_put_node;
> 		}
>
> That is also asymmetric with the sibling ^port@[1-2]$ subschema below,
> which does require reg.
>
>> +        patternProperties:
>> +          "^port@[1-2]$":
>> +            type: object
>> +            unevaluatedProperties: true
>> +            description:
>> +              External Ethernet port. port at 1 is wired to MAC1 and reached
>> +              through the mdio-1 bus, port at 2 is wired to MAC2 and reached
>> +              through the mdio-2 bus.
>> +
>> +            properties:
>> +              reg:
>> +                enum: [1, 2]
> [Severity: Medium]
>
> Since both external node names share this one subschema, reg is
> decoupled from the unit address.  This validates:
>
>      port at 1 { reg = <2>; phy-handle = <&phy0>; phy-mode = "rgmii-id"; };
>      port at 2 { reg = <1>; phy-handle = <&phy1>; phy-mode = "rgmii-id"; };
>
> as does giving both ports the same reg value.  The description keys the
> MAC and MDIO association by node name, while the DSA core selects the
> port by the reg value, so a swapped DT passes the schema and then
> associates the wrong PHY/MDIO bus with each MAC.
>
> Would per-port subschemas with reg: const: 1 and reg: const: 2, in the
> same style used for port at 0, work better here?
>
> [ ... ]
>