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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Probe failure for standard-compliant DT nodes due to unit-address stripping.
--

commit ce208f79c4ab3c3d36c98c28fc4120175bc8c6d9
Author: Srinivas Neeli <srinivas.neeli@xxxxxxx>

net: xilinx: tsn: parse endpoint DMA channel configuration

This commit introduces device tree parsing for the endpoint DMA channels in
the Xilinx TSN driver. It counts the available TX and RX channels and maps 
logical TX queues to their physical DMA channels based on child nodes.

> diff --git a/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c b/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c
> index 089f17a126f54..24025b1f6e66d 100644
> --- a/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c
> +++ b/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c

[ ... ]

> +/*
> + * 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)

[Severity: Medium]
Will this correctly parse standard device tree unit addresses?

The Open Firmware (OF) core strips the unit-address (the `@...` portion) 
when assigning the `name` field during node initialization. This means 
that for a standard-compliant node like `queue@0`, `qnode->name` will be 
exactly `"queue"`.

If `qnode->name` is exactly `"queue"`, skipping the prefix length leaves an
empty string (`"\0"`). Calling kstrtou32() on an empty string returns 
-EINVAL, which would erroneously abort the probe for compliant DTs.

Could this logic be adjusted to handle unit-address stripping, or perhaps
read the logical queue index from a standard DT property like `reg` instead
of relying on string parsing of the node name?

> +			return dev_err_probe(ep->dev, -EINVAL,
> +					     "tx-config: invalid queue node %pOFn (have %u queues)\n",
> +					     qnode, ep->num_tx_queues);
> +
> +		if (test_and_set_bit(queue, queue_seen))
> +			return dev_err_probe(ep->dev, -EINVAL,
> +					     "tx-config: queue %u described twice\n",
> +					     queue);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-patches_v2_external-v2-0-3a40babaff4c@xxxxxxx?part=4