Messages in this thread | | | Subject | Re: [PATCH 33/36] AArch64: Generic timers support | Date | Thu, 12 Jul 2012 12:09:24 +0200 | From | Marc Zyngier <> |
| |
On Thu, 12 Jul 2012 02:18:42 +0200, Linus Walleij <linus.ml.walleij@gmail.com> wrote:
Hi Linus,
> I'm reviewing the only patch I really understand... > > 2012/7/6 Catalin Marinas <catalin.marinas@arm.com>: > >> +/* This isn't really used any more */ >> +#define CLOCK_TICK_RATE 1000 > > Is it still necessary to even have it there?
It is used as part of the LATCH/TICK_* computation in include/linux/jiffies.h. It seems that any value could do, actually, and it only seem to be used in kernel/time/ntp.c. Any guidance on this much appreciated.
By the way, there is a very interesting comment about this in arch/ia64/include/asm/timex.h.
>> + /* Try to determine the frequency from the device tree or CNTFRQ >> */ >> + if (!of_property_read_u32(np, "clock-frequency", &freq)) >> + arch_timer_rate = freq; > > Have you documented the bindings for this thing?
It has been documented as part of the architected timer patch that has been merged for 3.5. See Documentation/devicetree/bindings/arm/arch_timer.txt.
>> + arch_timer_calibrate(); > > Why is the ability to get this from a clk not contemplated? > I think this will be common. You could make it optional I think, > just like in the SMP TWD.
_calibrate() is a misnomer here. It should really be _get_freq(). But your point still stand, and we could indeed use a clk to obtain the frequency.
It should probably be selected in the following order: clock-frequency (DT), clk, CNTFRQ.
>> diff --git a/include/clocksource/arm_generic.h >> b/include/clocksource/arm_generic.h >> new file mode 100644 >> index 0000000..6933f8e > (...) >> +#ifndef __ASM_ARCH_TIMER_H >> +#define __ASM_ARCH_TIMER_H > > This #ifdef name looks confusing, what about > #ifndef __CLKSOURCE_ARM_GENERIC_H? > > I noticed you already saw the problem with naming it "arch timers" > and renamed it to "arm generic" so a leftover here :)
Thanks! :-)
M. -- Fast, cheap, reliable. Pick two.
| |