Thankyou for the comments. On 24/06/13 23:08, Stephen Boyd wrote: > On 06/24/13 08:53, Srinivas KANDAGATLA wrote: >> +#include <linux/clkdev.h> > > Why do you need this include? > >> +#include <asm/mach/irq.h> > > And this one? Removed them. > >> +static u64 gt_counter_read(void) >> +{ >> + u64 counter; >> + u32 lower; >> + u32 upper, old_upper; >> + >> + upper = readl_relaxed(gt_base + GT_COUNTER1); >> + do { >> + old_upper = upper; >> + lower = readl_relaxed(gt_base + GT_COUNTER0); >> + upper = readl_relaxed(gt_base + GT_COUNTER1); > [snip] >> +static void gt_compare_set(unsigned long delta, int periodic) >> +{ >> + u64 counter = gt_counter_read(); >> + unsigned long ctrl = readl(gt_base + GT_CONTROL); >> + >> + counter += delta; >> + ctrl &= ~(GT_CONTROL_COMP_ENABLE | GT_CONTROL_IRQ_ENABLE); >> + >> + writel(ctrl, gt_base + GT_CONTROL); >> + writel(lower_32_bits(counter), gt_base + GT_COMP0); >> + writel(upper_32_bits(counter), gt_base + GT_COMP1); >> + >> + if (periodic) { >> + writel(delta, gt_base + GT_AUTO_INC); >> + ctrl |= GT_CONTROL_AUTO_INC; >> + } >> + >> + ctrl |= GT_CONTROL_COMP_ENABLE | GT_CONTROL_IRQ_ENABLE; >> + writel(ctrl, gt_base + GT_CONTROL); >> +} > > Why is there a mix of the relaxed and non-relaxed io accessors? gt_counter_read will be used very frequently, so using relaxed can reduce latencies involved in memory barriers. > >> +#ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK >> +static u32 gt_sched_clock_read(void) > > notrace > Ok, fixed it. >> +{ >> + if (!gt_base) >> + return 0; > > Seems impossible? Remove this check? Yep, I agree this case is impossible in the code flow. > Thanks, srini -- 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