All of lore.kernel.org
 help / color / mirror / Atom feed
* dmtimer: ack pending interrupt during suspend on am335x/am437x?
@ 2022-05-09  5:12 ` Drew Fustini
  0 siblings, 0 replies; 12+ messages in thread
From: Drew Fustini @ 2022-05-09  5:12 UTC (permalink / raw)
  To: Tony Lindgren, Daniel Lezcano
  Cc: Dave Gerlach, linux-omap, linux-arm-kernel, linux-kernel

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?

Thank you,
Drew

[1] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-linux-5.4.y&id=96f4c6e2ba8a
[2] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-linux-5.4.y&id=7ae7dd5f8272

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* dmtimer: ack pending interrupt during suspend on am335x/am437x?
@ 2022-05-09  5:12 ` Drew Fustini
  0 siblings, 0 replies; 12+ messages in thread
From: Drew Fustini @ 2022-05-09  5:12 UTC (permalink / raw)
  To: Tony Lindgren, Daniel Lezcano
  Cc: Dave Gerlach, linux-omap, linux-arm-kernel, linux-kernel

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?

Thank you,
Drew

[1] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-linux-5.4.y&id=96f4c6e2ba8a
[2] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-linux-5.4.y&id=7ae7dd5f8272

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

* Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?
  2022-05-09  5:12 ` Drew Fustini
@ 2022-05-10  6:49   ` Tony Lindgren
  -1 siblings, 0 replies; 12+ messages in thread
From: Tony Lindgren @ 2022-05-10  6:49 UTC (permalink / raw)
  To: Drew Fustini
  Cc: Daniel Lezcano, Dave Gerlach, linux-omap, linux-arm-kernel, linux-kernel

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

If there's still a pending interrupt status blocking deeper idle
states for system suspend after clockevent reset, then maybe the related
interrupt controller(s) should clear/reset the state for context lossy
suspend for CPU_CLUSTER_PM_ENTER for system suspend.

Looks like for GIC we have gic_dist_save() being called but it does not
seem to clear anything for system suspend.

For runtime PM, we don't want to clear any pending interrupts on
CPU_CLUSTER_PM_ENTER :)

Seems like a system suspend/resume test loop should be first run to
trigger the issue and see what is still missing.

Regards,

Tony


> [1] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-linux-5.4.y&id=96f4c6e2ba8a
> [2] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-linux-5.4.y&id=7ae7dd5f8272

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

* Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?
@ 2022-05-10  6:49   ` Tony Lindgren
  0 siblings, 0 replies; 12+ messages in thread
From: Tony Lindgren @ 2022-05-10  6:49 UTC (permalink / raw)
  To: Drew Fustini
  Cc: Daniel Lezcano, Dave Gerlach, linux-omap, linux-arm-kernel, linux-kernel

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

If there's still a pending interrupt status blocking deeper idle
states for system suspend after clockevent reset, then maybe the related
interrupt controller(s) should clear/reset the state for context lossy
suspend for CPU_CLUSTER_PM_ENTER for system suspend.

Looks like for GIC we have gic_dist_save() being called but it does not
seem to clear anything for system suspend.

For runtime PM, we don't want to clear any pending interrupts on
CPU_CLUSTER_PM_ENTER :)

Seems like a system suspend/resume test loop should be first run to
trigger the issue and see what is still missing.

Regards,

Tony


> [1] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-linux-5.4.y&id=96f4c6e2ba8a
> [2] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-linux-5.4.y&id=7ae7dd5f8272

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?
  2022-05-09  5:12 ` Drew Fustini
@ 2022-05-10  8:16   ` Marc Zyngier
  -1 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2022-05-10  8:16 UTC (permalink / raw)
  To: Drew Fustini
  Cc: Tony Lindgren, Daniel Lezcano, Dave Gerlach, linux-omap,
	linux-arm-kernel, linux-kernel

On 2022-05-09 06:12, Drew Fustini wrote:
> 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.

This isn´t an Ack. The Ack happens when you read the IAR register
(Interrupt Acknowledgement Register). Writing to EOI performs at least
a priority drop, and maybe a deactivation.

Simply writing to EOI doesn´t necessarily solve any problem if the
GIC is using EOIMode==1, because you´ĺl miss the deactivation.

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

The real issue is that you are apparently suspending from within an
interrupt handler. This is what should be addressed.

Please don´t randomly call into the irqchip code. It will eventually
break, and sooner rather than later.

         M.
-- 
Jazz is not dead. It just smells funny...

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

* Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?
@ 2022-05-10  8:16   ` Marc Zyngier
  0 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2022-05-10  8:16 UTC (permalink / raw)
  To: Drew Fustini
  Cc: Tony Lindgren, Daniel Lezcano, Dave Gerlach, linux-omap,
	linux-arm-kernel, linux-kernel

On 2022-05-09 06:12, Drew Fustini wrote:
> 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.

This isn´t an Ack. The Ack happens when you read the IAR register
(Interrupt Acknowledgement Register). Writing to EOI performs at least
a priority drop, and maybe a deactivation.

Simply writing to EOI doesn´t necessarily solve any problem if the
GIC is using EOIMode==1, because you´ĺl miss the deactivation.

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

The real issue is that you are apparently suspending from within an
interrupt handler. This is what should be addressed.

Please don´t randomly call into the irqchip code. It will eventually
break, and sooner rather than later.

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?
  2022-05-10  6:49   ` Tony Lindgren
@ 2022-05-10  8:19     ` Marc Zyngier
  -1 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2022-05-10  8:19 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Drew Fustini, Daniel Lezcano, Dave Gerlach, linux-omap,
	linux-arm-kernel, linux-kernel

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.

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

* Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?
@ 2022-05-10  8:19     ` Marc Zyngier
  0 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2022-05-10  8:19 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Drew Fustini, Daniel Lezcano, Dave Gerlach, linux-omap,
	linux-arm-kernel, linux-kernel

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?
  2022-05-10  8:19     ` Marc Zyngier
@ 2022-05-16  3:58       ` Drew Fustini
  -1 siblings, 0 replies; 12+ messages in thread
From: Drew Fustini @ 2022-05-16  3:58 UTC (permalink / raw)
  To: Marc Zyngier, Tony Lindgren
  Cc: Daniel Lezcano, Dave Gerlach, linux-omap, linux-arm-kernel, linux-kernel

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


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

* Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?
@ 2022-05-16  3:58       ` Drew Fustini
  0 siblings, 0 replies; 12+ messages in thread
From: Drew Fustini @ 2022-05-16  3:58 UTC (permalink / raw)
  To: Marc Zyngier, Tony Lindgren
  Cc: Daniel Lezcano, Dave Gerlach, linux-omap, linux-arm-kernel, linux-kernel

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?
  2022-05-16  3:58       ` Drew Fustini
@ 2022-05-16 16:00         ` Tony Lindgren
  -1 siblings, 0 replies; 12+ messages in thread
From: Tony Lindgren @ 2022-05-16 16:00 UTC (permalink / raw)
  To: Drew Fustini
  Cc: Marc Zyngier, Daniel Lezcano, Dave Gerlach, linux-omap,
	linux-arm-kernel, linux-kernel

* Drew Fustini <dfustini@baylibre.com> [220516 03:54]:
> 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.

OK good to hear, fingers crossed that the issue no longer exists :)

Regards,

Tony


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

* Re: dmtimer: ack pending interrupt during suspend on am335x/am437x?
@ 2022-05-16 16:00         ` Tony Lindgren
  0 siblings, 0 replies; 12+ messages in thread
From: Tony Lindgren @ 2022-05-16 16:00 UTC (permalink / raw)
  To: Drew Fustini
  Cc: Marc Zyngier, Daniel Lezcano, Dave Gerlach, linux-omap,
	linux-arm-kernel, linux-kernel

* Drew Fustini <dfustini@baylibre.com> [220516 03:54]:
> 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.

OK good to hear, fingers crossed that the issue no longer exists :)

Regards,

Tony


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2022-05-16 16:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-09  5:12 dmtimer: ack pending interrupt during suspend on am335x/am437x? Drew Fustini
2022-05-09  5:12 ` Drew Fustini
2022-05-10  6:49 ` Tony Lindgren
2022-05-10  6:49   ` Tony Lindgren
2022-05-10  8:19   ` Marc Zyngier
2022-05-10  8:19     ` Marc Zyngier
2022-05-16  3:58     ` Drew Fustini
2022-05-16  3:58       ` Drew Fustini
2022-05-16 16:00       ` Tony Lindgren
2022-05-16 16:00         ` Tony Lindgren
2022-05-10  8:16 ` Marc Zyngier
2022-05-10  8:16   ` Marc Zyngier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.