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, 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.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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