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

[Rob Herring]

On Tue, Oct 20, 2026 at 11:58:11AM +0000, netdev-bot+sashiko@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
> 
> Low priority / nit picks for your consideration:
> 
> - [Low] In Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-
>   ethernet-mac.yaml, the ethernet@ node's dma-names (lines 81-96) does…
> - [Low] Concern: tx-queues-config only enforces a lower bound tied to
>   xlnx,num-priorities, and the rest of the checking is left to a driver…
> - [Low] The child compatibles in
>   Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-ethernet-…
> 
> --
> 
> dt-bindings: net: add Xilinx TSN Endpoint Ethernet MAC
> 
> This adds a YAML binding for the Xilinx TSN Endpoint Ethernet MAC IP in
> its Bridged Endpoint configuration. The wrapper node owns the shared
> clocks and xlnx,num-priorities. It has two children: an xlnx,tsn-ep-mac
> endpoint that holds the DMA channels, and an xlnx,tsn-switch three-port
> switch with its MDIO buses and ports.
> 
> > 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 0000000000000..bca834120b3b1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-ethernet-mac.yaml
> > @@ -0,0 +1,491 @@
> 
> [ ... ]
> 
> > +properties:
> > +  compatible:
> > +    const: xlnx,tsn-endpoint-ethernet-mac-3.0
> 
> [ ... ]
> 
> > +patternProperties:
> > +  "^ethernet@[0-9a-f]+$":
> > +    type: object
> > +    $ref: /schemas/net/ethernet-controller.yaml#
> > +    unevaluatedProperties: false
> > +    description:
> > +      Endpoint MAC sub-node. Provides the host-side data path through the
> > +      DMA channels described by its dmas / dma-names.
> > +    properties:
> > +      compatible:
> > +        const: xlnx,tsn-ep-mac
> 
> [Severity: Low]
> The wrapper compatible is versioned (xlnx,tsn-endpoint-ethernet-mac-3.0).
> The child compatibles here and on the switch node (xlnx,tsn-switch) are
> generic and carry no version.
> 
> writing-bindings.rst asks for specific compatibles, with no device-family
> names. For sub-blocks of a bigger device, it asks for device-based
> compatibles.
> 
> If a later IP revision, or the planned Endpoint-only variant, changes the
> endpoint or switch register layout, how would these child nodes be told
> apart? The endpoint driver later in the series matches only on
> "xlnx,tsn-ep-mac" in xlnx_tsn_ep_of_match.
> 
> Should the child compatibles be tied to the device or version, the same
> way the wrapper compatible is?

It's fine. Being specific is less important given there's a specific 
parent compatible.

> 
> [ ... ]
> 
> > +      dma-names:
> > +        minItems: 2
> > +        maxItems: 16
> > +        items:
> > +          pattern: "^(tx|rx)_chan[0-7]$"
> > +        allOf:
> > +          - contains:
> > +              pattern: "^tx_chan[0-7]$"
> > +          - contains:
> > +              pattern: "^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. At least one TX and one RX
> > +          channel are required. Example: "tx_chan0", "tx_chan1", ...,
> > +          "rx_chan0", "rx_chan1", ....
> 
> [Severity: Low]
> The description says the names are "one per DMA channel". The v3
> changelog says "Cap TX at 8 and RX at 8 channels". Does the schema
> enforce either of these?
> 
> Without uniqueItems: true, dt_binding_check appears to accept a 16-entry
> list of fifteen "tx_chan0" plus one "rx_chan0". It also appears to accept
> "tx_chan0".."tx_chan7", then "tx_chan0".."tx_chan6", then "rx_chan0".
> 
> dtschema doesn't add uniqueness to -names properties by itself. For
> example, renesas,drif.yaml lists "rx", "rx", while renesas,sdhi.yaml sets
> uniqueItems: true on its clock-names.
> 
> Could uniqueItems: true be added here? The pattern only allows eight
> names per direction, so that would also enforce the 8 TX / 8 RX limit.
> 
> Later in the series, ep_count_dma_channels() rejects duplicate TX and RX
> names at probe time. So the runtime result is a DT that passes schema
> validation and then fails probe.

IIRC, dma-names is the exception here. So yes, you should add 
'uniqueItems: true'.

> 
> [ ... ]
> 
> > +        patternProperties:
> > +          "^queue[0-7]$":
> > +            type: object
> > +            additionalProperties: false
> 
> [ ... ]
> 
> > +        required:
> > +          - queue0
> > +          - queue1
> 
> [ ... ]
> 
> > +allOf:
> > +  - if:
> > +      properties:
> > +        xlnx,num-priorities:
> > +          const: 3
> > +    then:
> > +      patternProperties:
> > +        "^ethernet@[0-9a-f]+$":
> > +          properties:
> > +            tx-queues-config:
> > +              required:
> > +                - queue0
> > +                - queue1
> > +                - queue2
> 
> [Severity: Low]
> With xlnx,num-priorities = <2>, can a tx-queues-config that contains
> queue0 through queue7 still pass dt_binding_check?
> 
> The ^queue[0-7]$ pattern accepts every index from 0 to 7. The if/then
> blocks for num-priorities 3..8 only add entries to required. So the
> schema has a lower bound tied to xlnx,num-priorities, but no upper bound.
> 
> Later in the series, ep_parse_tx_queue_config() rejects queueN with
> N >= num_tx_queues at probe time, so only schema validation is affected.
> 
> Could each if/then block also set queueK: false for K >= N, so the
> schema rejects these nodes too?

Shrug. Sounds too complicated.

Rob