On Wed, 13 Aug 2008 16:34:19 +0400 Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx> wrote: > > -inline void pb1200_enable_irq(unsigned int irq_nr) > > +static void pb1200_unmask_irq(unsigned int irq_nr) > > { > > bcsr->intset_mask = 1 << (irq_nr - PB1200_INT_BEGIN); > > bcsr->intset = 1 << (irq_nr - PB1200_INT_BEGIN); > > } > > > > -inline void pb1200_disable_irq(unsigned int irq_nr) > > +static void pb1200_maskack_irq(unsigned int irq_nr) > > { > > bcsr->intclr_mask = 1 << (irq_nr - PB1200_INT_BEGIN); > > bcsr->intclr = 1 << (irq_nr - PB1200_INT_BEGIN); > > > > I wonder what's the difference between int{clr|set} and > int{clr|set}_mask registers... The irq assertion equation in the CPLD is: int_condition = inten & intmask & int_input_pin; In theory, the ->startup() and ->shutdown() methods should fiddle with the enable bits; the ->mask()/->unmask() methods should modfiy the mask bits. In practice, at least on my DB1200, if not BOTH of the enable and mask bits are cleared, the CPLD triggers tons of spurious interrupts. I read through the verilog sources but could not find a reason for this... > [...] > > + bcsr->int_status = 1 << (irq_nr - PB1200_INT_BEGIN); /* ack */ > > > > The above comment said that writing to this register has no effect on > the level-triggered interrupts, so this statement doesn't seem to make > sense since you're treating all interrupts as level-triggered below. They _all_ behave level-triggered, even the ones that should be edge. Again, looking at the verilog code, no edge detection or similar is implemented; for instance the SD card insert/eject irqs should be edge-triggered, but behave differently: the insert irq stays asserted as long as a card is in the socket, the eject irq is asserted as long as no card is present. The PCMCIA carrdetect irqs exhibit identical behaviour. > > @@ -113,12 +77,9 @@ static struct irq_chip external_irq_type = { > > #ifdef CONFIG_MIPS_DB1200 > > .name = "Db1200 Ext", > > #endif > > - .startup = pb1200_startup_irq, > > - .shutdown = pb1200_shutdown_irq, > > - .ack = pb1200_disable_irq, > > - .mask = pb1200_disable_irq, > > - .mask_ack = pb1200_disable_irq, > > - .unmask = pb1200_enable_irq, > > + .mask = pb1200_maskack_irq, > > + .mask_ack = pb1200_maskack_irq, > > > > You can use the same function for the mask() and mask_ack() methods > but it only should be masking IRQ, as clearing it doesn't make sense... It doesn't make a difference in practice, but I'll create a separate ->mask() method without the acking part of the supposedly-edge ints ;-) > > + for (irq = PB1200_INT_BEGIN; irq <= PB1200_INT_END; irq++) > > + set_irq_chip_and_handler_name(irq, &external_irq_type, > > + handle_level_irq, "level"); > > > > Are all those IRQs indeed level-triggered? See above. Thanks, Manuel Lauss