On Tue, Jan 27, 2026 at 12:51:45AM +0000, Russell King (Oracle) wrote:

On Mon, Jan 26, 2026 at 04:40:20PM -0800, Jakub Kicinski wrote:

quoted

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: stmmac: rk: add SoC specific ->init() method

This commit adds infrastructure for SoC-specific initialization by adding
an init callback to rk_gmac_ops and calling it from rk_gmac_setup().

quoted

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 7f8ffd1549bc..4117f6863ff0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c

[ ... ]

quoted

@@ -1618,6 +1619,14 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,

 	bsp_priv->dev = dev;

+	if (ops->init) {
+		ret = ops->init(bsp_priv);
+		if (ret) {
+			dev_err_probe(dev, ret, "failed to init BSP\n");
+			return ERR_PTR(ret);

                        ^^^^^^^^^^^^^^^

If ops->init() fails here, does bsp_priv->phy_reset need to be released?

Earlier in rk_gmac_setup(), at line 1610, the code calls:

    bsp_priv->phy_reset = of_reset_control_get(plat->phy_node, NULL);

of_reset_control_get() is not a devm-managed function, so the reset control
would need to be explicitly released with reset_control_put() on error paths.

This new error return happens after phy_reset could have been acquired (when
the device has an integrated PHY), but before the function successfully
returns. If this path is taken, it appears the reset control would be leaked.

Currently no SoC sets ops->init so this path cannot trigger, but when a
future SoC implements this callback, the leak would occur on init failure
for devices with integrated PHYs.

However, the driver does not release this resource when cleaning up, so
that's already a bug as the driver currently stands. I think this could
be converted to devm_reset_control_get(), which would resolve both
leakages, but not sure.

Note that fixing this is going to add yet another patch to the series,
because this is a pre-existing bug in the driver. It can't be replaced
with devm_reset_control_get(), because this driver is getting resources
for a foreign device (we don't have the struct device pointer.)

So, it isn't going to be a simple patch to fix this.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!