From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 12/13] ARM: move sp804 and integrator timers to drivers/clocksource
Date: Tue, 02 Apr 2013 21:31:48 -0500 [thread overview]
Message-ID: <515B9494.3060807@gmail.com> (raw)
In-Reply-To: <515B7AC1.9090403@linaro.org>
On 04/02/2013 07:41 PM, John Stultz wrote:
> On 04/02/2013 12:49 PM, Rob Herring wrote:
>> On 04/01/2013 06:26 PM, John Stultz wrote:
>>> On 04/01/2013 03:21 PM, Rob Herring wrote:
>>>> diff --git a/drivers/clocksource/integrator_ap_timer.c
>>>> b/drivers/clocksource/integrator_ap_timer.c
>>>> new file mode 100644
>>>> index 0000000..05e6204
>>>> --- /dev/null
>>>> +++ b/drivers/clocksource/integrator_ap_timer.c
>>> [snip]
>>
>>>> +CLOCKSOURCE_OF_DECLARE(integrator_ap, "integrator-timer",
>>>> ap_of_timer_init);
>>> So, most of the code here is really clockevent code and not clocksource
>>> code. I realize they were combined in the mach directory you're copying
>>> them from, but if we're going to move all this code out of the arch
>>> directory and into drivers/, I'd like to propose we not dump it all into
>>> drivers/clocksource.
>> Arguably, half the h/w is used for clocksource and half for clockevent
>> so it is equal. :) The sp804 and integrator are somewhat separated that
>> carving them up may be possible, but there are examples like i.MX timers
>> where the implementations are tied more closely together (the clockevent
>> compare value is the clocksource counter + delta).
>>
>> Where do I put the init functions which need to know about both?
>
> Either have them init themselves via *_initcall, or call out from one to
> the other?
initcalls are too late on ARM. This is why CLOCKSOURCE_OF_DECLARE was
created, but it is tied to 1 init callback per matched DT node. The nice
thing about it is we avoid any headers with arbitrary timer init
functions and data and calls from the platform code in the DT case.
We could also create a CLOCKEVENT_OF_DECLARE and match twice, but then
that is 2 searches thru the DT.
I don't think an undefined interface of shared data that's different for
every timer is a good idea. This is typically the mapped address(es),
clocks, spinlocks, register access functions, misc flags, etc.
>> What about all the other timers that are already in drivers/clocksource?
>
> I'm hoping to start moving those out soon.
I count 3 clkevt only, 4 clocksource only and 13 drivers with both. How
intertwined the 13 are, I don't know. I think we should get things moved
to a common spot first and look at all the various h/w first before
deciding how to best split things up.
Rob
>>> For more context here see:
>>> http://www.spinics.net/lists/arm-kernel/msg234074.html
>> Seems like this is more a maintainer issue than necessarily what is the
>> right split of code (not that those are completely unrelated). Perhaps
>> splitting maintainership between core and drivers is needed. It
>> definitely seems Thomas cannot keep up with the plethora of ARM related
>> timers and irqchips getting moved into drivers/.
>
> Well, I'm hoping Daniel can help offload Thomas, but I've still not
> heard back if Thomas is ok with this whole proposal or not.
>
> thanks
> -john
next prev parent reply other threads:[~2013-04-03 2:31 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-01 22:21 [PATCH v2 00/13] ARM arch, sp804 and integrator timer CLKSRC_OF support Rob Herring
2013-04-01 22:21 ` [PATCH v2 01/13] ARM: make machine_desc->init_time default to clocksource_of_init Rob Herring
2013-04-01 22:21 ` [PATCH v2 02/13] ARM: convert arm/arm64 arch timer to use CLKSRC_OF init Rob Herring
2013-04-02 3:23 ` Santosh Shilimkar
2013-04-04 5:56 ` Simon Horman
2013-04-10 23:17 ` Rob Herring
2013-04-11 0:09 ` Simon Horman
2013-04-15 9:39 ` Catalin Marinas
2013-04-01 22:21 ` [PATCH v2 03/13] OF: add empty of_device_is_available for !OF Rob Herring
2013-04-01 22:21 ` [PATCH v2 04/13] ARM: timer-sp: convert to use CLKSRC_OF init Rob Herring
2013-04-19 16:30 ` Russell King - ARM Linux
2013-04-01 22:21 ` [PATCH v2 05/13] ARM: highbank: use OF init for sp804 timer Rob Herring
2013-04-01 22:21 ` [PATCH v2 06/13] ARM: vexpress: remove sp804 OF init Rob Herring
2013-04-02 16:41 ` Pawel Moll
2013-04-01 22:21 ` [PATCH v2 07/13] ARM: dts: vexpress: disable CA9 core tile sp804 timer Rob Herring
2013-04-02 16:47 ` Pawel Moll
2013-04-01 22:21 ` [PATCH v2 08/13] ARM: vexpress: remove extra timer-sp control register clearing Rob Herring
2013-04-02 16:48 ` Pawel Moll
2013-04-01 22:21 ` [PATCH v2 09/13] ARM: versatile: add versatile dtbs to dtbs target Rob Herring
2013-04-01 22:21 ` [PATCH v2 10/13] ARM: versatile: use OF init for sp804 timer Rob Herring
2013-04-01 22:21 ` [PATCH v2 11/13] ARM: integrator-cp: convert use CLKSRC_OF for timer init Rob Herring
2013-04-01 22:21 ` [PATCH v2 12/13] ARM: move sp804 and integrator timers to drivers/clocksource Rob Herring
2013-04-01 23:26 ` John Stultz
2013-04-02 19:49 ` Rob Herring
2013-04-03 0:41 ` John Stultz
2013-04-03 2:31 ` Rob Herring [this message]
2013-04-10 23:23 ` Rob Herring
2013-04-11 8:33 ` Linus Walleij
2013-04-11 15:12 ` Rob Herring
2013-04-03 16:52 ` Linus Walleij
2013-04-04 11:06 ` Daniel Lezcano
2013-04-01 22:21 ` [PATCH v2 13/13] devtree: add binding documentation for sp804 Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=515B9494.3060807@gmail.com \
--to=robherring2@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).