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(-)

--

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

Hi Mark,
Given the fact that work will not be queued if it is pending anyway

why is this change needed?
--

Dmitry

--

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

--

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.

--

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

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/includ...
[ message continues ]

Hi Mark,
Hmm, another unique module option... How about using device_can_wakeup()

infrastructure that we already have. I'm adding David Brownell to CC since
--

Dmitry

--

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).
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
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
--

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
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
There is a bit of a question about which device this should be checked,
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
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 out...
[ message continues ]