* [PATCH] pmdomain: bcm: bcm2835-power: Raise ASB poll timeout to 100us
@ 2026-05-30 20:46 Maíra Canal
2026-05-31 10:41 ` Stefan Wahren
0 siblings, 1 reply; 3+ messages in thread
From: Maíra Canal @ 2026-05-30 20:46 UTC (permalink / raw)
To: Ulf Hansson, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Stefan Wahren
Cc: linux-pm, linux-rpi-kernel, linux-arm-kernel, kernel-dev,
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.
Signed-off-by: Maíra Canal <mcanal@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;
--
2.54.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] pmdomain: bcm: bcm2835-power: Raise ASB poll timeout to 100us 2026-05-30 20:46 [PATCH] pmdomain: bcm: bcm2835-power: Raise ASB poll timeout to 100us Maíra Canal @ 2026-05-31 10:41 ` Stefan Wahren 2026-05-31 12:18 ` Maíra Canal 0 siblings, 1 reply; 3+ messages in thread From: Stefan Wahren @ 2026-05-31 10:41 UTC (permalink / raw) To: Maíra Canal, Ulf Hansson, Florian Fainelli, Broadcom internal kernel review list, Ray Jui, Scott Branden Cc: linux-pm, linux-rpi-kernel, linux-arm-kernel, kernel-dev 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). 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? Best regards > > Signed-off-by: Maíra Canal <mcanal@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; ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] pmdomain: bcm: bcm2835-power: Raise ASB poll timeout to 100us 2026-05-31 10:41 ` Stefan Wahren @ 2026-05-31 12:18 ` Maíra Canal 0 siblings, 0 replies; 3+ messages in thread From: Maíra Canal @ 2026-05-31 12:18 UTC (permalink / raw) To: Stefan Wahren, Ulf Hansson, Florian Fainelli, Broadcom internal kernel review list, Ray Jui, Scott Branden Cc: linux-pm, linux-rpi-kernel, linux-arm-kernel, kernel-dev 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? [1] https://lore.kernel.org/dri-devel/20260530-v3d-fix-rpi4-freezes-v1-0-c2c8307da6ce@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@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; > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-31 12:18 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-30 20:46 [PATCH] pmdomain: bcm: bcm2835-power: Raise ASB poll timeout to 100us Maíra Canal 2026-05-31 10:41 ` Stefan Wahren 2026-05-31 12:18 ` Maíra Canal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox