* [PATCH] input: eeti_ts: cancel pending work when going to suspend
@ 2010-04-13 18:31 Daniel Mack
2010-04-14 18:14 ` Dmitry Torokhov
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Mack @ 2010-04-13 18:31 UTC (permalink / raw)
To: linux-input; +Cc: Daniel Mack, Dmitry Torokhov
This fixes a race between the suspend code and input events.
Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/eeti_ts.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/input/touchscreen/eeti_ts.c b/drivers/input/touchscreen/eeti_ts.c
index 204b8a1..2a01695 100644
--- a/drivers/input/touchscreen/eeti_ts.c
+++ b/drivers/input/touchscreen/eeti_ts.c
@@ -250,6 +250,9 @@ static int eeti_ts_suspend(struct i2c_client *client, pm_message_t mesg)
if (device_may_wakeup(&client->dev))
enable_irq_wake(priv->irq);
+ disable_irq(priv->irq);
+ cancel_work_sync(&priv->work);
+
return 0;
}
@@ -260,6 +263,10 @@ static int eeti_ts_resume(struct i2c_client *client)
if (device_may_wakeup(&client->dev))
disable_irq_wake(priv->irq);
+ /* If we have active users, read the events once to arm the IRQ */
+ if (priv->input->users)
+ eeti_ts_read(&priv->work);
+
return 0;
}
#else
--
1.7.0.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] input: eeti_ts: cancel pending work when going to suspend
2010-04-13 18:31 [PATCH] input: eeti_ts: cancel pending work when going to suspend Daniel Mack
@ 2010-04-14 18:14 ` Dmitry Torokhov
2010-04-19 14:15 ` Daniel Mack
0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Torokhov @ 2010-04-14 18:14 UTC (permalink / raw)
To: Daniel Mack; +Cc: linux-input
On Tue, Apr 13, 2010 at 08:31:45PM +0200, Daniel Mack wrote:
> This fixes a race between the suspend code and input events.
>
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/touchscreen/eeti_ts.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/touchscreen/eeti_ts.c b/drivers/input/touchscreen/eeti_ts.c
> index 204b8a1..2a01695 100644
> --- a/drivers/input/touchscreen/eeti_ts.c
> +++ b/drivers/input/touchscreen/eeti_ts.c
> @@ -250,6 +250,9 @@ static int eeti_ts_suspend(struct i2c_client *client, pm_message_t mesg)
> if (device_may_wakeup(&client->dev))
> enable_irq_wake(priv->irq);
>
> + disable_irq(priv->irq);
> + cancel_work_sync(&priv->work);
You need to check 'users' when disabling as well, otherwise you may get
enable/disable unbalanced when there are no users.
Does the patch below still work for you?
--
Dmitry
Input: eeti_ts - cancel pending work when going to suspend
From: Daniel Mack <daniel@caiaq.de>
This fixes a race between the suspend code and input events.
Signed-off-by: Daniel Mack <daniel@caiaq.de>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
drivers/input/touchscreen/eeti_ts.c | 56 +++++++++++++++++++++++++++++------
1 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/eeti_ts.c b/drivers/input/touchscreen/eeti_ts.c
index 204b8a1..75f8b73 100644
--- a/drivers/input/touchscreen/eeti_ts.c
+++ b/drivers/input/touchscreen/eeti_ts.c
@@ -124,14 +124,25 @@ static irqreturn_t eeti_ts_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
-static int eeti_ts_open(struct input_dev *dev)
+static void eeti_ts_start(struct eeti_ts_priv *priv)
{
- struct eeti_ts_priv *priv = input_get_drvdata(dev);
-
enable_irq(priv->irq);
/* Read the events once to arm the IRQ */
eeti_ts_read(&priv->work);
+}
+
+static void eeti_ts_stop(struct eeti_ts_priv *priv)
+{
+ disable_irq(priv->irq);
+ cancel_work_sync(&priv->work);
+}
+
+static int eeti_ts_open(struct input_dev *dev)
+{
+ struct eeti_ts_priv *priv = input_get_drvdata(dev);
+
+ eeti_ts_start(priv);
return 0;
}
@@ -140,8 +151,7 @@ static void eeti_ts_close(struct input_dev *dev)
{
struct eeti_ts_priv *priv = input_get_drvdata(dev);
- disable_irq(priv->irq);
- cancel_work_sync(&priv->work);
+ eeti_ts_stop(priv);
}
static int __devinit eeti_ts_probe(struct i2c_client *client,
@@ -153,10 +163,12 @@ static int __devinit eeti_ts_probe(struct i2c_client *client,
unsigned int irq_flags;
int err = -ENOMEM;
- /* In contrast to what's described in the datasheet, there seems
+ /*
+ * In contrast to what's described in the datasheet, there seems
* to be no way of probing the presence of that device using I2C
* commands. So we need to blindly believe it is there, and wait
- * for interrupts to occur. */
+ * for interrupts to occur.
+ */
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv) {
@@ -212,9 +224,11 @@ static int __devinit eeti_ts_probe(struct i2c_client *client,
goto err2;
}
- /* Disable the irq for now. It will be enabled once the input device
- * is opened. */
- disable_irq(priv->irq);
+ /*
+ * Disable the device for now. It will be enabled once the
+ * input device is opened.
+ */
+ eeti_ts_stop(priv);
device_init_wakeup(&client->dev, 0);
return 0;
@@ -235,6 +249,12 @@ static int __devexit eeti_ts_remove(struct i2c_client *client)
struct eeti_ts_priv *priv = i2c_get_clientdata(client);
free_irq(priv->irq, priv);
+ /*
+ * eeti_ts_stop() leaves IRQ disabled. We need to re-enable it
+ * so that device still works if we reload the driver.
+ */
+ enable_irq(priv->irq);
+
input_unregister_device(priv->input);
i2c_set_clientdata(client, NULL);
kfree(priv);
@@ -246,6 +266,14 @@ static int __devexit eeti_ts_remove(struct i2c_client *client)
static int eeti_ts_suspend(struct i2c_client *client, pm_message_t mesg)
{
struct eeti_ts_priv *priv = i2c_get_clientdata(client);
+ struct input_dev *input_dev = priv->input;
+
+ mutex_lock(&input_dev->mutex);
+
+ if (input_dev->users)
+ eeti_ts_stop(priv);
+
+ mutex_unlock(&input_dev->mutex);
if (device_may_wakeup(&client->dev))
enable_irq_wake(priv->irq);
@@ -256,10 +284,18 @@ static int eeti_ts_suspend(struct i2c_client *client, pm_message_t mesg)
static int eeti_ts_resume(struct i2c_client *client)
{
struct eeti_ts_priv *priv = i2c_get_clientdata(client);
+ struct input_dev *input_dev = priv->input;
if (device_may_wakeup(&client->dev))
disable_irq_wake(priv->irq);
+ mutex_lock(&input_dev->mutex);
+
+ if (input_dev->users)
+ eeti_ts_start(priv);
+
+ mutex_unlock(&input_dev->mutex);
+
return 0;
}
#else
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] input: eeti_ts: cancel pending work when going to suspend
2010-04-14 18:14 ` Dmitry Torokhov
@ 2010-04-19 14:15 ` Daniel Mack
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Mack @ 2010-04-19 14:15 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
On Wed, Apr 14, 2010 at 11:14:18AM -0700, Dmitry Torokhov wrote:
> > This fixes a race between the suspend code and input events.
> >
> > Signed-off-by: Daniel Mack <daniel@caiaq.de>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/touchscreen/eeti_ts.c | 7 +++++++
> > 1 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/eeti_ts.c b/drivers/input/touchscreen/eeti_ts.c
> > index 204b8a1..2a01695 100644
> > --- a/drivers/input/touchscreen/eeti_ts.c
> > +++ b/drivers/input/touchscreen/eeti_ts.c
> > @@ -250,6 +250,9 @@ static int eeti_ts_suspend(struct i2c_client *client, pm_message_t mesg)
> > if (device_may_wakeup(&client->dev))
> > enable_irq_wake(priv->irq);
> >
> > + disable_irq(priv->irq);
> > + cancel_work_sync(&priv->work);
>
> You need to check 'users' when disabling as well, otherwise you may get
> enable/disable unbalanced when there are no users.
>
> Does the patch below still work for you?
Yes, looks good. Thanks :)
Daniel
> Input: eeti_ts - cancel pending work when going to suspend
>
> From: Daniel Mack <daniel@caiaq.de>
>
> This fixes a race between the suspend code and input events.
>
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>
> drivers/input/touchscreen/eeti_ts.c | 56 +++++++++++++++++++++++++++++------
> 1 files changed, 46 insertions(+), 10 deletions(-)
>
>
> diff --git a/drivers/input/touchscreen/eeti_ts.c b/drivers/input/touchscreen/eeti_ts.c
> index 204b8a1..75f8b73 100644
> --- a/drivers/input/touchscreen/eeti_ts.c
> +++ b/drivers/input/touchscreen/eeti_ts.c
> @@ -124,14 +124,25 @@ static irqreturn_t eeti_ts_isr(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static int eeti_ts_open(struct input_dev *dev)
> +static void eeti_ts_start(struct eeti_ts_priv *priv)
> {
> - struct eeti_ts_priv *priv = input_get_drvdata(dev);
> -
> enable_irq(priv->irq);
>
> /* Read the events once to arm the IRQ */
> eeti_ts_read(&priv->work);
> +}
> +
> +static void eeti_ts_stop(struct eeti_ts_priv *priv)
> +{
> + disable_irq(priv->irq);
> + cancel_work_sync(&priv->work);
> +}
> +
> +static int eeti_ts_open(struct input_dev *dev)
> +{
> + struct eeti_ts_priv *priv = input_get_drvdata(dev);
> +
> + eeti_ts_start(priv);
>
> return 0;
> }
> @@ -140,8 +151,7 @@ static void eeti_ts_close(struct input_dev *dev)
> {
> struct eeti_ts_priv *priv = input_get_drvdata(dev);
>
> - disable_irq(priv->irq);
> - cancel_work_sync(&priv->work);
> + eeti_ts_stop(priv);
> }
>
> static int __devinit eeti_ts_probe(struct i2c_client *client,
> @@ -153,10 +163,12 @@ static int __devinit eeti_ts_probe(struct i2c_client *client,
> unsigned int irq_flags;
> int err = -ENOMEM;
>
> - /* In contrast to what's described in the datasheet, there seems
> + /*
> + * In contrast to what's described in the datasheet, there seems
> * to be no way of probing the presence of that device using I2C
> * commands. So we need to blindly believe it is there, and wait
> - * for interrupts to occur. */
> + * for interrupts to occur.
> + */
>
> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> if (!priv) {
> @@ -212,9 +224,11 @@ static int __devinit eeti_ts_probe(struct i2c_client *client,
> goto err2;
> }
>
> - /* Disable the irq for now. It will be enabled once the input device
> - * is opened. */
> - disable_irq(priv->irq);
> + /*
> + * Disable the device for now. It will be enabled once the
> + * input device is opened.
> + */
> + eeti_ts_stop(priv);
>
> device_init_wakeup(&client->dev, 0);
> return 0;
> @@ -235,6 +249,12 @@ static int __devexit eeti_ts_remove(struct i2c_client *client)
> struct eeti_ts_priv *priv = i2c_get_clientdata(client);
>
> free_irq(priv->irq, priv);
> + /*
> + * eeti_ts_stop() leaves IRQ disabled. We need to re-enable it
> + * so that device still works if we reload the driver.
> + */
> + enable_irq(priv->irq);
> +
> input_unregister_device(priv->input);
> i2c_set_clientdata(client, NULL);
> kfree(priv);
> @@ -246,6 +266,14 @@ static int __devexit eeti_ts_remove(struct i2c_client *client)
> static int eeti_ts_suspend(struct i2c_client *client, pm_message_t mesg)
> {
> struct eeti_ts_priv *priv = i2c_get_clientdata(client);
> + struct input_dev *input_dev = priv->input;
> +
> + mutex_lock(&input_dev->mutex);
> +
> + if (input_dev->users)
> + eeti_ts_stop(priv);
> +
> + mutex_unlock(&input_dev->mutex);
>
> if (device_may_wakeup(&client->dev))
> enable_irq_wake(priv->irq);
> @@ -256,10 +284,18 @@ static int eeti_ts_suspend(struct i2c_client *client, pm_message_t mesg)
> static int eeti_ts_resume(struct i2c_client *client)
> {
> struct eeti_ts_priv *priv = i2c_get_clientdata(client);
> + struct input_dev *input_dev = priv->input;
>
> if (device_may_wakeup(&client->dev))
> disable_irq_wake(priv->irq);
>
> + mutex_lock(&input_dev->mutex);
> +
> + if (input_dev->users)
> + eeti_ts_start(priv);
> +
> + mutex_unlock(&input_dev->mutex);
> +
> return 0;
> }
> #else
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-04-19 14:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-13 18:31 [PATCH] input: eeti_ts: cancel pending work when going to suspend Daniel Mack
2010-04-14 18:14 ` Dmitry Torokhov
2010-04-19 14:15 ` Daniel Mack
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).