Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down
@ 2019-02-10 22:51 ` Stuart Menefy
  2019-02-10 22:51   ` [PATCH 1/2] clocksource: exynos_mct: Move one-shot check from tick clear to ISR Stuart Menefy
                     ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Stuart Menefy @ 2019-02-10 22:51 UTC (permalink / raw)
  To: linux-samsung-soc, Daniel Lezcano, Thomas Gleixner, Kukjin Kim,
	Krzysztof Kozlowski, linux-kernel, linux-arm-kernel

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().

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(-)

-- 
2.13.6


_______________________________________________
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] 9+ messages in thread

* [PATCH 1/2] clocksource: exynos_mct: Move one-shot check from tick clear to ISR
  2019-02-10 22:51 ` [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down Stuart Menefy
@ 2019-02-10 22:51   ` Stuart Menefy
  2019-02-11  8:47     ` Krzysztof Kozlowski
  2019-02-10 22:51   ` [PATCH 2/2] clocksource: exynos_mct: Clear timer interrupt when shutdown Stuart Menefy
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Stuart Menefy @ 2019-02-10 22:51 UTC (permalink / raw)
  To: linux-samsung-soc, Daniel Lezcano, Thomas Gleixner, Kukjin Kim,
	Krzysztof Kozlowski, linux-kernel, linux-arm-kernel
  Cc: Stuart Menefy

When a timer tick occurs and the clock is in one-shot mode, the timer
needs to be stopped to prevent it triggering subsequent interrupts.
Currently this code is in exynos4_mct_tick_clear(), but as it is
only needed when an ISR occurs move it into exynos4_mct_tick_isr(),
leaving exynos4_mct_tick_clear() just doing what its name suggests it
should.

Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com>
---
 drivers/clocksource/exynos_mct.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 7a244b681876..1e325f89d408 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -388,6 +388,13 @@ static void exynos4_mct_tick_start(unsigned long cycles,
 	exynos4_mct_write(tmp, mevt->base + MCT_L_TCON_OFFSET);
 }
 
+static void exynos4_mct_tick_clear(struct mct_clock_event_device *mevt)
+{
+	/* Clear the MCT tick interrupt */
+	if (readl_relaxed(reg_base + mevt->base + MCT_L_INT_CSTAT_OFFSET) & 1)
+		exynos4_mct_write(0x1, mevt->base + MCT_L_INT_CSTAT_OFFSET);
+}
+
 static int exynos4_tick_set_next_event(unsigned long cycles,
 				       struct clock_event_device *evt)
 {
@@ -420,8 +427,11 @@ static int set_state_periodic(struct clock_event_device *evt)
 	return 0;
 }
 
-static void exynos4_mct_tick_clear(struct mct_clock_event_device *mevt)
+static irqreturn_t exynos4_mct_tick_isr(int irq, void *dev_id)
 {
+	struct mct_clock_event_device *mevt = dev_id;
+	struct clock_event_device *evt = &mevt->evt;
+
 	/*
 	 * This is for supporting oneshot mode.
 	 * Mct would generate interrupt periodically
@@ -430,16 +440,6 @@ static void exynos4_mct_tick_clear(struct mct_clock_event_device *mevt)
 	if (!clockevent_state_periodic(&mevt->evt))
 		exynos4_mct_tick_stop(mevt);
 
-	/* Clear the MCT tick interrupt */
-	if (readl_relaxed(reg_base + mevt->base + MCT_L_INT_CSTAT_OFFSET) & 1)
-		exynos4_mct_write(0x1, mevt->base + MCT_L_INT_CSTAT_OFFSET);
-}
-
-static irqreturn_t exynos4_mct_tick_isr(int irq, void *dev_id)
-{
-	struct mct_clock_event_device *mevt = dev_id;
-	struct clock_event_device *evt = &mevt->evt;
-
 	exynos4_mct_tick_clear(mevt);
 
 	evt->event_handler(evt);
-- 
2.13.6


_______________________________________________
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] 9+ messages in thread

* [PATCH 2/2] clocksource: exynos_mct: Clear timer interrupt when shutdown
  2019-02-10 22:51 ` [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down Stuart Menefy
  2019-02-10 22:51   ` [PATCH 1/2] clocksource: exynos_mct: Move one-shot check from tick clear to ISR Stuart Menefy
@ 2019-02-10 22:51   ` Stuart Menefy
  2019-02-11  8:47     ` Krzysztof Kozlowski
  2019-02-11  7:14   ` [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down Marek Szyprowski
  2019-02-11  8:45   ` Krzysztof Kozlowski
  3 siblings, 1 reply; 9+ messages in thread
From: Stuart Menefy @ 2019-02-10 22:51 UTC (permalink / raw)
  To: linux-samsung-soc, Daniel Lezcano, Thomas Gleixner, Kukjin Kim,
	Krzysztof Kozlowski, linux-kernel, linux-arm-kernel
  Cc: Stuart Menefy

When shutting down the timer, 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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 1e325f89d408..d55c30f6981d 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -411,6 +411,7 @@ static int set_state_shutdown(struct clock_event_device *evt)
 
 	mevt = container_of(evt, struct mct_clock_event_device, evt);
 	exynos4_mct_tick_stop(mevt);
+	exynos4_mct_tick_clear(mevt);
 	return 0;
 }
 
-- 
2.13.6


_______________________________________________
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] 9+ messages in thread

* Re: [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down
  2019-02-10 22:51 ` [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down Stuart Menefy
  2019-02-10 22:51   ` [PATCH 1/2] clocksource: exynos_mct: Move one-shot check from tick clear to ISR Stuart Menefy
  2019-02-10 22:51   ` [PATCH 2/2] clocksource: exynos_mct: Clear timer interrupt when shutdown Stuart Menefy
@ 2019-02-11  7:14   ` Marek Szyprowski
  2019-02-11 11:23     ` Daniel Lezcano
  2019-02-11  8:45   ` Krzysztof Kozlowski
  3 siblings, 1 reply; 9+ messages in thread
From: Marek Szyprowski @ 2019-02-11  7:14 UTC (permalink / raw)
  To: Stuart Menefy, linux-samsung-soc, Daniel Lezcano,
	Thomas Gleixner, Kukjin Kim, Krzysztof Kozlowski, linux-kernel,
	linux-arm-kernel

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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
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] 9+ messages in thread

* Re: [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down
  2019-02-10 22:51 ` [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down Stuart Menefy
                     ` (2 preceding siblings ...)
  2019-02-11  7:14   ` [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down Marek Szyprowski
@ 2019-02-11  8:45   ` Krzysztof Kozlowski
  3 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-11  8:45 UTC (permalink / raw)
  To: Stuart Menefy
  Cc: linux-samsung-soc, Daniel Lezcano, linux-kernel, Kukjin Kim,
	Thomas Gleixner, linux-arm-kernel

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

_______________________________________________
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] 9+ messages in thread

* Re: [PATCH 1/2] clocksource: exynos_mct: Move one-shot check from tick clear to ISR
  2019-02-10 22:51   ` [PATCH 1/2] clocksource: exynos_mct: Move one-shot check from tick clear to ISR Stuart Menefy
@ 2019-02-11  8:47     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-11  8:47 UTC (permalink / raw)
  To: Stuart Menefy
  Cc: linux-samsung-soc, Daniel Lezcano, linux-kernel, Kukjin Kim,
	Thomas Gleixner, linux-arm-kernel

On Sun, 10 Feb 2019 at 23:51, Stuart Menefy
<stuart.menefy@mathembedded.com> wrote:
>
> When a timer tick occurs and the clock is in one-shot mode, the timer
> needs to be stopped to prevent it triggering subsequent interrupts.
> Currently this code is in exynos4_mct_tick_clear(), but as it is
> only needed when an ISR occurs move it into exynos4_mct_tick_isr(),
> leaving exynos4_mct_tick_clear() just doing what its name suggests it
> should.
>
> Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com>
> ---
>  drivers/clocksource/exynos_mct.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

_______________________________________________
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] 9+ messages in thread

* Re: [PATCH 2/2] clocksource: exynos_mct: Clear timer interrupt when shutdown
  2019-02-10 22:51   ` [PATCH 2/2] clocksource: exynos_mct: Clear timer interrupt when shutdown Stuart Menefy
@ 2019-02-11  8:47     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-11  8:47 UTC (permalink / raw)
  To: Stuart Menefy
  Cc: linux-samsung-soc, Daniel Lezcano, linux-kernel, Kukjin Kim,
	Thomas Gleixner, linux-arm-kernel

On Sun, 10 Feb 2019 at 23:51, Stuart Menefy
<stuart.menefy@mathembedded.com> wrote:
>
> When shutting down the timer, 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 | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

_______________________________________________
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] 9+ messages in thread

* Re: [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down
  2019-02-11  7:14   ` [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down Marek Szyprowski
@ 2019-02-11 11:23     ` Daniel Lezcano
  2019-02-11 11:34       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Lezcano @ 2019-02-11 11:23 UTC (permalink / raw)
  To: Marek Szyprowski, Stuart Menefy, linux-samsung-soc,
	Thomas Gleixner, Kukjin Kim, Krzysztof Kozlowski, linux-kernel,
	linux-arm-kernel

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?


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


_______________________________________________
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] 9+ messages in thread

* Re: [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down
  2019-02-11 11:23     ` Daniel Lezcano
@ 2019-02-11 11:34       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-11 11:34 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: linux-samsung-soc, Stuart Menefy, linux-kernel, Kukjin Kim,
	Thomas Gleixner, linux-arm-kernel, Marek Szyprowski

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

_______________________________________________
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] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20190210225147epcas3p4cedebfc5692274bc6135ceabbcf2642d@epcas3p4.samsung.com>
2019-02-10 22:51 ` [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down Stuart Menefy
2019-02-10 22:51   ` [PATCH 1/2] clocksource: exynos_mct: Move one-shot check from tick clear to ISR Stuart Menefy
2019-02-11  8:47     ` Krzysztof Kozlowski
2019-02-10 22:51   ` [PATCH 2/2] clocksource: exynos_mct: Clear timer interrupt when shutdown Stuart Menefy
2019-02-11  8:47     ` Krzysztof Kozlowski
2019-02-11  7:14   ` [PATCH 0/2] Subject: [PATCH 0/2] clocksource: exynos_mct: Clear timer interrupt when shutting down Marek Szyprowski
2019-02-11 11:23     ` Daniel Lezcano
2019-02-11 11:34       ` Krzysztof Kozlowski
2019-02-11  8:45   ` Krzysztof Kozlowski

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org infradead-linux-arm-kernel@archiver.kernel.org
	public-inbox-index linux-arm-kernel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox