devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff LaBundy <jeff@labundy.com>
To: dmitry.torokhov@gmail.com, robh+dt@kernel.org
Cc: linux-input@vger.kernel.org, devicetree@vger.kernel.org,
	Jeff LaBundy <jeff@labundy.com>
Subject: [PATCH 6/9] Input: iqs5xx - prevent interrupt storm during removal
Date: Thu,  4 Mar 2021 22:12:33 -0600	[thread overview]
Message-ID: <20210305041236.3489-7-jeff@labundy.com> (raw)
In-Reply-To: <20210305041236.3489-1-jeff@labundy.com>

Unsolicited I2C communication causes the device to assert an interrupt; as
such the IRQ is disabled before any registers are written in iqs5xx_open()
and iqs5xx_close().

After the driver is unloaded, however, i2c_device_remove() sets the IRQ to
zero before any handlers may call input_close_device() while the device is
unregistered. This keeps iqs5xx_close() from disabling the IRQ, leading to
an interrupt storm during removal.

Placing input_register_device() in front of devm_request_threaded_irq() to
free the IRQ before iqs5xx_close() is called does not cover the case where
firmware is updated at the factory and the input device is registered well
after the driver has already probed.

The solution, therefore, is to remove the open and close callbacks as they
do not buy much in the first place. The device already starts in an active
state, then drops into a low-power mode based on activity.

As an added benefit, this change allows the 250-ms delay in initialization
to be removed as iqs5xx_open() no longer follows immediately. Instead, the
delay is replaced with a mere 50-us delay which allows the interrupt to be
deasserted before the handler is registered.

Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 drivers/input/touchscreen/iqs5xx.c | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/input/touchscreen/iqs5xx.c b/drivers/input/touchscreen/iqs5xx.c
index a990c176abf7..350466ff6bbd 100644
--- a/drivers/input/touchscreen/iqs5xx.c
+++ b/drivers/input/touchscreen/iqs5xx.c
@@ -468,20 +468,6 @@ static int iqs5xx_set_state(struct i2c_client *client, u8 state)
 	return error2;
 }
 
-static int iqs5xx_open(struct input_dev *input)
-{
-	struct iqs5xx_private *iqs5xx = input_get_drvdata(input);
-
-	return iqs5xx_set_state(iqs5xx->client, IQS5XX_RESUME);
-}
-
-static void iqs5xx_close(struct input_dev *input)
-{
-	struct iqs5xx_private *iqs5xx = input_get_drvdata(input);
-
-	iqs5xx_set_state(iqs5xx->client, IQS5XX_SUSPEND);
-}
-
 static int iqs5xx_axis_init(struct i2c_client *client)
 {
 	struct iqs5xx_private *iqs5xx = i2c_get_clientdata(client);
@@ -497,10 +483,7 @@ static int iqs5xx_axis_init(struct i2c_client *client)
 
 		input->name = client->name;
 		input->id.bustype = BUS_I2C;
-		input->open = iqs5xx_open;
-		input->close = iqs5xx_close;
 
-		input_set_drvdata(input, iqs5xx);
 		iqs5xx->input = input;
 	}
 
@@ -622,13 +605,7 @@ static int iqs5xx_dev_init(struct i2c_client *client)
 
 	iqs5xx->dev_id_info = *dev_id_info;
 
-	/*
-	 * The following delay allows ATI to complete before the open and close
-	 * callbacks are free to elicit I2C communication. Any attempts to read
-	 * from or write to the device during this time may face extended clock
-	 * stretching and prompt the I2C controller to report an error.
-	 */
-	msleep(250);
+	usleep_range(50, 100);
 
 	return 0;
 }
-- 
2.17.1


  parent reply	other threads:[~2021-03-05  4:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-05  4:12 [PATCH 0/9] Input: iqs5xx - more enhancements and optimizations Jeff LaBundy
2021-03-05  4:12 ` [PATCH 1/9] Input: iqs5xx - update vendor's URL Jeff LaBundy
2021-03-05  4:12 ` [PATCH 2/9] Input: iqs5xx - optimize axis definition and validation Jeff LaBundy
2021-03-05  4:12 ` [PATCH 3/9] Input: iqs5xx - expose firmware revision to user space Jeff LaBundy
2021-03-05  4:12 ` [PATCH 4/9] Input: iqs5xx - remove superfluous revision validation Jeff LaBundy
2021-03-05  4:12 ` [PATCH 5/9] Input: iqs5xx - close bootloader using hardware reset Jeff LaBundy
2021-03-05  4:12 ` Jeff LaBundy [this message]
2021-03-05  4:12 ` [PATCH 7/9] Input: iqs5xx - suspend or resume regardless of users Jeff LaBundy
2021-03-05  4:12 ` [PATCH 8/9] Input: iqs5xx - make reset GPIO optional Jeff LaBundy
2021-03-05  4:12 ` [PATCH 9/9] dt-bindings: input: iqs5xx: Convert to YAML Jeff LaBundy
2021-03-08 21:46   ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210305041236.3489-7-jeff@labundy.com \
    --to=jeff@labundy.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).