Re: [PATCH net-next v2 4/8] net: xilinx: tsn: parse endpoint DMA channel configuration

hi,

On 9/13/2026 2:06 AM, netdev-bot+sashiko@kernel.org wrote:

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] xlnx_tsn_ep_probe() and ep_parse_tx_queue_config() enforce two
   new mandatory device-tree constraints that the in-tree binding does…

--

net: xilinx: tsn: parse endpoint DMA channel configuration

This patch counts the TX and RX MCDMA channels listed in the endpoint's
"dma-names" property, allocates the conduit netdev with that many hardware
queues, and parses the "tx-queues-config" child node to map each logical TX
queue to a physical DMA channel.

quoted

diff --git a/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c b/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c
index 089f17a126f5..24025b1f6e66 100644
--- a/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c
+++ b/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c

[ ... ]

quoted

@@ -69,15 +88,173 @@ static const struct ethtool_ops ep_ethtool_ops = {
  	.get_drvinfo	= ep_get_drvinfo,
  };
  
+/*
+ * Parse the "tx-queues-config" child of the EP node. The logical queue
+ * index is taken from the "queue<N>" node name, so the mapping does not
+ * depend on the order the child nodes appear in the device tree.
+ */
+static int ep_parse_tx_queue_config(struct xlnx_tsn_ep *ep,
+				    struct device_node *txcfg_np, u16 tx_present)
+{
+	DECLARE_BITMAP(queue_seen, TSN_MAX_TX_QUEUE) = {};
+	DECLARE_BITMAP(chan_seen, TSN_MAX_TX_QUEUE) = {};
+	unsigned int count = 0;
+	int ret;
+
+	for_each_child_of_node_scoped(txcfg_np, qnode) {
+		u32 chan, queue;
+
+		if (!str_has_prefix(qnode->name, "queue") ||
+		    kstrtou32(qnode->name + strlen("queue"), 10, &queue) ||
+		    queue >= ep->num_tx_queues)
+			return dev_err_probe(ep->dev, -EINVAL,
+					     "tx-config: invalid queue node %pOFn (have %u queues)\n",
+					     qnode, ep->num_tx_queues);

[ ... ]

quoted

+		ep->tx_dma_chan_map[queue] = chan;
+		count++;
+	}
+
+	if (count != ep->num_tx_queues)
+		return dev_err_probe(ep->dev, -EINVAL,
+				     "tx-config: described %u queues but expected %u\n",
+				     count, ep->num_tx_queues);
+
+	return 0;
+}

[ ... ]

quoted

  static int xlnx_tsn_ep_probe(struct platform_device *pdev)
  {
+	u32 rx_chan_num[TSN_MAX_RX_QUEUE];
  	struct device *dev = &pdev->dev;
+	struct device_node *txcfg_np;
+	u32 num_tx, num_rx, num_prio;
+	struct device_node *ip_np;
  	struct net_device *ndev;
  	struct xlnx_tsn_ep *ep;
  	u8 mac_addr[ETH_ALEN];
+	u16 tx_present = 0;
  	int ret;
+	int i;
+
+	ret = ep_count_dma_queues(dev, &num_tx, &num_rx, &tx_present, rx_chan_num);
+	if (ret)
+		return ret;
+
+	ip_np = of_get_parent(dev->of_node);
+	if (!ip_np)
+		return dev_err_probe(dev, -EINVAL, "missing parent IP node\n");
+
+	ret = of_property_read_u32(ip_np, "xlnx,num-priorities", &num_prio);
+	of_node_put(ip_np);
+	if (ret)
+		return dev_err_probe(dev, ret, "missing xlnx,num-priorities\n");
+
+	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);

[Severity: Medium]
Can this equality check reject device trees that the in-tree binding
accepts?  Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-ethernet-mac.yaml
describes the two properties completely independently:

   xlnx,num-priorities:
     $ref: /schemas/types.yaml#/definitions/uint32
     minimum: 2
     maximum: 8

       dma-names:
         minItems: 2
         maxItems: 16
         items:
           pattern: "^(tx|rx)_chan[0-7]$"

With that schema, dma-names = "tx_chan0", "rx_chan0" together with
xlnx,num-priorities = <2> validates, but xlnx_tsn_ep_probe() now fails with
"TX channel count 1 must equal num-priorities 2".  A 4 TX + 4 RX endpoint
under xlnx,num-priorities = <8> fails the same way.

The queue node checks in ep_parse_tx_queue_config() add a second constraint
that the schema does not express: exactly one queue<N> node per TX channel,
with dense indices 0..num_tx-1.  The schema has no required queue children
under tx-queues-config, so an empty

   tx-queues-config {
   };

is schema-valid and gets "tx-config: described 0 queues but expected N",
and sparse names such as queue0/queue1/queue2/queue7 with 4 TX channels get
"tx-config: invalid queue node queue7 (have 4 queues)".

Because the endpoint MAC is the DSA conduit referenced by
port@0 { ethernet = <&ep_mac>; }, a probe failure here also takes away the
three-port switch, not just the ep0 netdev.

Semantically these look like different things: xlnx,num-priorities is an
IP-wide traffic class count on the parent node, while dma-names describes
host-side MCDMA wiring on the endpoint node, and the binding's own example
uses an arbitrary queueN -> tx_chanM permutation.  num_prio is read here for
no other purpose than this comparison.

Should the binding encode the coupling (required queue children and a
dma-names/num-priorities relation), or should the driver accept every
topology the binding advertises?

   The binding intentionally does not encode the coupling. 
xlnx,num-priorities is
   an IP-wide property on the parent node describing the number of 
egress priority
   queues the IP is synthesised with, while dma-names describes the host 
MCDMA
   wiring on the endpoint node. The equality holds in this MCDMA-only
   configuration, where the driver sets up one MCDMA TX channel per priority
   queue, but it is not the meaning of the property, with the time-aware and
   per-stream schedulers a priority queue can be served without an MCDMA TX
   channel. Encoding num_tx == num-priorities in the schema would 
describe this
   configuration rather than the hardware.

The YAML is unchanged by this patch and by the remainder of the series, and
only the 8 TX / 8 RX, xlnx,num-priorities = <8> example in the binding
satisfies the driver, so dt_binding_check does not surface the mismatch.

Could the commit message body state these new device tree requirements?
Right now they appear only in the below-the-line v2 changelog:

   - Require one queue node per tx_chan and num-priorities equal to the TX
     channel count.

[ ... ]

   v3 adds required: [queue0] under tx-queues-config, so an empty node 
is now rejected at
   dt_binding_check. The remaining constraint, exactly one queueN per TX 
channel with
   dense indices, depends on the tx_chanN count in another property, 
which dt-schema
  cannot express, so the driver validates it and reports it with 
dev_err_probe().

Thanks
Srinivas Neeli