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

[netdev-bot+sashiko@kernel.org]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The driver enforces DT rules that the in-tree binding
  (Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-ethernet-…

--

net: xilinx: tsn: parse endpoint DMA channel configuration

This commit counts the TX and RX MCDMA channels listed in dma-names and
allocates the conduit netdev with that many queues. It also parses the
tx-queues-config child node to map each logical TX queue to a tx_chanN
entry. Probe fails if the mapping is not one-to-one, or if the TX
channel count differs from xlnx,num-priorities.

> diff --git a/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c b/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c
> index 609d444c68356..2adee07f6f51c 100644
> --- a/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c
> +++ b/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c

[ ... ]

>  static int xlnx_tsn_ep_probe(struct platform_device *pdev)
>  {

[ ... ]

> +	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_dma_channels != num_prio)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "TX channel count %u must equal num-priorities %u\n",
> +				     num_tx_dma_channels, num_prio);

[Severity: Medium]
Do these rules match what the binding in
Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-ethernet-mac.yaml
allows?

Here xlnx_tsn_ep_probe() requires the number of tx_chanN entries in
dma-names to equal xlnx,num-priorities. ep_parse_tx_queue_config() adds
three more rules:

	if (!str_has_prefix(qnode->name, "queue") ||
	    kstrtou32(qnode->name + strlen("queue"), 10, &queue) ||
	    queue >= ep->num_tx_queues)
	...
	if (test_and_set_bit(chan, chan_seen))
	...
	if (count != ep->num_tx_queues)

The binding only has:

	dma-names:
	  items:
	    pattern: "^(tx|rx)_chan[0-7]$"
	  allOf:
	    - contains:
	        pattern: "^tx_chan[0-7]$"
	    - contains:
	        pattern: "^rx_chan[0-7]$"

	tx-queues-config:
	  patternProperties:
	    "^queue[0-7]$":

The allOf blocks at the end of the binding only require queue0 through
queue(num-priorities - 1). The binding does not tie the tx_chanN count to
xlnx,num-priorities. It does not forbid queueN nodes at or above
num-priorities, and it does not require each queue to use a distinct
channel.

Its description reads as a general mapping:

	For example, queue0 with xlnx,dma-channel-num set to 5 means
	priority queue 0 uses tx_chan5 for data transfer.

So some device trees pass schema validation but would make probe fail
with -EINVAL. For example:

  - xlnx,num-priorities = <4> with all eight tx_chanN entries listed and
    queue0..queue3 mapped to channels 4..7
  - xlnx,num-priorities = <2> with queue0, queue1 and queue2 nodes
  - xlnx,num-priorities = <2> with two tx channels and both queues
    mapped to channel 0

The binding also sets minimum: 2 for xlnx,num-priorities, and the driver
does not check that.

Later patches in the series do not change the binding, so the mismatch
is still there at the end of the series.

Should the binding describe and enforce the one-to-one queue/channel rule
and the num-priorities rule? Or should the driver accept the general
mapping that the binding describes?

>  
> -	ndev = alloc_netdev(sizeof(*ep), "eth%d", NET_NAME_UNKNOWN, ether_setup);
> +	ndev = alloc_netdev_mqs(sizeof(*ep), "eth%d", NET_NAME_UNKNOWN,
> +				ether_setup, num_tx_dma_channels,
> +				num_rx_dma_channels);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-patches_v2_external-v3-0-e33d3960ca1e%40amd.com