On Tue, Jan 05, 2021 at 11:26:12AM +1300, Barry Song wrote: > This patch originated from the discussion with Dmitry in the below thread: > https://lore.kernel.org/linux-input/[email protected]/ > there are many drivers which don't want interrupts enabled automatically > due to request_irq(). > So they are handling this issue by either way of the below two: > (1) > irq_set_status_flags(irq, IRQ_NOAUTOEN); > request_irq(dev, irq...); > (2) > request_irq(dev, irq...); > disable_irq(irq); > > The code in the second way is silly and unsafe. In the small time gap > between request_irq and disable_irq, interrupts can still come. > The code in the first way is safe though we might be able to do it in > the generic irq code. > > I guess Dmitry also prefers genirq handles this as he said > "What I would like to see is to allow passing something like IRQF_DISABLED > to request_irq() so that we would not need neither irq_set_status_flags() > nor disable_irq()" in the original email thread.
One of the reasons I dislike irq_set_status_flags() is that we have to call it before we actually granted our IRQ request... > > If this one is accepted, hundreds of drivers with this problem will be > handled afterwards. > > Cc: Dmitry Torokhov <[email protected]> > Signed-off-by: Barry Song <[email protected]> > --- > include/linux/interrupt.h | 3 +++ > kernel/irq/manage.c | 3 +++ > kernel/irq/settings.h | 10 ++++++++++ > 3 files changed, 16 insertions(+) > > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h > index bb8ff9083e7d..0f22d277078c 100644 > --- a/include/linux/interrupt.h > +++ b/include/linux/interrupt.h > @@ -61,6 +61,8 @@ > * interrupt handler after suspending interrupts. For system > * wakeup devices users need to implement wakeup detection in > * their interrupt handlers. > + * IRQF_NO_AUTOEN - Don't enable IRQ automatically when users request it. > Users > + * will enable it explicitly by enable_irq() later. > */ > #define IRQF_SHARED 0x00000080 > #define IRQF_PROBE_SHARED 0x00000100 > @@ -74,6 +76,7 @@ > #define IRQF_NO_THREAD 0x00010000 > #define IRQF_EARLY_RESUME 0x00020000 > #define IRQF_COND_SUSPEND 0x00040000 > +#define IRQF_NO_AUTOEN 0x00080000 > > #define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND | > IRQF_NO_THREAD) > > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > index ab8567f32501..364e8b47d9ba 100644 > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -1693,6 +1693,9 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, > struct irqaction *new) > irqd_set(&desc->irq_data, IRQD_NO_BALANCING); > } > > + if (new->flags & IRQF_NO_AUTOEN) > + irq_settings_set_noautoen(desc); Can we make sure we refuse this request if the caller also specified IRQF_SHARED? Thanks. -- Dmitry
