mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering
@ 2026-11-03 23:17 Ryan Brue
  2026-11-09  9:54 ` Chen-Yu Tsai
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ryan Brue @ 2026-11-03 23:17 UTC (permalink / raw)
  To: Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
	AngeloGioacchino Del Regno, Luca Leonardo Scorcia
  Cc: linux-kernel, linux-arm-kernel, linux-mediatek, Ryan Brue

Since the driver probes as a platform device, the GPT clock event is
registered late in boot, when PSCI CPU idle may already have put CPUs
into a state that stops their local timer. Those CPUs wait in the
oneshot broadcast mask on the hrtimer broadcast, and
tick_install_broadcast_device() arms the new broadcast device to expire
at once so that it wakes them.

mtk_gpt_init() sets the timer's bit in GPT_IRQ_EN only after
clockevents_config_and_register(). The forced event fires while the bit
is clear, nothing is latched in the status register, and the waiting
CPUs are never woken. On an MT8173 tablet with the CPU_SLEEP_0 idle
state in use, this hung the boot in 20 of 40 boots, and in none of 16
with this change.

Enable the interrupt before registering the clock event. The IRQ line
stays disabled until enable_irq(), so the handler still can't run before
the clock event is set up.

Fixes: 7a097aba7255 ("clocksource/drivers/timer-mediatek: Convert to OF probing")
Signed-off-by: Ryan Brue <ryanbrue.dev@gmail.com>
---
Since v7.4-rc1 the MediaTek timer probes as a platform device, and on
MT8173 with PSCI CPU idle the boot then hangs about half the time: the
broadcast device is armed at once to wake CPUs that are already idle,
and that event is lost because the GPT's interrupt is enabled only after
registration. Traced on the board with prints in
tick_install_broadcast_device() and mtk_gpt_init().
---
 drivers/clocksource/timer-mediatek.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-mediatek.c b/drivers/clocksource/timer-mediatek.c
index 842d6ee4259e..1b2cbcc2795a 100644
--- a/drivers/clocksource/timer-mediatek.c
+++ b/drivers/clocksource/timer-mediatek.c
@@ -339,10 +339,10 @@ static int mtk_gpt_init(struct platform_device *pdev)
 
 	/* Configure clock event */
 	mtk_gpt_setup(&to, TIMER_CLK_EVT, GPT_CTRL_OP_REPEAT);
+	mtk_gpt_enable_irq(&to, TIMER_CLK_EVT);
 	clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
 					TIMER_SYNC_TICKS, 0xffffffff);
 
-	mtk_gpt_enable_irq(&to, TIMER_CLK_EVT);
 	enable_irq(timer_of_irq(&to));
 
 	return 0;

---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261103-timer-mediatek-gpt-irq-order-a878ed869f84

Best regards,
-- 
Ryan Brue <ryanbrue.dev@gmail.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering
  2026-11-03 23:17 [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering Ryan Brue
@ 2026-11-09  9:54 ` Chen-Yu Tsai
  2026-11-09 12:35   ` AngeloGioacchino Del Regno
  2026-11-09 13:37 ` Luca Leonardo Scorcia
  2026-11-11  2:41 ` Ryan Brue
  2 siblings, 1 reply; 5+ messages in thread
From: Chen-Yu Tsai @ 2026-11-09  9:54 UTC (permalink / raw)
  To: Ryan Brue
  Cc: Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
	AngeloGioacchino Del Regno, Luca Leonardo Scorcia, linux-kernel,
	linux-arm-kernel, linux-mediatek

On Wed, Nov 4, 2026 at 8:18 AM Ryan Brue <ryanbrue.dev@gmail.com> wrote:
>
> Since the driver probes as a platform device, the GPT clock event is
> registered late in boot, when PSCI CPU idle may already have put CPUs
> into a state that stops their local timer. Those CPUs wait in the
> oneshot broadcast mask on the hrtimer broadcast, and
> tick_install_broadcast_device() arms the new broadcast device to expire
> at once so that it wakes them.
>
> mtk_gpt_init() sets the timer's bit in GPT_IRQ_EN only after
> clockevents_config_and_register(). The forced event fires while the bit
> is clear, nothing is latched in the status register, and the waiting
> CPUs are never woken. On an MT8173 tablet with the CPU_SLEEP_0 idle
> state in use, this hung the boot in 20 of 40 boots, and in none of 16
> with this change.
>
> Enable the interrupt before registering the clock event. The IRQ line
> stays disabled until enable_irq(), so the handler still can't run before
> the clock event is set up.
>
> Fixes: 7a097aba7255 ("clocksource/drivers/timer-mediatek: Convert to OF probing")
> Signed-off-by: Ryan Brue <ryanbrue.dev@gmail.com>

This seems to have fixed the cpuidle related stalls I was seeing. I
didn't do that many rounds of boot tests though.

Tested-by: Chen-Yu Tsai <wenst@chromium.org> # on Hana (Telesu)
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

I do think we should expedite this into the kernel, as a 50% chance
of stalling is not great. It was even higher for me and somehow I
had to do a hard power cycle by rebooting the EC to get it unstuck.

> ---
> Since v7.4-rc1 the MediaTek timer probes as a platform device, and on
> MT8173 with PSCI CPU idle the boot then hangs about half the time: the
> broadcast device is armed at once to wake CPUs that are already idle,
> and that event is lost because the GPT's interrupt is enabled only after
> registration. Traced on the board with prints in
> tick_install_broadcast_device() and mtk_gpt_init().
> ---
>  drivers/clocksource/timer-mediatek.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/timer-mediatek.c b/drivers/clocksource/timer-mediatek.c
> index 842d6ee4259e..1b2cbcc2795a 100644
> --- a/drivers/clocksource/timer-mediatek.c
> +++ b/drivers/clocksource/timer-mediatek.c
> @@ -339,10 +339,10 @@ static int mtk_gpt_init(struct platform_device *pdev)
>
>         /* Configure clock event */
>         mtk_gpt_setup(&to, TIMER_CLK_EVT, GPT_CTRL_OP_REPEAT);
> +       mtk_gpt_enable_irq(&to, TIMER_CLK_EVT);
>         clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
>                                         TIMER_SYNC_TICKS, 0xffffffff);
>
> -       mtk_gpt_enable_irq(&to, TIMER_CLK_EVT);
>         enable_irq(timer_of_irq(&to));
>
>         return 0;
>
> ---
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> change-id: 20261103-timer-mediatek-gpt-irq-order-a878ed869f84
>
> Best regards,
> --
> Ryan Brue <ryanbrue.dev@gmail.com>
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering
  2026-11-09  9:54 ` Chen-Yu Tsai
@ 2026-11-09 12:35   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 5+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-11-09 12:35 UTC (permalink / raw)
  To: Chen-Yu Tsai, Ryan Brue
  Cc: Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
	Luca Leonardo Scorcia, linux-kernel, linux-arm-kernel,
	linux-mediatek

On 11/9/26 10:54, Chen-Yu Tsai wrote:
> On Wed, Nov 4, 2026 at 8:18 AM Ryan Brue <ryanbrue.dev@gmail.com> wrote:
>>
>> Since the driver probes as a platform device, the GPT clock event is
>> registered late in boot, when PSCI CPU idle may already have put CPUs
>> into a state that stops their local timer. Those CPUs wait in the
>> oneshot broadcast mask on the hrtimer broadcast, and
>> tick_install_broadcast_device() arms the new broadcast device to expire
>> at once so that it wakes them.
>>
>> mtk_gpt_init() sets the timer's bit in GPT_IRQ_EN only after
>> clockevents_config_and_register(). The forced event fires while the bit
>> is clear, nothing is latched in the status register, and the waiting
>> CPUs are never woken. On an MT8173 tablet with the CPU_SLEEP_0 idle
>> state in use, this hung the boot in 20 of 40 boots, and in none of 16
>> with this change.
>>
>> Enable the interrupt before registering the clock event. The IRQ line
>> stays disabled until enable_irq(), so the handler still can't run before
>> the clock event is set up.
>>
>> Fixes: 7a097aba7255 ("clocksource/drivers/timer-mediatek: Convert to OF probing")
>> Signed-off-by: Ryan Brue <ryanbrue.dev@gmail.com>
> 
> This seems to have fixed the cpuidle related stalls I was seeing. I
> didn't do that many rounds of boot tests though.
> 
> Tested-by: Chen-Yu Tsai <wenst@chromium.org> # on Hana (Telesu)
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> 
> I do think we should expedite this into the kernel, as a 50% chance
> of stalling is not great. It was even higher for me and somehow I
> had to do a hard power cycle by rebooting the EC to get it unstuck.
> 

Agreed.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

>> ---
>> Since v7.4-rc1 the MediaTek timer probes as a platform device, and on
>> MT8173 with PSCI CPU idle the boot then hangs about half the time: the
>> broadcast device is armed at once to wake CPUs that are already idle,
>> and that event is lost because the GPT's interrupt is enabled only after
>> registration. Traced on the board with prints in
>> tick_install_broadcast_device() and mtk_gpt_init().
>> ---
>>   drivers/clocksource/timer-mediatek.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clocksource/timer-mediatek.c b/drivers/clocksource/timer-mediatek.c
>> index 842d6ee4259e..1b2cbcc2795a 100644
>> --- a/drivers/clocksource/timer-mediatek.c
>> +++ b/drivers/clocksource/timer-mediatek.c
>> @@ -339,10 +339,10 @@ static int mtk_gpt_init(struct platform_device *pdev)
>>
>>          /* Configure clock event */
>>          mtk_gpt_setup(&to, TIMER_CLK_EVT, GPT_CTRL_OP_REPEAT);
>> +       mtk_gpt_enable_irq(&to, TIMER_CLK_EVT);
>>          clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
>>                                          TIMER_SYNC_TICKS, 0xffffffff);
>>
>> -       mtk_gpt_enable_irq(&to, TIMER_CLK_EVT);
>>          enable_irq(timer_of_irq(&to));
>>
>>          return 0;
>>
>> ---
>> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
>> change-id: 20261103-timer-mediatek-gpt-irq-order-a878ed869f84
>>
>> Best regards,
>> --
>> Ryan Brue <ryanbrue.dev@gmail.com>
>>
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering
  2026-11-03 23:17 [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering Ryan Brue
  2026-11-09  9:54 ` Chen-Yu Tsai
@ 2026-11-09 13:37 ` Luca Leonardo Scorcia
  2026-11-11  2:41 ` Ryan Brue
  2 siblings, 0 replies; 5+ messages in thread
From: Luca Leonardo Scorcia @ 2026-11-09 13:37 UTC (permalink / raw)
  To: Ryan Brue
  Cc: Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
	AngeloGioacchino Del Regno, linux-kernel, linux-arm-kernel,
	linux-mediatek

> Fixes: 7a097aba7255 ("clocksource/drivers/timer-mediatek: Convert to OF probing")
> Signed-off-by: Ryan Brue <ryanbrue.dev@gmail.com>

Thank you!

Reviewed-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
Tested-by: Luca Leonardo Scorcia <l.scorcia@gmail.com> # on mt8167
Sony PSC and mt8167 Xiaomi Clock

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering
  2026-11-03 23:17 [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering Ryan Brue
  2026-11-09  9:54 ` Chen-Yu Tsai
  2026-11-09 13:37 ` Luca Leonardo Scorcia
@ 2026-11-11  2:41 ` Ryan Brue
  2 siblings, 0 replies; 5+ messages in thread
From: Ryan Brue @ 2026-11-11  2:41 UTC (permalink / raw)
  To: Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
	AngeloGioacchino Del Regno, Luca Leonardo Scorcia
  Cc: linux-kernel, linux-arm-kernel, linux-mediatek

On 11/3/26 5:17 PM, Ryan Brue wrote:
> [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering
> [...]
>   
>   	/* Configure clock event */
>   	mtk_gpt_setup(&to, TIMER_CLK_EVT, GPT_CTRL_OP_REPEAT);
> +	mtk_gpt_enable_irq(&to, TIMER_CLK_EVT);
>   	clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
>   					TIMER_SYNC_TICKS, 0xffffffff);
>   
> -	mtk_gpt_enable_irq(&to, TIMER_CLK_EVT);
>   	enable_irq(timer_of_irq(&to));
>   
>   	return 0;

Hi Daniel and Thomas,

Could this go in through timers/urgent for a 7.4-rc? The regression come 
in with 7.4-rc1 (7a097aba7255), and on MT8173 boards with PSCI CPU idle 
it hangs about half of all boots: 20 of 40 here, and Chen-Yu sees it on 
Hana as well. It now has Reviewed-by tags from Chen-Yu, Angelo, and 
Luca, and Tested-by tags from Chen-Yu (MT8173) and Luca (MT8167).

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-11  2:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03 23:17 [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering Ryan Brue
2026-11-09  9:54 ` Chen-Yu Tsai
2026-11-09 12:35   ` AngeloGioacchino Del Regno
2026-11-09 13:37 ` Luca Leonardo Scorcia
2026-11-11  2:41 ` Ryan Brue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®