Messages in this thread | | | From | Kumar Gala <> | Subject | Re: [RFC] IRQ type flags | Date | Tue, 13 Dec 2005 08:49:50 -0600 |
| |
On Dec 12, 2005, at 5:47 AM, Russell King wrote:
> Here's an updated patch, taking account of folks comments. The > changes > from the previous patch are: > > * Move SA_TRIGGER into linux/signal.h > * Change SA_TRIGGER_* to match IORESOURCE_IRQ_* definitions > * Change ARM __IRQT_* definitions to match IORESOURCE_IRQ_* > definitions > * Add a comment to explain the case where no SA_TRIGGER_* flags are > passed to request_irq. > > ---- > Some ARM platforms have the ability to program the interrupt > controller to > detect various interrupt edges and/or levels. For some platforms, > this is > critical to setup correctly, particularly those which the setting is > dependent on the device. > > Currently, ARM drivers do (eg) the following: > > err = request_irq(irq, ...); > > set_irq_type(irq, IRQT_RISING); > > However, if the interrupt has previously been programmed to be level > sensitive (for whatever reason) then this will cause an interrupt > storm. > > Hence, if we combine set_irq_type() with request_irq(), we can then > safely > set the type prior to unmasking the interrupt. The unfortunate > problem is > that in order to support this, these flags need to be visible > outside of > the ARM architecture - drivers such as smc91x need these flags and > they're > cross-architecture. > > Finally, the SA_TRIGGER_* flag passed to request_irq() should > reflect the > property that the device would like. The IRQ controller code > should do its > best to select the most appropriate supported mode. > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > > arch/arm/kernel/irq.c | 14 ++++++++++++-- > include/asm-arm/irq.h | 12 ++++++++---- > include/linux/signal.h | 13 +++++++++++++ > 3 files changed, 33 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c > --- a/arch/arm/kernel/irq.c > +++ b/arch/arm/kernel/irq.c > @@ -684,8 +684,12 @@ int setup_irq(unsigned int irq, struct i > spin_lock_irqsave(&irq_controller_lock, flags); > p = &desc->action; > if ((old = *p) != NULL) { > - /* Can't share interrupts unless both agree to */ > - if (!(old->flags & new->flags & SA_SHIRQ)) { > + /* > + * Can't share interrupts unless both agree to and are > + * the same type. > + */ > + if (!(old->flags & new->flags & SA_SHIRQ) || > + (~old->flags & new->flags) & SA_TRIGGER_MASK) { > spin_unlock_irqrestore(&irq_controller_lock, flags); > return -EBUSY; > } > @@ -705,6 +709,12 @@ int setup_irq(unsigned int irq, struct i > desc->running = 0; > desc->pending = 0; > desc->disable_depth = 1; > + > + if (new->flags & SA_TRIGGER_MASK) { > + unsigned int type = new->flags & SA_TRIGGER; > + desc->chip->set_type(irq, type); > + } > + > if (!desc->noautoenable) { > desc->disable_depth = 0; > desc->chip->unmask(irq); > diff --git a/include/asm-arm/irq.h b/include/asm-arm/irq.h > --- a/include/asm-arm/irq.h > +++ b/include/asm-arm/irq.h > @@ -25,10 +25,14 @@ extern void disable_irq_nosync(unsigned > extern void disable_irq(unsigned int); > extern void enable_irq(unsigned int); > > -#define __IRQT_FALEDGE (1 << 0) > -#define __IRQT_RISEDGE (1 << 1) > -#define __IRQT_LOWLVL (1 << 2) > -#define __IRQT_HIGHLVL (1 << 3) > +/* > + * These correspond with the SA_TRIGGER_* defines, and therefore the > + * IRQRESOURCE_IRQ_* defines.
comment nit. Should be IORESOURCE_IRQ_* not IRQRESOURCE_IRQ_*
> + */ > +#define __IRQT_RISEDGE (1 << 0) > +#define __IRQT_FALEDGE (1 << 1) > +#define __IRQT_HIGHLVL (1 << 2) > +#define __IRQT_LOWLVL (1 << 3) > > #define IRQT_NOEDGE (0) > #define IRQT_RISING (__IRQT_RISEDGE) > diff --git a/include/linux/signal.h b/include/linux/signal.h > --- a/include/linux/signal.h > +++ b/include/linux/signal.h > @@ -18,6 +18,19 @@ > #define SA_PROBE SA_ONESHOT > #define SA_SAMPLE_RANDOM SA_RESTART > #define SA_SHIRQ 0x04000000 > +/* > + * As above, these correspond to the IORESOURCE_IRQ_* defines in > + * linux/ioport.h to select the interrupt line behaviour. When > + * requesting an interrupt without specifying a SA_TRIGGER, the > + * setting should be assumed to be "as already configured", which > + * may be as per machine or firmware initialisation. > + */ > +#define SA_TRIGGER_LOW 0x00000008 > +#define SA_TRIGGER_HIGH 0x00000004 > +#define SA_TRIGGER_FALLING 0x00000002 > +#define SA_TRIGGER_RISING 0x00000001 > +#define SA_TRIGGER_MASK (SA_TRIGGER_HIGH|SA_TRIGGER_LOW|\ > + SA_TRIGGER_RISING|SA_TRIGGER_FALLING)
Do you mind expand the comment to convey that LOW/HIGH are related to level sensitive interrupts and FALLING/RISING are for edge. This is different naming that I'm used to with PowerPC and it can get a little confusing.
- kumar
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |