[PATCH] da9052: ensure da9052_ts_input_open will not lock up on irq flood

It is possible that da9052_ts_input_open is called and the touchscreen
is triggering continuous interrupts. In this case we will never leave
da9052_ts_input_open. The da9052_reg_update will never get the i2c bus
as the interrupt handler will continuously lock it.

This patch switch places of da9052_reg_update and the setting of
tsi->stopped. This way its ensured that da9052_ts_input_open will never
lockup on interrupt flood.

Signed-off-by: Michael Grzeschik <redacted>
---
 drivers/input/touchscreen/da9052_tsi.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/da9052_tsi.c b/drivers/input/touchscreen/da9052_tsi.c
index 5a013bb..1b136b8 100644
--- a/drivers/input/touchscreen/da9052_tsi.c
+++ b/drivers/input/touchscreen/da9052_tsi.c

@@ -189,17 +189,20 @@ static int da9052_configure_tsi(struct da9052_tsi *tsi)
 
 static int da9052_ts_input_open(struct input_dev *input_dev)
 {
+	int val;
 	struct da9052_tsi *tsi = input_get_drvdata(input_dev);
 
-	tsi->stopped = false;
-	mb();
+	val = da9052_reg_update(tsi->da9052, DA9052_TSI_CONT_A_REG,
+				 1 << 1, 1 << 1);
 
 	/* Unmask PEN_DOWN event */
 	da9052_enable_irq(tsi->da9052, DA9052_IRQ_PENDOWN);
 
+	tsi->stopped = false;
+	mb();
+
 	/* Enable Pen Detect Circuit */
-	return da9052_reg_update(tsi->da9052, DA9052_TSI_CONT_A_REG,
-				 1 << 1, 1 << 1);
+	return val;
 }
 
 static void da9052_ts_input_close(struct input_dev *input_dev)

-- 
2.7.0

On Thu, Mar 03, 2016 at 11:52:02AM +0100, Michael Grzeschik wrote:

It is possible that da9052_ts_input_open is called and the touchscreen
is triggering continuous interrupts. In this case we will never leave
da9052_ts_input_open. The da9052_reg_update will never get the i2c bus
as the interrupt handler will continuously lock it.

This patch switch places of da9052_reg_update and the setting of
tsi->stopped. This way its ensured that da9052_ts_input_open will never
lockup on interrupt flood.

I understand what you are saying about updating register, however I do
not understand why we would want to swap unmasking pendown event ans
clearing the "stopped" flag.

Thanks.

-- 
Dmitry

On Fri, Mar 11, 2016 at 11:33:11AM -0800, Dmitry Torokhov wrote:

On Thu, Mar 03, 2016 at 11:52:02AM +0100, Michael Grzeschik wrote:

quoted

It is possible that da9052_ts_input_open is called and the touchscreen
is triggering continuous interrupts. In this case we will never leave
da9052_ts_input_open. The da9052_reg_update will never get the i2c bus
as the interrupt handler will continuously lock it.

This patch switch places of da9052_reg_update and the setting of
tsi->stopped. This way its ensured that da9052_ts_input_open will never
lockup on interrupt flood.

I understand what you are saying about updating register, however I do
not understand why we would want to swap unmasking pendown event ans
clearing the "stopped" flag.

The "stopped" flag has an locking purpose in the irq handler. My patch
ensures that the irq handler will not trigger any da9052_reg_update
before da9052_ts_input_open has finished.

On second thought, it would probably be enough to swap the call of
da9052_reg_update and da9052_enable_irq in da9052_ts_input_open.

This will have the same effect. I will send an corrected v2.

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |