public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Jakub Kicinski <kuba@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org,
	linux-stm32@st-md-mailman.stormreply.com, netdev@vger.kernel.org,
	Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH net-next 1/3] net: stmmac: rk: fix missing reset_control_put()
Date: Wed, 28 Jan 2026 12:19:56 +0000	[thread overview]
Message-ID: <aXn-7LWRk5cZjno8@shell.armlinux.org.uk> (raw)
In-Reply-To: <c8a10b5355b750cfc83a7f746347175ab40b64d7.camel@pengutronix.de>

On Wed, Jan 28, 2026 at 01:04:21PM +0100, Philipp Zabel wrote:
> On Mi, 2026-01-28 at 10:58 +0000, Russell King (Oracle) wrote:
> > rk_gmac_setup() delves into the PHY's DT node to retrieve its reset
> > control using of_reset_control_get(). However, it never releases it
> > when the driver is removed. Add reset_control_put() to rk_gmac_exit()
> > to clean this up.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > index 5f8d2031b97c..bc69cbb5a7d4 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > @@ -1784,6 +1784,8 @@ static void rk_gmac_exit(struct device *dev, void *bsp_priv_)
> >  
> >  	if (priv->plat->phy_node && bsp_priv->integrated_phy)
> >  		clk_put(bsp_priv->clk_phy);
> > +
> > +	reset_control_put(bsp_priv->phy_reset);
> >  }
> >  
> >  static int rk_gmac_probe(struct platform_device *pdev)
> 
> This is fine because the driver sets plat_dat->suspend, and so
> rk_gmac_exit() is never called via stmmac_pltfr_exit() during suspend.
> 
> It does look a bit sketchy to release resources in the rk_gmac_exit()
> counterpart to rk_gmac_init(), which never requested the resources,
> though. Maybe use devm_add_action_or_reset() to register the release of
> the reset during remove?

Thanks, but I think a sense of proportion is required here. This
patch is the result of introducing the ->init() method, and AI
noticing that there was no cleanup of this resource. This was the
simplest way to implement that cleanup.

However, your review commit also applies to bsp_priv->clk_phy which
has the same problem - this also isn't obtained in rk_gmac_init(),
but in rk_gmac_clk_init().

Given that, and the fact that this entire series is already
considerably big (it was 21 patches, then 22, now 23, and with this
it's going to become 24 patches) I'm going to say that this issue
can be addressed at a later time.

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

  reply	other threads:[~2026-01-28 12:20 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-28 10:58 [PATCH net-next 0/3] net: stmmac: rk: second chunk of cleanups Russell King (Oracle)
2026-01-28 10:58 ` [PATCH net-next 1/3] net: stmmac: rk: fix missing reset_control_put() Russell King (Oracle)
2026-01-28 12:04   ` Philipp Zabel
2026-01-28 12:19     ` Russell King (Oracle) [this message]
2026-01-28 13:52       ` Philipp Zabel
2026-01-28 13:34   ` Andrew Lunn
2026-01-28 10:58 ` [PATCH net-next 2/3] net: stmmac: rk: add GMAC_CLK_xx constants, simplify RGMII definitions Russell King (Oracle)
2026-01-28 13:27   ` Maxime Chevallier
2026-01-28 10:58 ` [PATCH net-next 3/3] net: stmmac: rk: add SoC specific ->init() method Russell King (Oracle)
2026-01-30  2:50 ` [PATCH net-next 0/3] net: stmmac: rk: second chunk of cleanups patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aXn-7LWRk5cZjno8@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=heiko@sntech.de \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=netdev@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox