On 25/03/2021 02:35, Evan Benn wrote:
> set_state_shutdown is called during system suspend after interrupts have
> been disabled. If the timer has fired in the meantime, there will be
> a pending IRQ. So we ack that now and disable the timer. Without this
> ARM trusted firmware will abort the suspend due to the pending
> interrupt.
>
> Now always disable the IRQ in state transitions, and re-enable in
> set_periodic and next_event.
Why not put add the suspend/resume callbacks and put there the specific
code and let the irq untouched in the normal flow ?
> Signed-off-by: Evan Benn <evanb...@chromium.org>
> ---
>
> Changes in v2:
> Remove the patch that splits the drivers into 2 files.
>
> drivers/clocksource/timer-mediatek.c | 49 +++++++++++++++++-----------
> 1 file changed, 30 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/clocksource/timer-mediatek.c
> b/drivers/clocksource/timer-mediatek.c
> index 9318edcd8963..fba2f9494d90 100644
> --- a/drivers/clocksource/timer-mediatek.c
> +++ b/drivers/clocksource/timer-mediatek.c
> @@ -132,13 +132,33 @@ static u64 notrace mtk_gpt_read_sched_clock(void)
> return readl_relaxed(gpt_sched_reg);
> }
>
> +static void mtk_gpt_disable_ack_interrupts(struct timer_of *to, u8 timer)
> +{
> + u32 val;
> +
> + /* Disable interrupts */
> + val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
> + writel(val & ~GPT_IRQ_ENABLE(timer), timer_of_base(to) +
> + GPT_IRQ_EN_REG);
> +
> + /* Ack interrupts */
> + writel(GPT_IRQ_ACK(timer), timer_of_base(to) + GPT_IRQ_ACK_REG);
> +}
> +
> static void mtk_gpt_clkevt_time_stop(struct timer_of *to, u8 timer)
> {
> u32 val;
>
> + /* Disable timer */
> val = readl(timer_of_base(to) + GPT_CTRL_REG(timer));
> writel(val & ~GPT_CTRL_ENABLE, timer_of_base(to) +
> GPT_CTRL_REG(timer));
> +
> + /* This may be called with interrupts disabled,
> + * so we need to ack any interrupt that is pending
> + * Or for example ATF will prevent a suspend from completing.
> + */
> + mtk_gpt_disable_ack_interrupts(to, timer);
> }
>
> static void mtk_gpt_clkevt_time_setup(struct timer_of *to,
> @@ -152,8 +172,10 @@ static void mtk_gpt_clkevt_time_start(struct timer_of
> *to,
> {
> u32 val;
>
> - /* Acknowledge interrupt */
> - writel(GPT_IRQ_ACK(timer), timer_of_base(to) + GPT_IRQ_ACK_REG);
> + /* Enable interrupts */
> + val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
> + writel(val | GPT_IRQ_ENABLE(timer),
> + timer_of_base(to) + GPT_IRQ_EN_REG);
>
> val = readl(timer_of_base(to) + GPT_CTRL_REG(timer));
>
> @@ -226,21 +248,6 @@ __init mtk_gpt_setup(struct timer_of *to, u8 timer, u8
> option)
> timer_of_base(to) + GPT_CTRL_REG(timer));
> }
>
> -static void mtk_gpt_enable_irq(struct timer_of *to, u8 timer)
> -{
> - u32 val;
> -
> - /* Disable all interrupts */
> - writel(0x0, timer_of_base(to) + GPT_IRQ_EN_REG);
> -
> - /* Acknowledge all spurious pending interrupts */
> - writel(0x3f, timer_of_base(to) + GPT_IRQ_ACK_REG);
> -
> - val = readl(timer_of_base(to) + GPT_IRQ_EN_REG);
> - writel(val | GPT_IRQ_ENABLE(timer),
> - timer_of_base(to) + GPT_IRQ_EN_REG);
> -}
> -
> static struct timer_of to = {
> .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
>
> @@ -292,6 +299,12 @@ static int __init mtk_gpt_init(struct device_node *node)
> if (ret)
> return ret;
>
> + /* In case the firmware left the interrupts enabled
> + * disable and ack those now
> + */
> + mtk_gpt_disable_ack_interrupts(&to, TIMER_CLK_SRC);
> + mtk_gpt_disable_ack_interrupts(&to, TIMER_CLK_EVT);
> +
> /* Configure clock source */
> mtk_gpt_setup(&to, TIMER_CLK_SRC, GPT_CTRL_OP_FREERUN);
> clocksource_mmio_init(timer_of_base(&to) + GPT_CNT_REG(TIMER_CLK_SRC),
> @@ -305,8 +318,6 @@ static int __init mtk_gpt_init(struct device_node *node)
> clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
> TIMER_SYNC_TICKS, 0xffffffff);
>
> - mtk_gpt_enable_irq(&to, TIMER_CLK_EVT);
> -
> return 0;
> }
> TIMER_OF_DECLARE(mtk_mt6577, "mediatek,mt6577-timer", mtk_gpt_init);
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
