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

[Neeli, Srinivas]

Hi Rob,

On 10/21/2026 8:46 AM, Rob Herring wrote:
> 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'.

  I tried to add uniqueItems: true under dma-names and I observed the below
  error:

      xlnx,tsn-endpoint-ethernet-mac.yaml: patternProperties:^ethernet@...:
      properties:dma-names: Additional properties are not allowed
      ('uniqueItems' was unexpected)
        from schema $id: 
http://devicetree.org/meta-schemas/string-array.yaml#

  As per my understanding the core DT schema already has uniqueItems: true
  for dma-names:

https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/dma/dma.yaml

  I also tried to reproduce the negative test scenarios reported by
  netdev-bot using:

    make dt_binding_check DT_CHECKER_FLAGS=-m \
 DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-ethernet-mac.yaml

    With DT_CHECKER_FLAGS=-m I can reproduce the reported errors. 
Without it
    those negative scenarios pass. As per my understanding netdev-bot ran
    without DT_CHECKER_FLAGS, which is why it reported these cases as
    accepted. As all the scenarios are rejected once DT_CHECKER_FLAGS is
    passed, I am ignoring the warning and keeping the binding unchanged.
> > [ ... ]
> >
> > > +        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?
Understood. I will address the upper bound check in the next version.

Thanks
Neeli Srinivas

pw-bot: cr

> Shrug. Sounds too complicated.
>
> Rob