From: john.stultz@linaro.org (John Stultz)
To: linux-arm-kernel@lists.infradead.org
Subject: dw_apb_timer_of.c: remove parts that were picoxcell-specific
Date: Tue, 07 May 2013 17:30:52 -0700 [thread overview]
Message-ID: <51899CBC.6020003@linaro.org> (raw)
In-Reply-To: <20130507201126.GA8169@amd.pavel.ucw.cz>
On 05/07/2013 01:11 PM, Pavel Machek wrote:
> It seems we made a mistake when creating dw_apb_timer_of.c:
> picoxcell sched_clock had parts that were not related to
> dw_apb_timer, yet we moved them to dw_apb_timer_of, and tried to
> use them on socfpga.
>
> This results in system where user/system time is not measured
> properly, as demonstrated by
>
> time dd if=/dev/urandom of=/dev/zero bs=100000 count=100
>
> So this patch switches sched_clock to hardware that exists on both
> platforms, and adds missing of_node_put() in dw_apb_timer_init().
>
> Signed-off-by: Pavel Machek <pavel@denx.de>
> Acked-by: Jamie Iles <jamie@jamieiles.com>
Ok. I'm still not a happy about the general issue of
sched_clock/clockevent code being in drivers/clocksources (I know,
everyone is sick of my griping about it :), so reviewing this sucks, but
at least this patch technically isn't making that issue worse.
Additionally, this is an *ugly* driver in my opinion. Its split between
arch specific logic in arch/x86/kernel/apb_timer.c,
arch/arm/mach-picoxcell/common.c, and arch/arm/mach-socfpga/socfpga.c,
and then arch independent logic in drivers/clocksource/dw_apb_timer.c
and drivers/clocksource/dw_apb_timer_of.c, but then it seems like much
of drivers/clocksource/dw_apb_timer_of.c is actually ARM specific.
Are there any plans to clean this up in the future?
Also, next time please run checkpatch.pl to catch trivial issues like
trailing whitespace. :P
But I've gone ahead and queued this for 3.11.
thanks
-john
next prev parent reply other threads:[~2013-05-08 0:30 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-26 12:14 dw_apb_timer_of.c: remove parts that were picoxcell-specific Pavel Machek
2013-04-26 12:41 ` Jamie Iles
2013-04-26 21:37 ` Pavel Machek
2013-04-26 21:43 ` Pavel Machek
2013-05-06 12:48 ` Pavel Machek
2013-05-06 13:45 ` Arnd Bergmann
2013-05-06 15:53 ` Pavel Machek
2013-05-06 21:24 ` Arnd Bergmann
2013-05-07 13:57 ` Pavel Machek
2013-05-07 16:39 ` John Stultz
2013-05-07 20:11 ` Pavel Machek
2013-05-08 0:30 ` John Stultz [this message]
2013-05-10 12:38 ` Pavel Machek
2013-05-07 20:13 ` Pavel Machek
2013-05-07 9:36 ` Jamie Iles
-- strict thread matches above, loose matches on Subject: below --
2013-04-23 10:58 Pavel Machek
2013-04-24 22:44 ` Dinh Nguyen
2013-04-26 11:36 ` Pavel Machek
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=51899CBC.6020003@linaro.org \
--to=john.stultz@linaro.org \
--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).