[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-arm-kernel
Subject:    Re: [PATCH 2/3] sirf tn2x0bd platform support - sources
From:       Russell King - ARM Linux <linux () arm ! linux ! org ! uk>
Date:       2007-12-31 15:47:42
Message-ID: 20071231154741.GG31777 () flint ! arm ! linux ! org ! uk
[Download RAW message or body]

On Sat, Nov 24, 2007 at 08:26:13PM +0000, Russell King - ARM Linux wrote:
> > +static inline u32 tn2x0bd_readtimer_lo(void)
> > +{
> > +	__raw_writel(1, TIMER_BASE + TIMER_LATCH);
> > +	return __raw_readl(TIMER_BASE + TIMER_LATCHED_LO);
> > +
> > +}
> > +static unsigned long tn2x0bd_gettimeoffset(void)
> > +{
> > +	u32 ticks_to_match, elapsed;
> > +
> > +	ticks_to_match =
> > +	    __raw_readl(TIMER_BASE + TIMER_MATCH_0) - tn2x0bd_readtimer_lo();
> > +	elapsed = LATCH - ticks_to_match;
> > +	return (elapsed * (tick_nsec / 1000)) / LATCH;
> > +}
> 
> Suggest converting to the clocksource and clockevent infrastructure
> rather than the old (and some would say obsolete) gettimeoffset()
> stuff.  With the newer clocksource and clockevent code, you can
> support things like tickless kernels.

The patches need another round of review on the list.  The following
was taken from 4676/1 in the patch system.

+/* Note: all timers are UPCOUNTING */
+static inline u32 tn2x0bd_readtimer_lo(void)
+{
+	__raw_writel(TIMER0_MATCH, TIMER_BASE + TIMER_LATCH);
+	return __raw_readl(TIMER_BASE + TIMER_LATCHED_LO);
+
+}
+
+unsigned long long sched_clock(void)
+{
+	__raw_writel(TIMER0_MATCH, TIMER_BASE + TIMER_LATCH);
+	return __raw_readl(TIMER_BASE + TIMER_LATCHED_LO ) |
+		(((unsigned long long)__raw_readl(TIMER_BASE + TIMER_LATCHED_HI)) << 32);

What if something writes TIMER_LATCH between the two reads?

+}
+
+/*
+* IRQ handler for the timer
+*/
+static irqreturn_t tn2x0bd_timer_interrupt(int irq, void *dev_id)
+{
+        struct clock_event_device *c = dev_id;

Should be a tab.

+
+	switch (c->mode)
+	{
+	case CLOCK_EVT_MODE_ONESHOT:
+		c->event_handler(c);
+		break;
+	case CLOCK_EVT_MODE_PERIODIC:
+		do {
+			c->event_handler(c);
+			__raw_writel(__raw_readl(TIMER_BASE + TIMER_MATCH_0) + LATCH,
+				TIMER_BASE + TIMER_MATCH_0);
+			__raw_writel(TIMER0_MATCH, TIMER_BASE + TIMER_STATUS);
+		} while ((signed)
+                         (readl(TIMER_BASE + TIMER_MATCH_0) -
+                          tn2x0bd_readtimer_lo()) < 0);

Please don't copy what PXA did; it's wrong.  If you don't support periodic
interrupts in hardware then don't simulate them in software like this.
The core timer code knows how to accurately deal with one-shot hardware
timers itself.

+		break;
+	default:
+		/* unknown clock_event_device's mode ? */
+		BUG();
+		break;
+	}
+	return IRQ_HANDLED;
+}
+
+/*
+*/
+static inline unsigned long get_system_clock(void)
+{
+	unsigned long parent, ratio, cfg;
+	unsigned long freq = 12000000;
+	int f,r,d,pll;
+	unsigned long pll_configs[] = { -1, PWR_PLL1_CONFIG, PWR_PLL2_CONFIG };
+
+	parent = readl(PWRMGR_BASE + PWR_CLK_SWITCH);
+	pll = (parent >> 2) & 0x3;
+	if (pll == 1 || pll == 2) {
+		cfg = readl(PWRMGR_BASE + pll_configs[pll]);
+		if (!(cfg & (1<<21))) {
+		        d = ((cfg >> 18) & 3) + 1;
+        		r = ((cfg >> 12) & 0x1F) + 1;
+	        	f = (cfg & 0x7ff) + 1;
+			freq = freq * f / r / d;
+		}
+	}
+	ratio = readl(PWRMGR_BASE + PWR_SYS_RATIO);
+	freq = freq >> ((ratio >> 8) & 0xF);
+	return freq;
+}
+
+static cycle_t tn2x0bd_read_timer0(void)
+{
+	return (cycle_t)tn2x0bd_readtimer_lo();
+}
+
+static struct clocksource cksrc_tn2x0bd_timer0 = {
+        .name           = "timer0",
+        .rating         = 200,
+        .read           = tn2x0bd_read_timer0,
+        .mask           = CLOCKSOURCE_MASK(32),
+        .shift          = 1,
+        .flags          = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+static void tn2x0bd_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *dev)
+{
+        unsigned long irqflags;
+
+        switch (mode) {
+        case CLOCK_EVT_MODE_PERIODIC:
+                raw_local_irq_save(irqflags);
+		__raw_writel(TIMER0_MATCH, TIMER_BASE + TIMER_STATUS);
+		__raw_writel(TIMER0_EN, TIMER_BASE + TIMER_INT_EN);
+		__raw_writel(tn2x0bd_readtimer_lo() + LATCH,
+		     TIMER_BASE + TIMER_MATCH_0);
+                raw_local_irq_restore(irqflags);
+                break;
+
+        case CLOCK_EVT_MODE_ONESHOT:
+                raw_local_irq_save(irqflags);
+		__raw_writel(TIMER0_MATCH, TIMER_BASE + TIMER_STATUS);
+		__raw_writel(TIMER0_EN, TIMER_BASE + TIMER_INT_EN);
+		__raw_writel(tn2x0bd_readtimer_lo() + LATCH,
+		     TIMER_BASE + TIMER_MATCH_0);
+                raw_local_irq_restore(irqflags);
+                break;
+
+        case CLOCK_EVT_MODE_UNUSED:
+        case CLOCK_EVT_MODE_SHUTDOWN:
+		break;
+
+	case CLOCK_EVT_MODE_RESUME:
+                break;
+        }
+}
+
+static int
+tn2x0bd_timer_set_next_event(unsigned long delta, struct clock_event_device *dev)
+{
+        unsigned long flags;
+
+        raw_local_irq_save(flags);
+	__raw_writel(tn2x0bd_readtimer_lo() + delta,
+	 	    TIMER_BASE + TIMER_MATCH_0);

Weird spacing.

+        raw_local_irq_restore(flags);

Is this IRQ masking required to write one register?

+
+        return 0;
+}
+
+static struct clock_event_device ckevt_tn2x0bd_timer0 = {
+        .name           = "tn2x0bd_timer0",
+        .features       = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
+        .shift          = 0,
+        .rating         = 200,
+        .cpumask        = CPU_MASK_CPU0,
+        .set_next_event = tn2x0bd_timer_set_next_event,
+        .set_mode       = tn2x0bd_timer_set_mode,
+};
+
+static struct irqaction tn2x0bd_timer_irq = {
+	.name 		= "timer tick (tn2x0bd)",
+	.flags 		= IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
+	.handler 	= tn2x0bd_timer_interrupt,
+        .dev_id         = &ckevt_tn2x0bd_timer0,
+};
+
+/*
+* can't use clk here yet
+*/
+static void tn2x0bd_setup_timer(void)
+{
+	unsigned long sys = get_system_clock();
+
+	printk("setup timer\n");
+	writel(sys / CLOCK_TICK_RATE / 2 - 1, TIMER_BASE + TIMER_DIV);
+	cksrc_tn2x0bd_timer0.shift = 20;
+        cksrc_tn2x0bd_timer0.mult =
+                clocksource_khz2mult(1000, cksrc_tn2x0bd_timer0.shift);
+
+	setup_irq(IRQ_OS_TIMER0, &tn2x0bd_timer_irq);
+        clocksource_register(&cksrc_tn2x0bd_timer0);
+        clockevents_register_device(&ckevt_tn2x0bd_timer0);
+}
+
+static void tn2x0bd_suspend_timer(void)
+{
+#ifdef CONFIG_PM
+	writel(0, TIMER_BASE + TIMER_INT_EN);
+	writel(readl(TIMER_BASE + TIMER_MATCH_0) -
+			tn2x0bd_readtimer_lo(),
+		TIMER_BASE + TIMER_MATCH_0); /* save diff */
+#endif
+}
+
+static void tn2x0bd_resume_timer(void)
+{
+#ifdef CONFIG_PM
+	writel(readl(TIMER_BASE + TIMER_MATCH_0) +
+			tn2x0bd_readtimer_lo(),
+		TIMER_BASE + TIMER_MATCH_0);
+	writel(1, TIMER_BASE + TIMER_INT_EN);
+#endif
+}
+
+struct sys_timer tn2x0bd_timer = {
+	.init = tn2x0bd_setup_timer,
+	.suspend = tn2x0bd_suspend_timer,
+	.resume = tn2x0bd_resume_timer,
+};

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic