WM97xx touchscreen updates

WM97xx touchscreen updates

[Mark Brown]

This patch series updates the WM97xx touchscreen drivers with a couple
of small cleanups and the addition of support for use of the WM97xx
touchpanels to wake up from suspend.

It is also available in the git repository at:

  git://opensource.wolfsonmicro.com/linux-2.6-touch upstream

Mark Brown (3):
      wm97xx-core: Only schedule interrupt handler if not already scheduled
      wm97xx-core: Use IRQF_SAMPLE_RANDOM
      wm97xx-core: Support use as a wakeup source

 drivers/input/touchscreen/wm97xx-core.c |   39 ++++++++++++++++++++++++------
 include/linux/wm97xx.h                  |    3 ++
 2 files changed, 34 insertions(+), 8 deletions(-)

[PATCH 1/3] wm97xx-core: Only schedule interrupt handler if not already scheduled

[Mark Brown]

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/input/touchscreen/wm97xx-core.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
index 2910999..e27b1e0 100644
--- a/drivers/input/touchscreen/wm97xx-core.c
+++ b/drivers/input/touchscreen/wm97xx-core.c
@@ -328,18 +328,18 @@ static void wm97xx_pen_irq_worker(struct work_struct *work)
  *
  * We have to disable the codec interrupt in the handler because it
  * can take upto 1ms to clear the interrupt source. We schedule a task
- * in a work queue to do the actual interaction with the chip (it
- * doesn't matter if we end up reenqueing it before it is executed
- * since we don't touch the chip until it has run).  The interrupt is
- * then enabled again in the slow handler when the source has been
- * cleared.
+ * in a work queue to do the actual interaction with the chip.  The
+ * interrupt is then enabled again in the slow handler when the source
+ * has been cleared.
  */
 static irqreturn_t wm97xx_pen_interrupt(int irq, void *dev_id)
 {
 	struct wm97xx *wm = dev_id;
 
-	wm->mach_ops->irq_enable(wm, 0);
-	queue_work(wm->ts_workq, &wm->pen_event_work);
+	if (!work_pending(&wm->pen_event_work)) {
+		wm->mach_ops->irq_enable(wm, 0);
+		queue_work(wm->ts_workq, &wm->pen_event_work);
+	}
 
 	return IRQ_HANDLED;
 }
-- 
1.5.5

[PATCH 2/3] wm97xx-core: Use IRQF_SAMPLE_RANDOM

[Mark Brown]

The touchscreen interrupt is driven by human input which can reasonably
be used to provide entropy.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/input/touchscreen/wm97xx-core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
index e27b1e0..fec07c2 100644
--- a/drivers/input/touchscreen/wm97xx-core.c
+++ b/drivers/input/touchscreen/wm97xx-core.c
@@ -355,7 +355,8 @@ static int wm97xx_init_pen_irq(struct wm97xx *wm)
 	 * provided. */
 	BUG_ON(!wm->mach_ops->irq_enable);
 
-	if (request_irq(wm->pen_irq, wm97xx_pen_interrupt, IRQF_SHARED,
+	if (request_irq(wm->pen_irq, wm97xx_pen_interrupt,
+			IRQF_SHARED | IRQF_SAMPLE_RANDOM,
 			"wm97xx-pen", wm)) {
 		dev_err(wm->dev,
 			"Failed to register pen down interrupt, polling");
-- 
1.5.5

[PATCH 3/3] wm97xx-core: Support use as a wakeup source

[Mark Brown]

The PENDOWN signal from the WM97xx touch screen controllers can be used
to generate a wakeup event when the system is suspended. Provide a new
machine configuration option 'suspend_mode' allowing machine drivers to
enable this. If no suspend_mode is provided then the touch panel will be
powered down when the system is suspended.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/input/touchscreen/wm97xx-core.c |   22 ++++++++++++++++++++++
 include/linux/wm97xx.h                  |    3 +++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
index fec07c2..e1a1ca0 100644
--- a/drivers/input/touchscreen/wm97xx-core.c
+++ b/drivers/input/touchscreen/wm97xx-core.c
@@ -689,10 +689,32 @@ static int wm97xx_remove(struct device *dev)
 static int wm97xx_suspend(struct device *dev, pm_message_t state)
 {
 	struct wm97xx *wm = dev_get_drvdata(dev);
+	u16 reg;
+	int suspend_mode;
+
+	if (wm->mach_ops)
+		suspend_mode = wm->mach_ops->suspend_mode;
+	else
+		suspend_mode = 0;
 
 	if (wm->input_dev->users)
 		cancel_delayed_work_sync(&wm->ts_reader);
 
+	/* Power down the digitiser (bypassing the cache for resume) */
+	reg = wm97xx_reg_read(wm, AC97_WM97XX_DIGITISER2);
+	reg &= ~WM97XX_PRP_DET_DIG;
+	if (wm->input_dev->users && suspend_mode)
+		reg |= suspend_mode;
+	wm->ac97->bus->ops->write(wm->ac97, AC97_WM97XX_DIGITISER2, reg);
+
+	/* WM9713 has an additional power bit - turn it off if there
+	 * are no users or if suspend mode is zero. */
+	if (wm->id == WM9713_ID2 &&
+	    (!wm->input_dev->users || !suspend_mode)) {
+		reg = wm97xx_reg_read(wm, AC97_EXTENDED_MID) | 0x8000;
+		wm97xx_reg_write(wm, AC97_EXTENDED_MID, reg);
+	}
+
 	return 0;
 }
 
diff --git a/include/linux/wm97xx.h b/include/linux/wm97xx.h
index ed01c7d..2082833 100644
--- a/include/linux/wm97xx.h
+++ b/include/linux/wm97xx.h
@@ -258,6 +258,9 @@ struct wm97xx_mach_ops {
 	/* pre and post sample - can be used to minimise any analog noise */
 	void (*pre_sample) (int);  /* function to run before sampling */
 	void (*post_sample) (int);  /* function to run after sampling */
+
+	/* WM97XX_PRP value to configure while system is suspended */
+	u16 suspend_mode;
 };
 
 struct wm97xx {
-- 
1.5.5

Re: [PATCH 3/3] wm97xx-core: Support use as a wakeup source

[Dmitry Torokhov]

Hi Mark,

On Mon, Apr 14, 2008 at 06:39:35PM +0100, Mark Brown wrote:
> The PENDOWN signal from the WM97xx touch screen controllers can be used
> to generate a wakeup event when the system is suspended. Provide a new
> machine configuration option 'suspend_mode' allowing machine drivers to
> enable this. If no suspend_mode is provided then the touch panel will be
> powered down when the system is suspended.
> 

Hmm, another unique module option... How about using device_can_wakeup()
infrastructure that we already have. I'm adding David Brownell to CC since
I believe he's doing a lot of similar work.

> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/input/touchscreen/wm97xx-core.c |   22 ++++++++++++++++++++++
>  include/linux/wm97xx.h                  |    3 +++
>  2 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
> index fec07c2..e1a1ca0 100644
> --- a/drivers/input/touchscreen/wm97xx-core.c
> +++ b/drivers/input/touchscreen/wm97xx-core.c
> @@ -689,10 +689,32 @@ static int wm97xx_remove(struct device *dev)
>  static int wm97xx_suspend(struct device *dev, pm_message_t state)
>  {
>  	struct wm97xx *wm = dev_get_drvdata(dev);
> +	u16 reg;
> +	int suspend_mode;
> +
> +	if (wm->mach_ops)
> +		suspend_mode = wm->mach_ops->suspend_mode;
> +	else
> +		suspend_mode = 0;
>  
>  	if (wm->input_dev->users)
>  		cancel_delayed_work_sync(&wm->ts_reader);
>  
> +	/* Power down the digitiser (bypassing the cache for resume) */
> +	reg = wm97xx_reg_read(wm, AC97_WM97XX_DIGITISER2);
> +	reg &= ~WM97XX_PRP_DET_DIG;
> +	if (wm->input_dev->users && suspend_mode)
> +		reg |= suspend_mode;
> +	wm->ac97->bus->ops->write(wm->ac97, AC97_WM97XX_DIGITISER2, reg);
> +
> +	/* WM9713 has an additional power bit - turn it off if there
> +	 * are no users or if suspend mode is zero. */
> +	if (wm->id == WM9713_ID2 &&
> +	    (!wm->input_dev->users || !suspend_mode)) {
> +		reg = wm97xx_reg_read(wm, AC97_EXTENDED_MID) | 0x8000;
> +		wm97xx_reg_write(wm, AC97_EXTENDED_MID, reg);
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/include/linux/wm97xx.h b/include/linux/wm97xx.h
> index ed01c7d..2082833 100644
> --- a/include/linux/wm97xx.h
> +++ b/include/linux/wm97xx.h
> @@ -258,6 +258,9 @@ struct wm97xx_mach_ops {
>  	/* pre and post sample - can be used to minimise any analog noise */
>  	void (*pre_sample) (int);  /* function to run before sampling */
>  	void (*post_sample) (int);  /* function to run after sampling */
> +
> +	/* WM97XX_PRP value to configure while system is suspended */
> +	u16 suspend_mode;
>  };
>  
>  struct wm97xx {
> -- 
> 1.5.5
> 

-- 
Dmitry

Re: [PATCH 3/3] wm97xx-core: Support use as a wakeup source

[David Brownell]

On Monday 14 April 2008, Dmitry Torokhov wrote:
> Hi Mark,
> 
> On Mon, Apr 14, 2008 at 06:39:35PM +0100, Mark Brown wrote:
> > The PENDOWN signal from the WM97xx touch screen controllers can be used
> > to generate a wakeup event when the system is suspended. Provide a new
> > machine configuration option 'suspend_mode' allowing machine drivers to
> > enable this. If no suspend_mode is provided then the touch panel will be
> > powered down when the system is suspended.
> > 
> 
> Hmm, another unique module option... How about using device_can_wakeup()
> infrastructure that we already have. I'm adding David Brownell to CC since
> I believe he's doing a lot of similar work.

I don't quite follow the technical content of this "suspend_mode"
mask, but it sure looks as if it should use the driver model flags
in addition to whatever that mask is doing.

There are two aspects to this:

 (a) Whether on a given system the touchscreen *can* issue wakeup
     events ... call device_init_wakeup(dev, true) to say it can.
     In this case, presumably suspend_mode == 0 means it can't.
     Board setup, or at latest probe(), should device_init_wakeup().

 (b) Whether it *should* do so ... e.g. the Nokia N810 has a mode
     where it doesn't, and usually when you're carrying one around
     in a pocket (or purse, or whatever) you would say "doesn't".
     Userspace manages that answer via a sysfs file, and drivers
     test the answer with device_may_wakeup(dev).

So:  yes, this should initialize and use those driver model flags.


> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > ---
> >  drivers/input/touchscreen/wm97xx-core.c |   22 ++++++++++++++++++++++
> >  include/linux/wm97xx.h                  |    3 +++
> >  2 files changed, 25 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
> > index fec07c2..e1a1ca0 100644
> > --- a/drivers/input/touchscreen/wm97xx-core.c
> > +++ b/drivers/input/touchscreen/wm97xx-core.c
> > @@ -689,10 +689,32 @@ static int wm97xx_remove(struct device *dev)
> >  static int wm97xx_suspend(struct device *dev, pm_message_t state)
> >  {
> >  	struct wm97xx *wm = dev_get_drvdata(dev);
> > +	u16 reg;
> > +	int suspend_mode;
> > +
> > +	if (wm->mach_ops)
> > +		suspend_mode = wm->mach_ops->suspend_mode;
> > +	else
> > +		suspend_mode = 0;
> >  
> >  	if (wm->input_dev->users)
> >  		cancel_delayed_work_sync(&wm->ts_reader);
> >  
> > +	/* Power down the digitiser (bypassing the cache for resume) */
> > +	reg = wm97xx_reg_read(wm, AC97_WM97XX_DIGITISER2);
> > +	reg &= ~WM97XX_PRP_DET_DIG;
> > +	if (wm->input_dev->users && suspend_mode)

This should probably check device_may_wakeup(SOMEDEV), rather
than testing suspend_mode.

The wakeup mechanism here isn't clear to me.  Does this assume
that the touchscreen IRQ is wakeup-capable?  I don't see any
calls to enable_irq_wake() or disable_irq_wake() ... or for that
matter any other calls that might affect system behavior.


> > +		reg |= suspend_mode;
> > +	wm->ac97->bus->ops->write(wm->ac97, AC97_WM97XX_DIGITISER2, reg);
> > +
> > +	/* WM9713 has an additional power bit - turn it off if there
> > +	 * are no users or if suspend mode is zero. */
> > +	if (wm->id == WM9713_ID2 &&
> > +	    (!wm->input_dev->users || !suspend_mode)) {
> > +		reg = wm97xx_reg_read(wm, AC97_EXTENDED_MID) | 0x8000;
> > +		wm97xx_reg_write(wm, AC97_EXTENDED_MID, reg);
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > diff --git a/include/linux/wm97xx.h b/include/linux/wm97xx.h
> > index ed01c7d..2082833 100644
> > --- a/include/linux/wm97xx.h
> > +++ b/include/linux/wm97xx.h
> > @@ -258,6 +258,9 @@ struct wm97xx_mach_ops {
> >  	/* pre and post sample - can be used to minimise any analog noise */
> >  	void (*pre_sample) (int);  /* function to run before sampling */
> >  	void (*post_sample) (int);  /* function to run after sampling */
> > +
> > +	/* WM97XX_PRP value to configure while system is suspended */
> > +	u16 suspend_mode;

I'm more accustomed to thinking that the driver will
know "the" setup bit values to assign which will
ensure that the chip issues wakeup events ... that
stuff is usually not board-specific.


> >  };
> >  
> >  struct wm97xx {
> > -- 
> > 1.5.5
> > 
> 
> -- 
> Dmitry
> 

Re: [PATCH 3/3] wm97xx-core: Support use as a wakeup source

[Mark Brown]

On Mon, Apr 14, 2008 at 05:29:07PM -0700, David Brownell wrote:
> On Monday 14 April 2008, Dmitry Torokhov wrote:
> > Hmm, another unique module option... How about using device_can_wakeup()

The reason there are so many configuration options here is that many of
the features of the driver depend on exactly how the chip is connected
to the rest of the system and can't be automatically discovered.  In the
embedded applications where these devices are usually deployed this is
usually an OK user experience - if these hooks aren't provided what
tends to happen is that users patch the driver directly to get the
configurablilty they need.

> I don't quite follow the technical content of this "suspend_mode"
> mask, but it sure looks as if it should use the driver model flags
> in addition to whatever that mask is doing.

It's probably worth pointing out here that this part of the WM97xx
driver is split into two portions.  This part of the driver covers the
WM97xx itself but in order to actually use functionality like wakeup a
machine specific driver needs to be added.  The WM97xx chips have a
series of multi-function pins, some of which can be configured to
provide a signal mirroring the pen down status with most of the chip
powered down.  It can also be configured to do other things like try to
start the AC97 bus and/or start the touchscreen digitiser up if it
detects a pen down while suspended.

At the minute this is implemented by having the WM97xx register a child
device which platform code can provide.  When the child device probes it
configures both the WM97xx and the rest of the system.  The child device
can also provide other machine-specific things like use of the pen down
or interrupt signals from the chip to control when the touch screen is
read, synchronisation of the touch screen with writes to the display to
reduce interference from the video signals and accelerated reading of
the touchscreen if the AC97 controller is able to support it.

> There are two aspects to this:

>  (a) Whether on a given system the touchscreen *can* issue wakeup
>      events ... call device_init_wakeup(dev, true) to say it can.
>      In this case, presumably suspend_mode == 0 means it can't.
>      Board setup, or at latest probe(), should device_init_wakeup().

> So:  yes, this should initialize and use those driver model flags.

There is a bit of a question about which device this should be checked,
though the WM97xx itself is probably more sensible.

> > > +	reg = wm97xx_reg_read(wm, AC97_WM97XX_DIGITISER2);
> > > +	reg &= ~WM97XX_PRP_DET_DIG;
> > > +	if (wm->input_dev->users && suspend_mode)

> This should probably check device_may_wakeup(SOMEDEV), rather
> than testing suspend_mode.

That by itself isn't enough since if the system hasn't been configured
appropriately then the WM97xx won't be able to wake the system - we need
the suspend_mode configuration to know how to wake.  I'll do a spin of
the patch which pushes the configuration of suspend mode through an API
to let the core keep track of this.

> The wakeup mechanism here isn't clear to me.  Does this assume
> that the touchscreen IRQ is wakeup-capable?  I don't see any

No, it doesn't assume that.  For example, the system may wish to have
the chip initiate wakeup by bringing the AC97 bus up or have an output
from the WM97xx wired directly to some other component such as a PMU or
microcontroller rather than to an interrupt line visible to the CPU.

In theory this could also be used for some purpose other than waking the
system though I'm not aware of any such users so I don't think that's
worth worrying about right now.  In any case, the default is to enable
wakeup so most users won't notice this addition anyway.

> calls to enable_irq_wake() or disable_irq_wake() ... or for that
> matter any other calls that might affect system behavior.

Some combination of the machine-specific driver and other platform code
is expected to do this where required.

> > > +	/* WM97XX_PRP value to configure while system is suspended */
> > > +	u16 suspend_mode;

> I'm more accustomed to thinking that the driver will
> know "the" setup bit values to assign which will
> ensure that the chip issues wakeup events ... that
> stuff is usually not board-specific.

Unfortunately the fact that there are many different ways for the chip
to be used in a system (none of which can be automatically discovered)
means that this does need to be board-specific.

Re: [PATCH 1/3] wm97xx-core: Only schedule interrupt handler if not already scheduled

[Dmitry Torokhov]

Hi Mark,

On Mon, Apr 14, 2008 at 06:39:33PM +0100, Mark Brown wrote:
>  static irqreturn_t wm97xx_pen_interrupt(int irq, void *dev_id)
>  {
>  	struct wm97xx *wm = dev_id;
>  
> -	wm->mach_ops->irq_enable(wm, 0);
> -	queue_work(wm->ts_workq, &wm->pen_event_work);
> +	if (!work_pending(&wm->pen_event_work)) {
> +		wm->mach_ops->irq_enable(wm, 0);
> +		queue_work(wm->ts_workq, &wm->pen_event_work);
> +	}

Given the fact that work will not be queued if it is pending anyway
why is this change needed?

-- 
Dmitry

Re: [PATCH 1/3] wm97xx-core: Only schedule interrupt handler if not already scheduled

[Mark Brown]

On Mon, Apr 14, 2008 at 02:10:02PM -0400, Dmitry Torokhov wrote:
> On Mon, Apr 14, 2008 at 06:39:33PM +0100, Mark Brown wrote:

> > +	if (!work_pending(&wm->pen_event_work)) {
> > +		wm->mach_ops->irq_enable(wm, 0);
> > +		queue_work(wm->ts_workq, &wm->pen_event_work);
> > +	}

> Given the fact that work will not be queued if it is pending anyway
> why is this change needed?

As well as not queuing the work it ensures that the calls to
irq_enable() are balanced which helps with implementing that operation.

Re: [PATCH 1/3] wm97xx-core: Only schedule interrupt handler if not already scheduled

[Dmitry Torokhov]

On Tue, Apr 15, 2008 at 09:48:19AM +0100, Mark Brown wrote:
> On Mon, Apr 14, 2008 at 02:10:02PM -0400, Dmitry Torokhov wrote:
> > On Mon, Apr 14, 2008 at 06:39:33PM +0100, Mark Brown wrote:
> 
> > > +	if (!work_pending(&wm->pen_event_work)) {
> > > +		wm->mach_ops->irq_enable(wm, 0);
> > > +		queue_work(wm->ts_workq, &wm->pen_event_work);
> > > +	}
> 
> > Given the fact that work will not be queued if it is pending anyway
> > why is this change needed?
> 
> As well as not queuing the work it ensures that the calls to
> irq_enable() are balanced which helps with implementing that operation.

Hmm... another question then - what would fire up wm97xx_pen_interrupt()
again, before wm97xx_pen_irq_worker had a chance to run, if the very
first thing we do after getting interrupt is irq_enable(wm, 0)?

-- 
Dmitry

Re: [PATCH 1/3] wm97xx-core: Only schedule interrupt handler if not already scheduled

[Mark Brown]

On Tue, Apr 15, 2008 at 09:02:35AM -0400, Dmitry Torokhov wrote:
> On Tue, Apr 15, 2008 at 09:48:19AM +0100, Mark Brown wrote:

> > As well as not queuing the work it ensures that the calls to
> > irq_enable() are balanced which helps with implementing that operation.

> Hmm... another question then - what would fire up wm97xx_pen_interrupt()
> again, before wm97xx_pen_irq_worker had a chance to run, if the very
> first thing we do after getting interrupt is irq_enable(wm, 0)?

Normally nothing - in most systems the interrupt will not be shared
and/or the board irq_enable() operation will completely disable the IRQ
line.  The main use case would be if the interrupt is shared and the
board irq_enable() operation doesn't affect other sources, though boards
may also decide that they can do without disabling the interrupt.

WM97xx touchscreen updates

[Mark Brown]

This patch series updates the WM97xx touchscreen drivers with a couple
of small cleanups and the addition of support for use of the WM97xx
touchpanels to wake up from suspend.  Since the last submission I have:

 - Updated the commit message for scheduling of the interrupt handler. 
 - Hooked the touchscreen into the driver model wakeup infrastructure.

It is also available in the git repository at:

  git://opensource.wolfsonmicro.com/linux-2.6-touch upstream

Mark Brown (3):
      wm97xx-core: Only schedule interrupt handler if not already scheduled
      wm97xx-core: Use IRQF_SAMPLE_RANDOM
      wm97xx-core: Support use as a wakeup source

 drivers/input/touchscreen/wm97xx-core.c |   56 ++++++++++++++++++++++++++----
 include/linux/wm97xx.h                  |    3 ++
 2 files changed, 51 insertions(+), 8 deletions(-)

[PATCH 1/3] wm97xx-core: Only schedule interrupt handler if not already scheduled

[Mark Brown]

As well as clarifying the fact that the driver can cope if a second
interrupt occurs before the IRQ work is scheduled this also ensures
that calls to the machine irq_enable() are balanced, making that easier
to implement.  Normally this is redundant due to the interrupt disabling
but some unusal board configurations can trigger it.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/input/touchscreen/wm97xx-core.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
index 2910999..e27b1e0 100644
--- a/drivers/input/touchscreen/wm97xx-core.c
+++ b/drivers/input/touchscreen/wm97xx-core.c
@@ -328,18 +328,18 @@ static void wm97xx_pen_irq_worker(struct work_struct *work)
  *
  * We have to disable the codec interrupt in the handler because it
  * can take upto 1ms to clear the interrupt source. We schedule a task
- * in a work queue to do the actual interaction with the chip (it
- * doesn't matter if we end up reenqueing it before it is executed
- * since we don't touch the chip until it has run).  The interrupt is
- * then enabled again in the slow handler when the source has been
- * cleared.
+ * in a work queue to do the actual interaction with the chip.  The
+ * interrupt is then enabled again in the slow handler when the source
+ * has been cleared.
  */
 static irqreturn_t wm97xx_pen_interrupt(int irq, void *dev_id)
 {
 	struct wm97xx *wm = dev_id;
 
-	wm->mach_ops->irq_enable(wm, 0);
-	queue_work(wm->ts_workq, &wm->pen_event_work);
+	if (!work_pending(&wm->pen_event_work)) {
+		wm->mach_ops->irq_enable(wm, 0);
+		queue_work(wm->ts_workq, &wm->pen_event_work);
+	}
 
 	return IRQ_HANDLED;
 }
-- 
1.5.5