Re: [PATCH v3 11/24] C6X: time management

From: Thomas Gleixner
Date: Tue Sep 27 2011 - 19:41:33 EST


On Tue, 27 Sep 2011, Mark Salter wrote:

> The C6X architecture includes a 64-bit free running core clock counter which
> is used as the clocksource. The SoCs have a number of 64-bit programmable
> timers. One of these is used as the clockevent timer.
>
> + * Port on Texas Instruments TMS320C6x architecture
> + *
> + * Copyright (C) 2004, 2009, 2010, 2011 Texas Instruments Incorporated
> + * Author: Aurelien Jacquiot (aurelien.jacquiot@xxxxxxxxxx)
> + *
> + * Modified for 2.6.34: Mark Salter <msalter@xxxxxxxxxx>

Hmm. Who wrote that code and who is assigned the authorship? If the
original Author is Aurelian, then this patch is missing:

- An extra From: ....

- A SOB from the orginal author or an explnation why it's not
available.

If not, then this want's to be documented in a proper form.

Please do not attribute other peoples code to yourself w/o a fcking
good reason. And porting forward itself is not considered to be one.

> +/*
> + * This should be close enough...

What's the point of that comment ?

> + */
> +#define CLOCK_TICK_RATE ((1000 * 1000000UL) / 6)
> +
> +/* 64-bit timestamp */
> +typedef unsigned long long cycles_t;

...

> +static int next_event(unsigned long delta,
> + struct clock_event_device *evt)
> +{
> + soc_writel(soc_readl(&timer->tcr) & ~TCR_ENAMODELO_MASK, &timer->tcr);
> + soc_writel(delta - 1, &timer->prdlo);
> + soc_writel(0, &timer->cntlo);
> + soc_writel(soc_readl(&timer->tcr) | TCR_ENAMODELO_ONCE, &timer->tcr);
> +
> + return 0;
> +}
> +
> +static void set_clock_mode(enum clock_event_mode mode,
> + struct clock_event_device *evt)
> +{

So there is no way to disable that thing ?

> +}
> +
> +static struct clock_event_device t64_clockevent_device = {
> + .name = "TIMER64_EVT32_TIMER",
> + .features = CLOCK_EVT_FEAT_ONESHOT,
> + .rating = 200,
> + .set_mode = set_clock_mode,
> + .set_next_event = next_event,
> +};
> +
> +static irqreturn_t timer_interrupt(int irq, void *dev_id)
> +{
> + struct clock_event_device *cd = &t64_clockevent_device;

If proper set up, then you can just hand in dev_id to the handler.

> +
> + cd->event_handler(cd);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static struct irqaction timer_iact = {
> + .name = "timer",
> + .flags = IRQF_TIMER,
> + .handler = timer_interrupt,
> + .dev_id = &t64_clockevent_device,

Which seems to be the case :)

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/