Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?

[Drew Fustini <dfustini@baylibre.com>]

On Tue, May 10, 2022 at 09:19:51AM +0100, Marc Zyngier wrote:
> On 2022-05-10 07:49, Tony Lindgren wrote:
> > * Drew Fustini <dfustini@baylibre.com> [220509 05:07]:
> > > Hello Daniel, Tony suggested I mail you along with the list to get
> > > feedback. I'm attempting to upstream these two patches [1][2] from
> > > ti-linux-5.4.y for arch/arm/mach-omap2/timer.c:
> > > 96f4c6e2ba8a ("ARM: OMAP2+: timer: Ack pending interrupt during
> > > suspend")
> > > 7ae7dd5f8272 ("ARM: OMAP2+: timer: Extend pending interrupt ACK for
> > > gic")
> > > 
> > > On the TI AM335x and AM437x SoCs, it is possible for a late
> > > interrupt to
> > > be generated which will cause a suspend failure. The first patch makes
> > > omap_clkevt_idle() ack the irq both in the timer peripheral register
> > > and in the interrupt controller to avoid the issue.
> > > 
> > > On AM437x only, the GIC cannot be directly acked using only the
> > > irqchip
> > > calls. To workaround that, the second patch maps the GIC_CPU_BASE and
> > > reads the GIC_CPU_INTACK register before calling irq_eoi to properly
> > > ack
> > > the late timer interrupts that show up during suspend.
> > > 
> > > However, Tony removed most of arch/arm/mach-omap2/timer.c with:
> > > 2ee04b88547a ("ARM: OMAP2+: Drop old timer code for dmtimer and 32k
> > > counter")
> > > 
> > > The timers are now implemented in drivers/clocksource/timer-ti-dm.c
> > > and
> > > drivers/clocksource/timer-ti-dm-systimer.c. The function
> > > dmtimer_clocksource_suspend() disables the dmtimer and clock but does
> > > not ack any interrupts.
> > > 
> > > Tony suggested the right place to ack the interrupt during suspend is
> > > in CPU_CLUSTER_PM_ENTER inside omap_timer_context_notifier().
> > > 
> > > Do you think that would be an acceptable approach?
> > 
> > Based on what we chatted on irc yesterday, I'd suggest try resetting the
> > clockevent on suspend first for am3/4 at omap_clockevent_idle() and see
> > if
> > that takes care of the issue. If it's the timer hardware blocking the
> > deeper idle states, this should work, and GIC will lose it's context
> > on system suspend anyways.
> 
> Maybe, but the core tracking code will still know it is in the
> middle of an interrupt. I´d expect things like lockdep to shout
> at you...
> 
>         M.

Tony and Marc - thank you for your insights.

I have learned from Dave Gerlach that the occurance of suspend failures
on the downstream TI 5.4 kernel was only 1 in 10,000 prior to his
downstream patches.

I have now done 33,175 cycles of suspend/resume with rtcwake and no
failures have occurred. Thus, I have to conclude that mainline does not
exhibit the problem of late timer interrupts causing suspend to fail.

Thanks,
Drew