lkml.org 
[lkml]   [2008]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: 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;
    > _
    >
    >



    \
     
     \ /
      Last update: 2008-08-01 18:03    [W:0.031 / U:0.536 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site