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

[Neeli, Srinivas]

hi,

On 9/13/2026 2:06 AM, netdev-bot+sashiko@xxxxxxxxxx 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.
>
> > 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
> [ ... ]
>
> > @@ -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);
> [ ... ]
>
> > +		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;
> > +}
> [ ... ]
>
> >   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
>