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

[Maíra Canal]

Hi Ulf,

On 18/03/26 10:06, Ulf Hansson wrote:
> On Wed, 18 Mar 2026 at 13:54, Maíra Canal <mcanal at igalia.com> wrote:
>>
>> Hi Stefan,
>>
>> On 18/03/26 08:51, Stefan Wahren wrote:
>>> Hi Maíra,
>>>
>>> Am 17.03.26 um 23:41 schrieb Maíra Canal:
>>>> 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
> 
>>
>> [1]
>> https://lore.kernel.org/dri-devel/20260312-v3d-power-management-v7-0-9f006a1d4c55@igalia.com/T/#mf96146960ec7ffeea32e732c95ccf9548af21748
>>
>> Best regards,
>> - Maíra
>>
>>> Best regards
>>>>
>>>> Cc: stable at 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 at 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;
>>>>    }
>>>>
>>>
>>