diff mbox

ARM: architected timers: allow dt based discovery using clocksource_of_init

Message ID 1358731326-2979-1-git-send-email-thomas.abraham@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Thomas Abraham Jan. 21, 2013, 1:22 a.m. UTC
Add an entry in __clksrc_of_table so that ARMv7 architected timer is
discoverable using call to clocksource_of_init.

Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 arch/arm/kernel/arch_timer.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Mark Rutland Jan. 21, 2013, 12:13 p.m. UTC | #1
Hi Thomas,

This looks really useful, and it's going to conflict with my arch_timer /
arm_generic driver unification series [1].

I'm happy to take this as part of my series if that's ok with you?

On Mon, Jan 21, 2013 at 01:22:06AM +0000, Thomas Abraham wrote:
> Add an entry in __clksrc_of_table so that ARMv7 architected timer is
> discoverable using call to clocksource_of_init.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  arch/arm/kernel/arch_timer.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> index c8ef207..d21aada 100644
> --- a/arch/arm/kernel/arch_timer.c
> +++ b/arch/arm/kernel/arch_timer.c
> @@ -504,6 +504,9 @@ int __init arch_timer_of_register(void)
>  
>  	return arch_timer_register();
>  }
> +#ifdef CONFIG_CLKSRC_OF
> +CLOCKSOURCE_OF_DECLARE(armv7_timer, "arm,armv7-timer", arch_timer_of_register)
> +#endif
>  
>  int __init arch_timer_sched_clock_init(void)
>  {
> -- 
> 1.7.5.4
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/142070.html
Stephen Warren Jan. 21, 2013, 5:21 p.m. UTC | #2
On 01/20/2013 06:22 PM, Thomas Abraham wrote:
> Add an entry in __clksrc_of_table so that ARMv7 architected timer is
> discoverable using call to clocksource_of_init.

> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c

> +#ifdef CONFIG_CLKSRC_OF
> +CLOCKSOURCE_OF_DECLARE(armv7_timer, "arm,armv7-timer", arch_timer_of_register)
> +#endif

I wonder if we shouldn't enhance include/linux/clocksource.h to define
CLOCKSOURCE_OF_DECLARE even when !CONFIG_CLKSRC_OF; that way, drivers
wouldn't need that ifdef.
Thomas Abraham Jan. 21, 2013, 6:19 p.m. UTC | #3
Hi Mark,

On 21 January 2013 04:13, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Thomas,
>
> This looks really useful, and it's going to conflict with my arch_timer /
> arm_generic driver unification series [1].
>
> I'm happy to take this as part of my series if that's ok with you?

Sure, that's fine. This patch would probably be needed in 3.9 on
Exynos platforms. So you could squash this change into your series.

Thanks,
Thomas.

>
> On Mon, Jan 21, 2013 at 01:22:06AM +0000, Thomas Abraham wrote:
>> Add an entry in __clksrc_of_table so that ARMv7 architected timer is
>> discoverable using call to clocksource_of_init.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> ---
>>  arch/arm/kernel/arch_timer.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
>> index c8ef207..d21aada 100644
>> --- a/arch/arm/kernel/arch_timer.c
>> +++ b/arch/arm/kernel/arch_timer.c
>> @@ -504,6 +504,9 @@ int __init arch_timer_of_register(void)
>>
>>       return arch_timer_register();
>>  }
>> +#ifdef CONFIG_CLKSRC_OF
>> +CLOCKSOURCE_OF_DECLARE(armv7_timer, "arm,armv7-timer", arch_timer_of_register)
>> +#endif
>>
>>  int __init arch_timer_sched_clock_init(void)
>>  {
>> --
>> 1.7.5.4
>>
>> _______________________________________________
>> devicetree-discuss mailing list
>> devicetree-discuss@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>>
>
> Thanks,
> Mark.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/142070.html
>
Marc Zyngier Jan. 21, 2013, 6:25 p.m. UTC | #4
On Mon, 21 Jan 2013 10:21:08 -0700, Stephen Warren <swarren@wwwdotorg.org>
wrote:
> On 01/20/2013 06:22 PM, Thomas Abraham wrote:
>> Add an entry in __clksrc_of_table so that ARMv7 architected timer is
>> discoverable using call to clocksource_of_init.
> 
>> diff --git a/arch/arm/kernel/arch_timer.c
b/arch/arm/kernel/arch_timer.c
> 
>> +#ifdef CONFIG_CLKSRC_OF
>> +CLOCKSOURCE_OF_DECLARE(armv7_timer, "arm,armv7-timer",
>> arch_timer_of_register)
>> +#endif
> 
> I wonder if we shouldn't enhance include/linux/clocksource.h to define
> CLOCKSOURCE_OF_DECLARE even when !CONFIG_CLKSRC_OF; that way, drivers
> wouldn't need that ifdef.

Indeed. That would be a lot nicer.

        M.
Thomas Abraham Jan. 21, 2013, 6:30 p.m. UTC | #5
Hi Stephen,

On 21 January 2013 09:21, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 01/20/2013 06:22 PM, Thomas Abraham wrote:
>> Add an entry in __clksrc_of_table so that ARMv7 architected timer is
>> discoverable using call to clocksource_of_init.
>
>> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
>
>> +#ifdef CONFIG_CLKSRC_OF
>> +CLOCKSOURCE_OF_DECLARE(armv7_timer, "arm,armv7-timer", arch_timer_of_register)
>> +#endif
>
> I wonder if we shouldn't enhance include/linux/clocksource.h to define
> CLOCKSOURCE_OF_DECLARE even when !CONFIG_CLKSRC_OF; that way, drivers
> wouldn't need that ifdef.

Yes, it will be helpful to have a !CONFIG_CLKSRC_OF version of
CLOCKSOURCE_OF_DECLARE. And can CONFIG_CLKSRC_OF be enabled by default
for all ARM platforms?

Thanks,
Thomas.
Mark Rutland Jan. 22, 2013, 11:04 a.m. UTC | #6
On Mon, Jan 21, 2013 at 06:30:47PM +0000, Thomas Abraham wrote:
> Hi Stephen,
> 
> On 21 January 2013 09:21, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 01/20/2013 06:22 PM, Thomas Abraham wrote:
> >> Add an entry in __clksrc_of_table so that ARMv7 architected timer is
> >> discoverable using call to clocksource_of_init.
> >
> >> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> >
> >> +#ifdef CONFIG_CLKSRC_OF
> >> +CLOCKSOURCE_OF_DECLARE(armv7_timer, "arm,armv7-timer", arch_timer_of_register)
> >> +#endif
> >
> > I wonder if we shouldn't enhance include/linux/clocksource.h to define
> > CLOCKSOURCE_OF_DECLARE even when !CONFIG_CLKSRC_OF; that way, drivers
> > wouldn't need that ifdef.
> 
> Yes, it will be helpful to have a !CONFIG_CLKSRC_OF version of
> CLOCKSOURCE_OF_DECLARE. And can CONFIG_CLKSRC_OF be enabled by default
> for all ARM platforms?
> 
> Thanks,
> Thomas.

Thomas,

Once the above is worked out, could you send me the updated patch with a
pointer to whichever {branch,patch}(es) I need for CLKSRC_OF?

Thanks,
Mark.
Mark Rutland Jan. 30, 2013, 5:19 p.m. UTC | #7
Thomas,

On Tue, Jan 22, 2013 at 11:04:56AM +0000, Mark Rutland wrote:
> On Mon, Jan 21, 2013 at 06:30:47PM +0000, Thomas Abraham wrote:
> > Hi Stephen,
> > 
> > On 21 January 2013 09:21, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > > On 01/20/2013 06:22 PM, Thomas Abraham wrote:
> > >> Add an entry in __clksrc_of_table so that ARMv7 architected timer is
> > >> discoverable using call to clocksource_of_init.
> > >
> > >> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> > >
> > >> +#ifdef CONFIG_CLKSRC_OF
> > >> +CLOCKSOURCE_OF_DECLARE(armv7_timer, "arm,armv7-timer", arch_timer_of_register)
> > >> +#endif
> > >
> > > I wonder if we shouldn't enhance include/linux/clocksource.h to define
> > > CLOCKSOURCE_OF_DECLARE even when !CONFIG_CLKSRC_OF; that way, drivers
> > > wouldn't need that ifdef.
> > 
> > Yes, it will be helpful to have a !CONFIG_CLKSRC_OF version of
> > CLOCKSOURCE_OF_DECLARE. And can CONFIG_CLKSRC_OF be enabled by default
> > for all ARM platforms?
> > 
> > Thanks,
> > Thomas.
> 
> Thomas,
> 
> Once the above is worked out, could you send me the updated patch with a
> pointer to whichever {branch,patch}(es) I need for CLKSRC_OF?

Is there any news on this? I'd like to be able to push out a branch ready for
merging soon.

I've also realised that platforms using of_clocksource_init won't get the
architected timer registered as a sched_clock, which would seem like a loss.
I'm not sure what the relative requirements are for sched_clock and
clocksource, so there may be some platforms that want it and some that don't.

I see for exynos5440 the architected timer is regsitered as a
clock{source,_event_device}, but not as a sched_clock, and omap5 is similar,
but may register dmtimer_read_sched_clock first.

We can probably leave it out for now, but it's something we'll need to consider
in future.

Thanks,
Mark.
Stephen Warren Jan. 30, 2013, 5:49 p.m. UTC | #8
On 01/30/2013 10:19 AM, Mark Rutland wrote:
> Thomas,
> 
> On Tue, Jan 22, 2013 at 11:04:56AM +0000, Mark Rutland wrote:
>> On Mon, Jan 21, 2013 at 06:30:47PM +0000, Thomas Abraham wrote:
>>> Hi Stephen,
>>>
>>> On 21 January 2013 09:21, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 01/20/2013 06:22 PM, Thomas Abraham wrote:
>>>>> Add an entry in __clksrc_of_table so that ARMv7 architected timer is
>>>>> discoverable using call to clocksource_of_init.
>>>>
>>>>> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
>>>>
>>>>> +#ifdef CONFIG_CLKSRC_OF
>>>>> +CLOCKSOURCE_OF_DECLARE(armv7_timer, "arm,armv7-timer", arch_timer_of_register)
>>>>> +#endif
>>>>
>>>> I wonder if we shouldn't enhance include/linux/clocksource.h to define
>>>> CLOCKSOURCE_OF_DECLARE even when !CONFIG_CLKSRC_OF; that way, drivers
>>>> wouldn't need that ifdef.
>>>
>>> Yes, it will be helpful to have a !CONFIG_CLKSRC_OF version of
>>> CLOCKSOURCE_OF_DECLARE. And can CONFIG_CLKSRC_OF be enabled by default
>>> for all ARM platforms?
>>>
>>> Thanks,
>>> Thomas.
>>
>> Thomas,
>>
>> Once the above is worked out, could you send me the updated patch with a
>> pointer to whichever {branch,patch}(es) I need for CLKSRC_OF?
> 
> Is there any news on this? I'd like to be able to push out a branch ready for
> merging soon.

I just sent a patch for this.
diff mbox

Patch

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index c8ef207..d21aada 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -504,6 +504,9 @@  int __init arch_timer_of_register(void)
 
 	return arch_timer_register();
 }
+#ifdef CONFIG_CLKSRC_OF
+CLOCKSOURCE_OF_DECLARE(armv7_timer, "arm,armv7-timer", arch_timer_of_register)
+#endif
 
 int __init arch_timer_sched_clock_init(void)
 {