Re: [PATCH net-next 01/12] net: pcs: pcs-xpcs-regmap: support XPCS memory-mapped MDIO bus via regmap

On 5/2/26 10:56 AM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] devm_xpcs_regmap_register violates devm lifecycle semantics and causes a resource leak.
> --
> 
> commit 0a11082b5acea20f4d45fbd6df8fb9ec06db752d
> Author: Daniel Thompson <[email protected]>
> 
> net: pcs: pcs-xpcs-regmap: support XPCS memory-mapped MDIO bus via regmap
> 
> This commit introduces helpers to allow a driver to register an XPCS using a
> regmap for the memory-mapped MDIO bus. It adds the devm_xpcs_regmap_register
> function to manage the bus registration and interface abstractions.
> 
>> diff --git a/drivers/net/pcs/pcs-xpcs-regmap.c b/drivers/net/pcs/pcs-xpcs-regmap.c
>> new file mode 100644
>> index 000000000000..20a54a360595
>> --- /dev/null
>> +++ b/drivers/net/pcs/pcs-xpcs-regmap.c
> 
> [ ... ]
> 
>> +struct dw_xpcs *devm_xpcs_regmap_register(struct device *dev,
>> +					  const struct xpcs_regmap_config *config)
>> +{
> 
> [ ... ]
> 
>> +	ret = devm_mdiobus_register(dev, pxpcs->bus);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to create MDIO bus\n");
>> +		return ERR_PTR(ret);
>> +	}
>> +
>> +	return xpcs_create_mdiodev(pxpcs->bus, 0);
> 
> Could this cause a resource leak on driver unbind?

Yes it could.  This will be fixed by checking the return value here,
and if it does not indicate an error, setting up a devm action
to call xpcs_destroy() on the dw_xpcs structure created.

					-Alex

> The devm_xpcs_regmap_register() function uses the devm_ prefix, implying an API
> contract where the returned dw_xpcs object will be automatically managed and
> cleaned up.
> 
> Internally, it calls xpcs_create_mdiodev(), which allocates the dw_xpcs
> structure and increments the reference count of the underlying mdio_device.
> These resources normally must be explicitly freed via xpcs_destroy().
> 
> Should this function register a devres cleanup action (for example, using
> devm_add_action_or_reset()) to invoke xpcs_destroy() during unbind?
> 
>> +}
>> +EXPORT_SYMBOL_GPL(devm_xpcs_regmap_register);
>