On Tue, 12 Jan 2010 11:14:30 +0100 Jens Rottmann <[email protected]> wrote:
> geode-mfgpt: restore previous behavior for selecting IRQ > > The MFGPT IRQ used to be, in order of decreasing priority, > * IRQ supplied by the user as a boot-time parameter, > * IRQ previously set by the BIOS or another driver, > * default IRQ given at compile time. > > Return to this behavior, which got broken when splitting the > MFGPT/clocksource driver for 2.6.33-rc1. > > Signed-off-by: Jens Rottmann <[email protected]> > --- > > Hi Andres, > > a bit more detail. Up to 2.6.32 the MFGPT IRQ was chosen like this: > > static int irq; > static int __init mfgpt_setup(char *str) > { > get_option(&str, &irq); > return 1; > } > __setup("mfgpt_irq=", mfgpt_setup); > [...] > /* Choose IRQ: if none supplied, keep IRQ already set or use default > */ if (!*irq) > *irq = (zsel >> shift) & 0xF; // may have been set by > the BIOS if (!*irq) > *irq = MFGPT_DEFAULT_IRQ; // #defined as 7 > > The code in 2.6.33-rc3 after the MFGPT/clocksource split looks like > this: > > static int timer_irq = CONFIG_CS5535_MFGPT_DEFAULT_IRQ; // > defaults to 7 module_param_named(irq, timer_irq, int, 0644); > [...] > if (!*irq) > *irq = (zsel >> shift) & 0xF; // gets ignored > now if (!*irq) > *irq = CONFIG_CS5535_MFGPT_DEFAULT_IRQ; // again!? > > The patch removes the first of the two ocurrances of > CONFIG_CS5535_MFGPT_DEFAULT_IRQ, reverting the IRQ selection to the > previous (working) behavior. You're right, thanks. For the patch below: Acked-by: Andres Salomon <[email protected]> > > > BTW, while looking into this I stumbled upon the fact that > cs5535_mfgpt_init() doesn't free up the timer in the error path if > cs5535_mfgpt_setup_irq() or setup_irq() fail. However, some comments > in cs5535-mfgpt.c (stating that it's not possible to release the > individual timer hardware) make me think this might have been skipped > on purpose? Could you please have a look? Oh, you mean in the clockevt driver? Yeah, we can't really free the timer, unfortunately. It actually might not be a bad idea to reverse the code so that the IRQ allocation happens first, since we can clean that up if mfgpt allocation fails. Feel like sending a patch? :) > > Thanks, > Jens > > --- linux-2.6.33-rc3-git4/drivers/clocksource/cs5535-clockevt.c > +++ previous_behavior/drivers/clocksource/cs5535-clockevt.c > @@ -21,7 +21,7 @@ > > #define DRV_NAME "cs5535-clockevt" > > -static int timer_irq = CONFIG_CS5535_MFGPT_DEFAULT_IRQ; > +static int timer_irq; > module_param_named(irq, timer_irq, int, 0644); > MODULE_PARM_DESC(irq, "Which IRQ to use for the clock source MFGPT > ticks."); > _ > _______________________________________________ Linux-geode mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-geode
