Messages in this thread Patch in this message | | | Date | Fri, 1 Aug 2008 10:00:26 -0600 | From | Jordan Crouse <> | Subject | Re: MFGPT driver inhibits boot on some boards |
| |
On 01/08/08 17:25 +0200, Jens Rottmann wrote: > mfgpt: check IRQ before using MFGPT as clocksource > > Adds a simple IRQ autodetection to the AMD Geode MFGPT driver, and more > importantly, adds some checks, if IRQs can actually be received on the > chosen line. This fixes cases where MFGPT is selected as clocksource > though not producing any ticks, so the kernel simply starves during > boot. > > Signed-off-by: Jens Rottmann <JRottmann@LiPPERTEmbedded.de>
One comment below - otherwise, I reviewed it and it looks good, but I haven't tried it.
Jordan
> --- > > --- linux-2.6.26/include/asm-x86/geode.h > +++ mfgpt-irq-fix/include/asm-x86/geode.h > @@ -50,6 +50,7 @@ > #define MSR_PIC_YSEL_HIGH 0x51400021 > #define MSR_PIC_ZSEL_LOW 0x51400022 > #define MSR_PIC_ZSEL_HIGH 0x51400023 > +#define MSR_PIC_IRQM_LPC 0x51400025 > > #define MSR_MFGPT_IRQ 0x51400028 > #define MSR_MFGPT_NR 0x51400029 > @@ -237,7 +238,7 @@ > } > > extern int geode_mfgpt_toggle_event(int timer, int cmp, int event, int enable); > -extern int geode_mfgpt_set_irq(int timer, int cmp, int irq, int enable); > +extern int geode_mfgpt_set_irq(int timer, int cmp, int *irq, int enable); > extern int geode_mfgpt_alloc_timer(int timer, int domain); > > #define geode_mfgpt_setup_irq(t, c, i) geode_mfgpt_set_irq((t), (c), (i), 1) > --- linux-2.6.26/arch/x86/kernel/mfgpt_32.c > +++ mfgpt-irq-fix/arch/x86/kernel/mfgpt_32.c > @@ -33,6 +33,8 @@ > #include <linux/module.h> > #include <asm/geode.h> > > +#define MFGPT_DEFAULT_IRQ 7 > + > static struct mfgpt_timer_t { > unsigned int avail:1; > } mfgpt_timers[MFGPT_MAX_TIMERS]; > @@ -157,29 +159,41 @@ > } > EXPORT_SYMBOL_GPL(geode_mfgpt_toggle_event); > > -int geode_mfgpt_set_irq(int timer, int cmp, int irq, int enable) > +int geode_mfgpt_set_irq(int timer, int cmp, int *irq, int enable) > { > - u32 val, dummy; > - int offset; > + u32 zsel, lpc, dummy; > + int shift; > > if (timer < 0 || timer >= MFGPT_MAX_TIMERS) > return -EIO; > > - if (geode_mfgpt_toggle_event(timer, cmp, MFGPT_EVENT_IRQ, enable)) > + /* IRQ already set to 2 means VSA is using the timer's Siamese twin */
Please make this more descriptive - something like this: /* If the IRQ is set to 2, then VSA is using the other timer in the pair */
Siamese twin might be confusing.
> + shift = ((cmp == MFGPT_CMP1 ? 0 : 4) + timer % 4) * 4; > + rdmsr(MSR_PIC_ZSEL_LOW, zsel, dummy); > + if (((zsel >> shift) & 0xF) == 2) > return -EIO;
> - rdmsr(MSR_PIC_ZSEL_LOW, val, dummy); > - > - offset = (timer % 4) * 4; > + /* Choose IRQ: if none supplied, keep IRQ already set or use default */ > + if (!*irq) > + *irq = (zsel >> shift) & 0xF; > + if (!*irq) > + *irq = MFGPT_DEFAULT_IRQ;
> - val &= ~((0xF << offset) | (0xF << (offset + 16))); > + /* Can't use IRQ if it's 0 (=disabled), 2, or routed to LPC */ > + if (*irq < 1 || *irq == 2 || *irq > 15) > + return -EIO; > + rdmsr(MSR_PIC_IRQM_LPC, lpc, dummy); > + if (lpc & (1 << *irq)) > + return -EIO;
> + /* All chosen and checked - go for it */ > + if (geode_mfgpt_toggle_event(timer, cmp, MFGPT_EVENT_IRQ, enable)) > + return -EIO; > if (enable) { > - val |= (irq & 0x0F) << (offset); > - val |= (irq & 0x0F) << (offset + 16); > + zsel = (zsel & ~(0xF << shift)) | (*irq << shift); > + wrmsr(MSR_PIC_ZSEL_LOW, zsel, dummy); > } > > - wrmsr(MSR_PIC_ZSEL_LOW, val, dummy); > return 0; > } > > @@ -242,7 +256,7 @@ > static unsigned int mfgpt_tick_mode = CLOCK_EVT_MODE_SHUTDOWN; > static u16 mfgpt_event_clock; > > -static int irq = 7; > +static int irq; > static int __init mfgpt_setup(char *str) > { > get_option(&str, &irq); > @@ -346,7 +360,7 @@ int __init mfgpt_timer_setup(void) > mfgpt_event_clock = timer; > > /* Set up the IRQ on the MFGPT side */ > - if (geode_mfgpt_setup_irq(mfgpt_event_clock, MFGPT_CMP2, irq)) { > + if (geode_mfgpt_setup_irq(mfgpt_event_clock, MFGPT_CMP2, &irq)) { > printk(KERN_ERR "mfgpt-timer: Could not set up IRQ %d\n", irq); > return -EIO; > } > @@ -374,13 +388,14 @@ int __init mfgpt_timer_setup(void) > &mfgpt_clockevent); > > printk(KERN_INFO > - "mfgpt-timer: registering the MFGPT timer as a clock event.\n"); > + "mfgpt-timer: Registering MFGPT timer %d as a clock event, using IRQ %d\n", > + timer, irq); > clockevents_register_device(&mfgpt_clockevent); > > return 0; > > err: > - geode_mfgpt_release_irq(mfgpt_event_clock, MFGPT_CMP2, irq); > + geode_mfgpt_release_irq(mfgpt_event_clock, MFGPT_CMP2, &irq); > printk(KERN_ERR > "mfgpt-timer: Unable to set up the MFGPT clock source\n"); > return -EIO; > _ > >
| |