[PATCH 0/5] add GPT timer support for mt8173

[PATCH 0/5] add GPT timer support for mt8173

[Yingjoe Chen]

This series add GPT timer support for mt8173. This is based on v4.2-rc1
and Matthias' next branch (for dts parts).

The first 2 patches comes from 'Add SMP bringup support for mt65xx socs'
series [1]. I decide to move these 2 patches to this series, since it
is more relevent here. They are changed based on Matthias' and Daniel's
comments.

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

Daniel Kurtz (1):
  arm64: dts: mt8173: add timer node

Yingjoe Chen (4):
  clocksource: mediatek: do not enable GPT_CLK_EVT when setup
  clocksource: mediatek: Use GPT as sched clock source
  arm64: mediatek: enable MTK_TIMER
  clk: mediatek: add 13mhz clock for MT8173

 arch/arm64/Kconfig                       |  1 +
 arch/arm64/boot/dts/mediatek/mt8173.dtsi |  9 +++++++++
 drivers/clk/mediatek/clk-mt8173.c        |  5 +++++
 drivers/clocksource/mtk_timer.c          | 26 ++++++++++++++++++++------
 include/dt-bindings/clock/mt8173-clk.h   |  3 ++-
 5 files changed, 37 insertions(+), 7 deletions(-)

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

[Yingjoe Chen]

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. When irq is enabled afterward, we see
spurious interrupt.
Change init flow to only enable GPT_CLK_SRC at mtk_timer_init.

Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
---
 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..237c20b 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 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));
 }
 
 static void mtk_timer_enable_irq(struct mtk_clock_event_device *evt, u8 timer)
@@ -235,12 +239,12 @@ static void __init mtk_timer_init(struct device_node *node)
 	evt->ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
 
 	/* Configure clock source */
-	mtk_timer_setup(evt, GPT_CLK_SRC, TIMER_CTRL_OP_FREERUN);
+	mtk_timer_setup(evt, GPT_CLK_SRC, TIMER_CTRL_OP_FREERUN, true);
 	clocksource_mmio_init(evt->gpt_base + TIMER_CNT_REG(GPT_CLK_SRC),
 			node->name, rate, 300, 32, clocksource_mmio_readl_up);
 
 	/* Configure clock event */
-	mtk_timer_setup(evt, GPT_CLK_EVT, TIMER_CTRL_OP_REPEAT);
+	mtk_timer_setup(evt, GPT_CLK_EVT, TIMER_CTRL_OP_REPEAT, false);
 	clockevents_config_and_register(&evt->dev, rate, 0x3,
 					0xffffffff);
 
-- 
1.8.1.1.dirty

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

[Daniel Kurtz]

Hi Yingjoe,

On Mon, Jul 13, 2015 at 5:32 PM, Yingjoe Chen <yingjoe.chen@mediatek.com> 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. When irq is enabled afterward, we see
> spurious interrupt.
> Change init flow to only enable GPT_CLK_SRC at mtk_timer_init.
>
> Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
> ---
>  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..237c20b 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 mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer,
> +                           u8 option, bool enable)

This function can be: __init

Other than this tiny nit, and the small potential conflict in patch 4,
this whole series is:

Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>

(I do think it is a bit strange that the mediatek,mt6577-timer binding
does not use "clock-names", but that is independent of this patch
set).

Thanks!


>  {
> +       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));
>  }
>
>  static void mtk_timer_enable_irq(struct mtk_clock_event_device *evt, u8 timer)
> @@ -235,12 +239,12 @@ static void __init mtk_timer_init(struct device_node *node)
>         evt->ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
>
>         /* Configure clock source */
> -       mtk_timer_setup(evt, GPT_CLK_SRC, TIMER_CTRL_OP_FREERUN);
> +       mtk_timer_setup(evt, GPT_CLK_SRC, TIMER_CTRL_OP_FREERUN, true);
>         clocksource_mmio_init(evt->gpt_base + TIMER_CNT_REG(GPT_CLK_SRC),
>                         node->name, rate, 300, 32, clocksource_mmio_readl_up);
>
>         /* Configure clock event */
> -       mtk_timer_setup(evt, GPT_CLK_EVT, TIMER_CTRL_OP_REPEAT);
> +       mtk_timer_setup(evt, GPT_CLK_EVT, TIMER_CTRL_OP_REPEAT, false);
>         clockevents_config_and_register(&evt->dev, rate, 0x3,
>                                         0xffffffff);
>
> --
> 1.8.1.1.dirty
>

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

[Yingjoe Chen]

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. When irq is enabled afterward, we see
spurious interrupt.
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));
 }
 
 static void mtk_timer_enable_irq(struct mtk_clock_event_device *evt, u8 timer)
@@ -235,12 +239,12 @@ static void __init mtk_timer_init(struct device_node *node)
 	evt->ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
 
 	/* Configure clock source */
-	mtk_timer_setup(evt, GPT_CLK_SRC, TIMER_CTRL_OP_FREERUN);
+	mtk_timer_setup(evt, GPT_CLK_SRC, TIMER_CTRL_OP_FREERUN, true);
 	clocksource_mmio_init(evt->gpt_base + TIMER_CNT_REG(GPT_CLK_SRC),
 			node->name, rate, 300, 32, clocksource_mmio_readl_up);
 
 	/* Configure clock event */
-	mtk_timer_setup(evt, GPT_CLK_EVT, TIMER_CTRL_OP_REPEAT);
+	mtk_timer_setup(evt, GPT_CLK_EVT, TIMER_CTRL_OP_REPEAT, false);
 	clockevents_config_and_register(&evt->dev, rate, 0x3,
 					0xffffffff);
 
-- 
1.8.1.1.dirty

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

[Yingjoe Chen]

On Wed, 2015-07-22 at 16:14 +0800, 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. When irq is enabled afterward, we see
> spurious interrupt.
> Change init flow to only enable GPT_CLK_SRC at mtk_timer_init.
> 
> Acked-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> 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


Hi Daniel, Thomas,

Any suggestions for mtk_timer fixes in this series?
Should I resend and add tags from the reviewers?

Joe.C


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Daniel Lezcano]

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. When irq is enabled afterward, we see
> spurious interrupt.
> 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;

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.

>   }
>
>   static void mtk_timer_enable_irq(struct mtk_clock_event_device *evt, u8 timer)
> @@ -235,12 +239,12 @@ static void __init mtk_timer_init(struct device_node *node)
>   	evt->ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
>
>   	/* Configure clock source */
> -	mtk_timer_setup(evt, GPT_CLK_SRC, TIMER_CTRL_OP_FREERUN);
> +	mtk_timer_setup(evt, GPT_CLK_SRC, TIMER_CTRL_OP_FREERUN, true);
>   	clocksource_mmio_init(evt->gpt_base + TIMER_CNT_REG(GPT_CLK_SRC),
>   			node->name, rate, 300, 32, clocksource_mmio_readl_up);
>
>   	/* Configure clock event */
> -	mtk_timer_setup(evt, GPT_CLK_EVT, TIMER_CTRL_OP_REPEAT);
> +	mtk_timer_setup(evt, GPT_CLK_EVT, TIMER_CTRL_OP_REPEAT, false);
>   	clockevents_config_and_register(&evt->dev, rate, 0x3,
>   					0xffffffff);




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

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

[Yingjoe Chen]

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. When irq is enabled afterward, we see
> > spurious interrupt.
> > Change init flow to only enable GPT_CLK_SRC at mtk_timer_init.
> >
> > Acked-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Reviewed-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >
> > 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, 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

Joe.C

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

[Daniel Lezcano]

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

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

[Yingjoe Chen]

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

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

[Daniel Lezcano]

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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Yingjoe Chen]

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

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

[Yingjoe Chen]

On Tue, 2015-07-14 at 15:39 +0800, Daniel Kurtz wrote:
> Hi Yingjoe,
> 
> On Mon, Jul 13, 2015 at 5:32 PM, Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> 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. When irq is enabled afterward, we see
> > spurious interrupt.
> > Change init flow to only enable GPT_CLK_SRC at mtk_timer_init.
> >
> > Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  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..237c20b 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 mtk_timer_setup(struct mtk_clock_event_device *evt, u8 timer,
> > +                           u8 option, bool enable)
> 
> This function can be: __init
> 
> Other than this tiny nit, and the small potential conflict in patch 4,
> this whole series is:
> 
> Reviewed-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> (I do think it is a bit strange that the mediatek,mt6577-timer binding
> does not use "clock-names", but that is independent of this patch
> set).
> 


Hi Daniel,

Thanks for your review.

I added __init as you suggested, and Pi-Cheng already sent an updated
version of his patch to resolve the conflict[1].

Joe.C

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



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Matthias Brugger]

On Monday, July 13, 2015 05:32:45 PM 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. When irq is enabled afterward, we see
> spurious interrupt.
> Change init flow to only enable GPT_CLK_SRC at mtk_timer_init.
> 
> Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---

Acked-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] arm64: mediatek: enable MTK_TIMER

[Yingjoe Chen]

Enable MTK_TIMER for MediaTek plaform, which will be used as
schedule clock.

Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
---
 arch/arm64/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 0f6edb1..5934f51 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -193,6 +193,7 @@ config ARCH_MEDIATEK
 	bool "Mediatek MT65xx & MT81xx ARMv8 SoC"
 	select ARM_GIC
 	select PINCTRL
+	select MTK_TIMER
 	help
 	  Support for Mediatek MT65xx & MT81xx ARMv8 SoCs
 
-- 
1.8.1.1.dirty

Re: [PATCH 3/5] arm64: mediatek: enable MTK_TIMER

[Matthias Brugger]

On Monday, July 13, 2015 05:32:47 PM Yingjoe Chen wrote:
> Enable MTK_TIMER for MediaTek plaform, which will be used as
> schedule clock.
> 
> Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
> ---

Applied, thanks.

Re: [PATCH 3/5] arm64: mediatek: enable MTK_TIMER

[Thomas Gleixner]

On Fri, 17 Jul 2015, Matthias Brugger wrote:

> On Monday, July 13, 2015 05:32:47 PM Yingjoe Chen wrote:
> > Enable MTK_TIMER for MediaTek plaform, which will be used as
> > schedule clock.
> > 
> > Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> 
> Applied, thanks.

So you enable the current code w/o the modifications required for this
to work?

Thanks,

	tglx

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] arm64: mediatek: enable MTK_TIMER

[Matthias Brugger]

On Saturday, July 18, 2015 09:31:53 AM Thomas Gleixner wrote:
> On Fri, 17 Jul 2015, Matthias Brugger wrote:
> > On Monday, July 13, 2015 05:32:47 PM Yingjoe Chen wrote:
> > > Enable MTK_TIMER for MediaTek plaform, which will be used as
> > > schedule clock.
> > > 
> > > Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > ---
> > 
> > Applied, thanks.
> 
> So you enable the current code w/o the modifications required for this
> to work?
> 

Well as the timer is not defined in dts it should not break the board...
But yes you are right, I will drop this commit for now.

Thanks,
Matthias
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] clk: mediatek: add 13mhz clock for MT8173

[Yingjoe Chen]

Add 13mhz clock used by GPT timer in infracfg.

Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
---
 drivers/clk/mediatek/clk-mt8173.c      | 5 +++++
 include/dt-bindings/clock/mt8173-clk.h | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 4b9e04c..540c5c3 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -618,6 +618,10 @@ static const struct mtk_gate infra_clks[] __initconst = {
 	GATE_ICG(CLK_INFRA_PMICWRAP, "infra_pmicwrap", "axi_sel", 23),
 };
 
+static const struct mtk_fixed_factor infra_divs[] __initconst = {
+	FACTOR(CLK_INFRA_CLK_13M, "clk13m", "clk26m", 1, 2),
+};
+
 static const struct mtk_gate_regs peri0_cg_regs = {
 	.set_ofs = 0x0008,
 	.clr_ofs = 0x0010,
@@ -737,6 +741,7 @@ static void __init mtk_infrasys_init(struct device_node *node)
 
 	mtk_clk_register_gates(node, infra_clks, ARRAY_SIZE(infra_clks),
 						clk_data);
+	mtk_clk_register_factors(infra_divs, ARRAY_SIZE(infra_divs), clk_data);
 
 	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
 	if (r)
diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
index 4ad76ed..fa2a2bb 100644
--- a/include/dt-bindings/clock/mt8173-clk.h
+++ b/include/dt-bindings/clock/mt8173-clk.h
@@ -187,7 +187,8 @@
 #define CLK_INFRA_CEC			9
 #define CLK_INFRA_PMICSPI		10
 #define CLK_INFRA_PMICWRAP		11
-#define CLK_INFRA_NR_CLK		12
+#define CLK_INFRA_CLK_13M		12
+#define CLK_INFRA_NR_CLK		13
 
 /* PERI_SYS */
 
-- 
1.8.1.1.dirty

Re: [PATCH 4/5] clk: mediatek: add 13mhz clock for MT8173

[Stephen Boyd]

On 07/13, Yingjoe Chen wrote:
> Add 13mhz clock used by GPT timer in infracfg.
> 
> Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---

Acked-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] clk: mediatek: add 13mhz clock for MT8173

[Daniel Kurtz]

On Mon, Jul 13, 2015 at 5:32 PM, Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> Add 13mhz clock used by GPT timer in infracfg.
>
> Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/clk/mediatek/clk-mt8173.c      | 5 +++++
>  include/dt-bindings/clock/mt8173-clk.h | 3 ++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
> index 4b9e04c..540c5c3 100644
> --- a/drivers/clk/mediatek/clk-mt8173.c
> +++ b/drivers/clk/mediatek/clk-mt8173.c
> @@ -618,6 +618,10 @@ static const struct mtk_gate infra_clks[] __initconst = {
>         GATE_ICG(CLK_INFRA_PMICWRAP, "infra_pmicwrap", "axi_sel", 23),
>  };
>
> +static const struct mtk_fixed_factor infra_divs[] __initconst = {
> +       FACTOR(CLK_INFRA_CLK_13M, "clk13m", "clk26m", 1, 2),
> +};
> +
>  static const struct mtk_gate_regs peri0_cg_regs = {
>         .set_ofs = 0x0008,
>         .clr_ofs = 0x0010,
> @@ -737,6 +741,7 @@ static void __init mtk_infrasys_init(struct device_node *node)
>
>         mtk_clk_register_gates(node, infra_clks, ARRAY_SIZE(infra_clks),
>                                                 clk_data);
> +       mtk_clk_register_factors(infra_divs, ARRAY_SIZE(infra_divs), clk_data);
>
>         r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
>         if (r)
> diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
> index 4ad76ed..fa2a2bb 100644
> --- a/include/dt-bindings/clock/mt8173-clk.h
> +++ b/include/dt-bindings/clock/mt8173-clk.h
> @@ -187,7 +187,8 @@
>  #define CLK_INFRA_CEC                  9
>  #define CLK_INFRA_PMICSPI              10
>  #define CLK_INFRA_PMICWRAP             11
> -#define CLK_INFRA_NR_CLK               12
> +#define CLK_INFRA_CLK_13M              12
> +#define CLK_INFRA_NR_CLK               13

Note: this one conflicts slightly with pi-cheng's patch that adds CPU
mux clocks for cpufreq driver, since they both add more INFRA clocks:
https://patchwork.kernel.org/patch/6721511/

>
>  /* PERI_SYS */
>
> --
> 1.8.1.1.dirty
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] clocksource: mediatek: Use GPT as sched clock source

[Yingjoe Chen]

When cpu is in deep idle, arch timer will stop counting. Setup GPT as
sched clock source so it can keep counting in idle.

Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/clocksource/mtk_timer.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/clocksource/mtk_timer.c b/drivers/clocksource/mtk_timer.c
index 237c20b..ae95b29 100644
--- a/drivers/clocksource/mtk_timer.c
+++ b/drivers/clocksource/mtk_timer.c
@@ -24,6 +24,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/sched_clock.h>
 #include <linux/slab.h>
 
 #define GPT_IRQ_EN_REG		0x00
@@ -59,6 +60,13 @@ struct mtk_clock_event_device {
 	struct clock_event_device dev;
 };
 
+static void __iomem *gpt_sched_reg __read_mostly;
+
+static u64 notrace mtk_read_sched_clock(void)
+{
+	return readl_relaxed(gpt_sched_reg);
+}
+
 static inline struct mtk_clock_event_device *to_mtk_clk(
 				struct clock_event_device *c)
 {
@@ -242,6 +250,8 @@ static void __init mtk_timer_init(struct device_node *node)
 	mtk_timer_setup(evt, GPT_CLK_SRC, TIMER_CTRL_OP_FREERUN, true);
 	clocksource_mmio_init(evt->gpt_base + TIMER_CNT_REG(GPT_CLK_SRC),
 			node->name, rate, 300, 32, clocksource_mmio_readl_up);
+	gpt_sched_reg = evt->gpt_base + TIMER_CNT_REG(GPT_CLK_SRC);
+	sched_clock_register(mtk_read_sched_clock, 32, rate);
 
 	/* Configure clock event */
 	mtk_timer_setup(evt, GPT_CLK_EVT, TIMER_CTRL_OP_REPEAT, false);
-- 
1.8.1.1.dirty

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] clocksource: mediatek: Use GPT as sched clock source

[Stephen Boyd]

On 07/13, Yingjoe Chen wrote:
> When cpu is in deep idle, arch timer will stop counting. Setup GPT as
> sched clock source so it can keep counting in idle.
> 
> Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---

Acked-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] clocksource: mediatek: Use GPT as sched clock source

[Matthias Brugger]

On Monday, July 13, 2015 05:32:46 PM Yingjoe Chen wrote:
> When cpu is in deep idle, arch timer will stop counting. Setup GPT as
> sched clock source so it can keep counting in idle.
> 
> Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---

Acked-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] arm64: dts: mt8173: add timer node

[Yingjoe Chen]

From: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Add device node to enable GPT timer. This timer will be
used as sched clock source.

Signed-off-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Eddie Huang <eddie.huang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 0696f8f..04bdd8f 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -219,6 +219,15 @@
 			reg = <0 0x10007000 0 0x100>;
 		};
 
+		timer: timer@10008000 {
+			compatible = "mediatek,mt8173-timer",
+				     "mediatek,mt6577-timer";
+			reg = <0 0x10008000 0 0x1000>;
+			interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&infracfg CLK_INFRA_CLK_13M>,
+				 <&topckgen CLK_TOP_RTC_SEL>;
+		};
+
 		pwrap: pwrap@1000d000 {
 			compatible = "mediatek,mt8173-pwrap";
 			reg = <0 0x1000d000 0 0x1000>;
-- 
1.8.1.1.dirty

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] arm64: dts: mt8173: add timer node

[Daniel Kurtz]

On Mon, Jul 13, 2015 at 5:32 PM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> From: Daniel Kurtz <djkurtz@chromium.org>
>
> Add device node to enable GPT timer. This timer will be
> used as sched clock source.
>
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
> Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>

This binding needs documentation.

> ---
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index 0696f8f..04bdd8f 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -219,6 +219,15 @@
>                         reg = <0 0x10007000 0 0x100>;
>                 };
>
> +               timer: timer@10008000 {
> +                       compatible = "mediatek,mt8173-timer",
> +                                    "mediatek,mt6577-timer";
> +                       reg = <0 0x10008000 0 0x1000>;
> +                       interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_LOW>;
> +                       clocks = <&infracfg CLK_INFRA_CLK_13M>,
> +                                <&topckgen CLK_TOP_RTC_SEL>;

Why two clocks?  The driver only uses one.
Please use a clock-names property.

Thanks,
-Dan

> +               };
> +
>                 pwrap: pwrap@1000d000 {
>                         compatible = "mediatek,mt8173-pwrap";
>                         reg = <0 0x1000d000 0 0x1000>;
> --
> 1.8.1.1.dirty
>

Re: [PATCH 5/5] arm64: dts: mt8173: add timer node

[Daniel Kurtz]

On Tue, Jul 14, 2015 at 12:26 PM, Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Mon, Jul 13, 2015 at 5:32 PM, Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> From: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>
>> Add device node to enable GPT timer. This timer will be
>> used as sched clock source.
>>
>> Signed-off-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Signed-off-by: Eddie Huang <eddie.huang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>
> This binding needs documentation.

Whoops.  I just found it at:
Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt

>
>> ---
>>  arch/arm64/boot/dts/mediatek/mt8173.dtsi | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> index 0696f8f..04bdd8f 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> @@ -219,6 +219,15 @@
>>                         reg = <0 0x10007000 0 0x100>;
>>                 };
>>
>> +               timer: timer@10008000 {
>> +                       compatible = "mediatek,mt8173-timer",
>> +                                    "mediatek,mt6577-timer";
>> +                       reg = <0 0x10008000 0 0x1000>;
>> +                       interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_LOW>;
>> +                       clocks = <&infracfg CLK_INFRA_CLK_13M>,
>> +                                <&topckgen CLK_TOP_RTC_SEL>;
>
> Why two clocks?  The driver only uses one.
> Please use a clock-names property.
>
> Thanks,
> -Dan
>
>> +               };
>> +
>>                 pwrap: pwrap@1000d000 {
>>                         compatible = "mediatek,mt8173-pwrap";
>>                         reg = <0 0x1000d000 0 0x1000>;
>> --
>> 1.8.1.1.dirty
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] add GPT timer support for mt8173

[Daniel Kurtz]

On Mon, Jul 13, 2015 at 5:32 PM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> This series add GPT timer support for mt8173. This is based on v4.2-rc1
> and Matthias' next branch (for dts parts).
>
> The first 2 patches comes from 'Add SMP bringup support for mt65xx socs'
> series [1]. I decide to move these 2 patches to this series, since it
> is more relevent here. They are changed based on Matthias' and Daniel's
> comments.
>
> [1] http://lists.infradead.org/pipermail/linux-mediatek/2015-May/000714.html
>
> Daniel Kurtz (1):
>   arm64: dts: mt8173: add timer node
>
> Yingjoe Chen (4):
>   clocksource: mediatek: do not enable GPT_CLK_EVT when setup
>   clocksource: mediatek: Use GPT as sched clock source
>   arm64: mediatek: enable MTK_TIMER
>   clk: mediatek: add 13mhz clock for MT8173

Was this mediatek clocksource patch set get forgotten again?
All 5 patches have have been reviewed, and I think the ones that need
it have been Ack'ed by Matthias and/or Stephen.
What is the plan for merging them?

Thanks,
-Dan

>
>  arch/arm64/Kconfig                       |  1 +
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi |  9 +++++++++
>  drivers/clk/mediatek/clk-mt8173.c        |  5 +++++
>  drivers/clocksource/mtk_timer.c          | 26 ++++++++++++++++++++------
>  include/dt-bindings/clock/mt8173-clk.h   |  3 ++-
>  5 files changed, 37 insertions(+), 7 deletions(-)
>

Re: [PATCH 0/5] add GPT timer support for mt8173

[Daniel Lezcano]

On 08/11/2015 05:54 PM, Daniel Kurtz wrote:
> On Mon, Jul 13, 2015 at 5:32 PM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
>> This series add GPT timer support for mt8173. This is based on v4.2-rc1
>> and Matthias' next branch (for dts parts).
>>
>> The first 2 patches comes from 'Add SMP bringup support for mt65xx socs'
>> series [1]. I decide to move these 2 patches to this series, since it
>> is more relevent here. They are changed based on Matthias' and Daniel's
>> comments.
>>
>> [1] http://lists.infradead.org/pipermail/linux-mediatek/2015-May/000714.html
>>
>> Daniel Kurtz (1):
>>    arm64: dts: mt8173: add timer node
>>
>> Yingjoe Chen (4):
>>    clocksource: mediatek: do not enable GPT_CLK_EVT when setup
>>    clocksource: mediatek: Use GPT as sched clock source
>>    arm64: mediatek: enable MTK_TIMER
>>    clk: mediatek: add 13mhz clock for MT8173
>
> Was this mediatek clocksource patch set get forgotten again?

Yep. Sounds like it felt at the wrong moment.

> All 5 patches have have been reviewed, and I think the ones that need
> it have been Ack'ed by Matthias and/or Stephen.
> What is the plan for merging them?

So if I am not wrong we have two patches I can take in my tree as a 
couple of fixes (1 and 2), right ?




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

Re: [PATCH 0/5] add GPT timer support for mt8173

[Daniel Kurtz]

On Wed, Aug 12, 2015 at 8:32 PM, Daniel Lezcano
<daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 08/11/2015 05:54 PM, Daniel Kurtz wrote:
>>
>> On Mon, Jul 13, 2015 at 5:32 PM, Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> wrote:
>>>
>>> This series add GPT timer support for mt8173. This is based on v4.2-rc1
>>> and Matthias' next branch (for dts parts).
>>>
>>> The first 2 patches comes from 'Add SMP bringup support for mt65xx socs'
>>> series [1]. I decide to move these 2 patches to this series, since it
>>> is more relevent here. They are changed based on Matthias' and Daniel's
>>> comments.
>>>
>>> [1]
>>> http://lists.infradead.org/pipermail/linux-mediatek/2015-May/000714.html
>>>
>>> Daniel Kurtz (1):
>>>    arm64: dts: mt8173: add timer node
>>>
>>> Yingjoe Chen (4):
>>>    clocksource: mediatek: do not enable GPT_CLK_EVT when setup
>>>    clocksource: mediatek: Use GPT as sched clock source
>>>    arm64: mediatek: enable MTK_TIMER
>>>    clk: mediatek: add 13mhz clock for MT8173
>>
>>
>> Was this mediatek clocksource patch set get forgotten again?
>
>
> Yep. Sounds like it felt at the wrong moment.
>
>> All 5 patches have have been reviewed, and I think the ones that need
>> it have been Ack'ed by Matthias and/or Stephen.
>> What is the plan for merging them?
>
>
> So if I am not wrong we have two patches I can take in my tree as a couple
> of fixes (1 and 2), right ?

Yes.
Patches 3 & 5 are for Matthias, and patch 4 is Ack'ed by Stephen Boyd,
so I assume Matthias can pick that one up too.

Matthias, is that correct?
Or does Patch 4 need to go in via a clock maintainer's tree?

-Dan

Re: [PATCH 0/5] add GPT timer support for mt8173

[Daniel Lezcano]

On 07/13/2015 11:32 AM, Yingjoe Chen wrote:
> This series add GPT timer support for mt8173. This is based on v4.2-rc1
> and Matthias' next branch (for dts parts).
>
> The first 2 patches comes from 'Add SMP bringup support for mt65xx socs'
> series [1]. I decide to move these 2 patches to this series, since it
> is more relevent here. They are changed based on Matthias' and Daniel's
> comments.
>
> [1] http://lists.infradead.org/pipermail/linux-mediatek/2015-May/000714.html
>
> Daniel Kurtz (1):
>    arm64: dts: mt8173: add timer node
>
> Yingjoe Chen (4):
>    clocksource: mediatek: do not enable GPT_CLK_EVT when setup
>    clocksource: mediatek: Use GPT as sched clock source
>    arm64: mediatek: enable MTK_TIMER
>    clk: mediatek: add 13mhz clock for MT8173
>
>   arch/arm64/Kconfig                       |  1 +
>   arch/arm64/boot/dts/mediatek/mt8173.dtsi |  9 +++++++++
>   drivers/clk/mediatek/clk-mt8173.c        |  5 +++++
>   drivers/clocksource/mtk_timer.c          | 26 ++++++++++++++++++++------
>   include/dt-bindings/clock/mt8173-clk.h   |  3 ++-
>   5 files changed, 37 insertions(+), 7 deletions(-)

Who will take this patchset ? I can take the patch 2 if needed.


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

Re: [PATCH 0/5] add GPT timer support for mt8173

[Yingjoe Chen]

On Wed, 2015-08-26 at 09:37 +0200, Daniel Lezcano wrote:
> On 07/13/2015 11:32 AM, Yingjoe Chen wrote:
> > This series add GPT timer support for mt8173. This is based on v4.2-rc1
> > and Matthias' next branch (for dts parts).
> >
> > The first 2 patches comes from 'Add SMP bringup support for mt65xx socs'
> > series [1]. I decide to move these 2 patches to this series, since it
> > is more relevent here. They are changed based on Matthias' and Daniel's
> > comments.
> >
> > [1] http://lists.infradead.org/pipermail/linux-mediatek/2015-May/000714.html
> >
> > Daniel Kurtz (1):
> >    arm64: dts: mt8173: add timer node
> >
> > Yingjoe Chen (4):
> >    clocksource: mediatek: do not enable GPT_CLK_EVT when setup
> >    clocksource: mediatek: Use GPT as sched clock source
> >    arm64: mediatek: enable MTK_TIMER
> >    clk: mediatek: add 13mhz clock for MT8173
> >
> >   arch/arm64/Kconfig                       |  1 +
> >   arch/arm64/boot/dts/mediatek/mt8173.dtsi |  9 +++++++++
> >   drivers/clk/mediatek/clk-mt8173.c        |  5 +++++
> >   drivers/clocksource/mtk_timer.c          | 26 ++++++++++++++++++++------
> >   include/dt-bindings/clock/mt8173-clk.h   |  3 ++-
> >   5 files changed, 37 insertions(+), 7 deletions(-)
> 
> Who will take this patchset ? I can take the patch 2 if needed.
> 
> 

Hi Daniel,

Please take patch 2 with your fix. Thanks.

I think patch 3(enable MTK_TIMER in Kconfig) and 5 (add timer node)
should go through Matthias' tree to arm-soc and patch 4(add 13mhz clock)
should go through James' mtk-clk tree to clk maintainer.

Joe.C


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html