Many drivers 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.

With this patch, drivers can request_irq with IRQF_NO_AUTOEN flag. They will need neither irq_set_status_flags() nor disable_irq(). Hundreds of drivers with this problem will be handled afterwards.

Cc: Dmitry Torokhov dmitry.torokhov@gmail.com Signed-off-by: Barry Song song.bao.hua@hisilicon.com --- -v4: remove the irq_settings magic for NOAUTOEN

include/linux/interrupt.h | 3 +++ kernel/irq/manage.c | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-)

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 dec3f73e8db9..95014073bd2e 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -1693,7 +1693,8 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) irqd_set(&desc->irq_data, IRQD_NO_BALANCING); }

- if (irq_settings_can_autoenable(desc)) { + if (!(new->flags & IRQF_NO_AUTOEN) && + irq_settings_can_autoenable(desc)) { irq_startup(desc, IRQ_RESEND, IRQ_START_COND); } else { /* @@ -2086,10 +2087,15 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler, * which interrupt is which (messes up the interrupt freeing * logic etc). * + * Also shared interrupts do not go well with disabling auto enable. + * The sharing interrupt might request it while it's still disabled + * and then wait for interrupts forever. + * * Also IRQF_COND_SUSPEND only makes sense for shared interrupts and * it cannot be set along with IRQF_NO_SUSPEND. */ if (((irqflags & IRQF_SHARED) && !dev_id) || + ((irqflags & IRQF_SHARED) && (irqflags & IRQF_NO_AUTOEN)) || (!(irqflags & IRQF_SHARED) && (irqflags & IRQF_COND_SUSPEND)) || ((irqflags & IRQF_NO_SUSPEND) && (irqflags & IRQF_COND_SUSPEND))) return -EINVAL;

disable_irq() after request_irq() still has a time gap in which interrupts can come. request_irq() with IRQF_NO_AUTOEN flag will disable IRQ auto-enable because of requesting.

Signed-off-by: Barry Song song.bao.hua@hisilicon.com --- drivers/input/touchscreen/atmel_mxt_ts.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index 383a848eb601..3c837c7b24b3 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -3156,15 +3156,14 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id) }

error = devm_request_threaded_irq(&client->dev, client->irq, - NULL, mxt_interrupt, IRQF_ONESHOT, + NULL, mxt_interrupt, + IRQF_ONESHOT | IRQF_NO_AUTOEN, client->name, data); if (error) { dev_err(&client->dev, "Failed to register interrupt\n"); return error; }

- disable_irq(client->irq); - error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), data->regulators); if (error) {

Right now, irq core supports requesting irq with IRQF_NO_AUTOEN, this means the driver doesn't need to call irq_set_status_flags any more.

Signed-off-by: Barry Song song.bao.hua@hisilicon.com --- drivers/input/touchscreen/bu21029_ts.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/bu21029_ts.c b/drivers/input/touchscreen/bu21029_ts.c index 341925edb8e6..392950aa7856 100644 --- a/drivers/input/touchscreen/bu21029_ts.c +++ b/drivers/input/touchscreen/bu21029_ts.c @@ -401,10 +401,10 @@ static int bu21029_probe(struct i2c_client *client,

input_set_drvdata(in_dev, bu21029);

- irq_set_status_flags(client->irq, IRQ_NOAUTOEN); error = devm_request_threaded_irq(&client->dev, client->irq, NULL, bu21029_touch_soft_irq, - IRQF_ONESHOT, DRIVER_NAME, bu21029); + IRQF_ONESHOT | IRQF_NO_AUTOEN, + DRIVER_NAME, bu21029); if (error) { dev_err(&client->dev, "unable to request touch irq: %d\n", error);

Right now, irq core supports requesting irq with IRQF_NO_AUTOEN, this means the driver doesn't need to call irq_set_status_flags any more.

Signed-off-by: Barry Song song.bao.hua@hisilicon.com --- drivers/input/touchscreen/zinitix.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c index a3e3adbabc67..0d3c93e4bbb7 100644 --- a/drivers/input/touchscreen/zinitix.c +++ b/drivers/input/touchscreen/zinitix.c @@ -513,10 +513,10 @@ static int zinitix_ts_probe(struct i2c_client *client) return -EINVAL; }

- irq_set_status_flags(client->irq, IRQ_NOAUTOEN); error = devm_request_threaded_irq(&client->dev, client->irq, NULL, zinitix_ts_irq_handler, - IRQF_ONESHOT, client->name, bt541); + IRQF_ONESHOT | IRQF_NO_AUTOEN, + client->name, bt541); if (error) { dev_err(&client->dev, "Failed to request IRQ: %d\n", error); return error;

disable_irq() after request_irq() still has a time gap in which interrupts can come. request_irq() with IRQF_NO_AUTOEN flag will disable IRQ auto-enable because of requesting.

Signed-off-by: Barry Song song.bao.hua@hisilicon.com --- drivers/input/touchscreen/wm831x-ts.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/wm831x-ts.c b/drivers/input/touchscreen/wm831x-ts.c index bb1699e0d3c7..319f57fb9af5 100644 --- a/drivers/input/touchscreen/wm831x-ts.c +++ b/drivers/input/touchscreen/wm831x-ts.c @@ -317,14 +317,13 @@ static int wm831x_ts_probe(struct platform_device *pdev)

error = request_threaded_irq(wm831x_ts->data_irq, NULL, wm831x_ts_data_irq, - irqf | IRQF_ONESHOT, + irqf | IRQF_ONESHOT | IRQF_NO_AUTOEN, "Touchscreen data", wm831x_ts); if (error) { dev_err(&pdev->dev, "Failed to request data IRQ %d: %d\n", wm831x_ts->data_irq, error); goto err_alloc; } - disable_irq(wm831x_ts->data_irq);

if (pdata && pdata->pd_irqf) irqf = pdata->pd_irqf;

disable_irq() after request_irq() still has a time gap in which interrupts can come. request_irq() with IRQF_NO_AUTOEN flag will disable IRQ auto-enable because of requesting.

Signed-off-by: Barry Song song.bao.hua@hisilicon.com --- drivers/input/keyboard/tegra-kbc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c index 9671842a082a..570fe18c0ce9 100644 --- a/drivers/input/keyboard/tegra-kbc.c +++ b/drivers/input/keyboard/tegra-kbc.c @@ -694,14 +694,13 @@ static int tegra_kbc_probe(struct platform_device *pdev) input_set_drvdata(kbc->idev, kbc);

err = devm_request_irq(&pdev->dev, kbc->irq, tegra_kbc_isr, - IRQF_TRIGGER_HIGH, pdev->name, kbc); + IRQF_TRIGGER_HIGH | IRQF_NO_AUTOEN, + pdev->name, kbc); if (err) { dev_err(&pdev->dev, "failed to request keyboard IRQ\n"); return err; }

- disable_irq(kbc->irq); - err = input_register_device(kbc->idev); if (err) { dev_err(&pdev->dev, "failed to register input device\n");

-----Original Message----- From: Song Bao Hua (Barry Song) Sent: Friday, January 29, 2021 11:35 AM To: tglx@linutronix.de; dmitry.torokhov@gmail.com; maz@kernel.org; gregkh@linuxfoundation.org; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org Cc: linuxarm@openeuler.org; Song Bao Hua (Barry Song) song.bao.hua@hisilicon.com Subject: [PATCH v4 01/12] genirq: add IRQF_NO_AUTOEN for request_irq

Many drivers 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.

With this patch, drivers can request_irq with IRQF_NO_AUTOEN flag. They will need neither irq_set_status_flags() nor disable_irq(). Hundreds of drivers with this problem will be handled afterwards.

Cc: Dmitry Torokhov dmitry.torokhov@gmail.com Signed-off-by: Barry Song song.bao.hua@hisilicon.com

---

-v4: remove the irq_settings magic for NOAUTOEN

Hi Thomas, Any further comment on this? Does it get any opportunity to hit 5.12 so that we can begin to handle those drivers in 5.12?

Thanks Barry

include/linux/interrupt.h | 3 +++ kernel/irq/manage.c | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-)

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 dec3f73e8db9..95014073bd2e 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -1693,7 +1693,8 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) irqd_set(&desc->irq_data, IRQD_NO_BALANCING); }

• if (irq_settings_can_autoenable(desc)) {
  

• if (!(new->flags & IRQF_NO_AUTOEN) &&
  

•     irq_settings_can_autoenable(desc)) {
  irq_startup(desc, IRQ_RESEND, IRQ_START_COND);
  

  } else { /*

@@ -2086,10 +2087,15 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler, * which interrupt is which (messes up the interrupt freeing * logic etc). *

• * Also shared interrupts do not go well with disabling auto enable.
  

• * The sharing interrupt might request it while it's still disabled
  

• * and then wait for interrupts forever.
  

• *
  

  • Also IRQF_COND_SUSPEND only makes sense for shared interrupts and
  • it cannot be set along with IRQF_NO_SUSPEND.
  */ if (((irqflags & IRQF_SHARED) && !dev_id) ||

•    ((irqflags & IRQF_SHARED) && (irqflags & IRQF_NO_AUTOEN)) ||
   (!(irqflags & IRQF_SHARED) && (irqflags & IRQF_COND_SUSPEND)) ||
   ((irqflags & IRQF_NO_SUSPEND) && (irqflags & IRQF_COND_SUSPEND)))
  

  return -EINVAL;

-- 2.25.1