Re: [PATCH net-next v2 3/8] net: xilinx: tsn: add endpoint MAC driver skeleton

[netdev-bot+sashiko@kernel.org]

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

Critical: 0 · High: 0 · Medium: 0 · Low: 2

- [Low] Commit message contradicts the code: the message body states
  'Register the netdev named "ep"', but xlnx_tsn_ep_probe() calls…
- [Low] Reported concern (validated as NOT a defect at the end of the
  series): the binding Documentation/devicetree/bindings/net/xlnx,tsn-…

--

net: xilinx: tsn: add endpoint MAC driver skeleton

This adds a platform driver for the "xlnx,tsn-ep-mac" endpoint MAC that
registers an enumerated netdev with minimal netdev and ethtool ops, sets
the MAC address from DT or a random one, and drops all transmitted
frames.  The new driver is registered alongside the existing TSN
platform driver.

Two questions below, both minor.

> Add a platform driver (compatible "xlnx,tsn-ep-mac") for the endpoint.
> Register the netdev named "ep", set its MAC address, and provide minimal
> netdev and ethtool ops. ndo_open starts the queues and ndo_start_xmit
> drops frames.

[Severity: Low]
Should "the netdev named "ep"" say ep%d here?  xlnx_tsn_ep_probe() uses:

	ndev = alloc_netdev(sizeof(*ep), "ep%d", NET_NAME_ENUM, ether_setup);

so the interface shows up as ep0, and ep1 for a second IP instance.  The
kernel-doc for struct xlnx_tsn_ep in the same file already says '("ep0"
for the first IP instance)'.

The rename is described in the v2 changelog below the "---" separator,
which is dropped when the patch is applied, so the permanent log would
keep the old name.  Can the message body be updated to match?

> diff --git a/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c b/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c
> new file mode 100644
> index 000000000000..089f17a126f5
> --- /dev/null
> +++ b/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c
> @@ -0,0 +1,143 @@

[ ... ]

> +static int xlnx_tsn_ep_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct net_device *ndev;
> +	struct xlnx_tsn_ep *ep;
> +	u8 mac_addr[ETH_ALEN];
> +	int ret;
> +
> +	ndev = alloc_netdev(sizeof(*ep), "ep%d", NET_NAME_ENUM, ether_setup);
> +	if (!ndev)
> +		return -ENOMEM;
> +
> +	SET_NETDEV_DEV(ndev, dev);
> +	ndev->netdev_ops = &ep_netdev_ops;
> +	ndev->ethtool_ops = &ep_ethtool_ops;
> +	ndev->features = NETIF_F_SG;
> +
> +	ep = netdev_priv(ndev);
> +	ep->ndev = ndev;
> +	ep->dev = dev;
> +
> +	ret = of_get_mac_address(dev->of_node, mac_addr);

[Severity: Low]
This isn't a bug, but of_get_mac_address() is the only DT property read
here, while the binding
Documentation/devicetree/bindings/net/xlnx,tsn-endpoint-ethernet-mac.yaml
lists reg, dmas, dma-names and tx-queues-config as required for this
node.

The later patches in this series ("net: xilinx: tsn: parse endpoint DMA
channel configuration" and the MCDMA/RX/TX data path patches) do add
ep_count_dma_queues() and ep_parse_tx_queue_config(), so dmas,
dma-names, tx-queues-config and xlnx,num-priorities all end up parsed
and enforced with dev_err_probe() by the end of the series.

The one property that is still never consumed at the end of the series
is the required reg region, since the v2 changelog notes the ioremap
comes back with QBV support.  Is reg intended to stay documentation-only
for the node unit address until then?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909-patches_v2_external-v2-0-3a40babaff4c%40amd.com