Re: [PATCH 1/2] pmdomain: bcm: bcm2835-power: Increase ASB control timeout

Hi Ulf,

On 18/03/26 10:06, Ulf Hansson wrote:

On Wed, 18 Mar 2026 at 13:54, Maíra Canal [off-list ref] wrote:

quoted

Hi Stefan,

On 18/03/26 08:51, Stefan Wahren wrote:

quoted

Hi Maíra,

Am 17.03.26 um 23:41 schrieb Maíra Canal:

quoted

The bcm2835_asb_control() function uses a tight polling loop to wait
for the ASB bridge to acknowledge a request. During intensive workloads,
this handshake intermittently fails for V3D's master ASB on BCM2711,
resulting in "Failed to disable ASB master for v3d" errors during
runtime PM suspend. As a consequence, the failed power-off leaves V3D in
a broken state, leading to bus faults or system hangs on later accesses.

As the timeout is insufficient in some scenarios, increase the polling
timeout from 1us to 5us, which is still negligible in the context of a
power domain transition. Also, replace the open-coded ktime_get_ns()/
cpu_relax() polling loop with readl_poll_timeout_atomic().

personally I would have moved all readl_poll_timeout_atomic changes in
the second patch, to avoid possible conflicts in stable. But no strong
opinion about this.

TBH personally, I also agree. But, as I don't have a strong opinion
about it, I prioritized addressing Ulf's feedback in the last version
[1].

The first version of the patch moved the call to ktime_get_ns(), so I
thought we might as well use readl_poll_timeout_atomic() directly,
instead of fixing up the open-coded loop.

Yeah, it makes sense. I'm okay with both options, so if Stefan agrees
with it, I'm fine in moving forward with this approach.

Best regards,
- Maíra

Kind regards
Uffe

quoted

[1]
https://lore.kernel.org/dri-devel/20260312-v3d-power-management-v7-0-9f006a1d4c55@igalia.com/T/#mf96146960ec7ffeea32e732c95ccf9548af21748 (local)

Best regards,
- Maíra

quoted

Best regards

quoted

Cc: stable@vger.kernel.org
Fixes: 670c672608a1 ("soc: bcm: bcm2835-pm: Add support for power
domains under a new binding.")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
   drivers/pmdomain/bcm/bcm2835-power.c | 12 ++++--------
   1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/pmdomain/bcm/bcm2835-power.c b/drivers/pmdomain/

bcm/bcm2835-power.c
index
0450202bbee2513c9116a36abaa839b460550935..eee87a3005325848547ce1f5fd729b168a641460 100644

--- a/drivers/pmdomain/bcm/bcm2835-power.c
+++ b/drivers/pmdomain/bcm/bcm2835-power.c

@@ -9,6 +9,7 @@
   #include <linux/clk.h>
   #include <linux/delay.h>
   #include <linux/io.h>
+#include <linux/iopoll.h>
   #include <linux/mfd/bcm2835-pm.h>
   #include <linux/module.h>
   #include <linux/platform_device.h>

@@ -153,7 +154,6 @@ struct bcm2835_power {
   static int bcm2835_asb_control(struct bcm2835_power *power, u32 reg,

bool enable)
   {
       void __iomem *base = power->asb;
-    u64 start;
       u32 val;
       switch (reg) {

@@ -166,8 +166,6 @@ static int bcm2835_asb_control(struct

bcm2835_power *power, u32 reg, bool enable
           break;
       }
-    start = ktime_get_ns();
-
       /* Enable the module's async AXI bridges. */
       if (enable) {
           val = readl(base + reg) & ~ASB_REQ_STOP;

@@ -176,11 +174,9 @@ static int bcm2835_asb_control(struct

bcm2835_power *power, u32 reg, bool enable
       }
       writel(PM_PASSWORD | val, base + reg);
-    while (!!(readl(base + reg) & ASB_ACK) == enable) {
-        cpu_relax();
-        if (ktime_get_ns() - start >= 1000)
-            return -ETIMEDOUT;
-    }
+    if (readl_poll_timeout_atomic(base + reg, val,
+                      !!(val & ASB_ACK) != enable, 0, 5))
+        return -ETIMEDOUT;
       return 0;
   }