| Message ID | 20190210225114.20110-1-stuart.menefy@mathembedded.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down | expand |
Hi Stuart On 2019-02-10 23:51, Stuart Menefy wrote: > When debugging suspend problems on Exynos 5260, I had a large number > of debugging prints going to the serial port after interrupts > had been disabled but before the timer interrupt was shutdown. This > was long enough for a timer tick to occur, but as interrupts were > disabled the ISR didn't run, and so the interrupt wasn't cleared. > Later when the timer was shutdown the interrupt was left asserted and > so the wfi at the heart of the suspend code didn't wait, causing > the suspend to fail. > > Currently the code which stops the timer when it is on one-shot mode > and the interrupt occurs is in exynos4_mct_tick_clear(), meaning if we > called this from the shutdown code exynos4_mct_tick_stop() could be > called twice. So first restructure the existing code, so the check for > one-shot mode and stopping the timer is moved to the ISR, leaving > exynos4_mct_tick_clear() just clearing the interrupt flag. > > Once this has been done simply call exynos4_mct_tick_clear() from > set_state_shutdown(). This also fixes mysterious suspend failures on Odroid XU3/XU4/HC1 :) Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > Stuart Menefy (2): > clocksource: exynos_mct: Move one-shot check from tick clear to ISR > clocksource: exynos_mct: Clear timer interrupt when shutdown > > drivers/clocksource/exynos_mct.c | 23 ++++++++++++----------- > 1 file changed, 12 insertions(+), 11 deletions(-) > Best regards
On Sun, 10 Feb 2019 at 23:51, Stuart Menefy <stuart.menefy@mathembedded.com> wrote: > > When debugging suspend problems on Exynos 5260, I had a large number > of debugging prints going to the serial port after interrupts > had been disabled but before the timer interrupt was shutdown. This > was long enough for a timer tick to occur, but as interrupts were > disabled the ISR didn't run, and so the interrupt wasn't cleared. > Later when the timer was shutdown the interrupt was left asserted and > so the wfi at the heart of the suspend code didn't wait, causing > the suspend to fail. > > Currently the code which stops the timer when it is on one-shot mode > and the interrupt occurs is in exynos4_mct_tick_clear(), meaning if we > called this from the shutdown code exynos4_mct_tick_stop() could be > called twice. So first restructure the existing code, so the check for > one-shot mode and stopping the timer is moved to the ISR, leaving > exynos4_mct_tick_clear() just clearing the interrupt flag. > > Once this has been done simply call exynos4_mct_tick_clear() from > set_state_shutdown(). For sending the corrected version. This is second revision of the patchset so please remember to add v2 to the title and changelog in cover letter. v2 can be easily added with format-patch (-v2). Best regards, Krzysztof
On 11/02/2019 08:14, Marek Szyprowski wrote: > Hi Stuart > > On 2019-02-10 23:51, Stuart Menefy wrote: >> When debugging suspend problems on Exynos 5260, I had a large number >> of debugging prints going to the serial port after interrupts >> had been disabled but before the timer interrupt was shutdown. This >> was long enough for a timer tick to occur, but as interrupts were >> disabled the ISR didn't run, and so the interrupt wasn't cleared. >> Later when the timer was shutdown the interrupt was left asserted and >> so the wfi at the heart of the suspend code didn't wait, causing >> the suspend to fail. >> >> Currently the code which stops the timer when it is on one-shot mode >> and the interrupt occurs is in exynos4_mct_tick_clear(), meaning if we >> called this from the shutdown code exynos4_mct_tick_stop() could be >> called twice. So first restructure the existing code, so the check for >> one-shot mode and stopping the timer is moved to the ISR, leaving >> exynos4_mct_tick_clear() just clearing the interrupt flag. >> >> Once this has been done simply call exynos4_mct_tick_clear() from >> set_state_shutdown(). > > This also fixes mysterious suspend failures on Odroid XU3/XU4/HC1 :) > > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> Applied. Shall I add the stable@ tag?
On Mon, 11 Feb 2019 at 12:23, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 11/02/2019 08:14, Marek Szyprowski wrote: > > Hi Stuart > > > > On 2019-02-10 23:51, Stuart Menefy wrote: > >> When debugging suspend problems on Exynos 5260, I had a large number > >> of debugging prints going to the serial port after interrupts > >> had been disabled but before the timer interrupt was shutdown. This > >> was long enough for a timer tick to occur, but as interrupts were > >> disabled the ISR didn't run, and so the interrupt wasn't cleared. > >> Later when the timer was shutdown the interrupt was left asserted and > >> so the wfi at the heart of the suspend code didn't wait, causing > >> the suspend to fail. > >> > >> Currently the code which stops the timer when it is on one-shot mode > >> and the interrupt occurs is in exynos4_mct_tick_clear(), meaning if we > >> called this from the shutdown code exynos4_mct_tick_stop() could be > >> called twice. So first restructure the existing code, so the check for > >> one-shot mode and stopping the timer is moved to the ISR, leaving > >> exynos4_mct_tick_clear() just clearing the interrupt flag. > >> > >> Once this has been done simply call exynos4_mct_tick_clear() from > >> set_state_shutdown(). > > > > This also fixes mysterious suspend failures on Odroid XU3/XU4/HC1 :) > > > > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > > Applied. Shall I add the stable@ tag? Makes sense - for the second patch, although first is a prerequisite. It should be backported up to v4.3 (the code differ a lot on older versions). Best regards, Krzysztof