Re: [net-next,v2,06/22] net: stmmac: rk: add SoC specific ->init() method

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Mon, Jan 26, 2026 at 04:40:20PM -0800, Jakub Kicinski wrote:
> 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().
> 
> > 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
> 
> [ ... ]
> 
> > @@ -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.

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