On Mon, Jun 10, 2013 at 11:21 AM, Srinivas KANDAGATLA
[off-list ref] wrote:

From: Stuart Menefy <redacted>

This is a simple driver for the global timer module found in the Cortex
A9-MP cores from revision r1p0 onwards. This should be able to perform
the functions of the system timer and the local timer in an SMP system.

The global timer has the following features:
    The global timer is a 64-bit incrementing counter with an
auto-incrementing feature. It continues incrementing after sending
interrupts. The global timer is memory mapped in the private memory
region.
    The global timer is accessible to all Cortex-A9 processors in the
cluster. Each Cortex-A9 processor has a private 64-bit comparator that
is used to assert a private interrupt when the global timer has reached
the comparator value. All the Cortex-A9 processors in a design use the
banked ID, ID27, for this interrupt. ID27 is sent to the Interrupt
Controller as a Private Peripheral Interrupt. The global timer is
clocked by PERIPHCLK.

Signed-off-by: Stuart Menefy <redacted>
Signed-off-by: Srinivas Kandagatla <redacted>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Rob Herring <redacted>
CC: Linus Walleij <redacted>
CC: Will Deacon <redacted>
CC: Thomas Gleixner <redacted>

This is starting to look very good!

(...)

+static int __cpuinit gt_clockevents_init(struct clock_event_device *clk)
+{
+       struct clock_event_device **this_cpu_clk;
+       int cpu = smp_processor_id();
+
+       clk->name = "ARM global timer clock event";
+       clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+       clk->set_mode = gt_clockevent_set_mode;
+       clk->set_next_event = gt_clockevent_set_next_event;
+       this_cpu_clk = __this_cpu_ptr(gt_evt);
+       *this_cpu_clk = clk;
+       clk->cpumask = cpumask_of(cpu);
+       clk->irq = gt_ppi;
+       clockevents_config_and_register(clk, gt_clk_rate,
+                                       0, 0xffffffff);

What do you mean with being able to set event on
0?

This should most probably be:


clockevents_config_and_register(clk, gt_clk_rate,
                                      1, 0xffffffff);

(...)

+static struct clk *gt_get_clock(void)
+{
+       struct clk *clk;
+       int err;
+
+       clk = clk_get_sys("gt", NULL);
+       if (IS_ERR(clk)) {
+               pr_err("global-timer: clock not found: %ld\n", PTR_ERR(clk));
+               return clk;
+       }

(...)

+       clk = of_clk_get(np, 0);
+       if (!IS_ERR(clk))
+               clk_register_clkdev(clk, NULL, "gt");

Well that was clever.

Isn't it better to pass a struct device_node *np around and have that as
NULL in the non-DT boot path?

(Maybe somebody in the community asked you to do this, then I
will live with it.)

Yours,
Linus Walleij