[PATCH 1/6] Input: ucb1400_ts - convert to use dev_pm_ops

[PATCH 1/6] Input: ucb1400_ts - convert to use dev_pm_ops

[Dmitry Torokhov]

Instead of using legacy PM interfaces switch to using dev_pm_ops.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/touchscreen/ucb1400_ts.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c
index 3b5b5df..ead7123 100644
--- a/drivers/input/touchscreen/ucb1400_ts.c
+++ b/drivers/input/touchscreen/ucb1400_ts.c
@@ -428,10 +428,10 @@ static int ucb1400_ts_remove(struct platform_device *dev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int ucb1400_ts_resume(struct platform_device *dev)
+#ifdef CONFIG_PM_SLEEP
+static int ucb1400_ts_resume(struct device *dev)
 {
-	struct ucb1400_ts *ucb = dev->dev.platform_data;
+	struct ucb1400_ts *ucb = dev->platform_data;
 
 	if (ucb->ts_task) {
 		/*
@@ -444,16 +444,16 @@ static int ucb1400_ts_resume(struct platform_device *dev)
 	}
 	return 0;
 }
-#else
-#define ucb1400_ts_resume NULL
 #endif
 
+static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops, NULL, ucb1400_ts_resume);
+
 static struct platform_driver ucb1400_ts_driver = {
 	.probe	= ucb1400_ts_probe,
 	.remove	= ucb1400_ts_remove,
-	.resume	= ucb1400_ts_resume,
 	.driver	= {
 		.name	= "ucb1400_ts",
+		.pm	= &ucb1400_ts_pm_ops,
 	},
 };
 
-- 
1.7.6.4

[PATCH 2/6] Input: ucb1400_ts - set driver owner

[Dmitry Torokhov]

So that it has proper symlink to the containing module in sysfs.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/touchscreen/ucb1400_ts.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c
index ead7123..550249b 100644
--- a/drivers/input/touchscreen/ucb1400_ts.c
+++ b/drivers/input/touchscreen/ucb1400_ts.c
@@ -453,6 +453,7 @@ static struct platform_driver ucb1400_ts_driver = {
 	.remove	= ucb1400_ts_remove,
 	.driver	= {
 		.name	= "ucb1400_ts",
+		.owner	= THIS_MODULE,
 		.pm	= &ucb1400_ts_pm_ops,
 	},
 };
-- 
1.7.6.4

[PATCH 3/6] Input: usb1400_ts - add __devinit/__devexit section annotations

[Dmitry Torokhov]

Add __devinit/__devexit sections annotations so parts of code could be
discarded after completing driver initialization.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/touchscreen/ucb1400_ts.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c
index 550249b..db525ac 100644
--- a/drivers/input/touchscreen/ucb1400_ts.c
+++ b/drivers/input/touchscreen/ucb1400_ts.c
@@ -299,7 +299,7 @@ static void ucb1400_ts_close(struct input_dev *idev)
  * Try to probe our interrupt, rather than relying on lots of
  * hard-coded machine dependencies.
  */
-static int ucb1400_ts_detect_irq(struct ucb1400_ts *ucb)
+static int __devinit ucb1400_ts_detect_irq(struct ucb1400_ts *ucb)
 {
 	unsigned long mask, timeout;
 
@@ -342,7 +342,7 @@ static int ucb1400_ts_detect_irq(struct ucb1400_ts *ucb)
 	return 0;
 }
 
-static int ucb1400_ts_probe(struct platform_device *dev)
+static int __devinit ucb1400_ts_probe(struct platform_device *dev)
 {
 	int error, x_res, y_res;
 	u16 fcsr;
@@ -416,15 +416,15 @@ err_free_devs:
 	input_free_device(ucb->ts_idev);
 err:
 	return error;
-
 }
 
-static int ucb1400_ts_remove(struct platform_device *dev)
+static int __devexit ucb1400_ts_remove(struct platform_device *dev)
 {
 	struct ucb1400_ts *ucb = dev->dev.platform_data;
 
 	free_irq(ucb->irq, ucb);
 	input_unregister_device(ucb->ts_idev);
+
 	return 0;
 }
 
@@ -450,7 +450,7 @@ static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops, NULL, ucb1400_ts_resume);
 
 static struct platform_driver ucb1400_ts_driver = {
 	.probe	= ucb1400_ts_probe,
-	.remove	= ucb1400_ts_remove,
+	.remove	= __devexit_p(ucb1400_ts_remove),
 	.driver	= {
 		.name	= "ucb1400_ts",
 		.owner	= THIS_MODULE,
-- 
1.7.6.4

[PATCH 4/6] Input: ucb1400_ts - drop inline annotations

[Dmitry Torokhov]

Let compiler figure out which ones makes most sense to inline.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/touchscreen/ucb1400_ts.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c
index db525ac..5c6b7b3 100644
--- a/drivers/input/touchscreen/ucb1400_ts.c
+++ b/drivers/input/touchscreen/ucb1400_ts.c
@@ -35,7 +35,7 @@ static int ts_delay = 55; /* us */
 static int ts_delay_pressure;	/* us */
 
 /* Switch to interrupt mode. */
-static inline void ucb1400_ts_mode_int(struct snd_ac97 *ac97)
+static void ucb1400_ts_mode_int(struct snd_ac97 *ac97)
 {
 	ucb1400_reg_write(ac97, UCB_TS_CR,
 			UCB_TS_CR_TSMX_POW | UCB_TS_CR_TSPX_POW |
@@ -47,7 +47,7 @@ static inline void ucb1400_ts_mode_int(struct snd_ac97 *ac97)
  * Switch to pressure mode, and read pressure.  We don't need to wait
  * here, since both plates are being driven.
  */
-static inline unsigned int ucb1400_ts_read_pressure(struct ucb1400_ts *ucb)
+static unsigned int ucb1400_ts_read_pressure(struct ucb1400_ts *ucb)
 {
 	ucb1400_reg_write(ucb->ac97, UCB_TS_CR,
 			UCB_TS_CR_TSMX_POW | UCB_TS_CR_TSPX_POW |
@@ -63,7 +63,7 @@ static inline unsigned int ucb1400_ts_read_pressure(struct ucb1400_ts *ucb)
  * gives a faster response time.  Even so, we need to wait about 55us
  * for things to stabilise.
  */
-static inline unsigned int ucb1400_ts_read_xpos(struct ucb1400_ts *ucb)
+static unsigned int ucb1400_ts_read_xpos(struct ucb1400_ts *ucb)
 {
 	ucb1400_reg_write(ucb->ac97, UCB_TS_CR,
 			UCB_TS_CR_TSMX_GND | UCB_TS_CR_TSPX_POW |
@@ -86,7 +86,7 @@ static inline unsigned int ucb1400_ts_read_xpos(struct ucb1400_ts *ucb)
  * gives a faster response time.  Even so, we need to wait about 55us
  * for things to stabilise.
  */
-static inline unsigned int ucb1400_ts_read_ypos(struct ucb1400_ts *ucb)
+static int ucb1400_ts_read_ypos(struct ucb1400_ts *ucb)
 {
 	ucb1400_reg_write(ucb->ac97, UCB_TS_CR,
 			UCB_TS_CR_TSMY_GND | UCB_TS_CR_TSPY_POW |
@@ -107,7 +107,7 @@ static inline unsigned int ucb1400_ts_read_ypos(struct ucb1400_ts *ucb)
  * Switch to X plate resistance mode.  Set MX to ground, PX to
  * supply.  Measure current.
  */
-static inline unsigned int ucb1400_ts_read_xres(struct ucb1400_ts *ucb)
+static unsigned int ucb1400_ts_read_xres(struct ucb1400_ts *ucb)
 {
 	ucb1400_reg_write(ucb->ac97, UCB_TS_CR,
 			UCB_TS_CR_TSMX_GND | UCB_TS_CR_TSPX_POW |
@@ -119,7 +119,7 @@ static inline unsigned int ucb1400_ts_read_xres(struct ucb1400_ts *ucb)
  * Switch to Y plate resistance mode.  Set MY to ground, PY to
  * supply.  Measure current.
  */
-static inline unsigned int ucb1400_ts_read_yres(struct ucb1400_ts *ucb)
+static unsigned int ucb1400_ts_read_yres(struct ucb1400_ts *ucb)
 {
 	ucb1400_reg_write(ucb->ac97, UCB_TS_CR,
 			UCB_TS_CR_TSMY_GND | UCB_TS_CR_TSPY_POW |
@@ -127,21 +127,21 @@ static inline unsigned int ucb1400_ts_read_yres(struct ucb1400_ts *ucb)
 	return ucb1400_adc_read(ucb->ac97, 0, adcsync);
 }
 
-static inline int ucb1400_ts_pen_up(struct snd_ac97 *ac97)
+static int ucb1400_ts_pen_up(struct snd_ac97 *ac97)
 {
 	unsigned short val = ucb1400_reg_read(ac97, UCB_TS_CR);
 
 	return val & (UCB_TS_CR_TSPX_LOW | UCB_TS_CR_TSMX_LOW);
 }
 
-static inline void ucb1400_ts_irq_enable(struct snd_ac97 *ac97)
+static void ucb1400_ts_irq_enable(struct snd_ac97 *ac97)
 {
 	ucb1400_reg_write(ac97, UCB_IE_CLEAR, UCB_IE_TSPX);
 	ucb1400_reg_write(ac97, UCB_IE_CLEAR, 0);
 	ucb1400_reg_write(ac97, UCB_IE_FAL, UCB_IE_TSPX);
 }
 
-static inline void ucb1400_ts_irq_disable(struct snd_ac97 *ac97)
+static void ucb1400_ts_irq_disable(struct snd_ac97 *ac97)
 {
 	ucb1400_reg_write(ac97, UCB_IE_FAL, 0);
 }
-- 
1.7.6.4

[PATCH 5/6] Input: ucb1400_ts - convert to threaded IRQ

[Dmitry Torokhov]

Instead of manually create and handler kernel thread switch to threaded
IRQ and let kernel IRQ core manage thread for us.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/touchscreen/ucb1400_ts.c |  235 +++++++++++++++-----------------
 include/linux/ucb1400.h                |    6 +-
 2 files changed, 115 insertions(+), 126 deletions(-)

diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c
index 5c6b7b3..1862633 100644
--- a/drivers/input/touchscreen/ucb1400_ts.c
+++ b/drivers/input/touchscreen/ucb1400_ts.c
@@ -20,24 +20,24 @@
 
 #include <linux/module.h>
 #include <linux/init.h>
-#include <linux/completion.h>
 #include <linux/delay.h>
+#include <linux/sched.h>
+#include <linux/wait.h>
 #include <linux/input.h>
 #include <linux/device.h>
 #include <linux/interrupt.h>
-#include <linux/suspend.h>
-#include <linux/kthread.h>
-#include <linux/freezer.h>
 #include <linux/ucb1400.h>
 
+#define UCB1400_TS_POLL_PERIOD	10 /* ms */
+
 static int adcsync;
 static int ts_delay = 55; /* us */
 static int ts_delay_pressure;	/* us */
 
 /* Switch to interrupt mode. */
-static void ucb1400_ts_mode_int(struct snd_ac97 *ac97)
+static void ucb1400_ts_mode_int(struct ucb1400_ts *ucb)
 {
-	ucb1400_reg_write(ac97, UCB_TS_CR,
+	ucb1400_reg_write(ucb->ac97, UCB_TS_CR,
 			UCB_TS_CR_TSMX_POW | UCB_TS_CR_TSPX_POW |
 			UCB_TS_CR_TSMY_GND | UCB_TS_CR_TSPY_GND |
 			UCB_TS_CR_MODE_INT);
@@ -53,7 +53,9 @@ static unsigned int ucb1400_ts_read_pressure(struct ucb1400_ts *ucb)
 			UCB_TS_CR_TSMX_POW | UCB_TS_CR_TSPX_POW |
 			UCB_TS_CR_TSMY_GND | UCB_TS_CR_TSPY_GND |
 			UCB_TS_CR_MODE_PRES | UCB_TS_CR_BIAS_ENA);
+
 	udelay(ts_delay_pressure);
+
 	return ucb1400_adc_read(ucb->ac97, UCB_ADC_INP_TSPY, adcsync);
 }
 
@@ -127,26 +129,26 @@ static unsigned int ucb1400_ts_read_yres(struct ucb1400_ts *ucb)
 	return ucb1400_adc_read(ucb->ac97, 0, adcsync);
 }
 
-static int ucb1400_ts_pen_up(struct snd_ac97 *ac97)
+static int ucb1400_ts_pen_up(struct ucb1400_ts *ucb)
 {
-	unsigned short val = ucb1400_reg_read(ac97, UCB_TS_CR);
+	unsigned short val = ucb1400_reg_read(ucb->ac97, UCB_TS_CR);
 
 	return val & (UCB_TS_CR_TSPX_LOW | UCB_TS_CR_TSMX_LOW);
 }
 
-static void ucb1400_ts_irq_enable(struct snd_ac97 *ac97)
+static void ucb1400_ts_irq_enable(struct ucb1400_ts *ucb)
 {
-	ucb1400_reg_write(ac97, UCB_IE_CLEAR, UCB_IE_TSPX);
-	ucb1400_reg_write(ac97, UCB_IE_CLEAR, 0);
-	ucb1400_reg_write(ac97, UCB_IE_FAL, UCB_IE_TSPX);
+	ucb1400_reg_write(ucb->ac97, UCB_IE_CLEAR, UCB_IE_TSPX);
+	ucb1400_reg_write(ucb->ac97, UCB_IE_CLEAR, 0);
+	ucb1400_reg_write(ucb->ac97, UCB_IE_FAL, UCB_IE_TSPX);
 }
 
-static void ucb1400_ts_irq_disable(struct snd_ac97 *ac97)
+static void ucb1400_ts_irq_disable(struct ucb1400_ts *ucb)
 {
-	ucb1400_reg_write(ac97, UCB_IE_FAL, 0);
+	ucb1400_reg_write(ucb->ac97, UCB_IE_FAL, 0);
 }
 
-static void ucb1400_ts_evt_add(struct input_dev *idev, u16 pressure, u16 x, u16 y)
+static void ucb1400_ts_report_event(struct input_dev *idev, u16 pressure, u16 x, u16 y)
 {
 	input_report_abs(idev, ABS_X, x);
 	input_report_abs(idev, ABS_Y, y);
@@ -162,7 +164,7 @@ static void ucb1400_ts_event_release(struct input_dev *idev)
 	input_sync(idev);
 }
 
-static void ucb1400_handle_pending_irq(struct ucb1400_ts *ucb)
+static void ucb1400_clear_pending_irq(struct ucb1400_ts *ucb)
 {
 	unsigned int isr;
 
@@ -171,32 +173,34 @@ static void ucb1400_handle_pending_irq(struct ucb1400_ts *ucb)
 	ucb1400_reg_write(ucb->ac97, UCB_IE_CLEAR, 0);
 
 	if (isr & UCB_IE_TSPX)
-		ucb1400_ts_irq_disable(ucb->ac97);
+		ucb1400_ts_irq_disable(ucb);
 	else
-		dev_dbg(&ucb->ts_idev->dev, "ucb1400: unexpected IE_STATUS = %#x\n", isr);
-	enable_irq(ucb->irq);
+		dev_dbg(&ucb->ts_idev->dev,
+			"ucb1400: unexpected IE_STATUS = %#x\n", isr);
 }
 
-static int ucb1400_ts_thread(void *_ucb)
+/*
+ * A restriction with interrupts exists when using the ucb1400, as
+ * the codec read/write routines may sleep while waiting for codec
+ * access completion and uses semaphores for access control to the
+ * AC97 bus. Therefore the driver is forced to use threaded interrupt
+ * handler.
+ */
+static irqreturn_t ucb1400_irq(int irqnr, void *devid)
 {
-	struct ucb1400_ts *ucb = _ucb;
-	struct task_struct *tsk = current;
-	int valid = 0;
-	struct sched_param param = { .sched_priority = 1 };
+	struct ucb1400_ts *ucb = devid;
+	unsigned int x, y, p;
+	bool penup;
 
-	sched_setscheduler(tsk, SCHED_FIFO, &param);
+	if (unlikely(irqnr != ucb->irq))
+		return IRQ_NONE;
 
-	set_freezable();
-	while (!kthread_should_stop()) {
-		unsigned int x, y, p;
-		long timeout;
+	ucb1400_clear_pending_irq(ucb);
 
-		ucb->ts_restart = 0;
+	/* Start with a small delay before checking pendown state */
+	msleep(UCB1400_TS_POLL_PERIOD);
 
-		if (ucb->irq_pending) {
-			ucb->irq_pending = 0;
-			ucb1400_handle_pending_irq(ucb);
-		}
+	while (!ucb->stopped && !(penup = ucb1400_ts_pen_up(ucb))) {
 
 		ucb1400_adc_enable(ucb->ac97);
 		x = ucb1400_ts_read_xpos(ucb);
@@ -204,91 +208,62 @@ static int ucb1400_ts_thread(void *_ucb)
 		p = ucb1400_ts_read_pressure(ucb);
 		ucb1400_adc_disable(ucb->ac97);
 
-		/* Switch back to interrupt mode. */
-		ucb1400_ts_mode_int(ucb->ac97);
-
-		msleep(10);
-
-		if (ucb1400_ts_pen_up(ucb->ac97)) {
-			ucb1400_ts_irq_enable(ucb->ac97);
-
-			/*
-			 * If we spat out a valid sample set last time,
-			 * spit out a "pen off" sample here.
-			 */
-			if (valid) {
-				ucb1400_ts_event_release(ucb->ts_idev);
-				valid = 0;
-			}
-
-			timeout = MAX_SCHEDULE_TIMEOUT;
-		} else {
-			valid = 1;
-			ucb1400_ts_evt_add(ucb->ts_idev, p, x, y);
-			timeout = msecs_to_jiffies(10);
-		}
+		ucb1400_ts_report_event(ucb->ts_idev, p, x, y);
 
-		wait_event_freezable_timeout(ucb->ts_wait,
-			ucb->irq_pending || ucb->ts_restart ||
-			kthread_should_stop(), timeout);
+		wait_event_timeout(ucb->ts_wait, ucb->stopped,
+				   msecs_to_jiffies(UCB1400_TS_POLL_PERIOD));
 	}
 
-	/* Send the "pen off" if we are stopping with the pen still active */
-	if (valid)
-		ucb1400_ts_event_release(ucb->ts_idev);
+	ucb1400_ts_event_release(ucb->ts_idev);
 
-	ucb->ts_task = NULL;
-	return 0;
+	if (!ucb->stopped) {
+		/* Switch back to interrupt mode. */
+		ucb1400_ts_mode_int(ucb);
+		ucb1400_ts_irq_enable(ucb);
+	}
+
+	return IRQ_HANDLED;
 }
 
-/*
- * A restriction with interrupts exists when using the ucb1400, as
- * the codec read/write routines may sleep while waiting for codec
- * access completion and uses semaphores for access control to the
- * AC97 bus.  A complete codec read cycle could take  anywhere from
- * 60 to 100uSec so we *definitely* don't want to spin inside the
- * interrupt handler waiting for codec access.  So, we handle the
- * interrupt by scheduling a RT kernel thread to run in process
- * context instead of interrupt context.
- */
-static irqreturn_t ucb1400_hard_irq(int irqnr, void *devid)
+static void ucb1400_ts_stop(struct ucb1400_ts *ucb)
 {
-	struct ucb1400_ts *ucb = devid;
+	/* Signal IRQ thread to stop polling and disable the handler. */
+	ucb->stopped = true;
+	mb();
+	wake_up(&ucb->ts_wait);
+	disable_irq(ucb->irq);
 
-	if (irqnr == ucb->irq) {
-		disable_irq_nosync(ucb->irq);
-		ucb->irq_pending = 1;
-		wake_up(&ucb->ts_wait);
-		return IRQ_HANDLED;
-	}
-	return IRQ_NONE;
+	ucb1400_ts_irq_disable(ucb);
+	ucb1400_reg_write(ucb->ac97, UCB_TS_CR, 0);
+}
+
+/* Must be called with ts->lock held */
+static void ucb1400_ts_start(struct ucb1400_ts *ucb)
+{
+	/* Tell IRQ thread that it may poll the device. */
+	ucb->stopped = false;
+	mb();
+
+	ucb1400_ts_mode_int(ucb);
+	ucb1400_ts_irq_enable(ucb);
+
+	enable_irq(ucb->irq);
 }
 
 static int ucb1400_ts_open(struct input_dev *idev)
 {
 	struct ucb1400_ts *ucb = input_get_drvdata(idev);
-	int ret = 0;
-
-	BUG_ON(ucb->ts_task);
 
-	ucb->ts_task = kthread_run(ucb1400_ts_thread, ucb, "UCB1400_ts");
-	if (IS_ERR(ucb->ts_task)) {
-		ret = PTR_ERR(ucb->ts_task);
-		ucb->ts_task = NULL;
-	}
+	ucb1400_ts_start(ucb);
 
-	return ret;
+	return 0;
 }
 
 static void ucb1400_ts_close(struct input_dev *idev)
 {
 	struct ucb1400_ts *ucb = input_get_drvdata(idev);
 
-	if (ucb->ts_task)
-		kthread_stop(ucb->ts_task);
-
-	ucb1400_ts_irq_disable(ucb->ac97);
-	ucb1400_reg_write(ucb->ac97, UCB_TS_CR, 0);
+	ucb1400_ts_stop(ucb);
 }
 
 #ifndef NO_IRQ
@@ -342,11 +317,11 @@ static int __devinit ucb1400_ts_detect_irq(struct ucb1400_ts *ucb)
 	return 0;
 }
 
-static int __devinit ucb1400_ts_probe(struct platform_device *dev)
+static int __devinit ucb1400_ts_probe(struct platform_device *pdev)
 {
+	struct ucb1400_ts *ucb = pdev->dev.platform_data;
 	int error, x_res, y_res;
 	u16 fcsr;
-	struct ucb1400_ts *ucb = dev->dev.platform_data;
 
 	ucb->ts_idev = input_allocate_device();
 	if (!ucb->ts_idev) {
@@ -362,21 +337,13 @@ static int __devinit ucb1400_ts_probe(struct platform_device *dev)
 			goto err_free_devs;
 		}
 	}
+	printk(KERN_DEBUG "UCB1400: found IRQ %d\n", ucb->irq);
 
 	init_waitqueue_head(&ucb->ts_wait);
 
-	error = request_irq(ucb->irq, ucb1400_hard_irq, IRQF_TRIGGER_RISING,
-				"UCB1400", ucb);
-	if (error) {
-		printk(KERN_ERR "ucb1400: unable to grab irq%d: %d\n",
-				ucb->irq, error);
-		goto err_free_devs;
-	}
-	printk(KERN_DEBUG "UCB1400: found IRQ %d\n", ucb->irq);
-
 	input_set_drvdata(ucb->ts_idev, ucb);
 
-	ucb->ts_idev->dev.parent	= &dev->dev;
+	ucb->ts_idev->dev.parent	= &pdev->dev;
 	ucb->ts_idev->name		= "UCB1400 touchscreen interface";
 	ucb->ts_idev->id.vendor		= ucb1400_reg_read(ucb->ac97,
 						AC97_VENDOR_ID1);
@@ -404,6 +371,17 @@ static int __devinit ucb1400_ts_probe(struct platform_device *dev)
 	input_set_abs_params(ucb->ts_idev, ABS_Y, 0, y_res, 0, 0);
 	input_set_abs_params(ucb->ts_idev, ABS_PRESSURE, 0, 0, 0, 0);
 
+	ucb1400_ts_stop(ucb);
+
+	error = request_threaded_irq(ucb->irq, NULL, ucb1400_irq,
+				     IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+				     "UCB1400", ucb);
+	if (error) {
+		printk(KERN_ERR "ucb1400: unable to grab irq%d: %d\n",
+				ucb->irq, error);
+		goto err_free_devs;
+	}
+
 	error = input_register_device(ucb->ts_idev);
 	if (error)
 		goto err_free_irq;
@@ -418,9 +396,9 @@ err:
 	return error;
 }
 
-static int __devexit ucb1400_ts_remove(struct platform_device *dev)
+static int __devexit ucb1400_ts_remove(struct platform_device *pdev)
 {
-	struct ucb1400_ts *ucb = dev->dev.platform_data;
+	struct ucb1400_ts *ucb = pdev->dev.platform_data;
 
 	free_irq(ucb->irq, ucb);
 	input_unregister_device(ucb->ts_idev);
@@ -429,24 +407,37 @@ static int __devexit ucb1400_ts_remove(struct platform_device *dev)
 }
 
 #ifdef CONFIG_PM_SLEEP
+static int ucb1400_ts_suspend(struct device *dev)
+{
+	struct ucb1400_ts *ucb = dev->platform_data;
+	struct input_dev *idev = ucb->ts_idev;
+
+	mutex_lock(&idev->mutex);
+
+	if (idev->users)
+		ucb1400_ts_start(ucb);
+
+	mutex_unlock(&idev->mutex);
+	return 0;
+}
+
 static int ucb1400_ts_resume(struct device *dev)
 {
 	struct ucb1400_ts *ucb = dev->platform_data;
+	struct input_dev *idev = ucb->ts_idev;
 
-	if (ucb->ts_task) {
-		/*
-		 * Restart the TS thread to ensure the
-		 * TS interrupt mode is set up again
-		 * after sleep.
-		 */
-		ucb->ts_restart = 1;
-		wake_up(&ucb->ts_wait);
-	}
+	mutex_lock(&idev->mutex);
+
+	if (idev->users)
+		ucb1400_ts_stop(ucb);
+
+	mutex_unlock(&idev->mutex);
 	return 0;
 }
 #endif
 
-static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops, NULL, ucb1400_ts_resume);
+static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops,
+			 ucb1400_ts_suspend, ucb1400_ts_resume);
 
 static struct platform_driver ucb1400_ts_driver = {
 	.probe	= ucb1400_ts_probe,
diff --git a/include/linux/ucb1400.h b/include/linux/ucb1400.h
index 5c75153..d21b33c 100644
--- a/include/linux/ucb1400.h
+++ b/include/linux/ucb1400.h
@@ -96,13 +96,11 @@ struct ucb1400_gpio {
 
 struct ucb1400_ts {
 	struct input_dev	*ts_idev;
-	struct task_struct	*ts_task;
 	int			id;
-	wait_queue_head_t	ts_wait;
-	unsigned int		ts_restart:1;
 	int			irq;
-	unsigned int		irq_pending;	/* not bit field shared */
 	struct snd_ac97		*ac97;
+	wait_queue_head_t	ts_wait;
+	bool			stopped;
 };
 
 struct ucb1400 {
-- 
1.7.6.4

[PATCH 6/6] Input: ucb1400-ts - switch to using dev_xxx() for diagnostic messages

[Dmitry Torokhov]

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/touchscreen/ucb1400_ts.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c
index 1862633..67f56a5 100644
--- a/drivers/input/touchscreen/ucb1400_ts.c
+++ b/drivers/input/touchscreen/ucb1400_ts.c
@@ -274,7 +274,8 @@ static void ucb1400_ts_close(struct input_dev *idev)
  * Try to probe our interrupt, rather than relying on lots of
  * hard-coded machine dependencies.
  */
-static int __devinit ucb1400_ts_detect_irq(struct ucb1400_ts *ucb)
+static int __devinit ucb1400_ts_detect_irq(struct ucb1400_ts *ucb,
+					   struct platform_device *pdev)
 {
 	unsigned long mask, timeout;
 
@@ -296,7 +297,7 @@ static int __devinit ucb1400_ts_detect_irq(struct ucb1400_ts *ucb)
 						UCB_ADC_DAT_VALID)) {
 		cpu_relax();
 		if (time_after(jiffies, timeout)) {
-			printk(KERN_ERR "ucb1400: timed out in IRQ probe\n");
+			dev_err(&pdev->dev, "timed out in IRQ probe\n");
 			probe_irq_off(mask);
 			return -ENODEV;
 		}
@@ -331,13 +332,13 @@ static int __devinit ucb1400_ts_probe(struct platform_device *pdev)
 
 	/* Only in case the IRQ line wasn't supplied, try detecting it */
 	if (ucb->irq < 0) {
-		error = ucb1400_ts_detect_irq(ucb);
+		error = ucb1400_ts_detect_irq(ucb, pdev);
 		if (error) {
-			printk(KERN_ERR "UCB1400: IRQ probe failed\n");
+			dev_err(&pdev->dev, "IRQ probe failed\n");
 			goto err_free_devs;
 		}
 	}
-	printk(KERN_DEBUG "UCB1400: found IRQ %d\n", ucb->irq);
+	dev_dbg(&pdev->dev, "found IRQ %d\n", ucb->irq);
 
 	init_waitqueue_head(&ucb->ts_wait);
 
@@ -365,7 +366,7 @@ static int __devinit ucb1400_ts_probe(struct platform_device *pdev)
 	x_res = ucb1400_ts_read_xres(ucb);
 	y_res = ucb1400_ts_read_yres(ucb);
 	ucb1400_adc_disable(ucb->ac97);
-	printk(KERN_DEBUG "UCB1400: x/y = %d/%d\n", x_res, y_res);
+	dev_dbg(&pdev->dev, "x/y = %d/%d\n", x_res, y_res);
 
 	input_set_abs_params(ucb->ts_idev, ABS_X, 0, x_res, 0, 0);
 	input_set_abs_params(ucb->ts_idev, ABS_Y, 0, y_res, 0, 0);
@@ -377,8 +378,8 @@ static int __devinit ucb1400_ts_probe(struct platform_device *pdev)
 				     IRQF_TRIGGER_RISING | IRQF_ONESHOT,
 				     "UCB1400", ucb);
 	if (error) {
-		printk(KERN_ERR "ucb1400: unable to grab irq%d: %d\n",
-				ucb->irq, error);
+		dev_err(&pdev->dev,
+			"unable to grab irq%d: %d\n", ucb->irq, error);
 		goto err_free_devs;
 	}
 
-- 
1.7.6.4

Re: [PATCH 1/6] Input: ucb1400_ts - convert to use dev_pm_ops

[Marek Vasut]

> Instead of using legacy PM interfaces switch to using dev_pm_ops.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>  drivers/input/touchscreen/ucb1400_ts.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ucb1400_ts.c
> b/drivers/input/touchscreen/ucb1400_ts.c index 3b5b5df..ead7123 100644
> --- a/drivers/input/touchscreen/ucb1400_ts.c
> +++ b/drivers/input/touchscreen/ucb1400_ts.c
> @@ -428,10 +428,10 @@ static int ucb1400_ts_remove(struct platform_device
> *dev) return 0;
>  }
> 
> -#ifdef CONFIG_PM
> -static int ucb1400_ts_resume(struct platform_device *dev)
> +#ifdef CONFIG_PM_SLEEP
> +static int ucb1400_ts_resume(struct device *dev)
>  {
> -	struct ucb1400_ts *ucb = dev->dev.platform_data;
> +	struct ucb1400_ts *ucb = dev->platform_data;
> 
>  	if (ucb->ts_task) {
>  		/*
> @@ -444,16 +444,16 @@ static int ucb1400_ts_resume(struct platform_device
> *dev) }
>  	return 0;
>  }
> -#else
> -#define ucb1400_ts_resume NULL
>  #endif
> 
> +static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops, NULL, ucb1400_ts_resume);
> +
>  static struct platform_driver ucb1400_ts_driver = {
>  	.probe	= ucb1400_ts_probe,
>  	.remove	= ucb1400_ts_remove,
> -	.resume	= ucb1400_ts_resume,
>  	.driver	= {
>  		.name	= "ucb1400_ts",
> +		.pm	= &ucb1400_ts_pm_ops,
>  	},
>  };

Hi,

I went through the patchset, but didn't test it. Though the changes look 
reasonable. I'll have time to test this sometimes next week MAYBE, or it MIGHT 
tell someone else to test, I can't promise anything here though.

Please add to all patches my:

Reviewed-by: Marek Vasut <marek.vasut@gmail.com>

Re: [PATCH 1/6] Input: ucb1400_ts - convert to use dev_pm_ops

[Dmitry Torokhov]

On Mon, Nov 14, 2011 at 11:39:17AM +0100, Marek Vasut wrote:
> > Instead of using legacy PM interfaces switch to using dev_pm_ops.
> > 
> > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > ---
> >  drivers/input/touchscreen/ucb1400_ts.c |   12 ++++++------
> >  1 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/input/touchscreen/ucb1400_ts.c
> > b/drivers/input/touchscreen/ucb1400_ts.c index 3b5b5df..ead7123 100644
> > --- a/drivers/input/touchscreen/ucb1400_ts.c
> > +++ b/drivers/input/touchscreen/ucb1400_ts.c
> > @@ -428,10 +428,10 @@ static int ucb1400_ts_remove(struct platform_device
> > *dev) return 0;
> >  }
> > 
> > -#ifdef CONFIG_PM
> > -static int ucb1400_ts_resume(struct platform_device *dev)
> > +#ifdef CONFIG_PM_SLEEP
> > +static int ucb1400_ts_resume(struct device *dev)
> >  {
> > -	struct ucb1400_ts *ucb = dev->dev.platform_data;
> > +	struct ucb1400_ts *ucb = dev->platform_data;
> > 
> >  	if (ucb->ts_task) {
> >  		/*
> > @@ -444,16 +444,16 @@ static int ucb1400_ts_resume(struct platform_device
> > *dev) }
> >  	return 0;
> >  }
> > -#else
> > -#define ucb1400_ts_resume NULL
> >  #endif
> > 
> > +static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops, NULL, ucb1400_ts_resume);
> > +
> >  static struct platform_driver ucb1400_ts_driver = {
> >  	.probe	= ucb1400_ts_probe,
> >  	.remove	= ucb1400_ts_remove,
> > -	.resume	= ucb1400_ts_resume,
> >  	.driver	= {
> >  		.name	= "ucb1400_ts",
> > +		.pm	= &ucb1400_ts_pm_ops,
> >  	},
> >  };
> 
> Hi,
> 
> I went through the patchset, but didn't test it. Though the changes look 
> reasonable. I'll have time to test this sometimes next week MAYBE, or it MIGHT 
> tell someone else to test, I can't promise anything here though.
> 
> Please add to all patches my:
> 
> Reviewed-by: Marek Vasut <marek.vasut@gmail.com>

Thanks Marek. I have not tested these either so it would be nice to find
someone who could. However there is no urgency as we have some time
before 3.3 merge window opens.

Thanks.

-- 
Dmitry

Re: [PATCH 1/6] Input: ucb1400_ts - convert to use dev_pm_ops

[Marek Vasut]

> On Mon, Nov 14, 2011 at 11:39:17AM +0100, Marek Vasut wrote:
> > > Instead of using legacy PM interfaces switch to using dev_pm_ops.
> > > 
> > > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > > ---
> > > 
> > >  drivers/input/touchscreen/ucb1400_ts.c |   12 ++++++------
> > >  1 files changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/input/touchscreen/ucb1400_ts.c
> > > b/drivers/input/touchscreen/ucb1400_ts.c index 3b5b5df..ead7123 100644
> > > --- a/drivers/input/touchscreen/ucb1400_ts.c
> > > +++ b/drivers/input/touchscreen/ucb1400_ts.c
> > > @@ -428,10 +428,10 @@ static int ucb1400_ts_remove(struct
> > > platform_device *dev) return 0;
> > > 
> > >  }
> > > 
> > > -#ifdef CONFIG_PM
> > > -static int ucb1400_ts_resume(struct platform_device *dev)
> > > +#ifdef CONFIG_PM_SLEEP
> > > +static int ucb1400_ts_resume(struct device *dev)
> > > 
> > >  {
> > > 
> > > -	struct ucb1400_ts *ucb = dev->dev.platform_data;
> > > +	struct ucb1400_ts *ucb = dev->platform_data;
> > > 
> > >  	if (ucb->ts_task) {
> > >  	
> > >  		/*
> > > 
> > > @@ -444,16 +444,16 @@ static int ucb1400_ts_resume(struct
> > > platform_device *dev) }
> > > 
> > >  	return 0;
> > >  
> > >  }
> > > 
> > > -#else
> > > -#define ucb1400_ts_resume NULL
> > > 
> > >  #endif
> > > 
> > > +static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops, NULL, ucb1400_ts_resume);
> > > +
> > > 
> > >  static struct platform_driver ucb1400_ts_driver = {
> > >  
> > >  	.probe	= ucb1400_ts_probe,
> > >  	.remove	= ucb1400_ts_remove,
> > > 
> > > -	.resume	= ucb1400_ts_resume,
> > > 
> > >  	.driver	= {
> > >  	
> > >  		.name	= "ucb1400_ts",
> > > 
> > > +		.pm	= &ucb1400_ts_pm_ops,
> > > 
> > >  	},
> > >  
> > >  };
> > 
> > Hi,
> > 
> > I went through the patchset, but didn't test it. Though the changes look
> > reasonable. I'll have time to test this sometimes next week MAYBE, or it
> > MIGHT tell someone else to test, I can't promise anything here though.
> > 
> > Please add to all patches my:
> > 
> > Reviewed-by: Marek Vasut <marek.vasut@gmail.com>
> 
> Thanks Marek. I have not tested these either so it would be nice to find
> someone who could. However there is no urgency as we have some time
> before 3.3 merge window opens.

I can, I have devices with this chip available ... just not now, real life is 
haunting me :)

> 
> Thanks.

Sure.

M

Re: [PATCH 5/6] Input: ucb1400_ts - convert to threaded IRQ

[Daniel Kurtz]

Hi Dmitry,

On Mon, Nov 14, 2011 at 4:36 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Instead of manually create and handler kernel thread switch to threaded
> IRQ and let kernel IRQ core manage thread for us.
>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>  drivers/input/touchscreen/ucb1400_ts.c |  235 +++++++++++++++-----------------
>  include/linux/ucb1400.h                |    6 +-
>  2 files changed, 115 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c
> index 5c6b7b3..1862633 100644
> --- a/drivers/input/touchscreen/ucb1400_ts.c
> +++ b/drivers/input/touchscreen/ucb1400_ts.c
> @@ -20,24 +20,24 @@
>
>  #include <linux/module.h>
>  #include <linux/init.h>
> -#include <linux/completion.h>
>  #include <linux/delay.h>
> +#include <linux/sched.h>
> +#include <linux/wait.h>
>  #include <linux/input.h>
>  #include <linux/device.h>
>  #include <linux/interrupt.h>
> -#include <linux/suspend.h>
> -#include <linux/kthread.h>
> -#include <linux/freezer.h>
>  #include <linux/ucb1400.h>
>
> +#define UCB1400_TS_POLL_PERIOD 10 /* ms */
> +
>  static int adcsync;
>  static int ts_delay = 55; /* us */
>  static int ts_delay_pressure;  /* us */
>
>  /* Switch to interrupt mode. */
> -static void ucb1400_ts_mode_int(struct snd_ac97 *ac97)
> +static void ucb1400_ts_mode_int(struct ucb1400_ts *ucb)
>  {
> -       ucb1400_reg_write(ac97, UCB_TS_CR,
> +       ucb1400_reg_write(ucb->ac97, UCB_TS_CR,
>                        UCB_TS_CR_TSMX_POW | UCB_TS_CR_TSPX_POW |
>                        UCB_TS_CR_TSMY_GND | UCB_TS_CR_TSPY_GND |
>                        UCB_TS_CR_MODE_INT);
> @@ -53,7 +53,9 @@ static unsigned int ucb1400_ts_read_pressure(struct ucb1400_ts *ucb)
>                        UCB_TS_CR_TSMX_POW | UCB_TS_CR_TSPX_POW |
>                        UCB_TS_CR_TSMY_GND | UCB_TS_CR_TSPY_GND |
>                        UCB_TS_CR_MODE_PRES | UCB_TS_CR_BIAS_ENA);
> +
>        udelay(ts_delay_pressure);
> +
>        return ucb1400_adc_read(ucb->ac97, UCB_ADC_INP_TSPY, adcsync);
>  }
>
> @@ -127,26 +129,26 @@ static unsigned int ucb1400_ts_read_yres(struct ucb1400_ts *ucb)
>        return ucb1400_adc_read(ucb->ac97, 0, adcsync);
>  }
>
> -static int ucb1400_ts_pen_up(struct snd_ac97 *ac97)
> +static int ucb1400_ts_pen_up(struct ucb1400_ts *ucb)
>  {
> -       unsigned short val = ucb1400_reg_read(ac97, UCB_TS_CR);
> +       unsigned short val = ucb1400_reg_read(ucb->ac97, UCB_TS_CR);
>
>        return val & (UCB_TS_CR_TSPX_LOW | UCB_TS_CR_TSMX_LOW);
>  }
>
> -static void ucb1400_ts_irq_enable(struct snd_ac97 *ac97)
> +static void ucb1400_ts_irq_enable(struct ucb1400_ts *ucb)
>  {
> -       ucb1400_reg_write(ac97, UCB_IE_CLEAR, UCB_IE_TSPX);
> -       ucb1400_reg_write(ac97, UCB_IE_CLEAR, 0);
> -       ucb1400_reg_write(ac97, UCB_IE_FAL, UCB_IE_TSPX);
> +       ucb1400_reg_write(ucb->ac97, UCB_IE_CLEAR, UCB_IE_TSPX);
> +       ucb1400_reg_write(ucb->ac97, UCB_IE_CLEAR, 0);
> +       ucb1400_reg_write(ucb->ac97, UCB_IE_FAL, UCB_IE_TSPX);
>  }
>
> -static void ucb1400_ts_irq_disable(struct snd_ac97 *ac97)
> +static void ucb1400_ts_irq_disable(struct ucb1400_ts *ucb)
>  {
> -       ucb1400_reg_write(ac97, UCB_IE_FAL, 0);
> +       ucb1400_reg_write(ucb->ac97, UCB_IE_FAL, 0);
>  }
>
> -static void ucb1400_ts_evt_add(struct input_dev *idev, u16 pressure, u16 x, u16 y)
> +static void ucb1400_ts_report_event(struct input_dev *idev, u16 pressure, u16 x, u16 y)
>  {
>        input_report_abs(idev, ABS_X, x);
>        input_report_abs(idev, ABS_Y, y);
> @@ -162,7 +164,7 @@ static void ucb1400_ts_event_release(struct input_dev *idev)
>        input_sync(idev);
>  }
>
> -static void ucb1400_handle_pending_irq(struct ucb1400_ts *ucb)
> +static void ucb1400_clear_pending_irq(struct ucb1400_ts *ucb)
>  {
>        unsigned int isr;
>
> @@ -171,32 +173,34 @@ static void ucb1400_handle_pending_irq(struct ucb1400_ts *ucb)
>        ucb1400_reg_write(ucb->ac97, UCB_IE_CLEAR, 0);
>
>        if (isr & UCB_IE_TSPX)
> -               ucb1400_ts_irq_disable(ucb->ac97);
> +               ucb1400_ts_irq_disable(ucb);
>        else
> -               dev_dbg(&ucb->ts_idev->dev, "ucb1400: unexpected IE_STATUS = %#x\n", isr);
> -       enable_irq(ucb->irq);
> +               dev_dbg(&ucb->ts_idev->dev,
> +                       "ucb1400: unexpected IE_STATUS = %#x\n", isr);
>  }
>
> -static int ucb1400_ts_thread(void *_ucb)
> +/*
> + * A restriction with interrupts exists when using the ucb1400, as
> + * the codec read/write routines may sleep while waiting for codec
> + * access completion and uses semaphores for access control to the
> + * AC97 bus. Therefore the driver is forced to use threaded interrupt
> + * handler.
> + */
> +static irqreturn_t ucb1400_irq(int irqnr, void *devid)
>  {
> -       struct ucb1400_ts *ucb = _ucb;
> -       struct task_struct *tsk = current;
> -       int valid = 0;
> -       struct sched_param param = { .sched_priority = 1 };
> +       struct ucb1400_ts *ucb = devid;
> +       unsigned int x, y, p;
> +       bool penup;
>
> -       sched_setscheduler(tsk, SCHED_FIFO, &param);
> +       if (unlikely(irqnr != ucb->irq))
> +               return IRQ_NONE;
>
> -       set_freezable();
> -       while (!kthread_should_stop()) {
> -               unsigned int x, y, p;
> -               long timeout;
> +       ucb1400_clear_pending_irq(ucb);
>
> -               ucb->ts_restart = 0;
> +       /* Start with a small delay before checking pendown state */
> +       msleep(UCB1400_TS_POLL_PERIOD);
>
> -               if (ucb->irq_pending) {
> -                       ucb->irq_pending = 0;
> -                       ucb1400_handle_pending_irq(ucb);
> -               }
> +       while (!ucb->stopped && !(penup = ucb1400_ts_pen_up(ucb))) {
>
>                ucb1400_adc_enable(ucb->ac97);
>                x = ucb1400_ts_read_xpos(ucb);
> @@ -204,91 +208,62 @@ static int ucb1400_ts_thread(void *_ucb)
>                p = ucb1400_ts_read_pressure(ucb);
>                ucb1400_adc_disable(ucb->ac97);
>
> -               /* Switch back to interrupt mode. */
> -               ucb1400_ts_mode_int(ucb->ac97);
> -
> -               msleep(10);
> -
> -               if (ucb1400_ts_pen_up(ucb->ac97)) {
> -                       ucb1400_ts_irq_enable(ucb->ac97);
> -
> -                       /*
> -                        * If we spat out a valid sample set last time,
> -                        * spit out a "pen off" sample here.
> -                        */
> -                       if (valid) {
> -                               ucb1400_ts_event_release(ucb->ts_idev);
> -                               valid = 0;
> -                       }
> -
> -                       timeout = MAX_SCHEDULE_TIMEOUT;
> -               } else {
> -                       valid = 1;
> -                       ucb1400_ts_evt_add(ucb->ts_idev, p, x, y);
> -                       timeout = msecs_to_jiffies(10);
> -               }
> +               ucb1400_ts_report_event(ucb->ts_idev, p, x, y);
>
> -               wait_event_freezable_timeout(ucb->ts_wait,
> -                       ucb->irq_pending || ucb->ts_restart ||
> -                       kthread_should_stop(), timeout);
> +               wait_event_timeout(ucb->ts_wait, ucb->stopped,
> +                                  msecs_to_jiffies(UCB1400_TS_POLL_PERIOD));
>        }
>
> -       /* Send the "pen off" if we are stopping with the pen still active */
> -       if (valid)
> -               ucb1400_ts_event_release(ucb->ts_idev);
> +       ucb1400_ts_event_release(ucb->ts_idev);
>
> -       ucb->ts_task = NULL;
> -       return 0;
> +       if (!ucb->stopped) {
> +               /* Switch back to interrupt mode. */
> +               ucb1400_ts_mode_int(ucb);
> +               ucb1400_ts_irq_enable(ucb);
> +       }
> +
> +       return IRQ_HANDLED;
>  }
>
> -/*
> - * A restriction with interrupts exists when using the ucb1400, as
> - * the codec read/write routines may sleep while waiting for codec
> - * access completion and uses semaphores for access control to the
> - * AC97 bus.  A complete codec read cycle could take  anywhere from
> - * 60 to 100uSec so we *definitely* don't want to spin inside the
> - * interrupt handler waiting for codec access.  So, we handle the
> - * interrupt by scheduling a RT kernel thread to run in process
> - * context instead of interrupt context.
> - */
> -static irqreturn_t ucb1400_hard_irq(int irqnr, void *devid)
> +static void ucb1400_ts_stop(struct ucb1400_ts *ucb)
>  {
> -       struct ucb1400_ts *ucb = devid;
> +       /* Signal IRQ thread to stop polling and disable the handler. */
> +       ucb->stopped = true;
> +       mb();
> +       wake_up(&ucb->ts_wait);
> +       disable_irq(ucb->irq);
>
> -       if (irqnr == ucb->irq) {
> -               disable_irq_nosync(ucb->irq);
> -               ucb->irq_pending = 1;
> -               wake_up(&ucb->ts_wait);
> -               return IRQ_HANDLED;
> -       }
> -       return IRQ_NONE;
> +       ucb1400_ts_irq_disable(ucb);
> +       ucb1400_reg_write(ucb->ac97, UCB_TS_CR, 0);
> +}
> +
> +/* Must be called with ts->lock held */
> +static void ucb1400_ts_start(struct ucb1400_ts *ucb)
> +{
> +       /* Tell IRQ thread that it may poll the device. */
> +       ucb->stopped = false;
> +       mb();
> +
> +       ucb1400_ts_mode_int(ucb);
> +       ucb1400_ts_irq_enable(ucb);
> +
> +       enable_irq(ucb->irq);
>  }
>
>  static int ucb1400_ts_open(struct input_dev *idev)
>  {
>        struct ucb1400_ts *ucb = input_get_drvdata(idev);
> -       int ret = 0;
> -
> -       BUG_ON(ucb->ts_task);
>
> -       ucb->ts_task = kthread_run(ucb1400_ts_thread, ucb, "UCB1400_ts");
> -       if (IS_ERR(ucb->ts_task)) {
> -               ret = PTR_ERR(ucb->ts_task);
> -               ucb->ts_task = NULL;
> -       }
> +       ucb1400_ts_start(ucb);
>
> -       return ret;
> +       return 0;
>  }
>
>  static void ucb1400_ts_close(struct input_dev *idev)
>  {
>        struct ucb1400_ts *ucb = input_get_drvdata(idev);
>
> -       if (ucb->ts_task)
> -               kthread_stop(ucb->ts_task);
> -
> -       ucb1400_ts_irq_disable(ucb->ac97);
> -       ucb1400_reg_write(ucb->ac97, UCB_TS_CR, 0);
> +       ucb1400_ts_stop(ucb);
>  }
>
>  #ifndef NO_IRQ
> @@ -342,11 +317,11 @@ static int __devinit ucb1400_ts_detect_irq(struct ucb1400_ts *ucb)
>        return 0;
>  }
>
> -static int __devinit ucb1400_ts_probe(struct platform_device *dev)
> +static int __devinit ucb1400_ts_probe(struct platform_device *pdev)
>  {
> +       struct ucb1400_ts *ucb = pdev->dev.platform_data;
>        int error, x_res, y_res;
>        u16 fcsr;
> -       struct ucb1400_ts *ucb = dev->dev.platform_data;
>
>        ucb->ts_idev = input_allocate_device();
>        if (!ucb->ts_idev) {
> @@ -362,21 +337,13 @@ static int __devinit ucb1400_ts_probe(struct platform_device *dev)
>                        goto err_free_devs;
>                }
>        }
> +       printk(KERN_DEBUG "UCB1400: found IRQ %d\n", ucb->irq);
>
>        init_waitqueue_head(&ucb->ts_wait);
>
> -       error = request_irq(ucb->irq, ucb1400_hard_irq, IRQF_TRIGGER_RISING,
> -                               "UCB1400", ucb);
> -       if (error) {
> -               printk(KERN_ERR "ucb1400: unable to grab irq%d: %d\n",
> -                               ucb->irq, error);
> -               goto err_free_devs;
> -       }
> -       printk(KERN_DEBUG "UCB1400: found IRQ %d\n", ucb->irq);
> -
>        input_set_drvdata(ucb->ts_idev, ucb);
>
> -       ucb->ts_idev->dev.parent        = &dev->dev;
> +       ucb->ts_idev->dev.parent        = &pdev->dev;
>        ucb->ts_idev->name              = "UCB1400 touchscreen interface";
>        ucb->ts_idev->id.vendor         = ucb1400_reg_read(ucb->ac97,
>                                                AC97_VENDOR_ID1);
> @@ -404,6 +371,17 @@ static int __devinit ucb1400_ts_probe(struct platform_device *dev)
>        input_set_abs_params(ucb->ts_idev, ABS_Y, 0, y_res, 0, 0);
>        input_set_abs_params(ucb->ts_idev, ABS_PRESSURE, 0, 0, 0, 0);
>
> +       ucb1400_ts_stop(ucb);
> +
> +       error = request_threaded_irq(ucb->irq, NULL, ucb1400_irq,
> +                                    IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +                                    "UCB1400", ucb);
> +       if (error) {
> +               printk(KERN_ERR "ucb1400: unable to grab irq%d: %d\n",
> +                               ucb->irq, error);
> +               goto err_free_devs;
> +       }
> +
>        error = input_register_device(ucb->ts_idev);
>        if (error)
>                goto err_free_irq;
> @@ -418,9 +396,9 @@ err:
>        return error;
>  }
>
> -static int __devexit ucb1400_ts_remove(struct platform_device *dev)
> +static int __devexit ucb1400_ts_remove(struct platform_device *pdev)
>  {
> -       struct ucb1400_ts *ucb = dev->dev.platform_data;
> +       struct ucb1400_ts *ucb = pdev->dev.platform_data;
>
>        free_irq(ucb->irq, ucb);
>        input_unregister_device(ucb->ts_idev);
> @@ -429,24 +407,37 @@ static int __devexit ucb1400_ts_remove(struct platform_device *dev)
>  }
>
>  #ifdef CONFIG_PM_SLEEP
> +static int ucb1400_ts_suspend(struct device *dev)
> +{
> +       struct ucb1400_ts *ucb = dev->platform_data;
> +       struct input_dev *idev = ucb->ts_idev;
> +
> +       mutex_lock(&idev->mutex);
> +
> +       if (idev->users)
> +               ucb1400_ts_start(ucb);
> +
> +       mutex_unlock(&idev->mutex);
> +       return 0;
> +}
> +
>  static int ucb1400_ts_resume(struct device *dev)
>  {
>        struct ucb1400_ts *ucb = dev->platform_data;
> +       struct input_dev *idev = ucb->ts_idev;
>
> -       if (ucb->ts_task) {
> -               /*
> -                * Restart the TS thread to ensure the
> -                * TS interrupt mode is set up again
> -                * after sleep.
> -                */
> -               ucb->ts_restart = 1;
> -               wake_up(&ucb->ts_wait);
> -       }
> +       mutex_lock(&idev->mutex);
> +
> +       if (idev->users)
> +               ucb1400_ts_stop(ucb);
> +
> +       mutex_unlock(&idev->mutex);
>        return 0;
>  }
>  #endif

Do you need an #else here to set *_suspend/resume to NULL when
CONFIG_PM_SLEEP is not defined?
It seems to have gotten lost somewhere.

>
> -static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops, NULL, ucb1400_ts_resume);
> +static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops,
> +                        ucb1400_ts_suspend, ucb1400_ts_resume);
>
>  static struct platform_driver ucb1400_ts_driver = {
>        .probe  = ucb1400_ts_probe,
> diff --git a/include/linux/ucb1400.h b/include/linux/ucb1400.h
> index 5c75153..d21b33c 100644
> --- a/include/linux/ucb1400.h
> +++ b/include/linux/ucb1400.h
> @@ -96,13 +96,11 @@ struct ucb1400_gpio {
>
>  struct ucb1400_ts {
>        struct input_dev        *ts_idev;
> -       struct task_struct      *ts_task;
>        int                     id;
> -       wait_queue_head_t       ts_wait;
> -       unsigned int            ts_restart:1;
>        int                     irq;
> -       unsigned int            irq_pending;    /* not bit field shared */
>        struct snd_ac97         *ac97;
> +       wait_queue_head_t       ts_wait;
> +       bool                    stopped;
>  };
>
>  struct ucb1400 {
> --
> 1.7.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Daniel Kurtz | Software Engineer | djkurtz@google.com | 650.204.0722
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] Input: ucb1400_ts - convert to threaded IRQ

[Dmitry Torokhov]

Hi Daniel,

On Wed, Nov 16, 2011 at 03:36:37PM +0800, Daniel Kurtz wrote:
`> Hi Dmitry,
> 
> On Mon, Nov 14, 2011 at 4:36 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> >  #ifdef CONFIG_PM_SLEEP
> > +static int ucb1400_ts_suspend(struct device *dev)
> > +{
> > +       struct ucb1400_ts *ucb = dev->platform_data;
> > +       struct input_dev *idev = ucb->ts_idev;
> > +
> > +       mutex_lock(&idev->mutex);
> > +
> > +       if (idev->users)
> > +               ucb1400_ts_start(ucb);
> > +
> > +       mutex_unlock(&idev->mutex);
> > +       return 0;
> > +}
> > +
> >  static int ucb1400_ts_resume(struct device *dev)
> >  {
> >        struct ucb1400_ts *ucb = dev->platform_data;
> > +       struct input_dev *idev = ucb->ts_idev;
> >
> > -       if (ucb->ts_task) {
> > -               /*
> > -                * Restart the TS thread to ensure the
> > -                * TS interrupt mode is set up again
> > -                * after sleep.
> > -                */
> > -               ucb->ts_restart = 1;
> > -               wake_up(&ucb->ts_wait);
> > -       }
> > +       mutex_lock(&idev->mutex);
> > +
> > +       if (idev->users)
> > +               ucb1400_ts_stop(ucb);
> > +
> > +       mutex_unlock(&idev->mutex);
> >        return 0;
> >  }
> >  #endif
> 
> Do you need an #else here to set *_suspend/resume to NULL when
> CONFIG_PM_SLEEP is not defined?
> It seems to have gotten lost somewhere.

Without CONFIG_PM_SLEEP SIMPLE_DEV_PM_OPS simply ignores its 2nd and 3rd
arguments so the fact that they are missing does not matter.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] Input: ucb1400_ts - convert to use dev_pm_ops

[Dmitry Torokhov]

On Mon, Nov 14, 2011 at 06:04:12PM +0100, Marek Vasut wrote:
> > On Mon, Nov 14, 2011 at 11:39:17AM +0100, Marek Vasut wrote:
> > > > Instead of using legacy PM interfaces switch to using dev_pm_ops.
> > > > 
> > > > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > > > ---
> > > > 
> > > >  drivers/input/touchscreen/ucb1400_ts.c |   12 ++++++------
> > > >  1 files changed, 6 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/input/touchscreen/ucb1400_ts.c
> > > > b/drivers/input/touchscreen/ucb1400_ts.c index 3b5b5df..ead7123 100644
> > > > --- a/drivers/input/touchscreen/ucb1400_ts.c
> > > > +++ b/drivers/input/touchscreen/ucb1400_ts.c
> > > > @@ -428,10 +428,10 @@ static int ucb1400_ts_remove(struct
> > > > platform_device *dev) return 0;
> > > > 
> > > >  }
> > > > 
> > > > -#ifdef CONFIG_PM
> > > > -static int ucb1400_ts_resume(struct platform_device *dev)
> > > > +#ifdef CONFIG_PM_SLEEP
> > > > +static int ucb1400_ts_resume(struct device *dev)
> > > > 
> > > >  {
> > > > 
> > > > -	struct ucb1400_ts *ucb = dev->dev.platform_data;
> > > > +	struct ucb1400_ts *ucb = dev->platform_data;
> > > > 
> > > >  	if (ucb->ts_task) {
> > > >  	
> > > >  		/*
> > > > 
> > > > @@ -444,16 +444,16 @@ static int ucb1400_ts_resume(struct
> > > > platform_device *dev) }
> > > > 
> > > >  	return 0;
> > > >  
> > > >  }
> > > > 
> > > > -#else
> > > > -#define ucb1400_ts_resume NULL
> > > > 
> > > >  #endif
> > > > 
> > > > +static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops, NULL, ucb1400_ts_resume);
> > > > +
> > > > 
> > > >  static struct platform_driver ucb1400_ts_driver = {
> > > >  
> > > >  	.probe	= ucb1400_ts_probe,
> > > >  	.remove	= ucb1400_ts_remove,
> > > > 
> > > > -	.resume	= ucb1400_ts_resume,
> > > > 
> > > >  	.driver	= {
> > > >  	
> > > >  		.name	= "ucb1400_ts",
> > > > 
> > > > +		.pm	= &ucb1400_ts_pm_ops,
> > > > 
> > > >  	},
> > > >  
> > > >  };
> > > 
> > > Hi,
> > > 
> > > I went through the patchset, but didn't test it. Though the changes look
> > > reasonable. I'll have time to test this sometimes next week MAYBE, or it
> > > MIGHT tell someone else to test, I can't promise anything here though.
> > > 
> > > Please add to all patches my:
> > > 
> > > Reviewed-by: Marek Vasut <marek.vasut@gmail.com>
> > 
> > Thanks Marek. I have not tested these either so it would be nice to find
> > someone who could. However there is no urgency as we have some time
> > before 3.3 merge window opens.
> 
> I can, I have devices with this chip available ... just not now, real life is 
> haunting me :)

Hi Marek,

Any chance you will be able to try these patches before 3.3 merge window
opens?

Thanks,

-- 
Dmitry

Re: [PATCH 1/6] Input: ucb1400_ts - convert to use dev_pm_ops

[Marek Vasut]

> On Mon, Nov 14, 2011 at 06:04:12PM +0100, Marek Vasut wrote:
> > > On Mon, Nov 14, 2011 at 11:39:17AM +0100, Marek Vasut wrote:
> > > > > Instead of using legacy PM interfaces switch to using dev_pm_ops.
> > > > > 
> > > > > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > > > > ---
> > > > > 
> > > > >  drivers/input/touchscreen/ucb1400_ts.c |   12 ++++++------
> > > > >  1 files changed, 6 insertions(+), 6 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/input/touchscreen/ucb1400_ts.c
> > > > > b/drivers/input/touchscreen/ucb1400_ts.c index 3b5b5df..ead7123
> > > > > 100644 --- a/drivers/input/touchscreen/ucb1400_ts.c
> > > > > +++ b/drivers/input/touchscreen/ucb1400_ts.c
> > > > > @@ -428,10 +428,10 @@ static int ucb1400_ts_remove(struct
> > > > > platform_device *dev) return 0;
> > > > > 
> > > > >  }
> > > > > 
> > > > > -#ifdef CONFIG_PM
> > > > > -static int ucb1400_ts_resume(struct platform_device *dev)
> > > > > +#ifdef CONFIG_PM_SLEEP
> > > > > +static int ucb1400_ts_resume(struct device *dev)
> > > > > 
> > > > >  {
> > > > > 
> > > > > -	struct ucb1400_ts *ucb = dev->dev.platform_data;
> > > > > +	struct ucb1400_ts *ucb = dev->platform_data;
> > > > > 
> > > > >  	if (ucb->ts_task) {
> > > > >  	
> > > > >  		/*
> > > > > 
> > > > > @@ -444,16 +444,16 @@ static int ucb1400_ts_resume(struct
> > > > > platform_device *dev) }
> > > > > 
> > > > >  	return 0;
> > > > >  
> > > > >  }
> > > > > 
> > > > > -#else
> > > > > -#define ucb1400_ts_resume NULL
> > > > > 
> > > > >  #endif
> > > > > 
> > > > > +static SIMPLE_DEV_PM_OPS(ucb1400_ts_pm_ops, NULL,
> > > > > ucb1400_ts_resume); +
> > > > > 
> > > > >  static struct platform_driver ucb1400_ts_driver = {
> > > > >  
> > > > >  	.probe	= ucb1400_ts_probe,
> > > > >  	.remove	= ucb1400_ts_remove,
> > > > > 
> > > > > -	.resume	= ucb1400_ts_resume,
> > > > > 
> > > > >  	.driver	= {
> > > > >  	
> > > > >  		.name	= "ucb1400_ts",
> > > > > 
> > > > > +		.pm	= &ucb1400_ts_pm_ops,
> > > > > 
> > > > >  	},
> > > > >  
> > > > >  };
> > > > 
> > > > Hi,
> > > > 
> > > > I went through the patchset, but didn't test it. Though the changes
> > > > look reasonable. I'll have time to test this sometimes next week
> > > > MAYBE, or it MIGHT tell someone else to test, I can't promise
> > > > anything here though.
> > > > 
> > > > Please add to all patches my:
> > > > 
> > > > Reviewed-by: Marek Vasut <marek.vasut@gmail.com>
> > > 
> > > Thanks Marek. I have not tested these either so it would be nice to
> > > find someone who could. However there is no urgency as we have some
> > > time before 3.3 merge window opens.
> > 
> > I can, I have devices with this chip available ... just not now, real
> > life is haunting me :)
> 
> Hi Marek,
> 
> Any chance you will be able to try these patches before 3.3 merge window
> opens?
> 
> Thanks,

Hi Dmitry.

Honestly, reality is just crap. But, I'm fine with these, please add my:

Acked-by: Marek Vasut <marek.vasut@gmail.com>