Re: [PATCH v7 net-next 10/15] net: dsa: netc: introduce NXP NETC switch driver for i.MX94

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Resource leaks and sysfs duplication upon DSA master interface unbind/rebind due to misuse of `devm_*` APIs in the `.setup` callback.
- [Medium] Missing `pci_clear_master()` in the `.shutdown` callback risks memory corruption during kexec.
--

commit 61a3ea3e0b6e78c0e019f8c93113e82986d40e5e
Author: Wei Fang <wei.fang@nxp.com>
Subject: net: dsa: netc: introduce NXP NETC switch driver for i.MX94

This commit introduces the initial NXP NETC switch driver for the i.MX94
series, providing basic probe and remove functionality. The NETC IP provides
integrated 802.1Q Ethernet switch functionality and internal ENETC
connectivity via a pseudo MAC.

> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> index 0000000000000..8e3a3230226c3
> --- /dev/null
> +++ b/drivers/net/dsa/netc/netc_main.c
[ ... ]
> +static int netc_port_create_emdio_bus(struct netc_port *np,
> +				      struct device_node *node)
> +{
[ ... ]
> +	bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
> +	if (!bus)
> +		return -ENOMEM;
[ ... ]
> +	err = devm_of_mdiobus_register(dev, bus, node);
> +	if (err)
> +		return dev_err_probe(dev, err,
> +				     "Cannot register EMDIO bus\n");
[ ... ]
> +static int netc_init_all_ports(struct netc_switch *priv)
> +{
> +	struct device *dev = priv->dev;
> +	struct netc_port *np;
> +	struct dsa_port *dp;
> +	int err;
> +
> +	priv->ports = devm_kcalloc(dev, priv->info->num_ports,
> +				   sizeof(struct netc_port *),
> +				   GFP_KERNEL);
> +	if (!priv->ports)
> +		return -ENOMEM;
[ ... ]
> +	for (int i = 0; i < priv->info->num_ports; i++) {
> +		np = devm_kzalloc(dev, sizeof(*np), GFP_KERNEL);
> +		if (!np)
> +			return -ENOMEM;
[ ... ]
> +static int netc_setup(struct dsa_switch *ds)
> +{
> +	struct netc_switch *priv = ds->priv;
> +	struct dsa_port *dp;
> +	int err;
> +
> +	err = netc_init_switch_id(priv);
> +	if (err)
> +		return err;
> +
> +	err = netc_init_all_ports(priv);

The DSA subsystem can invoke the switch driver's .setup and .teardown
callbacks multiple times over the device's lifetime, such as when the
master netdevice is unbound and rebound.

Because devm_kcalloc(), devm_kzalloc(), and devm_of_mdiobus_register()
are tied to the PCI device's lifetime rather than the DSA switch tree,
these resources won't be freed when netc_teardown() is executed.

If the master interface is rebound and netc_setup() runs again, could
this cause memory leaks and a duplicate sysfs name collision when trying
to register the same MDIO bus?

[ ... ]
> +static void netc_switch_shutdown(struct pci_dev *pdev)
> +{
> +	struct netc_switch *priv = pci_get_drvdata(pdev);
> +
> +	if (!priv)
> +		return;
> +
> +	dsa_switch_shutdown(priv->ds);
> +	pci_set_drvdata(pdev, NULL);
> +}

While dsa_switch_shutdown() brings the network interfaces down, does it
guarantee that all hardware DMA engines (such as the switch-wide NTMP
Command BD rings) are immediately halted?

If not, should this function call pci_clear_master() to prevent
the device from continuing pending DMA operations and potentially
corrupting memory during a kexec boot?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513030454.1666570-1-wei.fang@nxp.com?part=10