Re: [PATCH] clocksource/drivers/timer-mediatek: Enable the GPT irq before registering

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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>
>>
>>