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

[Stefan Wahren]

Am 18.03.26 um 14:21 schrieb 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.
As I said, I've no strong opinion about this. So please go ahead.
>
> 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;
>>>>>    }
>>>>>
>>>>
>>>
>