macb_runtime_resume() calls clk_prepare_enable() five times without
checking the return value. If any clock fails to enable, the driver
silently continues with potentially unclocked hardware, which can
cause undefined behavior or system instability on resume from runtime
suspend.

The existing macb_clks_disable() already uses clk_bulk_disable_unprepare()
for the symmetric disable path. Align the enable path by using
clk_bulk_prepare_enable(), which atomically enables all clocks and
automatically rolls back any successfully enabled clocks on failure.

Signed-off-by: Gustavo Kenji Mendonça Kaneko <kaneko.dev@pm.me>
---
 drivers/net/ethernet/cadence/macb_main.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index a12aa21244e8..85bd4ff0e4e6 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -6185,16 +6185,19 @@ static int __maybe_unused macb_runtime_resume(struct device *dev)
 {
 	struct net_device *netdev = dev_get_drvdata(dev);
 	struct macb *bp = netdev_priv(netdev);
+	struct clk_bulk_data clks[] = {
+		{ .clk = bp->pclk },
+		{ .clk = bp->hclk },
+		{ .clk = bp->tx_clk },
+		{ .clk = bp->rx_clk },
+		{ .clk = bp->tsu_clk },
+	};
 
-	if (!(device_may_wakeup(dev))) {
-		clk_prepare_enable(bp->pclk);
-		clk_prepare_enable(bp->hclk);
-		clk_prepare_enable(bp->tx_clk);
-		clk_prepare_enable(bp->rx_clk);
-		clk_prepare_enable(bp->tsu_clk);
-	} else if (!(bp->caps & MACB_CAPS_NEED_TSUCLK)) {
-		clk_prepare_enable(bp->tsu_clk);
-	}
+	if (!(device_may_wakeup(dev)))
+		return clk_bulk_prepare_enable(ARRAY_SIZE(clks), clks);
+
+	if (!(bp->caps & MACB_CAPS_NEED_TSUCLK))
+		return clk_prepare_enable(bp->tsu_clk);
 
 	return 0;
 }
-- 
2.54.0

Hello Gustavo,

Your CC list is laking some net maintainers, how did you get who to send
this email to?

   ./scripts/get_maintainer.pl -f drivers/net/ethernet/cadence/macb_main.c

On Sun May 31, 2026 at 7:18 AM CEST, Gustavo Kenji Mendonça Kaneko wrote:
> macb_runtime_resume() calls clk_prepare_enable() five times without
> checking the return value. If any clock fails to enable, the driver
> silently continues with potentially unclocked hardware, which can
> cause undefined behavior or system instability on resume from runtime
> suspend.

Could you point out if this was figured out to address a bug or if it is
only a theoretical defense?

> The existing macb_clks_disable() already uses clk_bulk_disable_unprepare()
> for the symmetric disable path. Align the enable path by using
> clk_bulk_prepare_enable(), which atomically enables all clocks and
> automatically rolls back any successfully enabled clocks on failure.

"atomically" usually refers to some locking mechanism, is that the
signification you had in mind here? Because I see none.

> Signed-off-by: Gustavo Kenji Mendonça Kaneko <kaneko.dev@pm.me>

Thanks!

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Hi Théo,

Thanks for the review.

> Your CC list is lacking some net maintainers, how did you get who to
> send this email to?

You're right, and I apologize. I did run get_maintainer.pl, but I made a
mistake assembling the --to/--cc lists and dropped the networking
maintainers. The full output is:

  Théo Lebrun <theo.lebrun@bootlin.com>      (macb maintainer)
  Conor Dooley <conor.dooley@microchip.com>  (reviewer)
  Andrew Lunn <andrew+netdev@lunn.ch>        (netdev)
  David S. Miller <davem@davemloft.net>
  Eric Dumazet <edumazet@google.com>
  Jakub Kicinski <kuba@kernel.org>
  Paolo Abeni <pabeni@redhat.com>
  netdev@vger.kernel.org
  linux-kernel@vger.kernel.org

I'll Cc all of them in v2.

> Could you point out if this was figured out to address a bug or if it
> is only a theoretical defense?

It is not tied to an observed failure. I found it by code review and I do
not have macb hardware to test on, so this is a robustness fix rather
than a bug fix: clk_prepare_enable() is __must_check, and here its return
value is silently discarded in the runtime-resume path. If you feel this
isn't worth changing, I'm fine with dropping the patch.

> "atomically" usually refers to some locking mechanism [...]

That was a poor word choice on my part; there is no locking involved. I
meant only that clk_bulk_prepare_enable() enables the clocks as a group
and, on failure, unwinds the ones it already enabled. I've dropped
"atomically" and reworded the commit message in v2.

Thanks,
Gustavo