Re: [PATCH net-next v23 4/9] mfd: an8855: Add support for Airoha AN8855 Switch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] of: memory leak of mdio_np in an8855_mdio_register error path
- [Medium] mfd: sysfs naming collision for virtual MDIO bus with multiple switches
--

commit 802b31a10a6859408359f548c9b38d422fd3e7dc
Author: Christian Marangi <ansuelsmth@gmail.com>
Subject: mfd: an8855: Add support for Airoha AN8855 Switch

Add support for Airoha AN8855 Switch that provide support for a DSA
switch and a NVMEM provider. Also make use of the mdio-regmap driver
and register a regmap for each internal PHY of the switch.

> diff --git a/drivers/mfd/airoha-an8855.c b/drivers/mfd/airoha-an8855.c
> new file mode 100644
> index 0000000000000..a2a36704a9fca
> --- /dev/null
> +++ b/drivers/mfd/airoha-an8855.c
[ ... ]
> +static int an8855_phy_bus_register(struct device *dev, struct an8855_core_priv *priv,
> +				   struct device_node *phy_np,
> +				   struct an8855_phy_bus *phy_bus_info)
> +{
[ ... ]
> +	mrc.regmap = regmap;
> +	mrc.parent = dev;
> +	mrc.valid_addr = addr;
> +	snprintf(mrc.name, MII_BUS_ID_SIZE, "an8855:%u-phy%u", priv->switch_addr,
> +		 phy_offset);

[Severity: Medium]
Will this static naming scheme fail to disambiguate virtual buses across
multiple independent parent devices?

If a system contains multiple AN8855 switches on different MDIO buses but
configured at the same MDIO address (e.g., address 0), priv->switch_addr
will be identical for both.

Since devm_mdio_regmap_register attempts to register both virtual buses with
the exact same name (e.g., an8855:0-phy0), would this lead to a sysfs naming
collision and cause the second switch to fail probe? A parent device
identifier like dev_name(dev) might be needed to ensure uniqueness.

> +
> +	bus = devm_mdio_regmap_register(dev, &mrc);
[ ... ]
> +static int an8855_mdio_register(struct device *dev, struct an8855_core_priv *priv,
> +				struct an8855_phy_bus *phy_bus_infos)
> +{
> +	struct device_node *mdio_np;
> +	int ret = 0;
> +	int i = 0;
> +
> +	mdio_np = of_get_child_by_name(dev->of_node, "mdio");

[Severity: Medium]
Does this code leak the mdio_np device node on the error path?

of_get_child_by_name dynamically acquires and increments the reference
count of the returned device node mdio_np.

> +	if (!mdio_np)
> +		return -ENODEV;
> +
> +	for_each_available_child_of_node_scoped(mdio_np, phy_np) {
> +		if (i >= AN8855_MAX_PHY_PORT) {
> +			ret = -EINVAL;
> +			goto exit;
> +		}
> +
> +		ret = an8855_phy_bus_register(dev, priv, phy_np,
> +					      &phy_bus_infos[i]);
> +		if (ret)
> +			goto exit;
> +
> +		i++;
> +	}
> +
> +	of_node_put(mdio_np);
> +exit:

In the normal execution path, of_node_put(mdio_np) is correctly called.
However, if an error occurs during child node iteration, the code executes
goto exit, which jumps directly to the cleanup block and bypasses
of_node_put(mdio_np), leaving the reference count incremented.

> +	if (ret) {
> +		while (i--)
> +			fwnode_handle_put(phy_bus_infos[i].node);
> +
> +		return ret;
> +	}
> +
> +	return i;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925182435.21207-1-ansuelsmth@gmail.com?part=4