On Fri, 29 Sep 2006 23:58:33 -0000
Thomas Gleixner <[email protected]> wrote:
> From: Thomas Gleixner <[email protected]>
>
> add clockevent drivers for i386: lapic (local) and PIT (global).
> Update the timer IRQ to call into the PIT driver's event handler
> and the lapic-timer IRQ to call into the lapic clockevent driver.
>
What's the story on other clock sources? hpet, pm-timer?
> --- linux-2.6.18-mm2.orig/arch/i386/kernel/apic.c 2006-09-30 01:41:11.000000000 +0200
> +++ linux-2.6.18-mm2/arch/i386/kernel/apic.c 2006-09-30 01:41:18.000000000 +0200
> @@ -25,6 +25,7 @@
> #include <linux/kernel_stat.h>
> #include <linux/sysdev.h>
> #include <linux/cpu.h>
> +#include <linux/clockchips.h>
> #include <linux/module.h>
>
> #include <asm/atomic.h>
> @@ -70,6 +71,23 @@ static inline void lapic_enable(void)
> */
> int apic_verbosity;
>
> +static unsigned int calibration_result;
> +
> +static void lapic_next_event(unsigned long delta, struct clock_event *evt);
> +static void lapic_timer_setup(int mode, struct clock_event *evt);
> +
> +static struct clock_event lapic_clockevent = {
> + .name = "lapic",
> + .capabilities = CLOCK_CAP_NEXTEVT | CLOCK_CAP_PROFILE
> +#ifdef CONFIG_SMP
> + | CLOCK_CAP_UPDATE
> +#endif
I can't work out why this is SMP-only. Please add changelog information
or, better, a comment explaining this.
> -static void __devinit setup_APIC_timer(unsigned int clocks)
> +static void lapic_timer_setup(int mode, struct clock_event *evt)
> {
> unsigned long flags;
>
> local_irq_save(flags);
> + __setup_APIC_LVTT(calibration_result, mode != CLOCK_EVT_PERIODIC);
> + local_irq_restore(flags);
> +}
>
> - /*
> - * Wait for IRQ0's slice:
> - */
> - wait_timer_tick();
> - wait_timer_tick();
For what reason was setup_APIC_timer() "Waiting for IRQ0's slice" and why
is it safe to remove that?
> +#ifdef CONFIG_HIGH_RES_TIMERS
> + printk("Disabling NO_HZ and high resolution timers "
> + "due to timer broadcasting\n");
> + for_each_possible_cpu(cpu)
> + per_cpu(lapic_events, cpu).capabilities &=
> + ~CLOCK_CAP_NEXTEVT;
> +#endif
I think you mean "due to lack of timer broadcasting"?
No comment, no changelog entry -> I don't understand why this code is here.
> *
> */
> -#include <linux/clocksource.h>
> +#include <linux/clockchips.h>
> #include <linux/spinlock.h>
> #include <linux/jiffies.h>
> #include <linux/sysdev.h>
> @@ -19,19 +19,63 @@
> DEFINE_SPINLOCK(i8253_lock);
> EXPORT_SYMBOL(i8253_lock);
>
> -void setup_pit_timer(void)
> +static void init_pit_timer(int mode, struct clock_event *evt)
No. `mode' is not an integer. It has type `enum you_forgot_to_give_it_a_name'.
In several places the timer code is using enums as integers - basically
using them as a glorified #define.
This loses the main advantages of enums: readability. They permit the
reader to say "ah, that's a clock event type" instead of "oh, thats an
integer".
Please give those enums a name, and use it everywhere, rather than relying
upon implicit conversion to `int'.
(C sucks)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&i8253_lock, flags);
> +
> + switch(mode) {
> + case CLOCK_EVT_PERIODIC:
> + /* binary, mode 2, LSB/MSB, ch 0 */
> + outb_p(0x34, PIT_MODE);
> + udelay(10);
> + outb_p(LATCH & 0xff , PIT_CH0); /* LSB */
> + outb(LATCH >> 8 , PIT_CH0); /* MSB */
> + break;
> +
> + case CLOCK_EVT_ONESHOT:
> + case CLOCK_EVT_SHUTDOWN:
> + /* One shot setup */
> + outb_p(0x38, PIT_MODE);
> + udelay(10);
> + break;
> + }
> + spin_unlock_irqrestore(&i8253_lock, flags);
> +}
> +
> +static void pit_next_event(unsigned long delta, struct clock_event *evt)
> {
> unsigned long flags;
>
> spin_lock_irqsave(&i8253_lock, flags);
> - outb_p(0x34,PIT_MODE); /* binary, mode 2, LSB/MSB, ch 0 */
> - udelay(10);
> - outb_p(LATCH & 0xff , PIT_CH0); /* LSB */
> - udelay(10);
> - outb(LATCH >> 8 , PIT_CH0); /* MSB */
> + outb_p(delta & 0xff , PIT_CH0); /* LSB */
> + outb(delta >> 8 , PIT_CH0); /* MSB */
> spin_unlock_irqrestore(&i8253_lock, flags);
> }
>
> +struct clock_event pit_clockevent = {
> + .name = "pit",
> + .capabilities = CLOCK_CAP_TICK | CLOCK_CAP_PROFILE | CLOCK_CAP_UPDATE
> +#ifndef CONFIG_SMP
> + | CLOCK_CAP_NEXTEVT
> +#endif
Again, the CONFIG_SMP conditionality is a complete mystery to this reader.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
