From: Ulrich Prinz <ulrich.prinz@googlemail.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: "Heiko Stübner" <heiko@sntech.de>,
"John Stultz" <john.stultz@linaro.org>,
"Jamie Iles" <jamie@jamieiles.com>,
"Dinh Nguyen" <dinguyen@altera.com>,
"Grant Likely" <grant.likely@linaro.org>,
linux-arm-kernel@lists.infradead.org,
"Rob Herring" <rob.herring@calxeda.com>,
devicetree-discuss@lists.ozlabs.org,
linux-kernel@vger.kernel.org, "Arnd Bergmann" <arnd@arndb.de>,
"Olof Johansson" <olof@lixom.net>
Subject: Re: [PATCH 9/9] clocksource: dw_apb_timer: special variant for rockchip rk3188 timers
Date: Fri, 12 Jul 2013 00:44:29 +0200 [thread overview]
Message-ID: <51DF354D.6050401@googlemail.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1307062233480.32106@ionos.tec.linutronix.de>
Hey guys,
according the dw_apb_timer I need your statement...
Am 06.07.2013 23:00, schrieb Thomas Gleixner:
> Ulrich,
>
> On Sat, 6 Jul 2013, Ulrich Prinz wrote:
>
>> I got the message. With modifying the existing driver to support more
>> function pointers in its system struct and assigning them at the
>> beginning, and using them on runtime, these quirks are obsolete.
>
> Correct.
>
>>
>> Adding more function pointers to a system relevant structure, doubling
>> the number of functions and such didn't look non-invasive to me.
>
> Well, it always depends. If there is a single place to deal with some
> oddball hardware, a flag is often the simplest way to go.
>
> If you have to add 10 conditionals in several functions then in a
> first step converting the existing implementation into function
> pointer calls and then in the next step providing new implementations
> for your hardware is most of the time simpler and cleaner.
>
>> But, I totally agree with your argumentation and I even wanted to do it
>> in the way you explained in your replies. Just the courage was missing I
>> guess :)
>
> Gut feelings are often a better guidance than our self-doubting
> intellect. :)
You can look at my github repo for the new driver, rewriting the
interface as discussed above:
https://github.com/Astralix/linux-rockchip/blob/wip/dw_timer64/drivers/clocksource/dw_apb_timer.c
Summary: It is a 90% rewrite of the complete dw_apb_timer.c and
modifications of dw_apb_timer_of.c and dw_apb_timer.h and the related
x86 drivers are needed.
And after that the 64bit functionality is still not given as the
existing structures do not take it. But for now scheduler doesn't
support 64bit either, I guess.
The drivers local struct stays, but instead of carrying pointers to
registers it now consists of function pointers. This could be used to
fit almost any kind of timer.
I would really appreciate your comments. Sure I need to clean up a bit,
but we're still testing. But before investing more time into that code I
need you comment if I should continue or split to a separate driver for
these kind of timers.
Thanks for any comments
Regards
Ulrich
prev parent reply other threads:[~2013-07-11 22:44 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-05 22:51 [PATCH 0/9] clocksource: dw_apb_timer: support for timer variant used in rk3188 SoCs Heiko Stübner
2013-07-05 22:51 ` [PATCH 1/9] clocksource: dw_apb_timer: infrastructure to handle quirks Heiko Stübner
2013-07-05 22:52 ` [PATCH 2/9] clocksource: dw_apb_timer: flexible register addresses Heiko Stübner
2013-07-05 22:53 ` [PATCH 3/9] clocksource: dw_apb_timer: quirk for variants with 64bit counter Heiko Stübner
[not found] ` <201307060053.10182.heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
2013-07-05 23:45 ` Thomas Gleixner
2013-07-06 20:19 ` Ulrich Prinz
2013-07-05 22:53 ` [PATCH 4/9] clocksource: dw_apb_timer: use the eoi callback to clear pending interrupts Heiko Stübner
2013-07-05 22:59 ` Heiko Stübner
2013-07-05 22:54 ` [PATCH 5/9] clocksource: dw_apb_timer: quirk for variants without EOI register Heiko Stübner
[not found] ` <201307060054.07784.heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
2013-07-05 22:58 ` Heiko Stübner
2013-07-05 23:49 ` Thomas Gleixner
2013-07-05 22:54 ` [PATCH 6/9] clocksource: dw_apb_timer: quirk for inverted int mask Heiko Stübner
2013-07-05 22:58 ` Heiko Stübner
[not found] ` <201307060054.35996.heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
2013-07-05 23:51 ` Thomas Gleixner
[not found] ` <201307060051.09716.heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
2013-07-05 22:55 ` [PATCH 7/9] clocksource: dw_apb_timer: quirk for inverted timer mode setting Heiko Stübner
2013-07-05 22:56 ` [PATCH 8/9] clocksource: dw_apb_timer_of: add quirk handling Heiko Stübner
2013-07-05 22:56 ` [PATCH 9/9] clocksource: dw_apb_timer: special variant for rockchip rk3188 timers Heiko Stübner
[not found] ` <201307060056.29370.heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
2013-07-06 0:12 ` Thomas Gleixner
2013-07-06 20:28 ` Ulrich Prinz
[not found] ` <51D87E04.3060704-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
2013-07-06 21:00 ` Thomas Gleixner
2013-07-11 22:44 ` Ulrich Prinz [this message]
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=51DF354D.6050401@googlemail.com \
--to=ulrich.prinz@googlemail.com \
--cc=arnd@arndb.de \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=dinguyen@altera.com \
--cc=grant.likely@linaro.org \
--cc=heiko@sntech.de \
--cc=jamie@jamieiles.com \
--cc=john.stultz@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=olof@lixom.net \
--cc=rob.herring@calxeda.com \
--cc=tglx@linutronix.de \
/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).