Re: [PATCH] [v4, 1/1] clocksource/drivers/timer-mediatek: optimize systimer irq clear flow on shutdown

[Hsin-Yi Wang <hsinyi@chromium.org>]

On Fri, Apr 9, 2021 at 5:22 PM Fengquan Chen <Fengquan.Chen@mediatek.com> wrote:
>
> mtk_syst_clkevt_shutdown is called after irq disabled in suspend flow,
> clear any pending systimer irq when shutdown to avoid suspend aborted
> due to timer irq pending
>
> Also as for systimer in mediatek socs, there must be firstly enable
> timer before clear systimer irq
>
> Fixes: e3af677607d9("clocksource/drivers/timer-mediatek: Add support for system timer")
> Signed-off-by: Fengquan Chen <fengquan.chen@mediatek.com>
>

Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>

Tested on a mt8183 device. This patch solves an issue that device
would hang on resume during genpd_resume_noirq:

[  724.342834] mtk-smi-larb 17010000.larb: genpd_resume_noirq+0x0/0xd0
returned 0 after 976593 usecs
[  724.351721] mtk-smi-larb 1a001000.larb: calling
genpd_resume_noirq+0x0/0xd0 @ 6629, parent: soc
[  736.845047] Kernel panic - not syncing: Watchdog detected hard
LOCKUP on cpu 3
[  736.852268] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G        W
  5.10.26 #17
[  736.859743] Hardware name: MediaTek krane sku176 board (DT)
[  736.865309] Call trace:
[  736.867756]  dump_backtrace+0x0/0x1bc
[  736.871415]  show_stack+0x20/0x2c
[  736.874728]  dump_stack+0xc0/0x11c
[  736.878124]  panic+0x174/0x378
[  736.881178]  watchdog_check_hardlockup+0x11c/0x124
[  736.885963]  watchdog_timer_fn+0x70/0x274
<snip>



> ---
>  drivers/clocksource/timer-mediatek.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/timer-mediatek.c b/drivers/clocksource/timer-mediatek.c
> index 9318edc..6461fd3 100644
> --- a/drivers/clocksource/timer-mediatek.c
> +++ b/drivers/clocksource/timer-mediatek.c
> @@ -60,9 +60,9 @@
>   * SYST_CON_EN: Clock enable. Shall be set to
>   *   - Start timer countdown.
>   *   - Allow timeout ticks being updated.
> - *   - Allow changing interrupt functions.
> + *   - Allow changing interrupt status,like clear irq pending.
>   *
> - * SYST_CON_IRQ_EN: Set to allow interrupt.
> + * SYST_CON_IRQ_EN: Set to enable interrupt.
>   *
>   * SYST_CON_IRQ_CLR: Set to clear interrupt.
>   */
> @@ -75,6 +75,7 @@
>  static void mtk_syst_ack_irq(struct timer_of *to)
>  {
>         /* Clear and disable interrupt */
> +       writel(SYST_CON_EN, SYST_CON_REG(to));
>         writel(SYST_CON_IRQ_CLR | SYST_CON_EN, SYST_CON_REG(to));
>  }
>
> @@ -111,6 +112,9 @@ static int mtk_syst_clkevt_next_event(unsigned long ticks,
>
>  static int mtk_syst_clkevt_shutdown(struct clock_event_device *clkevt)
>  {
> +       /* Clear any irq */
> +       mtk_syst_ack_irq(to_timer_of(clkevt));
> +
>         /* Disable timer */
>         writel(0, SYST_CON_REG(to_timer_of(clkevt)));
>
> --
> 1.8.1.1.dirty
>