From: ivan.khoronzhuk@ti•com (ivan.khoronzhuk)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone
Date: Tue, 17 Dec 2013 11:42:17 +0200 [thread overview]
Message-ID: <52B01C79.7040508@ti.com> (raw)
In-Reply-To: <52AF68DA.9000104@ti.com>
On 12/16/2013 10:55 PM, Santosh Shilimkar wrote:
> On Monday 16 December 2013 03:40 PM, Stephen Boyd wrote:
>> On 12/16, ivan.khoronzhuk wrote:
>>> On 12/13/2013 03:42 AM, Stephen Boyd wrote:
>>>> On 12/11/13 10:00, Ivan Khoronzhuk wrote:
>>>>> +
>>>>> +static inline u32 keystone_timer_readl(unsigned long rg)
>>>>> +{
>>>>> + return readl(timer.base + rg);
>>>>> +}
>>>>> +
>>>>> +static inline void keystone_timer_writel(u32 val, unsigned long rg)
>>>>> +{
>>>>> + writel(val, timer.base + rg);
>>>>> +}
>>>>
>>>> It's probably better to use the relaxed variants here to avoid any
>>>> memory barriers that aren't necessary.
>>>>
>>>
>>> Yes, but the code has places where I cannot use relaxed variants.
>>>
>>> From timer user guide:
>>> "Writes from the configuration bus to the timer registers are not allowed
>>> when the timer is active, except for stopping or resetting the timers.
>>> Registers that are protected by hardware include CNTLO, CNTHI, PRDLO,
>>> PRDHI, TGCR (except the TIMLORS and TIMHIRS bits), and TCR (except the
>>> ENAMODE bits)."
>>>
>>> According to this I have to add keystone readl/write relaxed functions
>>> and use mixed calls of writel/writel_relaxed functions.
>>>
>>> For instance, for keystone_timer_config() which is used by
>>> keystone_set_next_event(), I will do following:
>>> -----
>>> tcr = keystone_timer_readl_relaxed(TCR);
>>>
>>> /* disable timer */
>>> tcr &= ~(TCR_ENAMODE_MASK);
>>> keystone_timer_writel_relaxed(tcr, TCR);
>>> ...
>>> /* reset counter to zero, set new period */
>>> *** here I have to be sure the timer is disabled ***
>>> keystone_timer_writel(0, TIM12);
>>> keystone_timer_writel_relaxed(0, TIM34);
>>> keystone_timer_writel_relaxed(period & 0xffffffff, PRD12);
>>> keystone_timer_writel_relaxed(period >> 32, PRD34);
>>>
>>> /* enable timer */
>>> *** here I have to be sure that TIM and PRD registers are written ***
>>> keystone_timer_writel(tcr, TCR);
>>> ---
>>>
>>> The same for keystone_timer_init().
>>>
>>> Will it be better?
>>
>> Why not just put the explicit memory barriers where you need them
>> and always use the relaxed variants in the mmio wrappers? That
>> way you're always sure the memory barriers are there and that
>> they're properly documented.
>>
> I agree with Stephen.
>
> Regards,
> Santosh
>
Yes, it will be better
--
Regards,
Ivan Khoronzhuk
next prev parent reply other threads:[~2013-12-17 9:42 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-11 18:00 [PATCH 0/3] Introduce clocksource driver for Keystone platform Ivan Khoronzhuk
2013-12-11 18:00 ` [PATCH 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone Ivan Khoronzhuk
2013-12-12 15:51 ` Daniel Lezcano
2013-12-12 17:36 ` ivan.khoronzhuk
2013-12-13 6:55 ` Daniel Lezcano
2013-12-16 13:58 ` ivan.khoronzhuk
2013-12-13 1:42 ` Stephen Boyd
2013-12-16 12:43 ` ivan.khoronzhuk
2013-12-16 20:40 ` Stephen Boyd
2013-12-16 20:55 ` Santosh Shilimkar
2013-12-17 9:42 ` ivan.khoronzhuk [this message]
2013-12-11 18:00 ` [PATCH 2/3] clocksource: keystone: add bindings for keystone timer Ivan Khoronzhuk
2013-12-11 18:00 ` [PATCH 3/3] arm: dts: keystone: add keystone timer entry Ivan Khoronzhuk
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=52B01C79.7040508@ti.com \
--to=ivan.khoronzhuk@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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