[PATCH v3 00/12] add IRQF_NO_AUTOEN for request_irq

[PATCH v3 00/12] add IRQF_NO_AUTOEN for request_irq

[Barry Song]

This patchset added IRQF_NO_AUTOEN for request_irq() and converted
drivers/input to this new API.
Other drivers will be handled afterwards.

-v3:
  added examples with respect to Greg's comment

Barry Song (12):
  genirq: add IRQF_NO_AUTOEN for request_irq
  Input: ar1021 - request_irq by IRQF_NO_AUTOEN and remove disable_irq
  Input: atmel_mxt_ts - request_irq by IRQF_NO_AUTOEN and remove
    disable_irq
  Input: melfas_mip4 - request_irq by IRQF_NO_AUTOEN and remove
    disable_irq
  Input: bu21029_ts - request_irq by IRQF_NO_AUTOEN and remove
    irq_set_status_flags
  Input: stmfts - request_irq by IRQF_NO_AUTOEN and remove
    irq_set_status_flags
  Input: zinitix - request_irq by IRQF_NO_AUTOEN and remove
    irq_set_status_flags
  Input: mms114 - request_irq by IRQF_NO_AUTOEN and remove disable_irq
  Input: wm831x-ts - request_irq by IRQF_NO_AUTOEN and remove
    disable_irq
  Input: cyttsp - request_irq by IRQF_NO_AUTOEN and remove disable_irq
  Input: tegra-kbc - request_irq by IRQF_NO_AUTOEN and remove
    disable_irq
  Input: tca6416-keypad - request_irq by IRQF_NO_AUTOEN and remove
    disable_irq

 drivers/input/keyboard/tca6416-keypad.c  |  3 +--
 drivers/input/keyboard/tegra-kbc.c       |  5 ++---
 drivers/input/touchscreen/ar1021_i2c.c   |  5 +----
 drivers/input/touchscreen/atmel_mxt_ts.c |  5 ++---
 drivers/input/touchscreen/bu21029_ts.c   |  4 ++--
 drivers/input/touchscreen/cyttsp_core.c  |  5 ++---
 drivers/input/touchscreen/melfas_mip4.c  |  5 ++---
 drivers/input/touchscreen/mms114.c       |  4 ++--
 drivers/input/touchscreen/stmfts.c       |  3 +--
 drivers/input/touchscreen/wm831x-ts.c    |  3 +--
 drivers/input/touchscreen/zinitix.c      |  4 ++--
 include/linux/interrupt.h                |  3 +++
 kernel/irq/manage.c                      |  8 ++++++++
 kernel/irq/settings.h                    | 10 ++++++++++
 14 files changed, 39 insertions(+), 28 deletions(-)

-- 
2.25.1

[PATCH v3 01/12] genirq: add IRQF_NO_AUTOEN for request_irq

[Barry Song]

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>
---
 include/linux/interrupt.h |  3 +++
 kernel/irq/manage.c       |  8 ++++++++
 kernel/irq/settings.h     | 10 ++++++++++
 3 files changed, 21 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..2b28314e2572 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);
+
 		if (irq_settings_can_autoenable(desc)) {
 			irq_startup(desc, IRQ_RESEND, IRQ_START_COND);
 		} else {
@@ -2086,10 +2089,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;
diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
index 403378b9947b..a28958a9c548 100644
--- a/kernel/irq/settings.h
+++ b/kernel/irq/settings.h
@@ -145,6 +145,16 @@ static inline bool irq_settings_can_move_pcntxt(struct irq_desc *desc)
 	return desc->status_use_accessors & _IRQ_MOVE_PCNTXT;
 }
 
+static inline void irq_settings_clr_noautoen(struct irq_desc *desc)
+{
+	desc->status_use_accessors &= ~_IRQ_NOAUTOEN;
+}
+
+static inline void irq_settings_set_noautoen(struct irq_desc *desc)
+{
+	desc->status_use_accessors |= _IRQ_NOAUTOEN;
+}
+
 static inline bool irq_settings_can_autoenable(struct irq_desc *desc)
 {
 	return !(desc->status_use_accessors & _IRQ_NOAUTOEN);
-- 
2.25.1

[PATCH v3 02/12] Input: ar1021 - request_irq by IRQF_NO_AUTOEN and remove disable_irq

[Barry Song]

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/ar1021_i2c.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/ar1021_i2c.c b/drivers/input/touchscreen/ar1021_i2c.c
index c0d5c2413356..dc6a85362a40 100644
--- a/drivers/input/touchscreen/ar1021_i2c.c
+++ b/drivers/input/touchscreen/ar1021_i2c.c
@@ -125,7 +125,7 @@ static int ar1021_i2c_probe(struct i2c_client *client,
 
 	error = devm_request_threaded_irq(&client->dev, client->irq,
 					  NULL, ar1021_i2c_irq,
-					  IRQF_ONESHOT,
+					  IRQF_ONESHOT | IRQF_NO_AUTOEN,
 					  "ar1021_i2c", ar1021);
 	if (error) {
 		dev_err(&client->dev,
@@ -133,9 +133,6 @@ static int ar1021_i2c_probe(struct i2c_client *client,
 		return error;
 	}
 
-	/* Disable the IRQ, we'll enable it in ar1021_i2c_open() */
-	disable_irq(client->irq);
-
 	error = input_register_device(ar1021->input);
 	if (error) {
 		dev_err(&client->dev,
-- 
2.25.1

[PATCH v3 03/12] Input: atmel_mxt_ts - request_irq by IRQF_NO_AUTOEN and remove disable_irq

[Barry Song]

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) {
-- 
2.25.1

[PATCH v3 04/12] Input: melfas_mip4 - request_irq by IRQF_NO_AUTOEN and remove disable_irq

[Barry Song]

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/melfas_mip4.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/melfas_mip4.c b/drivers/input/touchscreen/melfas_mip4.c
index c0050044a5a9..dd6d4cc5f8c2 100644
--- a/drivers/input/touchscreen/melfas_mip4.c
+++ b/drivers/input/touchscreen/melfas_mip4.c
@@ -1502,7 +1502,8 @@ static int mip4_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 	error = devm_request_threaded_irq(&client->dev, client->irq,
 					  NULL, mip4_interrupt,
-					  IRQF_ONESHOT, MIP4_DEVICE_NAME, ts);
+					  IRQF_ONESHOT | IRQF_NO_AUTOEN,
+					  MIP4_DEVICE_NAME, ts);
 	if (error) {
 		dev_err(&client->dev,
 			"Failed to request interrupt %d: %d\n",
@@ -1510,8 +1511,6 @@ static int mip4_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return error;
 	}
 
-	disable_irq(client->irq);
-
 	error = input_register_device(input);
 	if (error) {
 		dev_err(&client->dev,
-- 
2.25.1

[PATCH v3 05/12] Input: bu21029_ts - request_irq by IRQF_NO_AUTOEN and remove irq_set_status_flags

[Barry Song]

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);
-- 
2.25.1

[PATCH v3 06/12] Input: stmfts - request_irq by IRQF_NO_AUTOEN and remove irq_set_status_flags

[Barry Song]

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/stmfts.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
index 9a64e1dbc04a..bc11203c9cf7 100644
--- a/drivers/input/touchscreen/stmfts.c
+++ b/drivers/input/touchscreen/stmfts.c
@@ -691,10 +691,9 @@ static int stmfts_probe(struct i2c_client *client,
 	 * interrupts. To be on the safe side it's better to not enable
 	 * the interrupts during their request.
 	 */
-	irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
 	err = devm_request_threaded_irq(&client->dev, client->irq,
 					NULL, stmfts_irq_handler,
-					IRQF_ONESHOT,
+					IRQF_ONESHOT | IRQF_NO_AUTOEN,
 					"stmfts_irq", sdata);
 	if (err)
 		return err;
-- 
2.25.1

[PATCH v3 07/12] Input: zinitix - request_irq by IRQF_NO_AUTOEN and remove irq_set_status_flags

[Barry Song]

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;
-- 
2.25.1

[PATCH v3 08/12] Input: mms114 - request_irq by IRQF_NO_AUTOEN and remove disable_irq

[Barry Song]

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/mms114.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 16557f51b09d..7043f57ea2dd 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -530,13 +530,13 @@ static int mms114_probe(struct i2c_client *client,
 	}
 
 	error = devm_request_threaded_irq(&client->dev, client->irq,
-					  NULL, mms114_interrupt, IRQF_ONESHOT,
+					  NULL, mms114_interrupt,
+					  IRQF_ONESHOT | IRQF_NO_AUTOEN,
 					  dev_name(&client->dev), data);
 	if (error) {
 		dev_err(&client->dev, "Failed to register interrupt\n");
 		return error;
 	}
-	disable_irq(client->irq);
 
 	error = input_register_device(data->input_dev);
 	if (error) {
-- 
2.25.1

[PATCH v3 09/12] Input: wm831x-ts - request_irq by IRQF_NO_AUTOEN and remove disable_irq

[Barry Song]

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;
-- 
2.25.1

[PATCH v3 10/12] Input: cyttsp - request_irq by IRQF_NO_AUTOEN and remove disable_irq

[Barry Song]

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/cyttsp_core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
index 73c854f35f33..d5c933604168 100644
--- a/drivers/input/touchscreen/cyttsp_core.c
+++ b/drivers/input/touchscreen/cyttsp_core.c
@@ -652,7 +652,8 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 	}
 
 	error = devm_request_threaded_irq(dev, ts->irq, NULL, cyttsp_irq,
-					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
+					  IRQF_NO_AUTOEN,
 					  "cyttsp", ts);
 	if (error) {
 		dev_err(ts->dev, "failed to request IRQ %d, err: %d\n",
@@ -660,8 +661,6 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 		return ERR_PTR(error);
 	}
 
-	disable_irq(ts->irq);
-
 	cyttsp_hard_reset(ts);
 
 	error = cyttsp_power_on(ts);
-- 
2.25.1

[PATCH v3 11/12] Input: tegra-kbc - request_irq by IRQF_NO_AUTOEN and remove disable_irq

[Barry Song]

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");
-- 
2.25.1

[PATCH v3 12/12] Input: tca6416-keypad - request_irq by IRQF_NO_AUTOEN and remove disable_irq

[Barry Song]

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/tca6416-keypad.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/tca6416-keypad.c b/drivers/input/keyboard/tca6416-keypad.c
index 9b0f9665dcb0..2a9755910065 100644
--- a/drivers/input/keyboard/tca6416-keypad.c
+++ b/drivers/input/keyboard/tca6416-keypad.c
@@ -274,7 +274,7 @@ static int tca6416_keypad_probe(struct i2c_client *client,
 		error = request_threaded_irq(chip->irqnum, NULL,
 					     tca6416_keys_isr,
 					     IRQF_TRIGGER_FALLING |
-						IRQF_ONESHOT,
+					     IRQF_ONESHOT | IRQF_NO_AUTOEN,
 					     "tca6416-keypad", chip);
 		if (error) {
 			dev_dbg(&client->dev,
@@ -282,7 +282,6 @@ static int tca6416_keypad_probe(struct i2c_client *client,
 				chip->irqnum, error);
 			goto fail1;
 		}
-		disable_irq(chip->irqnum);
 	}
 
 	error = input_register_device(input);
-- 
2.25.1

RE: [PATCH v3 00/12] add IRQF_NO_AUTOEN for request_irq

[Song Bao Hua (Barry Song)]

Hi Thomas, Greg, Dmitry, Marc,
Any further comment on this new API? 

Thanks
Barry

> -----Original Message-----
> From: Song Bao Hua (Barry Song)
> Sent: Friday, January 8, 2021 11:39 AM
> To: dmitry.torokhov@gmail.com; tglx@linutronix.de; 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 v3 00/12] add IRQF_NO_AUTOEN for request_irq
> 
> This patchset added IRQF_NO_AUTOEN for request_irq() and converted
> drivers/input to this new API.
> Other drivers will be handled afterwards.
> 
> -v3:
>   added examples with respect to Greg's comment
> 
> Barry Song (12):
>   genirq: add IRQF_NO_AUTOEN for request_irq
>   Input: ar1021 - request_irq by IRQF_NO_AUTOEN and remove disable_irq
>   Input: atmel_mxt_ts - request_irq by IRQF_NO_AUTOEN and remove
>     disable_irq
>   Input: melfas_mip4 - request_irq by IRQF_NO_AUTOEN and remove
>     disable_irq
>   Input: bu21029_ts - request_irq by IRQF_NO_AUTOEN and remove
>     irq_set_status_flags
>   Input: stmfts - request_irq by IRQF_NO_AUTOEN and remove
>     irq_set_status_flags
>   Input: zinitix - request_irq by IRQF_NO_AUTOEN and remove
>     irq_set_status_flags
>   Input: mms114 - request_irq by IRQF_NO_AUTOEN and remove disable_irq
>   Input: wm831x-ts - request_irq by IRQF_NO_AUTOEN and remove
>     disable_irq
>   Input: cyttsp - request_irq by IRQF_NO_AUTOEN and remove disable_irq
>   Input: tegra-kbc - request_irq by IRQF_NO_AUTOEN and remove
>     disable_irq
>   Input: tca6416-keypad - request_irq by IRQF_NO_AUTOEN and remove
>     disable_irq
> 
>  drivers/input/keyboard/tca6416-keypad.c  |  3 +--
>  drivers/input/keyboard/tegra-kbc.c       |  5 ++---
>  drivers/input/touchscreen/ar1021_i2c.c   |  5 +----
>  drivers/input/touchscreen/atmel_mxt_ts.c |  5 ++---
>  drivers/input/touchscreen/bu21029_ts.c   |  4 ++--
>  drivers/input/touchscreen/cyttsp_core.c  |  5 ++---
>  drivers/input/touchscreen/melfas_mip4.c  |  5 ++---
>  drivers/input/touchscreen/mms114.c       |  4 ++--
>  drivers/input/touchscreen/stmfts.c       |  3 +--
>  drivers/input/touchscreen/wm831x-ts.c    |  3 +--
>  drivers/input/touchscreen/zinitix.c      |  4 ++--
>  include/linux/interrupt.h                |  3 +++
>  kernel/irq/manage.c                      |  8 ++++++++
>  kernel/irq/settings.h                    | 10 ++++++++++
>  14 files changed, 39 insertions(+), 28 deletions(-)
> 
> --
> 2.25.1

Re: [PATCH v3 00/12] add IRQF_NO_AUTOEN for request_irq

[gregkh]

On Thu, Jan 21, 2021 at 09:38:28PM +0000, Song Bao Hua (Barry Song) wrote:
> Hi Thomas, Greg, Dmitry, Marc,
> Any further comment on this new API? 

It's not my subsystem, I'll let the irq maintainers handle it :)

thanks,

greg k-h

Re: [PATCH v3 00/12] add IRQF_NO_AUTOEN for request_irq

[dmitry.torokhov]

On Wed, Jan 27, 2021 at 02:49:21PM +0100, gregkh@linuxfoundation.org wrote:
> On Thu, Jan 21, 2021 at 09:38:28PM +0000, Song Bao Hua (Barry Song) wrote:
> > Hi Thomas, Greg, Dmitry, Marc,
> > Any further comment on this new API? 
> 
> It's not my subsystem, I'll let the irq maintainers handle it :)

Not my subsystem either, but I would like to have this feature
available. I do not like calling irq_set_status_flags() before
request_irq() as at that time we are not ensured of irq ownership, and
using disable_irq() afterwards is indeed awkward.

Thanks.

-- 
Dmitry

Re: [Linuxarm] Re: [PATCH v3 00/12] add IRQF_NO_AUTOEN for request_irq

[Jonathan Cameron]

On Wed, 27 Jan 2021 11:45:45 -0800
"dmitry.torokhov@gmail.com" <dmitry.torokhov@gmail.com> wrote:

> On Wed, Jan 27, 2021 at 02:49:21PM +0100, gregkh@linuxfoundation.org wrote:
> > On Thu, Jan 21, 2021 at 09:38:28PM +0000, Song Bao Hua (Barry Song) wrote:  
> > > Hi Thomas, Greg, Dmitry, Marc,
> > > Any further comment on this new API?   
> > 
> > It's not my subsystem, I'll let the irq maintainers handle it :)  
> 
> Not my subsystem either, but I would like to have this feature
> available. I do not like calling irq_set_status_flags() before
> request_irq() as at that time we are not ensured of irq ownership, and
> using disable_irq() afterwards is indeed awkward.
> 

Likewise, keen on having this available.

Jonathan

> Thanks.
> 

Re: [PATCH v3 01/12] genirq: add IRQF_NO_AUTOEN for request_irq

[Thomas Gleixner]

Barry,

On Fri, Jan 08 2021 at 11:39, Barry Song wrote:
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index ab8567f32501..2b28314e2572 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);

If we move this to request time flags, then setting the noautoen bit on
the irq descriptor is pretty pointless. See below.

I rather get rid of the irq_settings magic for NOAUTOEN completely.

Thanks,

        tglx
---
--- 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)
 
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1693,7 +1693,8 @@ static int
 			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 ir
 	 * 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;

RE: [PATCH v3 01/12] genirq: add IRQF_NO_AUTOEN for request_irq

[Song Bao Hua (Barry Song)]

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Friday, January 29, 2021 8:55 AM
> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>;
> 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: Re: [PATCH v3 01/12] genirq: add IRQF_NO_AUTOEN for request_irq
> 
> Barry,
> 
> On Fri, Jan 08 2021 at 11:39, Barry Song wrote:
> > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> > index ab8567f32501..2b28314e2572 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);
> 
> If we move this to request time flags, then setting the noautoen bit on
> the irq descriptor is pretty pointless. See below.
> 
> I rather get rid of the irq_settings magic for NOAUTOEN completely.

Thanks for your comment, Thomas.

Got this issue fixed in v4:
https://lore.kernel.org/lkml/20210128223538.20272-1-song.bao.hua@hisilicon.com/

btw, for those drivers which are using the first pattern:
irq_set_status_flags(irq, IRQ_NOAUTOEN);
request_irq(dev, irq...);

Simply running "git grep IRQ_NOAUTOEN"  will help figure where to fix.

For those drivers which are using the second pattern:
request_irq(dev, irq...);
disable_irq(irq);

I wrote a script as below:

#!/bin/bash
if [ $# != 1 -o ! -d $1 ] ; then
        echo "USAGE: $0 dir"
        exit 1;
fi

find $1 -iname "*.c" | while read i
do
        if [ -d "$i" ]; then
                break
        fi

        irq=`grep -n -A 10 -E "request_irq|request_threaded_irq|request_any_context_irq" $i | grep disable_irq` 
        if [ "$irq" != "" ]; then
                echo "$i":"$irq"
        fi
done

The script says there are more than 70 cases in 5.11-rc6.
We are going to fix all of them after this one settles down.

Thanks
Barry

> 
> Thanks,
> 
>         tglx
> ---
> --- 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)
> 
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1693,7 +1693,8 @@ static int
>  			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 ir
>  	 * 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;