All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: [PATCH] net: stmmac: dwmac-meson8b: fix meson8b_devm_clk_prepare_enable()
Date: Fri,  4 Nov 2022 09:30:04 +0100	[thread overview]
Message-ID: <20221104083004.2212520-1-linux@rasmusvillemoes.dk> (raw)

There are two problems with meson8b_devm_clk_prepare_enable(),
introduced in a54dc4a49045 (net: stmmac: dwmac-meson8b: Make the clock
enabling code re-usable):

- It doesn't pass the clk argument, but instead always the
  rgmii_tx_clk of the device.

- It silently ignores the return value of devm_add_action_or_reset().

The former didn't become an actual bug until another user showed up in
the next commit, 9308c47640d5 (net: stmmac: dwmac-meson8b: add support
for the RX delay configuration). The latter means the callers could
end up with the clock not actually prepared/enabled.

Fixes: a54dc4a49045 (net: stmmac: dwmac-meson8b: Make the clock enabling code re-usable)
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
Not tested in any way, just stumbled on this which didn't seem right.

It seems that the timing_adj_clk could be changed (with a bit of
refactoring) to use devm_clk_get_enabled() - i.e. don't make it
optional, but only get/prepare/enable it in the (delay_config &
PRG_ETH0_ADJ_ENABLE) branch where the code actually requires it.

But since there's no official devm_clk_prepare_enable, and the
rgmii_tx_clk isn't obtained via a clk_get, I don't see a simple way to
avoid this private implmentation of devm_clk_prepare_enable().

drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index c7a6588d9398..e8b507f88fbc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -272,11 +272,9 @@ static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
 	if (ret)
 		return ret;
 
-	devm_add_action_or_reset(dwmac->dev,
-				 (void(*)(void *))clk_disable_unprepare,
-				 dwmac->rgmii_tx_clk);
-
-	return 0;
+	return devm_add_action_or_reset(dwmac->dev,
+					(void(*)(void *))clk_disable_unprepare,
+					clk);
 }
 
 static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
-- 
2.37.2


             reply	other threads:[~2022-11-04  8:30 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-04  8:30 Rasmus Villemoes [this message]
2022-11-04 21:02 ` [PATCH] net: stmmac: dwmac-meson8b: fix meson8b_devm_clk_prepare_enable() Martin Blumenstingl
2022-11-05 22:55   ` Martin Blumenstingl
2022-11-08  2:00 ` 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=20221104083004.2212520-1-linux@rasmusvillemoes.dk \
    --to=linux@rasmusvillemoes.dk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=netdev@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.