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

["Neeli, Srinivas" <srneeli@amd.com>]

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@kernel.org 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