* [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-09-17 14:56 ` Yingjoe Chen
0 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-09-17 14:56 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
>
> On 16/09/15 03:04, Yingjoe Chen wrote:
> > From: Daniel Kurtz <djkurtz@chromium.org>
> >
> > Add device node to enable GPT timer. This timer will be
> > used as sched clock source.
> >
>
> Interesting any known issues with or advantage over the arch timers
> to prefer it as sched clock source. I see even arch timers are present
> in DT, hence the question. Or is it just a incorrect commit log ?
>
> How does this get selected as sched clock source ? I don't see
> sched_clock_register in mtk_timer.c
>
> To be clear, I am not against adding this timer support, but just want
> to know is it preferred for sched clock source ? if yes why ? better
> resolution ?
Hi Sudeep,
Thanks for your review.
I hit the send too soon and missed cover letter, please see:
http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
The main reason to use GPT as sched clock is it won't stop during idle.
> > Change-Id: Idc4e3f0ee80b5c36cae6f0f2328f94aafcca1253
>
> ^ Should be dropped
>
> > 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>
> > ---
> > 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 d18ee42..d763803 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> > @@ -238,6 +238,15 @@
> > reg = <0 0x10007000 0 0x100>;
> > };
> >
> > + timer: timer at 10008000 {
> > + compatible = "mediatek,mt8173-timer",
>
> Missing documentation ? I am referring upstream and it might be in some
> patches already queued perhaps ?
This is documented in
Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt.
Do you mean I should add "mediatek,mt8173-timer" to that file?
Joe.C
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-09-17 14:56 ` Yingjoe Chen
0 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-09-17 14:56 UTC (permalink / raw)
To: Sudeep Holla
Cc: Matthias Brugger, Daniel Lezcano, Thomas Gleixner, Stephen Boyd,
Michael Turquette, James Liao,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Arnd Bergmann,
Catalin Marinas,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Sascha Hauer, Olof Johansson,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Daniel Kurtz, linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
>
> On 16/09/15 03:04, Yingjoe Chen 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.
> >
>
> Interesting any known issues with or advantage over the arch timers
> to prefer it as sched clock source. I see even arch timers are present
> in DT, hence the question. Or is it just a incorrect commit log ?
>
> How does this get selected as sched clock source ? I don't see
> sched_clock_register in mtk_timer.c
>
> To be clear, I am not against adding this timer support, but just want
> to know is it preferred for sched clock source ? if yes why ? better
> resolution ?
Hi Sudeep,
Thanks for your review.
I hit the send too soon and missed cover letter, please see:
http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
The main reason to use GPT as sched clock is it won't stop during idle.
> > Change-Id: Idc4e3f0ee80b5c36cae6f0f2328f94aafcca1253
>
> ^ Should be dropped
>
> > 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 d18ee42..d763803 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> > @@ -238,6 +238,15 @@
> > reg = <0 0x10007000 0 0x100>;
> > };
> >
> > + timer: timer@10008000 {
> > + compatible = "mediatek,mt8173-timer",
>
> Missing documentation ? I am referring upstream and it might be in some
> patches already queued perhaps ?
This is documented in
Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt.
Do you mean I should add "mediatek,mt8173-timer" to that file?
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
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
2015-09-17 14:56 ` Yingjoe Chen
(?)
@ 2015-09-17 16:13 ` Sudeep Holla
-1 siblings, 0 replies; 35+ messages in thread
From: Sudeep Holla @ 2015-09-17 16:13 UTC (permalink / raw)
To: Yingjoe Chen
Cc: Sudeep Holla, Matthias Brugger, Daniel Lezcano, Thomas Gleixner,
Stephen Boyd, Michael Turquette, James Liao,
devicetree@vger.kernel.org, Arnd Bergmann, Catalin Marinas,
linux-kernel@vger.kernel.org, Rob Herring,
linux-mediatek@lists.infradead.org, Sascha Hauer, Olof Johansson,
srv_heupstream@mediatek.com, linux-arm-kernel@lists.infradead.org,
Daniel Kurtz, linux-clk@vger.kernel.org, Eddie Huang
On 17/09/15 15:56, Yingjoe Chen wrote:
> On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
>>
>> On 16/09/15 03:04, Yingjoe Chen wrote:
>>> From: Daniel Kurtz <djkurtz@chromium.org>
>>>
>>> Add device node to enable GPT timer. This timer will be
>>> used as sched clock source.
>>>
>>
>> Interesting any known issues with or advantage over the arch timers
>> to prefer it as sched clock source. I see even arch timers are present
>> in DT, hence the question. Or is it just a incorrect commit log ?
>>
>> How does this get selected as sched clock source ? I don't see
>> sched_clock_register in mtk_timer.c
>>
>> To be clear, I am not against adding this timer support, but just want
>> to know is it preferred for sched clock source ? if yes why ? better
>> resolution ?
>
> Hi Sudeep,
>
> Thanks for your review.
>
> I hit the send too soon and missed cover letter, please see:
> http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
>
OK
> The main reason to use GPT as sched clock is it won't stop during idle.
>
>
I think your are confusing the system counter with arch timers. System
counter is always-on, but the arch timers(logic implementing timers
comparators) might not be off when the processor is powered down.
I think you need this timer and are using it for low power idle states
in which case you will use this as a clock event and not clock source.
It will be used as a hardware broadcast event source.
There's no call to sched_clock_register in mtk_timer.c, so it can't be
the sched clock, so you need to fix the commit log.
[...]
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> index d18ee42..d763803 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> @@ -238,6 +238,15 @@
>>> reg = <0 0x10007000 0 0x100>;
>>> };
>>>
>>> + timer: timer@10008000 {
>>> + compatible = "mediatek,mt8173-timer",
>>
>> Missing documentation ? I am referring upstream and it might be in some
>> patches already queued perhaps ?
>
> This is documented in
> Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt.
> Do you mean I should add "mediatek,mt8173-timer" to that file?
>
Yes
Regards,
Sudeep
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-09-17 16:13 ` Sudeep Holla
0 siblings, 0 replies; 35+ messages in thread
From: Sudeep Holla @ 2015-09-17 16:13 UTC (permalink / raw)
To: linux-arm-kernel
On 17/09/15 15:56, Yingjoe Chen wrote:
> On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
>>
>> On 16/09/15 03:04, Yingjoe Chen wrote:
>>> From: Daniel Kurtz <djkurtz@chromium.org>
>>>
>>> Add device node to enable GPT timer. This timer will be
>>> used as sched clock source.
>>>
>>
>> Interesting any known issues with or advantage over the arch timers
>> to prefer it as sched clock source. I see even arch timers are present
>> in DT, hence the question. Or is it just a incorrect commit log ?
>>
>> How does this get selected as sched clock source ? I don't see
>> sched_clock_register in mtk_timer.c
>>
>> To be clear, I am not against adding this timer support, but just want
>> to know is it preferred for sched clock source ? if yes why ? better
>> resolution ?
>
> Hi Sudeep,
>
> Thanks for your review.
>
> I hit the send too soon and missed cover letter, please see:
> http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
>
OK
> The main reason to use GPT as sched clock is it won't stop during idle.
>
>
I think your are confusing the system counter with arch timers. System
counter is always-on, but the arch timers(logic implementing timers
comparators) might not be off when the processor is powered down.
I think you need this timer and are using it for low power idle states
in which case you will use this as a clock event and not clock source.
It will be used as a hardware broadcast event source.
There's no call to sched_clock_register in mtk_timer.c, so it can't be
the sched clock, so you need to fix the commit log.
[...]
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> index d18ee42..d763803 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> @@ -238,6 +238,15 @@
>>> reg = <0 0x10007000 0 0x100>;
>>> };
>>>
>>> + timer: timer at 10008000 {
>>> + compatible = "mediatek,mt8173-timer",
>>
>> Missing documentation ? I am referring upstream and it might be in some
>> patches already queued perhaps ?
>
> This is documented in
> Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt.
> Do you mean I should add "mediatek,mt8173-timer" to that file?
>
Yes
Regards,
Sudeep
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-09-17 16:13 ` Sudeep Holla
0 siblings, 0 replies; 35+ messages in thread
From: Sudeep Holla @ 2015-09-17 16:13 UTC (permalink / raw)
To: Yingjoe Chen
Cc: Sudeep Holla, Matthias Brugger, Daniel Lezcano, Thomas Gleixner,
Stephen Boyd, Michael Turquette, James Liao,
devicetree@vger.kernel.org, Arnd Bergmann, Catalin Marinas,
linux-kernel@vger.kernel.org, Rob Herring,
linux-mediatek@lists.infradead.org, Sascha Hauer, Olof Johansson,
srv_heupstream@mediatek.com, linux-arm-kernel@lists.infradead.org,
Daniel Kurtz,
"linux-clk@vger.kernel.org" <linux-cl>
On 17/09/15 15:56, Yingjoe Chen wrote:
> On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
>>
>> On 16/09/15 03:04, Yingjoe Chen wrote:
>>> From: Daniel Kurtz <djkurtz@chromium.org>
>>>
>>> Add device node to enable GPT timer. This timer will be
>>> used as sched clock source.
>>>
>>
>> Interesting any known issues with or advantage over the arch timers
>> to prefer it as sched clock source. I see even arch timers are present
>> in DT, hence the question. Or is it just a incorrect commit log ?
>>
>> How does this get selected as sched clock source ? I don't see
>> sched_clock_register in mtk_timer.c
>>
>> To be clear, I am not against adding this timer support, but just want
>> to know is it preferred for sched clock source ? if yes why ? better
>> resolution ?
>
> Hi Sudeep,
>
> Thanks for your review.
>
> I hit the send too soon and missed cover letter, please see:
> http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
>
OK
> The main reason to use GPT as sched clock is it won't stop during idle.
>
>
I think your are confusing the system counter with arch timers. System
counter is always-on, but the arch timers(logic implementing timers
comparators) might not be off when the processor is powered down.
I think you need this timer and are using it for low power idle states
in which case you will use this as a clock event and not clock source.
It will be used as a hardware broadcast event source.
There's no call to sched_clock_register in mtk_timer.c, so it can't be
the sched clock, so you need to fix the commit log.
[...]
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> index d18ee42..d763803 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>>> @@ -238,6 +238,15 @@
>>> reg = <0 0x10007000 0 0x100>;
>>> };
>>>
>>> + timer: timer@10008000 {
>>> + compatible = "mediatek,mt8173-timer",
>>
>> Missing documentation ? I am referring upstream and it might be in some
>> patches already queued perhaps ?
>
> This is documented in
> Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt.
> Do you mean I should add "mediatek,mt8173-timer" to that file?
>
Yes
Regards,
Sudeep
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
2015-09-17 16:13 ` Sudeep Holla
(?)
@ 2015-10-01 14:33 ` Yingjoe Chen
-1 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-10-01 14:33 UTC (permalink / raw)
To: Sudeep Holla
Cc: Matthias Brugger, Daniel Lezcano, Thomas Gleixner, Stephen Boyd,
Michael Turquette, James Liao, devicetree@vger.kernel.org,
Arnd Bergmann, Catalin Marinas, linux-kernel@vger.kernel.org,
Rob Herring, linux-mediatek@lists.infradead.org, Sascha Hauer,
Olof Johansson, srv_heupstream@mediatek.com,
linux-arm-kernel@lists.infradead.org, Daniel Kurtz,
linux-clk@vger.kernel.org, Eddie Huang
On Thu, 2015-09-17 at 17:13 +0100, Sudeep Holla wrote:
>
> On 17/09/15 15:56, Yingjoe Chen wrote:
> > On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
> >>
> >> On 16/09/15 03:04, Yingjoe Chen wrote:
> >>> From: Daniel Kurtz <djkurtz@chromium.org>
> >>>
> >>> Add device node to enable GPT timer. This timer will be
> >>> used as sched clock source.
> >>>
> >>
> >> Interesting any known issues with or advantage over the arch timers
> >> to prefer it as sched clock source. I see even arch timers are present
> >> in DT, hence the question. Or is it just a incorrect commit log ?
> >>
> >> How does this get selected as sched clock source ? I don't see
> >> sched_clock_register in mtk_timer.c
> >>
> >> To be clear, I am not against adding this timer support, but just want
> >> to know is it preferred for sched clock source ? if yes why ? better
> >> resolution ?
> >
> > Hi Sudeep,
> >
> > Thanks for your review.
> >
> > I hit the send too soon and missed cover letter, please see:
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
> >
>
> OK
>
> > The main reason to use GPT as sched clock is it won't stop during idle.
> >
> >
>
> I think your are confusing the system counter with arch timers. System
> counter is always-on, but the arch timers(logic implementing timers
> comparators) might not be off when the processor is powered down.
>
> I think you need this timer and are using it for low power idle states
> in which case you will use this as a clock event and not clock source.
> It will be used as a hardware broadcast event source.
>
> There's no call to sched_clock_register in mtk_timer.c, so it can't be
> the sched clock, so you need to fix the commit log.
Hi Sudeep,
Sorry for late reply.
For sched_clock_register, please see
http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001547.html
which was accepted in
https://git.linaro.org/people/daniel.lezcano/linux.git/shortlog/refs/heads/clockevents/4.4
You are right it is also used as clock event. I think we don't need to
mention those detail in commit message, so I'll change to just:
"Add device node to enable GPT timer."
>
> [...]
>
> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>> index d18ee42..d763803 100644
> >>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>> @@ -238,6 +238,15 @@
> >>> reg = <0 0x10007000 0 0x100>;
> >>> };
> >>>
> >>> + timer: timer@10008000 {
> >>> + compatible = "mediatek,mt8173-timer",
> >>
> >> Missing documentation ? I am referring upstream and it might be in some
> >> patches already queued perhaps ?
> >
> > This is documented in
> > Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt.
> > Do you mean I should add "mediatek,mt8173-timer" to that file?
> >
>
> Yes
Will do in next round.
Thanks
Joe.C
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-10-01 14:33 ` Yingjoe Chen
0 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-10-01 14:33 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2015-09-17 at 17:13 +0100, Sudeep Holla wrote:
>
> On 17/09/15 15:56, Yingjoe Chen wrote:
> > On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
> >>
> >> On 16/09/15 03:04, Yingjoe Chen wrote:
> >>> From: Daniel Kurtz <djkurtz@chromium.org>
> >>>
> >>> Add device node to enable GPT timer. This timer will be
> >>> used as sched clock source.
> >>>
> >>
> >> Interesting any known issues with or advantage over the arch timers
> >> to prefer it as sched clock source. I see even arch timers are present
> >> in DT, hence the question. Or is it just a incorrect commit log ?
> >>
> >> How does this get selected as sched clock source ? I don't see
> >> sched_clock_register in mtk_timer.c
> >>
> >> To be clear, I am not against adding this timer support, but just want
> >> to know is it preferred for sched clock source ? if yes why ? better
> >> resolution ?
> >
> > Hi Sudeep,
> >
> > Thanks for your review.
> >
> > I hit the send too soon and missed cover letter, please see:
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
> >
>
> OK
>
> > The main reason to use GPT as sched clock is it won't stop during idle.
> >
> >
>
> I think your are confusing the system counter with arch timers. System
> counter is always-on, but the arch timers(logic implementing timers
> comparators) might not be off when the processor is powered down.
>
> I think you need this timer and are using it for low power idle states
> in which case you will use this as a clock event and not clock source.
> It will be used as a hardware broadcast event source.
>
> There's no call to sched_clock_register in mtk_timer.c, so it can't be
> the sched clock, so you need to fix the commit log.
Hi Sudeep,
Sorry for late reply.
For sched_clock_register, please see
http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001547.html
which was accepted in
https://git.linaro.org/people/daniel.lezcano/linux.git/shortlog/refs/heads/clockevents/4.4
You are right it is also used as clock event. I think we don't need to
mention those detail in commit message, so I'll change to just:
"Add device node to enable GPT timer."
>
> [...]
>
> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>> index d18ee42..d763803 100644
> >>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>> @@ -238,6 +238,15 @@
> >>> reg = <0 0x10007000 0 0x100>;
> >>> };
> >>>
> >>> + timer: timer at 10008000 {
> >>> + compatible = "mediatek,mt8173-timer",
> >>
> >> Missing documentation ? I am referring upstream and it might be in some
> >> patches already queued perhaps ?
> >
> > This is documented in
> > Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt.
> > Do you mean I should add "mediatek,mt8173-timer" to that file?
> >
>
> Yes
Will do in next round.
Thanks
Joe.C
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-10-01 14:33 ` Yingjoe Chen
0 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-10-01 14:33 UTC (permalink / raw)
To: Sudeep Holla
Cc: Matthias Brugger, Daniel Lezcano, Thomas Gleixner, Stephen Boyd,
Michael Turquette, James Liao, devicetree@vger.kernel.org,
Arnd Bergmann, Catalin Marinas, linux-kernel@vger.kernel.org,
Rob Herring, linux-mediatek@lists.infradead.org, Sascha Hauer,
Olof Johansson, srv_heupstream@mediatek.com,
linux-arm-kernel@lists.infradead.org, Daniel Kurtz,
linux-clk@vger.kernel.org
On Thu, 2015-09-17 at 17:13 +0100, Sudeep Holla wrote:
>
> On 17/09/15 15:56, Yingjoe Chen wrote:
> > On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
> >>
> >> On 16/09/15 03:04, Yingjoe Chen wrote:
> >>> From: Daniel Kurtz <djkurtz@chromium.org>
> >>>
> >>> Add device node to enable GPT timer. This timer will be
> >>> used as sched clock source.
> >>>
> >>
> >> Interesting any known issues with or advantage over the arch timers
> >> to prefer it as sched clock source. I see even arch timers are present
> >> in DT, hence the question. Or is it just a incorrect commit log ?
> >>
> >> How does this get selected as sched clock source ? I don't see
> >> sched_clock_register in mtk_timer.c
> >>
> >> To be clear, I am not against adding this timer support, but just want
> >> to know is it preferred for sched clock source ? if yes why ? better
> >> resolution ?
> >
> > Hi Sudeep,
> >
> > Thanks for your review.
> >
> > I hit the send too soon and missed cover letter, please see:
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
> >
>
> OK
>
> > The main reason to use GPT as sched clock is it won't stop during idle.
> >
> >
>
> I think your are confusing the system counter with arch timers. System
> counter is always-on, but the arch timers(logic implementing timers
> comparators) might not be off when the processor is powered down.
>
> I think you need this timer and are using it for low power idle states
> in which case you will use this as a clock event and not clock source.
> It will be used as a hardware broadcast event source.
>
> There's no call to sched_clock_register in mtk_timer.c, so it can't be
> the sched clock, so you need to fix the commit log.
Hi Sudeep,
Sorry for late reply.
For sched_clock_register, please see
http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001547.html
which was accepted in
https://git.linaro.org/people/daniel.lezcano/linux.git/shortlog/refs/heads/clockevents/4.4
You are right it is also used as clock event. I think we don't need to
mention those detail in commit message, so I'll change to just:
"Add device node to enable GPT timer."
>
> [...]
>
> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>> index d18ee42..d763803 100644
> >>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>> @@ -238,6 +238,15 @@
> >>> reg = <0 0x10007000 0 0x100>;
> >>> };
> >>>
> >>> + timer: timer@10008000 {
> >>> + compatible = "mediatek,mt8173-timer",
> >>
> >> Missing documentation ? I am referring upstream and it might be in some
> >> patches already queued perhaps ?
> >
> > This is documented in
> > Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt.
> > Do you mean I should add "mediatek,mt8173-timer" to that file?
> >
>
> Yes
Will do in next round.
Thanks
Joe.C
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
2015-10-01 14:33 ` Yingjoe Chen
(?)
@ 2015-10-01 15:32 ` Sudeep Holla
-1 siblings, 0 replies; 35+ messages in thread
From: Sudeep Holla @ 2015-10-01 15:32 UTC (permalink / raw)
To: Yingjoe Chen
Cc: Sudeep Holla, Matthias Brugger, Daniel Lezcano, Thomas Gleixner,
Stephen Boyd, Michael Turquette, James Liao,
devicetree@vger.kernel.org, Arnd Bergmann, Catalin Marinas,
linux-kernel@vger.kernel.org, Rob Herring,
linux-mediatek@lists.infradead.org, Sascha Hauer, Olof Johansson,
srv_heupstream@mediatek.com, linux-arm-kernel@lists.infradead.org,
Daniel Kurtz, linux-clk@vger.kernel.org, Eddie Huang
On 01/10/15 15:33, Yingjoe Chen wrote:
> On Thu, 2015-09-17 at 17:13 +0100, Sudeep Holla wrote:
>>
[...]
>>
>> I think your are confusing the system counter with arch timers. System
>> counter is always-on, but the arch timers(logic implementing timers
>> comparators) might not be off when the processor is powered down.
>>
>> I think you need this timer and are using it for low power idle states
>> in which case you will use this as a clock event and not clock source.
>> It will be used as a hardware broadcast event source.
>>
>> There's no call to sched_clock_register in mtk_timer.c, so it can't be
>> the sched clock, so you need to fix the commit log.
>
> Hi Sudeep,
>
> Sorry for late reply.
>
> For sched_clock_register, please see
> http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001547.html
> which was accepted in
> https://git.linaro.org/people/daniel.lezcano/linux.git/shortlog/refs/heads/clockevents/4.4
>
The commit message makes no sense to me. The counters should continue to
work as long as they are in always-on domain. Only timers are lost
when you enter deeper idle states. So I agree with using MTK timer as
broadcast timer/eventsource. You still didn't answer what's the need
to use MTK timer as sched clocksource ?
Regards,
Sudeep
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-10-01 15:32 ` Sudeep Holla
0 siblings, 0 replies; 35+ messages in thread
From: Sudeep Holla @ 2015-10-01 15:32 UTC (permalink / raw)
To: linux-arm-kernel
On 01/10/15 15:33, Yingjoe Chen wrote:
> On Thu, 2015-09-17 at 17:13 +0100, Sudeep Holla wrote:
>>
[...]
>>
>> I think your are confusing the system counter with arch timers. System
>> counter is always-on, but the arch timers(logic implementing timers
>> comparators) might not be off when the processor is powered down.
>>
>> I think you need this timer and are using it for low power idle states
>> in which case you will use this as a clock event and not clock source.
>> It will be used as a hardware broadcast event source.
>>
>> There's no call to sched_clock_register in mtk_timer.c, so it can't be
>> the sched clock, so you need to fix the commit log.
>
> Hi Sudeep,
>
> Sorry for late reply.
>
> For sched_clock_register, please see
> http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001547.html
> which was accepted in
> https://git.linaro.org/people/daniel.lezcano/linux.git/shortlog/refs/heads/clockevents/4.4
>
The commit message makes no sense to me. The counters should continue to
work as long as they are in always-on domain. Only timers are lost
when you enter deeper idle states. So I agree with using MTK timer as
broadcast timer/eventsource. You still didn't answer what's the need
to use MTK timer as sched clocksource ?
Regards,
Sudeep
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-10-01 15:32 ` Sudeep Holla
0 siblings, 0 replies; 35+ messages in thread
From: Sudeep Holla @ 2015-10-01 15:32 UTC (permalink / raw)
To: Yingjoe Chen
Cc: Sudeep Holla, Matthias Brugger, Daniel Lezcano, Thomas Gleixner,
Stephen Boyd, Michael Turquette, James Liao,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Arnd Bergmann,
Catalin Marinas,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Sascha Hauer, Olof Johansson,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Daniel Kurtz,
"linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-cl>
On 01/10/15 15:33, Yingjoe Chen wrote:
> On Thu, 2015-09-17 at 17:13 +0100, Sudeep Holla wrote:
>>
[...]
>>
>> I think your are confusing the system counter with arch timers. System
>> counter is always-on, but the arch timers(logic implementing timers
>> comparators) might not be off when the processor is powered down.
>>
>> I think you need this timer and are using it for low power idle states
>> in which case you will use this as a clock event and not clock source.
>> It will be used as a hardware broadcast event source.
>>
>> There's no call to sched_clock_register in mtk_timer.c, so it can't be
>> the sched clock, so you need to fix the commit log.
>
> Hi Sudeep,
>
> Sorry for late reply.
>
> For sched_clock_register, please see
> http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001547.html
> which was accepted in
> https://git.linaro.org/people/daniel.lezcano/linux.git/shortlog/refs/heads/clockevents/4.4
>
The commit message makes no sense to me. The counters should continue to
work as long as they are in always-on domain. Only timers are lost
when you enter deeper idle states. So I agree with using MTK timer as
broadcast timer/eventsource. You still didn't answer what's the need
to use MTK timer as sched clocksource ?
Regards,
Sudeep
--
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
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
2015-10-01 15:32 ` Sudeep Holla
(?)
@ 2015-10-02 14:00 ` Yingjoe Chen
-1 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-10-02 14:00 UTC (permalink / raw)
To: Sudeep Holla
Cc: Matthias Brugger, Daniel Lezcano, Thomas Gleixner, Stephen Boyd,
Michael Turquette, James Liao, devicetree@vger.kernel.org,
Arnd Bergmann, Catalin Marinas, linux-kernel@vger.kernel.org,
Rob Herring, linux-mediatek@lists.infradead.org, Sascha Hauer,
Olof Johansson, srv_heupstream@mediatek.com,
linux-arm-kernel@lists.infradead.org, Daniel Kurtz,
linux-clk@vger.kernel.org, Eddie Huang
On Thu, 2015-10-01 at 16:32 +0100, Sudeep Holla wrote:
>
> On 01/10/15 15:33, Yingjoe Chen wrote:
> > On Thu, 2015-09-17 at 17:13 +0100, Sudeep Holla wrote:
> >>
>
> [...]
>
> >>
> >> I think your are confusing the system counter with arch timers. System
> >> counter is always-on, but the arch timers(logic implementing timers
> >> comparators) might not be off when the processor is powered down.
> >>
> >> I think you need this timer and are using it for low power idle states
> >> in which case you will use this as a clock event and not clock source.
> >> It will be used as a hardware broadcast event source.
> >>
> >> There's no call to sched_clock_register in mtk_timer.c, so it can't be
> >> the sched clock, so you need to fix the commit log.
> >
> > Hi Sudeep,
> >
> > Sorry for late reply.
> >
> > For sched_clock_register, please see
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001547.html
> > which was accepted in
> > https://git.linaro.org/people/daniel.lezcano/linux.git/shortlog/refs/heads/clockevents/4.4
> >
>
> The commit message makes no sense to me. The counters should continue to
> work as long as they are in always-on domain. Only timers are lost
> when you enter deeper idle states. So I agree with using MTK timer as
> broadcast timer/eventsource. You still didn't answer what's the need
> to use MTK timer as sched clocksource ?
Hi, Sudeep,
ARM ARM said the counter should be in always-on domain, but
unfortunately that not true for mt8173. The last CPU enter idle can
choose to enter deep idle mode and the counter value would be lost. Our
firmware backup/recover the counter so it looks like it is stopped.
That's why I thought we need to use it as sched clocksource.
On mt8173, we will fix the firmware to add missing counts, so it will
looks like the counter keep counting. But other mediatek platform have
similar issue, and the 2 counter have same resolution, so I still want
to keep using GPT as sched clocksource.
Joe.C
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-10-02 14:00 ` Yingjoe Chen
0 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-10-02 14:00 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2015-10-01 at 16:32 +0100, Sudeep Holla wrote:
>
> On 01/10/15 15:33, Yingjoe Chen wrote:
> > On Thu, 2015-09-17 at 17:13 +0100, Sudeep Holla wrote:
> >>
>
> [...]
>
> >>
> >> I think your are confusing the system counter with arch timers. System
> >> counter is always-on, but the arch timers(logic implementing timers
> >> comparators) might not be off when the processor is powered down.
> >>
> >> I think you need this timer and are using it for low power idle states
> >> in which case you will use this as a clock event and not clock source.
> >> It will be used as a hardware broadcast event source.
> >>
> >> There's no call to sched_clock_register in mtk_timer.c, so it can't be
> >> the sched clock, so you need to fix the commit log.
> >
> > Hi Sudeep,
> >
> > Sorry for late reply.
> >
> > For sched_clock_register, please see
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001547.html
> > which was accepted in
> > https://git.linaro.org/people/daniel.lezcano/linux.git/shortlog/refs/heads/clockevents/4.4
> >
>
> The commit message makes no sense to me. The counters should continue to
> work as long as they are in always-on domain. Only timers are lost
> when you enter deeper idle states. So I agree with using MTK timer as
> broadcast timer/eventsource. You still didn't answer what's the need
> to use MTK timer as sched clocksource ?
Hi, Sudeep,
ARM ARM said the counter should be in always-on domain, but
unfortunately that not true for mt8173. The last CPU enter idle can
choose to enter deep idle mode and the counter value would be lost. Our
firmware backup/recover the counter so it looks like it is stopped.
That's why I thought we need to use it as sched clocksource.
On mt8173, we will fix the firmware to add missing counts, so it will
looks like the counter keep counting. But other mediatek platform have
similar issue, and the 2 counter have same resolution, so I still want
to keep using GPT as sched clocksource.
Joe.C
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-10-02 14:00 ` Yingjoe Chen
0 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-10-02 14:00 UTC (permalink / raw)
To: Sudeep Holla
Cc: Matthias Brugger, Daniel Lezcano, Thomas Gleixner, Stephen Boyd,
Michael Turquette, James Liao, devicetree@vger.kernel.org,
Arnd Bergmann, Catalin Marinas, linux-kernel@vger.kernel.org,
Rob Herring, linux-mediatek@lists.infradead.org, Sascha Hauer,
Olof Johansson, srv_heupstream@mediatek.com,
linux-arm-kernel@lists.infradead.org, Daniel Kurtz,
linux-clk@vger.kernel.org
On Thu, 2015-10-01 at 16:32 +0100, Sudeep Holla wrote:
>
> On 01/10/15 15:33, Yingjoe Chen wrote:
> > On Thu, 2015-09-17 at 17:13 +0100, Sudeep Holla wrote:
> >>
>
> [...]
>
> >>
> >> I think your are confusing the system counter with arch timers. System
> >> counter is always-on, but the arch timers(logic implementing timers
> >> comparators) might not be off when the processor is powered down.
> >>
> >> I think you need this timer and are using it for low power idle states
> >> in which case you will use this as a clock event and not clock source.
> >> It will be used as a hardware broadcast event source.
> >>
> >> There's no call to sched_clock_register in mtk_timer.c, so it can't be
> >> the sched clock, so you need to fix the commit log.
> >
> > Hi Sudeep,
> >
> > Sorry for late reply.
> >
> > For sched_clock_register, please see
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001547.html
> > which was accepted in
> > https://git.linaro.org/people/daniel.lezcano/linux.git/shortlog/refs/heads/clockevents/4.4
> >
>
> The commit message makes no sense to me. The counters should continue to
> work as long as they are in always-on domain. Only timers are lost
> when you enter deeper idle states. So I agree with using MTK timer as
> broadcast timer/eventsource. You still didn't answer what's the need
> to use MTK timer as sched clocksource ?
Hi, Sudeep,
ARM ARM said the counter should be in always-on domain, but
unfortunately that not true for mt8173. The last CPU enter idle can
choose to enter deep idle mode and the counter value would be lost. Our
firmware backup/recover the counter so it looks like it is stopped.
That's why I thought we need to use it as sched clocksource.
On mt8173, we will fix the firmware to add missing counts, so it will
looks like the counter keep counting. But other mediatek platform have
similar issue, and the 2 counter have same resolution, so I still want
to keep using GPT as sched clocksource.
Joe.C
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
2015-09-17 14:56 ` Yingjoe Chen
(?)
@ 2015-09-17 16:41 ` Mark Rutland
-1 siblings, 0 replies; 35+ messages in thread
From: Mark Rutland @ 2015-09-17 16:41 UTC (permalink / raw)
To: Yingjoe Chen
Cc: Sudeep Holla, James Liao, srv_heupstream@mediatek.com,
Arnd Bergmann, devicetree@vger.kernel.org, Catalin Marinas,
Michael Turquette, Daniel Lezcano, Stephen Boyd,
linux-kernel@vger.kernel.org, Daniel Kurtz, Olof Johansson,
Rob Herring, linux-mediatek@lists.infradead.org, Sascha Hauer,
Matthias Brugger, Thomas Gleixner, Eddie Huang,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org
On Thu, Sep 17, 2015 at 03:56:56PM +0100, Yingjoe Chen wrote:
> On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
> >
> > On 16/09/15 03:04, Yingjoe Chen wrote:
> > > From: Daniel Kurtz <djkurtz@chromium.org>
> > >
> > > Add device node to enable GPT timer. This timer will be
> > > used as sched clock source.
> > >
> >
> > Interesting any known issues with or advantage over the arch timers
> > to prefer it as sched clock source. I see even arch timers are present
> > in DT, hence the question. Or is it just a incorrect commit log ?
> >
> > How does this get selected as sched clock source ? I don't see
> > sched_clock_register in mtk_timer.c
> >
> > To be clear, I am not against adding this timer support, but just want
> > to know is it preferred for sched clock source ? if yes why ? better
> > resolution ?
>
> Hi Sudeep,
>
> Thanks for your review.
>
> I hit the send too soon and missed cover letter, please see:
> http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
>
> The main reason to use GPT as sched clock is it won't stop during idle.
You don't mean sched clock, you just mean a clock_event_device.
A sched_clock is a high-precision clocksource that is read from (which
by definition requires the CPUs to be non-idle). It doesn't have
anything to do with interrupts and therefore cannot wake devices from
idle.
While the clock_event_device for the generic timer can't necessarily
wake CPUs from idle. The generic timer system counter counts even if
CPUs are idle, so the generic timer is fine as a sched_clock.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-09-17 16:41 ` Mark Rutland
0 siblings, 0 replies; 35+ messages in thread
From: Mark Rutland @ 2015-09-17 16:41 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 17, 2015 at 03:56:56PM +0100, Yingjoe Chen wrote:
> On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
> >
> > On 16/09/15 03:04, Yingjoe Chen wrote:
> > > From: Daniel Kurtz <djkurtz@chromium.org>
> > >
> > > Add device node to enable GPT timer. This timer will be
> > > used as sched clock source.
> > >
> >
> > Interesting any known issues with or advantage over the arch timers
> > to prefer it as sched clock source. I see even arch timers are present
> > in DT, hence the question. Or is it just a incorrect commit log ?
> >
> > How does this get selected as sched clock source ? I don't see
> > sched_clock_register in mtk_timer.c
> >
> > To be clear, I am not against adding this timer support, but just want
> > to know is it preferred for sched clock source ? if yes why ? better
> > resolution ?
>
> Hi Sudeep,
>
> Thanks for your review.
>
> I hit the send too soon and missed cover letter, please see:
> http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
>
> The main reason to use GPT as sched clock is it won't stop during idle.
You don't mean sched clock, you just mean a clock_event_device.
A sched_clock is a high-precision clocksource that is read from (which
by definition requires the CPUs to be non-idle). It doesn't have
anything to do with interrupts and therefore cannot wake devices from
idle.
While the clock_event_device for the generic timer can't necessarily
wake CPUs from idle. The generic timer system counter counts even if
CPUs are idle, so the generic timer is fine as a sched_clock.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-09-17 16:41 ` Mark Rutland
0 siblings, 0 replies; 35+ messages in thread
From: Mark Rutland @ 2015-09-17 16:41 UTC (permalink / raw)
To: Yingjoe Chen
Cc: Sudeep Holla, James Liao, srv_heupstream@mediatek.com,
Arnd Bergmann, devicetree@vger.kernel.org, Catalin Marinas,
Michael Turquette, Daniel Lezcano, Stephen Boyd,
linux-kernel@vger.kernel.org, Daniel Kurtz, Olof Johansson,
Rob Herring, linux-mediatek@lists.infradead.org, Sascha Hauer,
Matthias Brugger, Thomas Gleixner, Eddie Huang,
linux-clk@vger.kernel.org, linux-arm-kernel
On Thu, Sep 17, 2015 at 03:56:56PM +0100, Yingjoe Chen wrote:
> On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
> >
> > On 16/09/15 03:04, Yingjoe Chen wrote:
> > > From: Daniel Kurtz <djkurtz@chromium.org>
> > >
> > > Add device node to enable GPT timer. This timer will be
> > > used as sched clock source.
> > >
> >
> > Interesting any known issues with or advantage over the arch timers
> > to prefer it as sched clock source. I see even arch timers are present
> > in DT, hence the question. Or is it just a incorrect commit log ?
> >
> > How does this get selected as sched clock source ? I don't see
> > sched_clock_register in mtk_timer.c
> >
> > To be clear, I am not against adding this timer support, but just want
> > to know is it preferred for sched clock source ? if yes why ? better
> > resolution ?
>
> Hi Sudeep,
>
> Thanks for your review.
>
> I hit the send too soon and missed cover letter, please see:
> http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
>
> The main reason to use GPT as sched clock is it won't stop during idle.
You don't mean sched clock, you just mean a clock_event_device.
A sched_clock is a high-precision clocksource that is read from (which
by definition requires the CPUs to be non-idle). It doesn't have
anything to do with interrupts and therefore cannot wake devices from
idle.
While the clock_event_device for the generic timer can't necessarily
wake CPUs from idle. The generic timer system counter counts even if
CPUs are idle, so the generic timer is fine as a sched_clock.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
2015-09-17 16:41 ` Mark Rutland
(?)
@ 2015-10-01 14:50 ` Yingjoe Chen
-1 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-10-01 14:50 UTC (permalink / raw)
To: Mark Rutland
Cc: Sudeep Holla, James Liao, srv_heupstream@mediatek.com,
Arnd Bergmann, devicetree@vger.kernel.org, Catalin Marinas,
Michael Turquette, Daniel Lezcano, Stephen Boyd,
linux-kernel@vger.kernel.org, Daniel Kurtz, Olof Johansson,
Rob Herring, linux-mediatek@lists.infradead.org, Sascha Hauer,
Matthias Brugger, Thomas Gleixner, Eddie Huang,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org
On Thu, 2015-09-17 at 17:41 +0100, Mark Rutland wrote:
> On Thu, Sep 17, 2015 at 03:56:56PM +0100, Yingjoe Chen wrote:
> > On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
> > >
> > > On 16/09/15 03:04, Yingjoe Chen wrote:
> > > > From: Daniel Kurtz <djkurtz@chromium.org>
> > > >
> > > > Add device node to enable GPT timer. This timer will be
> > > > used as sched clock source.
> > > >
> > >
> > > Interesting any known issues with or advantage over the arch timers
> > > to prefer it as sched clock source. I see even arch timers are present
> > > in DT, hence the question. Or is it just a incorrect commit log ?
> > >
> > > How does this get selected as sched clock source ? I don't see
> > > sched_clock_register in mtk_timer.c
> > >
> > > To be clear, I am not against adding this timer support, but just want
> > > to know is it preferred for sched clock source ? if yes why ? better
> > > resolution ?
> >
> > Hi Sudeep,
> >
> > Thanks for your review.
> >
> > I hit the send too soon and missed cover letter, please see:
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
> >
> > The main reason to use GPT as sched clock is it won't stop during idle.
>
> You don't mean sched clock, you just mean a clock_event_device.
>
> A sched_clock is a high-precision clocksource that is read from (which
> by definition requires the CPUs to be non-idle). It doesn't have
> anything to do with interrupts and therefore cannot wake devices from
> idle.
>
> While the clock_event_device for the generic timer can't necessarily
> wake CPUs from idle. The generic timer system counter counts even if
> CPUs are idle, so the generic timer is fine as a sched_clock.
Hi, Mark,
Thanks for your info and sorry for late reply.
I notice ARM ARM said the arch timer shouldn't stop when idle, but
unfortunately that not true for mt8173. The last CPU enter idle can
choose to enter deep idle mode and the counter value would be lost. Our
firmware backup/recover the counter so it looks like it is stopped.
We will change the firmware to add missing count when back from deep
idle to make it looks like the counter never stop.
Joe.C
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-10-01 14:50 ` Yingjoe Chen
0 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-10-01 14:50 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2015-09-17 at 17:41 +0100, Mark Rutland wrote:
> On Thu, Sep 17, 2015 at 03:56:56PM +0100, Yingjoe Chen wrote:
> > On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
> > >
> > > On 16/09/15 03:04, Yingjoe Chen wrote:
> > > > From: Daniel Kurtz <djkurtz@chromium.org>
> > > >
> > > > Add device node to enable GPT timer. This timer will be
> > > > used as sched clock source.
> > > >
> > >
> > > Interesting any known issues with or advantage over the arch timers
> > > to prefer it as sched clock source. I see even arch timers are present
> > > in DT, hence the question. Or is it just a incorrect commit log ?
> > >
> > > How does this get selected as sched clock source ? I don't see
> > > sched_clock_register in mtk_timer.c
> > >
> > > To be clear, I am not against adding this timer support, but just want
> > > to know is it preferred for sched clock source ? if yes why ? better
> > > resolution ?
> >
> > Hi Sudeep,
> >
> > Thanks for your review.
> >
> > I hit the send too soon and missed cover letter, please see:
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
> >
> > The main reason to use GPT as sched clock is it won't stop during idle.
>
> You don't mean sched clock, you just mean a clock_event_device.
>
> A sched_clock is a high-precision clocksource that is read from (which
> by definition requires the CPUs to be non-idle). It doesn't have
> anything to do with interrupts and therefore cannot wake devices from
> idle.
>
> While the clock_event_device for the generic timer can't necessarily
> wake CPUs from idle. The generic timer system counter counts even if
> CPUs are idle, so the generic timer is fine as a sched_clock.
Hi, Mark,
Thanks for your info and sorry for late reply.
I notice ARM ARM said the arch timer shouldn't stop when idle, but
unfortunately that not true for mt8173. The last CPU enter idle can
choose to enter deep idle mode and the counter value would be lost. Our
firmware backup/recover the counter so it looks like it is stopped.
We will change the firmware to add missing count when back from deep
idle to make it looks like the counter never stop.
Joe.C
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/2] arm64: dts: mt8173: add timer node
@ 2015-10-01 14:50 ` Yingjoe Chen
0 siblings, 0 replies; 35+ messages in thread
From: Yingjoe Chen @ 2015-10-01 14:50 UTC (permalink / raw)
To: Mark Rutland
Cc: Sudeep Holla, James Liao, srv_heupstream@mediatek.com,
Arnd Bergmann, devicetree@vger.kernel.org, Catalin Marinas,
Michael Turquette, Daniel Lezcano, Stephen Boyd,
linux-kernel@vger.kernel.org, Daniel Kurtz, Olof Johansson,
Rob Herring, linux-mediatek@lists.infradead.org, Sascha Hauer,
Matthias Brugger, Thomas Gleixner, Eddie Huang,
linux-clk@vger.kernel.org, linux-arm-kernel@
On Thu, 2015-09-17 at 17:41 +0100, Mark Rutland wrote:
> On Thu, Sep 17, 2015 at 03:56:56PM +0100, Yingjoe Chen wrote:
> > On Thu, 2015-09-17 at 14:51 +0100, Sudeep Holla wrote:
> > >
> > > On 16/09/15 03:04, Yingjoe Chen wrote:
> > > > From: Daniel Kurtz <djkurtz@chromium.org>
> > > >
> > > > Add device node to enable GPT timer. This timer will be
> > > > used as sched clock source.
> > > >
> > >
> > > Interesting any known issues with or advantage over the arch timers
> > > to prefer it as sched clock source. I see even arch timers are present
> > > in DT, hence the question. Or is it just a incorrect commit log ?
> > >
> > > How does this get selected as sched clock source ? I don't see
> > > sched_clock_register in mtk_timer.c
> > >
> > > To be clear, I am not against adding this timer support, but just want
> > > to know is it preferred for sched clock source ? if yes why ? better
> > > resolution ?
> >
> > Hi Sudeep,
> >
> > Thanks for your review.
> >
> > I hit the send too soon and missed cover letter, please see:
> > http://lists.infradead.org/pipermail/linux-mediatek/2015-September/002303.html
> >
> > The main reason to use GPT as sched clock is it won't stop during idle.
>
> You don't mean sched clock, you just mean a clock_event_device.
>
> A sched_clock is a high-precision clocksource that is read from (which
> by definition requires the CPUs to be non-idle). It doesn't have
> anything to do with interrupts and therefore cannot wake devices from
> idle.
>
> While the clock_event_device for the generic timer can't necessarily
> wake CPUs from idle. The generic timer system counter counts even if
> CPUs are idle, so the generic timer is fine as a sched_clock.
Hi, Mark,
Thanks for your info and sorry for late reply.
I notice ARM ARM said the arch timer shouldn't stop when idle, but
unfortunately that not true for mt8173. The last CPU enter idle can
choose to enter deep idle mode and the counter value would be lost. Our
firmware backup/recover the counter so it looks like it is stopped.
We will change the firmware to add missing count when back from deep
idle to make it looks like the counter never stop.
Joe.C
^ permalink raw reply [flat|nested] 35+ messages in thread