[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260127081804.64841f65@kernel.org>
Date: Tue, 27 Jan 2026 08:18:04 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: linux-stm32@...md-mailman.stormreply.com, andrew+netdev@...n.ch,
linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org,
pabeni@...hat.com, linux-rockchip@...ts.infradead.org, heiko@...ech.de,
alexandre.torgue@...s.st.com, andrew@...n.ch, davem@...emloft.net,
edumazet@...gle.com
Subject: Re: [net-next,v2,06/22] net: stmmac: rk: add SoC specific ->init()
method
On Tue, 27 Jan 2026 11:09:58 +0000 Russell King (Oracle) wrote:
> > > 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.
Would it work to make that plus patches 1-4,6 a separate series?
Powered by blists - more mailing lists