[PATCH] pmdomain: bcm: bcm2835-power: Raise ASB poll timeout to 100us

[Maíra Canal]

Hi,

On 31/05/26 09:18, Maíra Canal wrote:
> Hi Stefan,
> 
> On 31/05/26 07:41, Stefan Wahren wrote:
>> Hi Maíra,
>>
>> Am 30.05.26 um 22:46 schrieb Maíra Canal:
>>> Commit 18605b1b936b ("pmdomain: bcm: bcm2835-power: Increase ASB control
>>> timeout") raised the ASB handshake polling budget from 1us to 5us.
>>> Surveying the pmdomain subsystem, 5us is still one of the smallest 
>>> polling
>>> budgets by a wide margin. Comparable handshakes in other drivers use:
>>>
>>>    - 100us : starfive jh71xx-pmu, apple pmgr-pwrstate
>>>    - 1ms   : renesas rcar-sysc, rmobile-sysc (power-on)
>>>    - 10ms  : renesas rcar-gen4-sysc, sunxi sun55i-pck600
>>>    - 1s    : mediatek mtk-pm-domains, mtk-scpsys
>>>
>>> Raise the BCM2835 timeout to 100us, matching analogous drivers. 100us is
>>> still negligible relative to a power-domain transition and gives the V3D
>>> master ASB substantially more headroom to drain under heavy workloads,
>>> assuring us that the timeout is enough for any scenario.
>> tbh I'm not convinced by this explanation. Starting with a timeout 
>> comparison across different pmdomain driver looks strange to me.
>>
>> My expectation that the reason for such a patch is that there is some 
>> kind of scenario to trigger unexpected timeouts.
>> If this is the case, please provide more information about the 
>> scenario (specific platform, scenario, link to the bug report).
> 
> The context: I was debugging this issue [1] and initially I had the
> intuition that it could be related to a timeout in the ASB polling loop.
> As I don't have access to the BCM2835 SoC datasheet (the public one
> doesn't have PM information), I started to check other driver's
> handshake timeout to see if ours was comparable to similar drivers. As
> you can see, our timeout is much smaller.
> 
> In the end, issue [1] wasn't related to the pmdomain driver, so I don't
> have a specific scenario to trigger this issue. Even though, I sent this
> patch considering the comparison to other drivers with the goal to be
> conservative and use a larger timeout that could accommodate an extreme
> scenario. However, if you believe it's unreasonable, I'm okay dropping
> this patch.
> 
> Ideally, it would be great to have information from Broadcom on what's
> the largest possible time required to perform a power transition. Maybe
> Florian could help us with that?
> 

Gentle ping, any ideas about this?

Best regards,
- Maíra

> [1] https://lore.kernel.org/dri-devel/20260530-v3d-fix-rpi4-freezes- 
> v1-0-c2c8307da6ce at igalia.com/T/
> 
>>
>> I'm not against the change in general, but please start your commit 
>> message with the problem and not with the solution.
>>
>> A related question, does it make sense to add an error message here 
>> for the timeout case just like in the rest of the driver?
> 
> Before commit 18605b1b936b, I was getting a "Failed to disable ASB
> master for v3d" error message, so I believe the error message already
> exists where the function is called.
> 
> Thanks for your feedback!
> 
> Best regards,
> - Maíra
> 
>>
>> Best regards
>>
>>>
>>> Signed-off-by: Maíra Canal <mcanal at igalia.com>
>>> ---
>>>   drivers/pmdomain/bcm/bcm2835-power.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/pmdomain/bcm/bcm2835-power.c b/drivers/pmdomain/ 
>>> bcm/bcm2835-power.c
>>> index b76d74e3849b..d507bb675e29 100644
>>> --- a/drivers/pmdomain/bcm/bcm2835-power.c
>>> +++ b/drivers/pmdomain/bcm/bcm2835-power.c
>>> @@ -175,7 +175,7 @@ static int bcm2835_asb_control(struct 
>>> bcm2835_power *power, u32 reg, bool enable
>>>       writel(PM_PASSWORD | val, base + reg);
>>>       if (readl_poll_timeout_atomic(base + reg, val,
>>> -                      !!(val & ASB_ACK) != enable, 0, 5))
>>> +                      !!(val & ASB_ACK) != enable, 0, 100))
>>>           return -ETIMEDOUT;
>>>       return 0;
>>
>