Re: [PATCH 1/3] clocksource: timer-keystone: introduce clocksource driver for Keystone

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux