Re: [PATCH] Alchemy: modernize Pb1200 IRQ cascade handling code.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux