| Message ID | 20190128230700.7325-2-stuart.menefy@mathembedded.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Resuscitate Exynos 5260 support | expand |
On Tue, 29 Jan 2019 at 00:07, Stuart Menefy <stuart.menefy@mathembedded.com> wrote: > > Ensure that after we have stopped the timer any pending interrupts > are cleared. This fixes a problem when suspending, as interrupts are > disabled before the timer is stopped, so the timer interrupt may still > be asserted, preventing the system entering a low power state when the > wfi is executed. > > Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com> > --- > drivers/clocksource/exynos_mct.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c > index 7a244b681876..c99d0b6e8b5f 100644 > --- a/drivers/clocksource/exynos_mct.c > +++ b/drivers/clocksource/exynos_mct.c > @@ -398,12 +398,15 @@ static int exynos4_tick_set_next_event(unsigned long cycles, > return 0; > } > > +static void exynos4_mct_tick_clear(struct mct_clock_event_device *mevt); > + > static int set_state_shutdown(struct clock_event_device *evt) > { > struct mct_clock_event_device *mevt; > > mevt = container_of(evt, struct mct_clock_event_device, evt); > exynos4_mct_tick_stop(mevt); > + exynos4_mct_tick_clear(mevt); exynos4_mct_tick_clear() calls stop for periodic clockevents so this could be duplicated stop. The exynos4_mct_tick_clear() should then only clear, not stop+clear. In general, are you sure that this is needed? After stopping the timer, still if any interrupt fires the request handler should be called and be processed. Best regards, Krzysztof > return 0; > } > > -- > 2.13.6 >
On Thu, 31 Jan 2019 at 13:02, Stuart Menefy <stuart.menefy@mathembedded.com> wrote: > Good point, I'll move the call to exynos4_mct_tick_stop() into > exynos4_mct_tick_isr(), leaving exynos4_mct_tick_clear() just > doing what its name suggests it should do. > > > In general, are you sure that this is needed? After stopping the > > timer, still if any interrupt fires the request handler should be > > called and be processed. > > The timer is only shutdown after interrupts have been disabled. > Just to double check I made the following change: > > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c > index 0bd595a0b610..a37a3b25da14 100644 > --- a/kernel/power/suspend.c > +++ b/kernel/power/suspend.c > @@ -434,6 +434,7 @@ static int suspend_enter(suspend_state_t state, > bool *wakeup) > > arch_suspend_disable_irqs(); > BUG_ON(!irqs_disabled()); > + mdelay(1000); > > system_state = SYSTEM_SUSPEND; > > which guarantees that a tick occurs after interrupts have been > disabled, and the system > no longer suspends. Obviously in normal operations this is extremely > unlikely, but it is > possible. However while debugging I had a huge number of printks in > place which caused > a long delay before the suspend, and which initially highlighted the problem. Indeed, make sense. Please go with v2. Nice work in general! Best regards, Krzysztof
diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c index 7a244b681876..c99d0b6e8b5f 100644 --- a/drivers/clocksource/exynos_mct.c +++ b/drivers/clocksource/exynos_mct.c @@ -398,12 +398,15 @@ static int exynos4_tick_set_next_event(unsigned long cycles, return 0; } +static void exynos4_mct_tick_clear(struct mct_clock_event_device *mevt); + static int set_state_shutdown(struct clock_event_device *evt) { struct mct_clock_event_device *mevt; mevt = container_of(evt, struct mct_clock_event_device, evt); exynos4_mct_tick_stop(mevt); + exynos4_mct_tick_clear(mevt); return 0; }
Ensure that after we have stopped the timer any pending interrupts are cleared. This fixes a problem when suspending, as interrupts are disabled before the timer is stopped, so the timer interrupt may still be asserted, preventing the system entering a low power state when the wfi is executed. Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com> --- drivers/clocksource/exynos_mct.c | 3 +++ 1 file changed, 3 insertions(+)