Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1210525 > unrolled thread

Re: [PATCH v2 1/5] clocksource: mediatek: do not enable GPT_CLK_EVT when setup

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 1/5] clocksource: mediatek: do not enable GPT_CLK_EVT  when setup Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-08-20 16:30 +0200
    Re: [PATCH v2 1/5] clocksource: mediatek: do not enable GPT_CLK_EVT  when setup Yingjoe Chen <yingjoe.chen@mediatek.com> - 2015-08-21 16:40 +0200
      Re: [PATCH v2 1/5] clocksource: mediatek: do not enable GPT_CLK_EVT  when setup Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-08-24 10:00 +0200
        Re: [PATCH v2 1/5] clocksource: mediatek: do not enable GPT_CLK_EVT  when setup Yingjoe Chen <yingjoe.chen@mediatek.com> - 2015-08-24 16:30 +0200
      [PATCH] clockevents/drivers/mtk: Fix spurious interrupt leading to crash Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-08-24 15:40 +0200
        Re: [PATCH] clockevents/drivers/mtk: Fix spurious interrupt leading  to crash Yingjoe Chen <yingjoe.chen@mediatek.com> - 2015-08-24 16:40 +0200
          [PATCH V2] clockevents/drivers/mtk: Fix spurious interrupt leading to crash Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-08-25 00:00 +0200
            Re: [PATCH V2] clockevents/drivers/mtk: Fix spurious interrupt  leading to crash Yingjoe Chen <yingjoe.chen@mediatek.com> - 2015-08-25 15:30 +0200
              Re: [PATCH V2] clockevents/drivers/mtk: Fix spurious interrupt leading  to crash Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-08-26 16:30 +0200
                Re: [PATCH V2] clockevents/drivers/mtk: Fix spurious interrupt  leading to crash Yingjoe Chen <yingjoe.chen@mediatek.com> - 2015-09-04 09:20 +0200
                  Re: [PATCH V2] clockevents/drivers/mtk: Fix spurious interrupt leading  to crash Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-09-04 09:40 +0200

#1210525 — Re: [PATCH v2 1/5] clocksource: mediatek: do not enable GPT_CLK_EVT when setup

On 08/17/2015 04:10 PM, Yingjoe Chen wrote:
> On Thu, 2015-08-13 at 10:35 +0200, Daniel Lezcano wrote:
>> On 07/22/2015 10:14 AM, Yingjoe Chen wrote:
>>> Spurious mtk timer interrupt is noticed at boot and cause kernel
>>> crash. It seems if GPT is enabled, it will latch irq status even
>>> when its IRQ is disabled.

It "seems" ?

>>> When irq is enabled afterward, we see
>>> spurious interrupt.

Doesn't have the firmware something to do with that ?

I have a mtk 8173 board I can use next week. How do you reproduce the 
issue ?

>>> Change init flow to only enable GPT_CLK_SRC at mtk_timer_init.
>>>
>>> Acked-by: Matthias Brugger <matthias.bgg@gmail.com>
>>> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
>>> Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
>>> ---
>>>
>>> Update to my patch [1], added __init as Daniel suggest. This is the
>>> only patch that need to change in that series, so I only sent this one.
>>>
>>> http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001545.html
>>>
>>>    drivers/clocksource/mtk_timer.c | 16 ++++++++++------
>>>    1 file changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/clocksource/mtk_timer.c b/drivers/clocksource/mtk_timer.c
>>> index 68ab423..2ba5b66 100644
>>> --- a/drivers/clocksource/mtk_timer.c
>>> +++ b/drivers/clocksource/mtk_timer.c
>>> @@ -156,9 +156,11 @@ static void mtk_timer_global_reset(struct mtk_clock_event_device *evt)
>>>    	writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);
>>>    }
>>>
>>> -static void
>>> -mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer, u8 option)
>>> +static void __init mtk_timer_setup(struct mtk_clock_event_device *evt,
>>> +				   u8 timer, u8 option, bool enable)
>>>    {
>>> +	u32 val;
>>> +
>>>    	writel(TIMER_CTRL_CLEAR | TIMER_CTRL_DISABLE,
>>>    		evt->gpt_base + TIMER_CTRL_REG(timer));
>>>
>>> @@ -167,8 +169,10 @@ mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer, u8 option)
>>>
>>>    	writel(0x0, evt->gpt_base + TIMER_CMP_REG(timer));
>>>
>>> -	writel(TIMER_CTRL_OP(option) | TIMER_CTRL_ENABLE,
>>> -			evt->gpt_base + TIMER_CTRL_REG(timer));
>>> +	val = TIMER_CTRL_OP(option);
>>> +	if (enable)
>>> +		val |= TIMER_CTRL_ENABLE;
>>> +	writel(val, evt->gpt_base + TIMER_CTRL_REG(timer));
>>
>> Instead of the 'enable' new option, I prefer a test with 'timer' with a
>> comment:
>>
>> 	/*
>> 	 * the timer hw is broken in that way ... bla bla, so we only
>> 	 * enable the clocksource ...
>> 	 */
>> 	if (timer == GPT_CLK_SRC)
>> 		val |= TIMER_CTRL_ENABLE;
>
> Hi Daniel,
>
> Thanks for your review.
> Since this bug happens to anyone using interrupt,

Can you elaborate ? I don't get the point.

> I'm not sure checking
> timer and only enable it for GPT_CLK_SRC is easier to read. Anyway, I'll
> change to this in next version.
>> That said, can you have a look at commit 1096be08 ?
>> "clockevents: sun5i: Fix setup_irq init sequence"
>>
>> first and check if moving the interrupt request after the
>> clockevents_config_and_register could fix your issue.
>
> I've tested this before, see:
>
> http://lists.infradead.org/pipermail/linux-mediatek/2015-May/000539.html
> http://lists.infradead.org/pipermail/linux-mediatek/2015-May/000551.html

I could take or ack this patch trusting it fixes the issue but there are 
some points that need clarifications.

  - Does the spurious interrupt occurs *every* time ? at each boot ?

The previous patches were supposed to fix the issue but they actually 
didn't. So how is tested the patch ?

Regarding the different fixes for this problem, it sounds like you are 
proceeding by trial and error.

Please give a more detailed analysis of the problem and especially check 
the timer is not altered by the firmware leaving it in a transient state 
or whatever.

   -- Daniel

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1211204

On Thu, 2015-08-20 at 16:28 +0200, Daniel Lezcano wrote:
> On 08/17/2015 04:10 PM, Yingjoe Chen wrote:
> > On Thu, 2015-08-13 at 10:35 +0200, Daniel Lezcano wrote:
> >> On 07/22/2015 10:14 AM, Yingjoe Chen wrote:
> >>> Spurious mtk timer interrupt is noticed at boot and cause kernel
> >>> crash. It seems if GPT is enabled, it will latch irq status even
> >>> when its IRQ is disabled.
> 
> It "seems" ?

Hi,

Datasheet doesn't mention detail. So I did some experiments, playing
around with registers. Based on my observation, I think this is what
happens:

For each GPT timer, it has ENABLE, IRQ_EN, IRQ status, IRQ_ACK, counter
& compare.

When mtk_timer_init calls mtk_timer_setup to setup GPT_CLK_EVT, it
enable the timer but didn't set counter or compare. Both counter &
compare is zero on reset, so GPT immediately raise IRQ status. IRQ_EN is
still disabled now, so it didn't trigger interrupt right away.

At end of mtk_timer_init, it calls mtk_timer_enable_irq to enable irq.
Since IRQ status is 1 now, GPT trigger interrupt immediately. The
interrupt is serviced by mtk_timer_interrupt. Since this is not an
expected event, evt->dev.event_handler will be NULL and system crashed
in the handler.

> >>> When irq is enabled afterward, we see
> >>> spurious interrupt.
> 
> Doesn't have the firmware something to do with that ?

We have 6 GPT on mt8173, mtk timer use 2 of them. The spurious interrupt
only happens on GPT_CLK_EVT (GPT1). Our firmware didn't touch that one,
so it is in reset default when mtk timer driver try to enable it.

> I have a mtk 8173 board I can use next week. How do you reproduce the 
> issue ?
>
> >>> Change init flow to only enable GPT_CLK_SRC at mtk_timer_init.
> >>>
> >>> Acked-by: Matthias Brugger <matthias.bgg@gmail.com>
> >>> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> >>> Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
> >>> ---
> >>>
> >>> Update to my patch [1], added __init as Daniel suggest. This is the
> >>> only patch that need to change in that series, so I only sent this one.
> >>>
> >>> http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001545.html
> >>>
> >>>    drivers/clocksource/mtk_timer.c | 16 ++++++++++------
> >>>    1 file changed, 10 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/clocksource/mtk_timer.c b/drivers/clocksource/mtk_timer.c
> >>> index 68ab423..2ba5b66 100644
> >>> --- a/drivers/clocksource/mtk_timer.c
> >>> +++ b/drivers/clocksource/mtk_timer.c
> >>> @@ -156,9 +156,11 @@ static void mtk_timer_global_reset(struct mtk_clock_event_device *evt)
> >>>    	writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);
> >>>    }
> >>>
> >>> -static void
> >>> -mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer, u8 option)
> >>> +static void __init mtk_timer_setup(struct mtk_clock_event_device *evt,
> >>> +				   u8 timer, u8 option, bool enable)
> >>>    {
> >>> +	u32 val;
> >>> +
> >>>    	writel(TIMER_CTRL_CLEAR | TIMER_CTRL_DISABLE,
> >>>    		evt->gpt_base + TIMER_CTRL_REG(timer));
> >>>
> >>> @@ -167,8 +169,10 @@ mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer, u8 option)
> >>>
> >>>    	writel(0x0, evt->gpt_base + TIMER_CMP_REG(timer));
> >>>
> >>> -	writel(TIMER_CTRL_OP(option) | TIMER_CTRL_ENABLE,
> >>> -			evt->gpt_base + TIMER_CTRL_REG(timer));
> >>> +	val = TIMER_CTRL_OP(option);
> >>> +	if (enable)
> >>> +		val |= TIMER_CTRL_ENABLE;
> >>> +	writel(val, evt->gpt_base + TIMER_CTRL_REG(timer));
> >>
> >> Instead of the 'enable' new option, I prefer a test with 'timer' with a
> >> comment:
> >>
> >> 	/*
> >> 	 * the timer hw is broken in that way ... bla bla, so we only
> >> 	 * enable the clocksource ...
> >> 	 */
> >> 	if (timer == GPT_CLK_SRC)
> >> 		val |= TIMER_CTRL_ENABLE;
> >
> > Hi Daniel,
> >
> > Thanks for your review.
> > Since this bug happens to anyone using interrupt,
> 
> Can you elaborate ? I don't get the point.
> 
> > I'm not sure checking
> > timer and only enable it for GPT_CLK_SRC is easier to read. Anyway, I'll
> > change to this in next version.
> >> That said, can you have a look at commit 1096be08 ?
> >> "clockevents: sun5i: Fix setup_irq init sequence"
> >>
> >> first and check if moving the interrupt request after the
> >> clockevents_config_and_register could fix your issue.
> >
> > I've tested this before, see:
> >
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-May/000539.html
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-May/000551.html
> 
> I could take or ack this patch trusting it fixes the issue but there are 
> some points that need clarifications.
> 
>   - Does the spurious interrupt occurs *every* time ? at each boot ?

Yes. If you applied this series to enable mtk timer without this fix on
mt8173 or mt8135 you can reproduce it. It occurs for every boot.

It crash before uart driver is ready, so you'll have to use earlycon to
see the crash log.

> The previous patches were supposed to fix the issue but they actually 
> didn't. So how is tested the patch ?

I'm not sure. I believe Matthias test that one on mt6589, maybe it have
different behavior, or different timing.

> Regarding the different fixes for this problem, it sounds like you are 
> proceeding by trial and error.
> 
> Please give a more detailed analysis of the problem and especially check 
> the timer is not altered by the firmware leaving it in a transient state 
> or whatever.

See above for my analysis.
Thanks.

Joe.C


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211875

On 08/21/2015 04:39 PM, Yingjoe Chen wrote:

[ ... ]

>>    - Does the spurious interrupt occurs *every* time ? at each boot ?
>
> Yes. If you applied this series to enable mtk timer without this fix on
> mt8173 or mt8135 you can reproduce it. It occurs for every boot.
>
> It crash before uart driver is ready, so you'll have to use earlycon to
> see the crash log.

Can you give me the earlycon params ?

Thanks.

  -- Daniel

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212239

On Mon, 2015-08-24 at 09:51 +0200, Daniel Lezcano wrote:
> On 08/21/2015 04:39 PM, Yingjoe Chen wrote:
> 
> [ ... ]
> 
> >>    - Does the spurious interrupt occurs *every* time ? at each boot ?
> >
> > Yes. If you applied this series to enable mtk timer without this fix on
> > mt8173 or mt8135 you can reproduce it. It occurs for every boot.
> >
> > It crash before uart driver is ready, so you'll have to use earlycon to
> > see the crash log.
> 
> Can you give me the earlycon params ?

Hi Daniel,

You probably already figure this out. anyway I'm using this to enable
earlycon:

Add 'earlycon' in bootargs, and in device tree chosen part, add
"linux,stdout-path=&uart0;"

Joe.C



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212191 — [PATCH] clockevents/drivers/mtk: Fix spurious interrupt leading to crash

After analysis done by Yingjoe Chen, the timer appears to have a pending
interrupt when it is enabled.

Fix this by acknowledging the pending interrupt when enabling the timer
interrupt.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/mtk_timer.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/clocksource/mtk_timer.c b/drivers/clocksource/mtk_timer.c
index 4cd16fb..13543a8 100644
--- a/drivers/clocksource/mtk_timer.c
+++ b/drivers/clocksource/mtk_timer.c
@@ -156,14 +156,6 @@ static irqreturn_t mtk_timer_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void mtk_timer_global_reset(struct mtk_clock_event_device *evt)
-{
-	/* Disable all interrupts */
-	writel(0x0, evt->gpt_base + GPT_IRQ_EN_REG);
-	/* Acknowledge all interrupts */
-	writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);
-}
-
 static void
 mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer, u8 option)
 {
@@ -183,6 +175,9 @@ static void mtk_timer_enable_irq(struct mtk_clock_event_device *evt, u8 timer)
 {
 	u32 val;
 
+        /* Acknowledge all spurious pending interrupts */
+        writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);
+
 	val = readl(evt->gpt_base + GPT_IRQ_EN_REG);
 	writel(val | GPT_IRQ_ENABLE(timer),
 			evt->gpt_base + GPT_IRQ_EN_REG);
@@ -232,8 +227,6 @@ static void __init mtk_timer_init(struct device_node *node)
 	}
 	rate = clk_get_rate(clk);
 
-	mtk_timer_global_reset(evt);
-
 	if (request_irq(evt->dev.irq, mtk_timer_interrupt,
 			IRQF_TIMER | IRQF_IRQPOLL, "mtk_timer", evt)) {
 		pr_warn("failed to setup irq %d\n", evt->dev.irq);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212258 — Re: [PATCH] clockevents/drivers/mtk: Fix spurious interrupt leading to crash

On Mon, 2015-08-24 at 15:30 +0200, Daniel Lezcano wrote:
> After analysis done by Yingjoe Chen, the timer appears to have a pending
> interrupt when it is enabled.
>
> Fix this by acknowledging the pending interrupt when enabling the timer
> interrupt.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Hi Daniel,


Thanks for your patch, this can fix the boot issue.
Tested-by: Yingjoe Chen <yingjoe.chen@mediatek.com>

> ---
>  drivers/clocksource/mtk_timer.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/clocksource/mtk_timer.c b/drivers/clocksource/mtk_timer.c
> index 4cd16fb..13543a8 100644
> --- a/drivers/clocksource/mtk_timer.c
> +++ b/drivers/clocksource/mtk_timer.c
> @@ -156,14 +156,6 @@ static irqreturn_t mtk_timer_interrupt(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> -static void mtk_timer_global_reset(struct mtk_clock_event_device *evt)
> -{
> -	/* Disable all interrupts */
> -	writel(0x0, evt->gpt_base + GPT_IRQ_EN_REG);
> -	/* Acknowledge all interrupts */
> -	writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);
> -}
> -
>  static void
>  mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer, u8 option)
>  {
> @@ -183,6 +175,9 @@ static void mtk_timer_enable_irq(struct mtk_clock_event_device *evt, u8 timer)
>  {
>  	u32 val;
>  
> +        /* Acknowledge all spurious pending interrupts */
> +        writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);

This should use tab to indent.

> +
>  	val = readl(evt->gpt_base + GPT_IRQ_EN_REG);
>  	writel(val | GPT_IRQ_ENABLE(timer),
>  			evt->gpt_base + GPT_IRQ_EN_REG);
> @@ -232,8 +227,6 @@ static void __init mtk_timer_init(struct device_node *node)
>  	}
>  	rate = clk_get_rate(clk);
>  
> -	mtk_timer_global_reset(evt);
> -

I think we should keep this one, or at least disable irq first in
mtk_timer_enable_irq. MT8173 firmware didn't use this GPT, but I think
it is a good ideat to do it just in case firmware in some other platform
use it.

Joe.C


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212540 — [PATCH V2] clockevents/drivers/mtk: Fix spurious interrupt leading to crash

After analysis done by Yingjoe Chen, the timer appears to have a pending
interrupt when it is enabled.

Fix this by acknowledging the pending interrupt when enabling the timer
interrupt.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/mtk_timer.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/clocksource/mtk_timer.c b/drivers/clocksource/mtk_timer.c
index 4cd16fb..505f53d 100644
--- a/drivers/clocksource/mtk_timer.c
+++ b/drivers/clocksource/mtk_timer.c
@@ -156,14 +156,6 @@ static irqreturn_t mtk_timer_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void mtk_timer_global_reset(struct mtk_clock_event_device *evt)
-{
-	/* Disable all interrupts */
-	writel(0x0, evt->gpt_base + GPT_IRQ_EN_REG);
-	/* Acknowledge all interrupts */
-	writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);
-}
-
 static void
 mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer, u8 option)
 {
@@ -183,6 +175,12 @@ static void mtk_timer_enable_irq(struct mtk_clock_event_device *evt, u8 timer)
 {
 	u32 val;
 
+	/* Disable all interrupts */
+	writel(0x0, evt->gpt_base + GPT_IRQ_EN_REG);
+
+	/* Acknowledge all spurious pending interrupts */
+	writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);
+
 	val = readl(evt->gpt_base + GPT_IRQ_EN_REG);
 	writel(val | GPT_IRQ_ENABLE(timer),
 			evt->gpt_base + GPT_IRQ_EN_REG);
@@ -232,8 +230,6 @@ static void __init mtk_timer_init(struct device_node *node)
 	}
 	rate = clk_get_rate(clk);
 
-	mtk_timer_global_reset(evt);
-
 	if (request_irq(evt->dev.irq, mtk_timer_interrupt,
 			IRQF_TIMER | IRQF_IRQPOLL, "mtk_timer", evt)) {
 		pr_warn("failed to setup irq %d\n", evt->dev.irq);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1213035 — Re: [PATCH V2] clockevents/drivers/mtk: Fix spurious interrupt leading to crash

On Mon, 2015-08-24 at 23:57 +0200, Daniel Lezcano wrote:
> After analysis done by Yingjoe Chen, the timer appears to have a pending
> interrupt when it is enabled.
> 
> Fix this by acknowledging the pending interrupt when enabling the timer
> interrupt.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/clocksource/mtk_timer.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/clocksource/mtk_timer.c b/drivers/clocksource/mtk_timer.c
> index 4cd16fb..505f53d 100644
> --- a/drivers/clocksource/mtk_timer.c
> +++ b/drivers/clocksource/mtk_timer.c
> @@ -156,14 +156,6 @@ static irqreturn_t mtk_timer_interrupt(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> -static void mtk_timer_global_reset(struct mtk_clock_event_device *evt)
> -{
> -	/* Disable all interrupts */
> -	writel(0x0, evt->gpt_base + GPT_IRQ_EN_REG);
> -	/* Acknowledge all interrupts */
> -	writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);
> -}
> -
>  static void
>  mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer, u8 option)
>  {
> @@ -183,6 +175,12 @@ static void mtk_timer_enable_irq(struct mtk_clock_event_device *evt, u8 timer)
>  {
>  	u32 val;
>  
> +	/* Disable all interrupts */
> +	writel(0x0, evt->gpt_base + GPT_IRQ_EN_REG);
> +
> +	/* Acknowledge all spurious pending interrupts */
> +	writel(0x3f, evt->gpt_base + GPT_IRQ_ACK_REG);
> +
>  	val = readl(evt->gpt_base + GPT_IRQ_EN_REG);
>  	writel(val | GPT_IRQ_ENABLE(timer),
>  			evt->gpt_base + GPT_IRQ_EN_REG);
> @@ -232,8 +230,6 @@ static void __init mtk_timer_init(struct device_node *node)
>  	}
>  	rate = clk_get_rate(clk);
>  
> -	mtk_timer_global_reset(evt);
> -
>  	if (request_irq(evt->dev.irq, mtk_timer_interrupt,
>  			IRQF_TIMER | IRQF_IRQPOLL, "mtk_timer", evt)) {
>  		pr_warn("failed to setup irq %d\n", evt->dev.irq);


Hi Daniel,


Thanks for your patch, tested on mt8173
Tested-by: Yingjoe Chen <yingjoe.chen@mediatek.com>

Joe.C


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1213919 — Re: [PATCH V2] clockevents/drivers/mtk: Fix spurious interrupt leading to crash

On 08/25/2015 03:21 PM, Yingjoe Chen wrote:
> Tested-by: Yingjoe Chen<yingjoe.chen@mediatek.com>

Applied to my tree for 4.4.

   -- Daniel

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1218700 — Re: [PATCH V2] clockevents/drivers/mtk: Fix spurious interrupt leading to crash

On Wed, 2015-08-26 at 16:25 +0200, Daniel Lezcano wrote:
> On 08/25/2015 03:21 PM, Yingjoe Chen wrote:
> > Tested-by: Yingjoe Chen<yingjoe.chen@mediatek.com>
> 
> Applied to my tree for 4.4.
> 
>    -- Daniel
> 

Hi Daniel,

I can't find this patch in
https://git.linaro.org/people/daniel.lezcano/linux.git
Is this tree public now? Where can I see it?

Also, will you take patch[1] "clocksource: mediatek: Use GPT as sched
clock source" in your tree?

Joe.C

[1]:
http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001547.html


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1218705 — Re: [PATCH V2] clockevents/drivers/mtk: Fix spurious interrupt leading to crash

On 09/04/2015 09:15 AM, Yingjoe Chen wrote:
> On Wed, 2015-08-26 at 16:25 +0200, Daniel Lezcano wrote:
>> On 08/25/2015 03:21 PM, Yingjoe Chen wrote:
>>> Tested-by: Yingjoe Chen<yingjoe.chen@mediatek.com>
>>
>> Applied to my tree for 4.4.
>>
>>     -- Daniel
>>
>
> Hi Daniel,
>
> I can't find this patch in
> https://git.linaro.org/people/daniel.lezcano/linux.git
> Is this tree public now? Where can I see it?
>
> Also, will you take patch[1] "clocksource: mediatek: Use GPT as sched
> clock source" in your tree?

Yep. I did not push my branch. Done.

Note it will be also available for linux-next.

Thanks for the head up.

   -- Daniel


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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web