Re: [PATCH net-next v2 4/8] net: xilinx: tsn: parse endpoint DMA channel configuration
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-12 20:36:23
Also in:
linux-arm-kernel, linux-devicetree, linux-rt-devel, lkml
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 hunk ↗ jump to hunk
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 hunk ↗ jump to hunk
@@ -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 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.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909-patches_v2_external-v2-0-3a40babaff4c%40amd.com