lkml.org 
[lkml]   [2009]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/1] [ARM] ep93xx clockevent support
    On Wed, Jul 22, 2009 at 12:09:46PM +0300, Ahmed Ammar wrote:
    > @@ -849,6 +851,7 @@ config HZ
    > default AT91_TIMER_HZ if ARCH_AT91
    > default 100
    >
    > +

    Please don't add unrelated blank lines.

    > @@ -100,55 +103,129 @@ void __init ep93xx_map_io(void)
    > * to use this timer for something else. We also use timer 4 for keeping
    > * track of lost jiffies.
    > */
    > -static unsigned int last_jiffy_time;
    > -
    > -#define TIMER4_TICKS_PER_JIFFY ((CLOCK_TICK_RATE + (HZ/2)) / HZ)
    > +static struct clock_event_device clockevent_ep93xx;
    >
    > static irqreturn_t ep93xx_timer_interrupt(int irq, void *dev_id)
    > {
    > - __raw_writel(1, EP93XX_TIMER1_CLEAR);
    > - while ((signed long)
    > - (__raw_readl(EP93XX_TIMER4_VALUE_LOW) - last_jiffy_time)
    > - >= TIMER4_TICKS_PER_JIFFY) {
    > - last_jiffy_time += TIMER4_TICKS_PER_JIFFY;
    > - timer_tick();
    > - }
    > -
    > + __raw_writel(EP93XX_TC_CLEAR, EP93XX_TIMER1_CLEAR);
    > + clockevent_ep93xx.event_handler(&clockevent_ep93xx);

    You can avoid the forward declaration by doing:

    struct clk_event_device *evt = dev_id;

    evt->event_handler(evt);

    here, and:

    > +static struct clock_event_device clockevent_ep93xx = {
    > + .name = "ep93xx-timer1",
    > + .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
    > + .shift = 32,
    > + .set_mode = ep93xx_set_mode,
    > + .set_next_event = ep93xx_set_next_event,
    > +};
    > +
    > static struct irqaction ep93xx_timer_irq = {
    > .name = "ep93xx timer",
    > .flags = IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
    > .handler = ep93xx_timer_interrupt,

    adding here:

    .dev_id = &clockevent_ep93xx,

    > diff --git a/arch/arm/mach-ep93xx/include/mach/timex.h b/arch/arm/mach-ep93xx/include/mach/timex.h
    > index 6b3503b..ddf3a8a 100644
    > --- a/arch/arm/mach-ep93xx/include/mach/timex.h
    > +++ b/arch/arm/mach-ep93xx/include/mach/timex.h
    > @@ -1,5 +1,11 @@
    > /*
    > * arch/arm/mach-ep93xx/include/mach/timex.h
    > */
    > +#include <mach/ep93xx-regs.h>
    > +#include <asm/io.h>
    >
    > #define CLOCK_TICK_RATE 983040
    > +
    > +#define mach_read_cycles() __raw_readl(EP93XX_TIMER4_VALUE_LOW)
    > +#define mach_cycles_to_usecs(d) (((d) * ((1000000LL << 32) / CLOCK_TICK_RATE)) >> 32)
    > +#define mach_usecs_to_cycles(d) (((d) * (((long long)CLOCK_TICK_RATE << 32) / 1000000)) >> 32)

    Two points here:

    1. You don't seem to use these in this patch, they're unrelated so please
    send as a separate patch.
    2. Please don't pollute mach/timex.h with anything other than the
    CLOCK_TICK_RATE definition. That's what it's there for, and it gets
    included by almost the entire kernel, so please don't pollute this
    file.


    \
     
     \ /
      Last update: 2009-07-22 11:29    [W:0.026 / U:0.028 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site